* Re: [PATCH 4/6] net: sh_eth: modify the PHY_INTERFACE_MODE
From: David Miller @ 2011-03-14 21:11 UTC (permalink / raw)
To: yoshihiro.shimoda.uh; +Cc: netdev, linux-sh
In-Reply-To: <4D75E1F1.5090306@renesas.com>
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date: Tue, 08 Mar 2011 16:59:45 +0900
> The previous code had hardcoded the PHY_INTERFACE_MODE_MII of phy_connect.
> So some Gigabit PHYs will not behave correctly.
> The patch adds the phy_interface in sh_eth_plat_data, so we can select
> the phy interface.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 3/6] net: sh_eth: remove almost #ifdef of SH7763
From: David Miller @ 2011-03-14 21:10 UTC (permalink / raw)
To: yoshihiro.shimoda.uh; +Cc: netdev, linux-sh
In-Reply-To: <4D75E1EA.50006@renesas.com>
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date: Tue, 08 Mar 2011 16:59:38 +0900
> The SH7763 has GETHER. So the specification of some registers differs than
> other CPUs. This patch removes almost #ifdef of CONFIG_CPU_SUBTYPE_SH7763.
> Then we are able to add other CPU's GETHER easily.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 2/6] net: sh_eth: remove the SH_TSU_ADDR
From: David Miller @ 2011-03-14 21:10 UTC (permalink / raw)
To: yoshihiro.shimoda.uh; +Cc: netdev, linux-sh
In-Reply-To: <4D75E1E6.10108@renesas.com>
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date: Tue, 08 Mar 2011 16:59:34 +0900
> The defination is hardcoded in this driver for some CPUs. This patch
> modifies to get resource of TSU address from platform_device.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Applied.
^ permalink raw reply
* Re: fcoe: correct checking for bonding
From: David Miller @ 2011-03-14 21:04 UTC (permalink / raw)
To: James.Bottomley
Cc: jpirko, robert.w.love, linux-scsi, devel, netdev, fubar,
joe.eykholt
In-Reply-To: <1300136015.31662.13.camel@mulgrave.site>
From: James Bottomley <James.Bottomley@suse.de>
Date: Mon, 14 Mar 2011 15:53:35 -0500
> That said, I think we're pretty close to the end of the patch series
> for the merge window and it's a simple patch, so as long as it
> applies to net-next, I think we have an pretty low probability for
> non trivial merges. You can do it with my and Rob's acked-by.
Ok, thanks.
^ permalink raw reply
* Re: [PATCH 0/3] bonding: various fixes
From: Andy Gospodarek @ 2011-03-14 21:04 UTC (permalink / raw)
To: Phil Oester; +Cc: netdev, fubar, andy
In-Reply-To: <1300119726-17529-1-git-send-email-kernel@linuxace.com>
On Mon, Mar 14, 2011 at 09:22:03AM -0700, Phil Oester wrote:
> A few collected fixes to bonding. Patches are against net-next,
> but should apply fine to 38-rc.
>
> Phil
>
>
> bond_main.c | 15 +++++++++++----
> bond_sysfs.c | 5 ++++-
> 2 files changed, 15 insertions(+), 5 deletions(-)
All in the series look good. Thanks for posting the queue cleanup,
Phil.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
^ permalink raw reply
* [PATCH v2] tcp: avoid cwnd moderation in undo
From: Yuchung Cheng @ 2011-03-14 20:57 UTC (permalink / raw)
To: David Miller, Ilpo Jarvinen; +Cc: Nandita Dukkipati, netdev, Yuchung Cheng
In-Reply-To: <201103141106.19679.carsten@wolffcarsten.de>
In the current undo logic, cwnd is moderated after it was restored
to the value prior entering fast-recovery. It was moderated first
in tcp_try_undo_recovery then again in tcp_complete_cwr.
Since the undo indicates recovery was false, these moderations
are not necessary. If the undo is triggered when most of the
outstanding data have been acknowledged, the (restored) cwnd is
falsely pulled down to a small value.
This patch removes these cwnd moderations if cwnd is undone
a) during fast-recovery
b) by receiving DSACKs past fast-recovery
Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
Changelog since v1:
Revise the patch description to include undos after recovery
net/ipv4/tcp_input.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 08ea735..1bd0f38 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2659,7 +2659,7 @@ static void DBGUNDO(struct sock *sk, const char *msg)
#define DBGUNDO(x...) do { } while (0)
#endif
-static void tcp_undo_cwr(struct sock *sk, const int undo)
+static void tcp_undo_cwr(struct sock *sk, const int undo_ssthresh)
{
struct tcp_sock *tp = tcp_sk(sk);
@@ -2671,14 +2671,13 @@ static void tcp_undo_cwr(struct sock *sk, const int undo)
else
tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh << 1);
- if (undo && tp->prior_ssthresh > tp->snd_ssthresh) {
+ if (undo_ssthresh && tp->prior_ssthresh > tp->snd_ssthresh) {
tp->snd_ssthresh = tp->prior_ssthresh;
TCP_ECN_withdraw_cwr(tp);
}
} else {
tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh);
}
- tcp_moderate_cwnd(tp);
tp->snd_cwnd_stamp = tcp_time_stamp;
}
@@ -2822,8 +2821,11 @@ static int tcp_try_undo_loss(struct sock *sk)
static inline void tcp_complete_cwr(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
- tp->snd_cwnd = min(tp->snd_cwnd, tp->snd_ssthresh);
- tp->snd_cwnd_stamp = tcp_time_stamp;
+ /* Do not moderate cwnd if it's already undone in cwr or recovery */
+ if (tp->undo_marker && tp->snd_cwnd > tp->snd_ssthresh) {
+ tp->snd_cwnd = tp->snd_ssthresh;
+ tp->snd_cwnd_stamp = tcp_time_stamp;
+ }
tcp_ca_event(sk, CA_EVENT_COMPLETE_CWR);
}
--
1.7.3.1
^ permalink raw reply related
* Re: fcoe: correct checking for bonding
From: James Bottomley @ 2011-03-14 20:53 UTC (permalink / raw)
To: David Miller
Cc: jpirko, robert.w.love, linux-scsi, devel, netdev, fubar,
joe.eykholt
In-Reply-To: <20110314.132007.193707878.davem@davemloft.net>
On Mon, 2011-03-14 at 13:20 -0700, David Miller wrote:
> From: Jiri Pirko <jpirko@redhat.com>
> Date: Mon, 14 Mar 2011 20:22:02 +0100
> > Mon, Mar 14, 2011 at 08:04:07PM CET, robert.w.love@intel.com wrote:
> >> Taking a patch like this through net{-next} could cause a merge
> >>problem at Linus' level if a later patch makes it though the normal
> >>process and conflicts. This is what I want to avoid.
> >>
> >> This patch, although appreciated, isn't critical. I have collected it
> >>into my tree and will re-post it to scsi-misc. I see no reason to treat
> >>this patch differently from other patches.
> >
> > Well I have another set of patches dependent on this one :(
>
> True, also I think Rob is overreacting.
>
> Any merge problems created will be handled properly by Linus.
>
> I recently changed the interface to ipv4 and ipv6 route lookups, and
> this required all kinds of changes to stuff under Infiniband and elsewhere.
> It's the only sane way to handle this kind of thing.
What Rob means is that fcoe has been in pretty heavy flux and so
parallel patches can often cause non trivial merge nasties because of
code motion. That said, I think we're pretty close to the end of the
patch series for the merge window and it's a simple patch, so as long as
it applies to net-next, I think we have an pretty low probability for
non trivial merges. You can do it with my and Rob's acked-by.
James
^ permalink raw reply
* Re: [PATCH net-next] tipc: delete extra semicolon blocking node deletion
From: David Miller @ 2011-03-14 20:50 UTC (permalink / raw)
To: paul.gortmaker; +Cc: netdev, Allan.Stephens
In-Reply-To: <1300122682-13752-1-git-send-email-paul.gortmaker@windriver.com>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Mon, 14 Mar 2011 13:11:22 -0400
> Remove bogus semicolon only recently introduced in 34e46258cb9f5
> that blocks cleanup of nodes for N>1 on shutdown.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>
> Also available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/paulg/net-next-2.6.git tipc-Mar14-2011
Pulled, thanks Paul.
We should probably change all text editors in the world to display
semi-colon characters large and flashing red when editing C code :)
^ permalink raw reply
* Re: [PATCH net-next-2.6] pktgen: bug fix in transmission headers with frags=0
From: David Miller @ 2011-03-14 20:47 UTC (permalink / raw)
To: eric.dumazet; +Cc: daniel.turull, netdev, robert, jens.laas, voravit
In-Reply-To: <1300113350.3423.32.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 14 Mar 2011 15:35:50 +0100
> Le lundi 14 mars 2011 à 15:31 +0100, Daniel Turull a écrit :
>> (bug introduced by commit 26ad787962ef84677a48c560
>> (pktgen: speedup fragmented skbs)
>>
>> The headers of pktgen were incorrectly added in a pktgen packet
>> without frags (frags=0). There was an offset in the pktgen headers.
>>
>> The cause was in reusing the pgh variable as a return variable in skb_put
>> when adding the payload to the skb.
>>
>> Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH] tcp: avoid cwnd moderation in undo
From: John Heffner @ 2011-03-14 20:44 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Carsten Wolff, David Miller, Ilpo Jarvinen, Nandita Dukkipati,
netdev
In-Reply-To: <AANLkTimra1e-ONMx2hTeN90HFEvxqb3jPAr4v9_C+9wR@mail.gmail.com>
On Mon, Mar 14, 2011 at 3:10 PM, Yuchung Cheng <ycheng@google.com> wrote:
> On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote:
>> The moderation is in place to avoid gigantic segment bursts, which could cause
>> unnecessary pressure on buffers. In my eyes it's already suboptimal that the
>> moderation is weakened in the presence of (detected) reordering, let alone
>> removing it completely.
>
> In the presence of reordering, cwnd is already moderated in Disorder
> state before
> entering the (false) recovery.
I've always been somewhat skeptical of the usefulness of cwnd
moderation. First, I don't know that its behavior is well defined.
When *should* tcp_moderate_cwnd() actually be called, and why?
Second, I've never liked the idea in general. Reducing cwnd has an
effect lasting many RTTs, so reducing it in response to a transient
event like reordering seems dubious. And it does not address many
causes of bursts, such as ack compression or stretch acks.
-John
^ permalink raw reply
* Re: [PATCH net-2.6] bnx2x: fix swap of rx-ticks and tx-ticks parameters in interrupt coalescing flow
From: David Miller @ 2011-03-14 20:43 UTC (permalink / raw)
To: ariele; +Cc: eilong, netdev
In-Reply-To: <1300100840.2090.21.camel@lb-tlvb-ariel.il.broadcom.com>
From: "Ariel Elior" <ariele@broadcom.com>
Date: Mon, 14 Mar 2011 13:07:20 +0200
> Signed-off-by: Ariel Elior <ariele@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
Applied, thanks.
^ permalink raw reply
* [PATCH][v3] dev : fix mtu check when TSO is enabled
From: Daniel Lezcano @ 2011-03-14 20:39 UTC (permalink / raw)
To: davem; +Cc: eric.dumazet, kaber, nightnord, netdev
In case the device where is coming from the packet has TSO enabled,
we should not check the mtu size value as this one could be bigger
than the expected value.
This is the case for the macvlan driver when the lower device has
TSO enabled. The macvlan inherit this feature and forward the packets
without fragmenting them. Then the packets go through dev_forward_skb
and are dropped. This patch fix this by checking TSO is not enabled
when we want to check the mtu size.
Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Andrian Nord <nightnord@gmail.com>
---
net/core/dev.c | 24 ++++++++++++++++++++++--
1 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 6561021..29329d3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1503,6 +1503,27 @@ static inline void net_timestamp_check(struct sk_buff *skb)
__net_timestamp(skb);
}
+static inline bool is_skb_forwardable(struct net_device *dev,
+ struct sk_buff *skb)
+{
+ unsigned int len;
+
+ if (!(dev->flags & IFF_UP))
+ return false;
+
+ len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
+ if (skb->len < len)
+ return true;
+
+ /* if TSO is enabled, we don't care about the length as the packet
+ * could be forwarded without being segmented before
+ */
+ if (skb->dev && skb->dev->features & NETIF_F_TSO)
+ return true;
+
+ return false;
+}
+
/**
* dev_forward_skb - loopback an skb to another netif
*
@@ -1526,8 +1547,7 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
skb_orphan(skb);
nf_reset(skb);
- if (unlikely(!(dev->flags & IFF_UP) ||
- (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
+ if (unlikely(!is_skb_forwardable(dev, skb))) {
atomic_long_inc(&dev->rx_dropped);
kfree_skb(skb);
return NET_RX_DROP;
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] virtio: put last seen used index into ring itself
From: Michael S. Tsirkin @ 2011-03-14 20:30 UTC (permalink / raw)
To: Rusty Russell
Cc: Shirley Ma, Tom Lendacky, Krishna Kumar2, David Miller, kvm,
netdev, steved, jasowang
In-Reply-To: <cover.1300134326.git.mst@redhat.com>
Generally, the Host end of the virtio ring doesn't need to see where
Guest is up to in consuming the ring. However, to completely understand
what's going on from the outside, this information must be exposed.
For example, host can reduce the number of interrupts by detecting
that the guest is currently handling previous buffers.
Fortunately, we have room to expand: the ring is always a whole number
of pages and there's hundreds of bytes of padding after the avail ring
and the used ring, whatever the number of descriptors (which must be a
power of 2).
We add a feature bit so the guest can tell the host that it's writing
out the current value there, if it wants to use that.
This is based on a patch by Rusty Russell, with the main difference
being that we dedicate a feature bit to guest to tell the host it is
writing the used index. This way we don't need to force host to publish
the last available index until we have a use for it.
Memory barriers use has been rationalized (hopefully correctly).
To avoid the cost of an extra memory barrier on each update, we only
commit for the index to be up to date when interrupts
are enabled.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_ring.c | 25 ++++++++++++++++---------
include/linux/virtio_ring.h | 11 +++++++++++
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cc2f73e..a6fc537 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -89,9 +89,6 @@ struct vring_virtqueue
/* Number we've added since last sync. */
unsigned int num_added;
- /* Last used index we've seen. */
- u16 last_used_idx;
-
/* How to notify other side. FIXME: commonalize hcalls! */
void (*notify)(struct virtqueue *vq);
@@ -283,12 +280,13 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head)
static inline bool more_used(const struct vring_virtqueue *vq)
{
- return vq->last_used_idx != vq->vring.used->idx;
+ return vring_last_used(vq) != vq->vring.used->idx;
}
void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ struct vring_used_elem *u;
void *ret;
unsigned int i;
@@ -308,9 +306,9 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
/* Only get used array entries after they have been exposed by host. */
virtio_rmb();
- i = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].id;
- *len = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].len;
-
+ u = &vq->vring.used->ring[vring_last_used(vq) % vq->vring.num];
+ i = u->id;
+ *len = u->len;
if (unlikely(i >= vq->vring.num)) {
BAD_RING(vq, "id %u out of range\n", i);
return NULL;
@@ -323,7 +321,12 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
/* detach_buf clears data, so grab it now. */
ret = vq->data[i];
detach_buf(vq, i);
- vq->last_used_idx++;
+ (vring_last_used(vq))++;
+ /* If we expect an interrupt for the next entry, flush out
+ * last used index write. */
+ if (!(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
+ virtio_mb();
+
END_USE(vq);
return ret;
}
@@ -346,6 +349,8 @@ bool virtqueue_enable_cb(struct virtqueue *_vq)
/* We optimistically turn back on interrupts, then check if there was
* more to do. */
vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
+ /* Besides flags write, this barrier also flushes out
+ * last available index write. */
virtio_mb();
if (unlikely(more_used(vq))) {
END_USE(vq);
@@ -429,7 +434,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
vq->vq.name = name;
vq->notify = notify;
vq->broken = false;
- vq->last_used_idx = 0;
+ vring_last_used(vq) = 0;
vq->num_added = 0;
list_add_tail(&vq->vq.list, &vdev->vqs);
#ifdef DEBUG
@@ -471,6 +476,8 @@ void vring_transport_features(struct virtio_device *vdev)
switch (i) {
case VIRTIO_RING_F_INDIRECT_DESC:
break;
+ case VIRTIO_RING_F_PUBLISH_USED:
+ break;
default:
/* We don't understand this bit. */
clear_bit(i, vdev->features);
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index e4d144b..4587ea2 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -29,6 +29,9 @@
/* We support indirect buffer descriptors */
#define VIRTIO_RING_F_INDIRECT_DESC 28
+/* The Guest publishes last-seen used index at the end of the avail ring. */
+#define VIRTIO_RING_F_PUBLISH_USED 29
+
/* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
struct vring_desc {
/* Address (guest-physical). */
@@ -83,6 +86,7 @@ struct vring {
* __u16 avail_flags;
* __u16 avail_idx;
* __u16 available[num];
+ * __u16 last_used_idx;
*
* // Padding to the next align boundary.
* char pad[];
@@ -93,6 +97,10 @@ struct vring {
* struct vring_used_elem used[num];
* };
*/
+
+/* We publish the last-seen used index at the end of the available ring.
+ * It is at the end for backwards compatibility. */
+#define vring_last_used(vr) ((vr)->avail->ring[num])
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
@@ -101,6 +109,9 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
vr->avail = p + num*sizeof(struct vring_desc);
vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
& ~(align - 1));
+ /* Verify that last used index does not spill over the used ring. */
+ BUG_ON((void *)&vring_last_used(vr) +
+ sizeof vring_last_used(vr) > (void *)vr->used);
}
static inline unsigned vring_size(unsigned int num, unsigned long align)
--
1.7.3.2.91.g446ac
^ permalink raw reply related
* [PATCH 0/2] publish last used index (was Re: TX from KVM guest virtio_net to vhost issues)
From: Michael S. Tsirkin @ 2011-03-14 20:29 UTC (permalink / raw)
To: Rusty Russell
Cc: Shirley Ma, Tom Lendacky, Krishna Kumar2, David Miller, kvm,
netdev, steved, jasowang
In-Reply-To: <87d3ly2n81.fsf@rustcorp.com.au>
On Fri, Mar 11, 2011 at 04:49:26PM +1030, Rusty Russell wrote:
> But if one side outruns the other, it does a lot of unnecessary work
> notifying/interrupting it over and over again before the host/guest gets
> a chance to shut notifications/interrupts off. Hence the last_used
> publishing patch (Xen does this right, I screwed it up).
>
> Long weekend here, and I'm otherwise committed. But if noone has
> cleaned up that patch by early next week, I'll try to do so and see if
> we can make a real difference.
>
> Cheers,
> Rusty.
I actually had the last_used publishing patch working. Here it is
again: it's the part that publishes the used index from guest and the
part that uses it in host to avoid spurious interrupts
(this is Tom's case, not Shirley's case)
updated to the latest bits, and clarified the code a bit more.
Note: there's some tricky logic in the vhost part, I think I got it right
but please review.
Warning: compiled only, completely untested, I intend to add the bits
that use the last available index in guest to reduce exits, test it all
together.
Would appreciate review and/or testing.
Michael S. Tsirkin (2):
virtio: put last seen used index into ring itself
vhost-net: utilize PUBLISH_USED_IDX feature
drivers/vhost/vhost.c | 58 +++++++++++++++++++++++++++++++++++++----
drivers/vhost/vhost.h | 9 ++++++
drivers/virtio/virtio_ring.c | 25 +++++++++++------
include/linux/virtio_ring.h | 11 ++++++++
4 files changed, 88 insertions(+), 15 deletions(-)
--
1.7.3.2.91.g446ac
^ permalink raw reply
* [PATCH 2/2] vhost-net: utilize PUBLISH_USED_IDX feature
From: Michael S. Tsirkin @ 2011-03-14 20:30 UTC (permalink / raw)
To: Rusty Russell
Cc: Shirley Ma, Tom Lendacky, Krishna Kumar2, David Miller, kvm,
netdev, steved, jasowang
In-Reply-To: <cover.1300134326.git.mst@redhat.com>
With PUBLISH_USED_IDX, guest tells us which used entries
it has consumed. This can be used to reduce the number
of interrupts: after we write a used entry, if the guest has not yet
consumed the previous entry, or if the guest has already consumed the
new entry, we do not need to interrupt.
This imporves bandwidth by 30% under some workflows.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 58 +++++++++++++++++++++++++++++++++++++++++++-----
drivers/vhost/vhost.h | 9 +++++++
2 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2ab2912..8d5d56a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -161,6 +161,8 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->last_avail_idx = 0;
vq->avail_idx = 0;
vq->last_used_idx = 0;
+ vq->signal_used = 0;;
+ vq->signal_next = true;;
vq->used_flags = 0;
vq->log_used = false;
vq->log_addr = -1ull;
@@ -489,14 +491,15 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
return 1;
}
-static int vq_access_ok(unsigned int num,
+static int vq_access_ok(struct vhost_dev *d, unsigned int num,
struct vring_desc __user *desc,
struct vring_avail __user *avail,
struct vring_used __user *used)
{
+ size_t s = vhost_has_feature(d, VIRTIO_RING_F_PUBLISH_USED) ? 2 : 0;
return access_ok(VERIFY_READ, desc, num * sizeof *desc) &&
access_ok(VERIFY_READ, avail,
- sizeof *avail + num * sizeof *avail->ring) &&
+ sizeof *avail + num * sizeof *avail->ring + s) &&
access_ok(VERIFY_WRITE, used,
sizeof *used + num * sizeof *used->ring);
}
@@ -531,7 +534,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
/* Caller should have vq mutex and device mutex */
int vhost_vq_access_ok(struct vhost_virtqueue *vq)
{
- return vq_access_ok(vq->num, vq->desc, vq->avail, vq->used) &&
+ return vq_access_ok(vq->dev, vq->num, vq->desc, vq->avail, vq->used) &&
vq_log_access_ok(vq, vq->log_base);
}
@@ -577,6 +580,7 @@ static int init_used(struct vhost_virtqueue *vq,
if (r)
return r;
+ vq->signal_next = true;
return get_user(vq->last_used_idx, &used->idx);
}
@@ -674,7 +678,7 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
* If it is not, we don't as size might not have been setup.
* We will verify when backend is configured. */
if (vq->private_data) {
- if (!vq_access_ok(vq->num,
+ if (!vq_access_ok(d, vq->num,
(void __user *)(unsigned long)a.desc_user_addr,
(void __user *)(unsigned long)a.avail_user_addr,
(void __user *)(unsigned long)a.used_user_addr)) {
@@ -699,6 +703,7 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
vq->log_used = !!(a.flags & (0x1 << VHOST_VRING_F_LOG));
vq->desc = (void __user *)(unsigned long)a.desc_user_addr;
vq->avail = (void __user *)(unsigned long)a.avail_user_addr;
+ vq->last_used = (u16 __user *)&vq->avail->ring[vq->num];
vq->log_addr = a.log_guest_addr;
vq->used = (void __user *)(unsigned long)a.used_user_addr;
break;
@@ -1230,7 +1235,8 @@ void vhost_discard_vq_desc(struct vhost_virtqueue *vq, int n)
/* After we've used one of their buffers, we tell them about it. We'll then
* want to notify the guest, using eventfd. */
-int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
+int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head,
+ int len)
{
struct vring_used_elem __user *used;
@@ -1267,6 +1273,8 @@ int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
eventfd_signal(vq->log_ctx, 1);
}
vq->last_used_idx++;
+ if (unlikely(vq->last_used_idx == vq->signal_used))
+ vq->signal_next = true;
return 0;
}
@@ -1275,6 +1283,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
unsigned count)
{
struct vring_used_elem __user *used;
+ u16 last_used_idx;
int start;
start = vq->last_used_idx % vq->num;
@@ -1292,7 +1301,14 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
((void __user *)used - (void __user *)vq->used),
count * sizeof *used);
}
+ last_used_idx = vq->last_used_idx;
vq->last_used_idx += count;
+ /* make sure we signal the next entry in an unlikely event of a
+ * wrap-around without signalling once. */
+ /* Note: this should never happen with current vhost-net code. */
+ if (unlikely(last_used_idx < vq->signal_used &&
+ vq->last_used_idx >= vq->signal_used))
+ vq->signal_next = true;
return 0;
}
@@ -1335,7 +1351,8 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
{
__u16 flags;
-
+ __u16 n;
+ bool s;
/* Flush out used index updates. This is paired
* with the barrier that the Guest executes when enabling
* interrupts. */
@@ -1346,12 +1363,41 @@ void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
return;
}
+ n = vq->signal_used;
+ s = vq->signal_next;
+ vq->signal_used = vq->last_used_idx;
+ vq->signal_next = false;
+
/* If they don't want an interrupt, don't signal, unless empty. */
if ((flags & VRING_AVAIL_F_NO_INTERRUPT) &&
(vq->avail_idx != vq->last_avail_idx ||
!vhost_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY)))
return;
+ if (vhost_has_feature(dev, VIRTIO_RING_F_PUBLISH_USED)) {
+ __u16 used;
+ if (get_user(used, vq->last_used)) {
+ vq_err(vq, "Failed to get last used idx");
+ return;
+ }
+
+ /* Do not notify if guest did not yet see the last update. */
+ /* We compare used from guest to signal_used, which is
+ * the old value of last_used_idx where we called signal previously.
+ Let's assume w.l.o.g that signalled_used is 0 (we will later
+ subtract this old value to get the generic case);
+ now if used > last_used_idx this means guest is processing
+ old entries and if used == last_used_idx it has processed
+ all the new ones.
+ Thus our logic *for when signal_used is 0* is:
+ if (used >= last_used_idx) return;
+ and in the generic case, subtract signalled_used,
+ modulo 0x10000.
+ */
+ if (s || ((u16)(used - n) >= (u16)(vq->last_used_idx - n)))
+ return;
+ }
+
/* Signal the Guest tell them we used something up. */
if (vq->call_ctx)
eventfd_signal(vq->call_ctx, 1);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index b3363ae..b14b994 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -59,6 +59,7 @@ struct vhost_virtqueue {
unsigned int num;
struct vring_desc __user *desc;
struct vring_avail __user *avail;
+ u16 __user *last_used;
struct vring_used __user *used;
struct file *kick;
struct file *call;
@@ -84,6 +85,13 @@ struct vhost_virtqueue {
/* Used flags */
u16 used_flags;
+ /* last_used_idx value when we signalled. */
+ u16 signal_used;
+
+ /* Flag to ignore signal_used above
+ * and signal on the next entry. */
+ bool signal_next;
+
/* Log writes to used structure. */
bool log_used;
u64 log_addr;
@@ -164,6 +172,7 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
enum {
VHOST_FEATURES = (1 << VIRTIO_F_NOTIFY_ON_EMPTY) |
(1 << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1 << VIRTIO_RING_F_PUBLISH_USED) |
(1 << VHOST_F_LOG_ALL) |
(1 << VHOST_NET_F_VIRTIO_NET_HDR) |
(1 << VIRTIO_NET_F_MRG_RXBUF),
--
1.7.3.2.91.g446ac
^ permalink raw reply related
* Re: [PATCH] Fix overflow of name in struct net_device, replaced str* and sprintf with strn* and snprintf respectivly.
From: Ben Hutchings @ 2011-03-14 20:29 UTC (permalink / raw)
To: Sasikanth V; +Cc: davem, netdev
In-Reply-To: <1300133248-2927-1-git-send-email-sasikanth.v19@gmail.com>
On Tue, 2011-03-15 at 01:37 +0530, Sasikanth V wrote:
> Signed-off-by: Sasikanth V <sasikanth.v19@gmail.com>
> ---
> net/core/dev.c | 14 +++++++-------
> 1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6561021..9d06c1e 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -510,7 +510,7 @@ int netdev_boot_setup_check(struct net_device *dev)
>
> for (i = 0; i < NETDEV_BOOT_SETUP_MAX; i++) {
> if (s[i].name[0] != '\0' && s[i].name[0] != ' ' &&
> - !strcmp(dev->name, s[i].name)) {
> + !strncmp(dev->name, s[i].name, IFNAMSIZ)) {
If s[i].name is too long, so what? No change is required here.
> dev->irq = s[i].map.irq;
> dev->base_addr = s[i].map.base_addr;
> dev->mem_start = s[i].map.mem_start;
> @@ -539,7 +539,7 @@ unsigned long netdev_boot_base(const char *prefix, int unit)
> char name[IFNAMSIZ];
> int i;
>
> - sprintf(name, "%s%d", prefix, unit);
> + snprintf(name, IFNAMSIZ, "%s%d", prefix, unit);
This looks reasonable.
> /*
> * If device already registered then return base of 1
> @@ -549,7 +549,7 @@ unsigned long netdev_boot_base(const char *prefix, int unit)
> return 1;
>
> for (i = 0; i < NETDEV_BOOT_SETUP_MAX; i++)
> - if (!strcmp(name, s[i].name))
> + if (!strncmp(name, s[i].name, IFNAMSIZ))
Not required.
> return s[i].map.base_addr;
> return 0;
> }
> @@ -836,7 +836,7 @@ EXPORT_SYMBOL(dev_get_by_flags_rcu);
> */
> int dev_valid_name(const char *name)
> {
> - if (*name == '\0')
> + if (!name || *name == '\0')
Not required; passing NULL is a bug.
> return 0;
> if (strlen(name) >= IFNAMSIZ)
> return 0;
> @@ -3834,7 +3834,7 @@ static int dev_ifname(struct net *net, struct ifreq __user *arg)
> return -ENODEV;
> }
>
> - strcpy(ifr.ifr_name, dev->name);
> + strncpy(ifr.ifr_name, dev->name, IFNAMSIZ);
If dev->name is longer than IFNAMSIZ (including null terminator) than we
already failed. This change is not required.
> rcu_read_unlock();
>
> if (copy_to_user(arg, &ifr, sizeof(struct ifreq)))
> @@ -4821,7 +4821,7 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg)
> if (copy_from_user(&ifr, arg, sizeof(struct ifreq)))
> return -EFAULT;
>
> - ifr.ifr_name[IFNAMSIZ-1] = 0;
> + ifr.ifr_name[IFNAMSIZ-1] = '\0';
This is just noise.
> colon = strchr(ifr.ifr_name, ':');
> if (colon)
> @@ -5694,7 +5694,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> goto free_all;
> #endif
>
> - strcpy(dev->name, name);
> + strncpy(dev->name, name, IFNAMSIZ);
This doesn't help, as strncpy() does not ensure null termination. And I
don't think truncating the name is helpful either. Perhaps we should do
this right at the beginning of the function:
if (WARN_ON(strlen(name) >= IFNAMSIZ))
return NULL;
Ben.
> return dev;
>
> free_all:
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [Bloat] shaper team forming up
From: Eric Dumazet @ 2011-03-14 20:24 UTC (permalink / raw)
To: Jonathan Morton; +Cc: Dave Täht, bloat-devel, bloat, netdev
In-Reply-To: <5BC42741-852B-4699-BA5D-D70B8D610D96@gmail.com>
Le lundi 14 mars 2011 à 21:55 +0200, Jonathan Morton a écrit :
> On 14 Mar, 2011, at 9:26 pm, Dave Täht wrote:
>
> > Over the weekend, Dan Siemons uncovered a possible bad interaction
> > between ECN and the default pfifo_fast qdisc in Linux.
> >
> > http://www.coverfire.com/archives/2011/03/13/pfifo_fast-and-ecn/
>
> This seems to be more complicated that it appears. It looks as though
> Linux has re-used the LSB of the old TOS field for some "link local"
> flag which is used by routing.
>
> It's not immediately obvious whether pfifo_fast is using this new
> interpretation though. If it isn't, the fix should be to remove the
> RTO_ONLINK bit from the mask it's using on the tos field. The other
> half of the mask correctly excludes the ECN bits from the field.
>
CC netdev, where linux network dev can take a look.
I would say that this is a wrong analysis :
1) ECN uses two low order bits of TOS byte
2) pfifo_fast uses skb->priority
skb->priority = rt_tos2priority(iph->tos);
#define IPTOS_TOS_MASK 0x1E
#define IPTOS_TOS(tos) ((tos)&IPTOS_TOS_MASK)
static inline char rt_tos2priority(u8 tos)
{
return ip_tos2prio[IPTOS_TOS(tos)>>1];
}
No interference between two mechanisms, unless sysadmin messed up things
(skb_edit)
^ permalink raw reply
* Re: [GIT/PATCH v5] xen network backend driver
From: David Miller @ 2011-03-14 20:23 UTC (permalink / raw)
To: Ian.Campbell
Cc: bhutchings, netdev, xen-devel, jeremy, herbert, konrad.wilk,
romieu, shemminger, mirqus
In-Reply-To: <1300131040.32696.23.camel@localhost.localdomain>
From: Ian Campbell <Ian.Campbell@eu.citrix.com>
Date: Mon, 14 Mar 2011 19:30:40 +0000
> On Mon, 2011-03-14 at 18:40 +0000, Ben Hutchings wrote:
>> I know you have asked for this to be pulled, in order to retain its
>> history, but I'm not sure that David Miller will do this unless the
>> driver is at least buildable at each stage in the history. (However,
>> given that the driver wouldn't be enabled before it's added, I'm not
>> sure it matters in this case.)
>
> I think it should be, at least from the second commit onwards (modulo
> any mistakes made in the past).
I don't want to retain all of that history, there are so many broken
points in the history that the only thing you can do is go back and
add Kconfig commits to depend the driver upon BROKEN, and that in effect
changes the history devalueing it even further.
I just want one patch that adds the driver in it's most current state,
and that's all.
Thanks.
^ permalink raw reply
* Re: fcoe: correct checking for bonding
From: David Miller @ 2011-03-14 20:20 UTC (permalink / raw)
To: jpirko
Cc: robert.w.love, James.Bottomley, linux-scsi, devel, netdev, fubar,
joe.eykholt
In-Reply-To: <20110314192201.GA2873@psychotron.redhat.com>
From: Jiri Pirko <jpirko@redhat.com>
Date: Mon, 14 Mar 2011 20:22:02 +0100
> Mon, Mar 14, 2011 at 08:04:07PM CET, robert.w.love@intel.com wrote:
>>On Sat, 2011-03-12 at 10:59 -0800, David Miller wrote:
>>> From: Jiri Pirko <jpirko@redhat.com>
>>> Date: Sat, 12 Mar 2011 13:01:10 +0100
>>>
>>> > Thu, Mar 03, 2011 at 02:09:18AM CET, robert.w.love@intel.com wrote:
>>> >>On Wed, 2011-03-02 at 01:55 -0800, Jiri Pirko wrote:
>>> >>> Or perhaps this should be applied to net-next?
>>> >>>
>>> >>I think this should go through scsi-misc as all the other
>>> >>libfc/libfcoe/fcoe patches do.
>>> >>
>>> >>> Wed, Mar 02, 2011 at 07:05:35AM CET, jpirko@redhat.com wrote:
>>> >>> >Check for bonding master and refuse to use that.
>>> >>> >
>>> >>> >Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>>> >>> >---
>>> >>> > drivers/scsi/fcoe/fcoe.c | 4 +---
>>> >>> > 1 files changed, 1 insertions(+), 3 deletions(-)
>>> >>> >
>>> >>> >diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>>> >>> >index 9f9600b..3becc6a 100644
>>> >>> >--- a/drivers/scsi/fcoe/fcoe.c
>>> >>> >+++ b/drivers/scsi/fcoe/fcoe.c
>>> >>> >@@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
>>> >>> > }
>>> >>> >
>>> >>> > /* Do not support for bonding device */
>>> >>> >- if ((netdev->priv_flags & IFF_MASTER_ALB) ||
>>> >>> >- (netdev->priv_flags & IFF_SLAVE_INACTIVE) ||
>>> >>> >- (netdev->priv_flags & IFF_MASTER_8023AD)) {
>>> >>> >+ if (netdev->priv_flags & IFF_BONDING && netdev->flags & IFF_MASTER) {
>>> >>> > FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
>>> >>> > return -EOPNOTSUPP;
>>> >>> > }
>>> >>> >--
>>> >>> >1.7.3.4
>>> >>> >
>>> >>
>>> >>James, feel free to pick up this patch. I don't have anything in my fcoe
>>> >>tree right now that it would conflict with. I'll also put it in my tree
>>> >>and resend if you don't put it into scsi-misc directly.
>>> >
>>> > What's the status of this? Maybe this should rather go thru net-next
>>>
>>> Sure, I can take this. I'll look at it later.
>>
>>Hi Dave,
>>
>> I'd rather have this patch go through scsi-misc. Most, if not all,
>>libfc, libfcoe and fcoe patches have taken this path. The way it has
>>been working is that I have been collecting fcoe patches and re-posting
>>them to scsi-misc after I have reviewed them and done some basic
>>testing.
>>
>> Taking a patch like this through net{-next} could cause a merge
>>problem at Linus' level if a later patch makes it though the normal
>>process and conflicts. This is what I want to avoid.
>>
>> This patch, although appreciated, isn't critical. I have collected it
>>into my tree and will re-post it to scsi-misc. I see no reason to treat
>>this patch differently from other patches.
>
> Well I have another set of patches dependent on this one :(
True, also I think Rob is overreacting.
Any merge problems created will be handled properly by Linus.
I recently changed the interface to ipv4 and ipv6 route lookups, and
this required all kinds of changes to stuff under Infiniband and elsewhere.
It's the only sane way to handle this kind of thing.
^ permalink raw reply
* [PATCH] Fix overflow of name in struct net_device, replaced str* and sprintf with strn* and snprintf respectivly.
From: Sasikanth V @ 2011-03-14 20:07 UTC (permalink / raw)
To: davem; +Cc: netdev, Sasikanth V
Signed-off-by: Sasikanth V <sasikanth.v19@gmail.com>
---
net/core/dev.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 6561021..9d06c1e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -510,7 +510,7 @@ int netdev_boot_setup_check(struct net_device *dev)
for (i = 0; i < NETDEV_BOOT_SETUP_MAX; i++) {
if (s[i].name[0] != '\0' && s[i].name[0] != ' ' &&
- !strcmp(dev->name, s[i].name)) {
+ !strncmp(dev->name, s[i].name, IFNAMSIZ)) {
dev->irq = s[i].map.irq;
dev->base_addr = s[i].map.base_addr;
dev->mem_start = s[i].map.mem_start;
@@ -539,7 +539,7 @@ unsigned long netdev_boot_base(const char *prefix, int unit)
char name[IFNAMSIZ];
int i;
- sprintf(name, "%s%d", prefix, unit);
+ snprintf(name, IFNAMSIZ, "%s%d", prefix, unit);
/*
* If device already registered then return base of 1
@@ -549,7 +549,7 @@ unsigned long netdev_boot_base(const char *prefix, int unit)
return 1;
for (i = 0; i < NETDEV_BOOT_SETUP_MAX; i++)
- if (!strcmp(name, s[i].name))
+ if (!strncmp(name, s[i].name, IFNAMSIZ))
return s[i].map.base_addr;
return 0;
}
@@ -836,7 +836,7 @@ EXPORT_SYMBOL(dev_get_by_flags_rcu);
*/
int dev_valid_name(const char *name)
{
- if (*name == '\0')
+ if (!name || *name == '\0')
return 0;
if (strlen(name) >= IFNAMSIZ)
return 0;
@@ -3834,7 +3834,7 @@ static int dev_ifname(struct net *net, struct ifreq __user *arg)
return -ENODEV;
}
- strcpy(ifr.ifr_name, dev->name);
+ strncpy(ifr.ifr_name, dev->name, IFNAMSIZ);
rcu_read_unlock();
if (copy_to_user(arg, &ifr, sizeof(struct ifreq)))
@@ -4821,7 +4821,7 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg)
if (copy_from_user(&ifr, arg, sizeof(struct ifreq)))
return -EFAULT;
- ifr.ifr_name[IFNAMSIZ-1] = 0;
+ ifr.ifr_name[IFNAMSIZ-1] = '\0';
colon = strchr(ifr.ifr_name, ':');
if (colon)
@@ -5694,7 +5694,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
goto free_all;
#endif
- strcpy(dev->name, name);
+ strncpy(dev->name, name, IFNAMSIZ);
return dev;
free_all:
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH][v2] dev : fix mtu check when TSO is enabled
From: Daniel Lezcano @ 2011-03-14 20:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, kaber, nightnord, netdev
In-Reply-To: <1300132932.2649.2.camel@edumazet-laptop>
On 03/14/2011 09:02 PM, Eric Dumazet wrote:
> Le lundi 14 mars 2011 à 20:10 +0100, Daniel Lezcano a écrit :
>> In case the device where is coming from the packet has TSO enabled,
>> we should not check the mtu size value as this one could be bigger
>> than the expected value.
>>
>> This is the case for the macvlan driver when the lower device has
>> TSO enabled. The macvlan inherit this feature and forward the packets
>> without fragmenting them. Then the packets go through dev_forward_skb
>> and are dropped. This patch fix this by checking TSO is not enabled
>> when we want to check the mtu size.
>>
>> Signed-off-by: Daniel Lezcano<daniel.lezcano@free.fr>
>> Cc: Eric Dumazet<eric.dumazet@gmail.com>
>> Cc: Patrick McHardy<kaber@trash.net>
>> Cc: Andrian Nord<nightnord@gmail.com>
>> ---
>> net/core/dev.c | 24 ++++++++++++++++++++++--
>> 1 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 6561021..f1607a0 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1503,6 +1503,27 @@ static inline void net_timestamp_check(struct sk_buff *skb)
>> __net_timestamp(skb);
>> }
>>
>> +static inline bool is_skb_forwardable(struct net_device *dev,
>> + struct sk_buff *skb)
>> +{
>> + unsigned int len;
>> +
>> + if (!dev->flags& IFF_UP)
>> + return false;
> if (!(dev->flags& IFF_UP))
> return false;
Argh ! Hopefully you saw it.
>> +
>> + /* we should not check the mtu size if TSO is enabled otherwise
>> + * we may have a packet with a length bigger than the expected
>> + * one as it was not segmented before */
>> + if (skb->dev&& skb->dev->features& NETIF_F_TSO)
>> + return true;
>> +
>> + len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
>> + if (skb->len> len)
>> + return false;
>> +
> I suggest you reorder tests so that the first one is discriminant most
> of the time
>
> maxlen = dev->mtu + dev->hard_header_len + VLAN_HLEN;
> if (skb->len< maxlen)
> return true;
>
> if (skb->dev&& skb->dev->features& NETIF_F_TSO)
> return true;
>
> return false;
Ok. Thanks !
-- Daniel
^ permalink raw reply
* Re: [PATCH][v2] dev : fix mtu check when TSO is enabled
From: Eric Dumazet @ 2011-03-14 20:02 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: davem, kaber, nightnord, netdev
In-Reply-To: <1300129859-9525-1-git-send-email-daniel.lezcano@free.fr>
Le lundi 14 mars 2011 à 20:10 +0100, Daniel Lezcano a écrit :
> In case the device where is coming from the packet has TSO enabled,
> we should not check the mtu size value as this one could be bigger
> than the expected value.
>
> This is the case for the macvlan driver when the lower device has
> TSO enabled. The macvlan inherit this feature and forward the packets
> without fragmenting them. Then the packets go through dev_forward_skb
> and are dropped. This patch fix this by checking TSO is not enabled
> when we want to check the mtu size.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Andrian Nord <nightnord@gmail.com>
> ---
> net/core/dev.c | 24 ++++++++++++++++++++++--
> 1 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6561021..f1607a0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1503,6 +1503,27 @@ static inline void net_timestamp_check(struct sk_buff *skb)
> __net_timestamp(skb);
> }
>
> +static inline bool is_skb_forwardable(struct net_device *dev,
> + struct sk_buff *skb)
> +{
> + unsigned int len;
> +
> + if (!dev->flags & IFF_UP)
> + return false;
if (!(dev->flags & IFF_UP))
return false;
> +
> + /* we should not check the mtu size if TSO is enabled otherwise
> + * we may have a packet with a length bigger than the expected
> + * one as it was not segmented before */
> + if (skb->dev && skb->dev->features & NETIF_F_TSO)
> + return true;
> +
> + len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
> + if (skb->len > len)
> + return false;
> +
I suggest you reorder tests so that the first one is discriminant most
of the time
maxlen = dev->mtu + dev->hard_header_len + VLAN_HLEN;
if (skb->len < maxlen)
return true;
if (skb->dev && skb->dev->features & NETIF_F_TSO)
return true;
return false;
> + return true;
> +}
> +
> /**
> * dev_forward_skb - loopback an skb to another netif
> *
> @@ -1526,8 +1547,7 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
> skb_orphan(skb);
> nf_reset(skb);
>
> - if (unlikely(!(dev->flags & IFF_UP) ||
> - (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
> + if (unlikely(!is_skb_forwardable(dev, skb))) {
> atomic_long_inc(&dev->rx_dropped);
> kfree_skb(skb);
> return NET_RX_DROP;
^ permalink raw reply
* Re: [GIT/PATCH v5] xen network backend driver
From: Ian Campbell @ 2011-03-14 19:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
Herbert Xu, Konrad Rzeszutek Wilk, Francois Romieu, Ben Hutchings,
Stephen Hemminger, Michał Mirosław
In-Reply-To: <1300129032.3423.170.camel@edumazet-laptop>
On Mon, 2011-03-14 at 18:57 +0000, Eric Dumazet wrote:
> Le lundi 14 mars 2011 à 14:27 +0000, Ian Campbell a écrit :
> > + /* Statistics */
> > + unsigned long rx_gso_checksum_fixup;
>
> this is an "unsigned long" field
> > +static void xenvif_get_ethtool_stats(struct net_device *dev,
> > + struct ethtool_stats *stats, u64 * data)
> > +{
> > + void *vif = netdev_priv(dev);
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(xenvif_stats); i++)
> > + data[i] = *(int *)(vif + xenvif_stats[i].offset);
>
> so use : data[i] = *(unsigned long *)(vif + xenvif_stats[i].offset);
Oops, will do, thanks.
Ian.
^ permalink raw reply
* Re: [GIT/PATCH v5] xen network backend driver
From: Ian Campbell @ 2011-03-14 19:30 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
Herbert Xu, Konrad Rzeszutek Wilk, Francois Romieu,
Stephen Hemminger, Michał Mirosław
In-Reply-To: <1300128035.2584.6.camel@bwh-desktop>
On Mon, 2011-03-14 at 18:40 +0000, Ben Hutchings wrote:
> On Mon, 2011-03-14 at 14:27 +0000, Ian Campbell wrote:
> > The following patch is the fifth iteration of the Xen network backend
> > driver for upstream Linux.
> >
> > This driver ("netback") is the host side counterpart to the frontend
> > driver in drivers/net/xen-netfront.c. The PV protocol is also
> > implemented by frontend drivers in other OSes too, such as the BSDs and
> > even Windows.
> >
> > Changes since the fourth posting, due to review from Stephen Hemminger:
> > * Treat dev->features as u32 (it effectively is already and
> > becomes literally so in net-next -- there's no harm in netback
> > switching earlier)
> > * Use stats from struct netdevice instead of a local copy.
> [...]
>
> All looks good (or good enough) to me.
Thanks.
> I would add Reviewed-by, but this isn't a single patch.
Sure.
> I know you have asked for this to be pulled, in order to retain its
> history, but I'm not sure that David Miller will do this unless the
> driver is at least buildable at each stage in the history. (However,
> given that the driver wouldn't be enabled before it's added, I'm not
> sure it matters in this case.)
I think it should be, at least from the second commit onwards (modulo
any mistakes made in the past).
To be sure perhaps I should rebase to add a depends BROKEN to (or
otherwise nobble) the Kconfig entry at the beginning of time and only
remove it at the end of the series?
I'd definitely like to keep the cleanups I made while upstreaming
separated from the version imported from xen.git though, but I could
collapse things before that point if that would be preferred. The
upstream/dom0/backend/netback-base branch is the imported version,
everything after that until upstream/dom0/backend/netback is what I
really want to keep as separate commits, the stuff before is just (very)
nice to have.
Ian.
^ permalink raw reply
* Re: Occasional link flap on Intel 82599 on boot in XAUI mode
From: Brent Cook @ 2011-03-14 19:21 UTC (permalink / raw)
To: Skidmore, Donald C; +Cc: netdev@vger.kernel.org
In-Reply-To: <29F4ED941D916B48B88B4D2A4F3D1B9C01D30140EF@orsmsx509.amr.corp.intel.com>
On Friday 25 February 2011 12:11:11 Skidmore, Donald C wrote:
> >-----Original Message-----
> >From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> >On Behalf Of Brent Cook
> >Sent: Friday, February 25, 2011 8:12 AM
> >To: netdev@vger.kernel.org
> >Subject: Occasional link flap on Intel 82599 on boot in XAUI mode
> >
> >We have a custom system with dual 82599's in XAUI mode. One has its pair
> >of ports connected to a 10G switch, the other has its pair of ports
> >connected to an FPGA.
> >
> >Occasionally, on any of the interfaces, we will see the links flapping
> >up and down when the system initially boots. This happens maybe one in
> >20 boots.
> >
> >Feb 23 14:58:10 mfg kernel: [ 594.254977] ixgbe: eth1 NIC Link is Down
> >Feb 23 14:58:12 mfg kernel: [ 596.230039] ixgbe: eth1 NIC Link is Up 10
> >Gbps, Flow Control: RX/TX
> >Feb 23 14:58:12 mfg kernel: [ 596.256537] ixgbe: eth1 NIC Link is Down
> >Feb 23 14:58:16 mfg kernel: [ 600.228096] ixgbe: eth1 NIC Link is Up 10
> >Gbps, Flow Control: RX/TX
> >Feb 23 14:58:16 mfg kernel: [ 600.240135] ixgbe: eth1 NIC Link is Down
> >Feb 23 14:58:18 mfg kernel: [ 602.227047] ixgbe: eth1 NIC Link is Up 10
> >Gbps, Flow Control: RX/TX
> >
> >Simply forcing a down/up on the interface seems to correct the problem:
> >
> >ip link set eth1 down
> >ip link set eth1 up
> >
> >Is anyone else using XAUI mode and has seen this? Here is the kernel
> >information that we are using currently:
> >
>
> By any chance have you tried it with a newer driver? Latest Source Forge is 3.2.9.
>
> Also do you only see this link flap on boot, can you recreate the flap by unload and loading the ixgbe module?
>
We updated to the latest source forge driver and had the same behavior. Reloading the driver does not reproduce the issue.
It seems to be related to the remote XAUI peer coming out of reset after the 82599. The FPGAs on one pair of links are loaded in software at boot time, and the switch reset is also delayed slightly compared to the 82599. We fixed the issue by doing 'ethtool -r' once for each link at boot time.
Thanks
- Brent
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox