* Re: [patch net-next 2/3] netfilter: ip6_tables: use reasm skb for matching
From: Jiri Pirko @ 2013-11-06 15:29 UTC (permalink / raw)
To: Patrick McHardy
Cc: Florian Westphal, netdev, davem, pablo, netfilter-devel, yoshfuji,
kadlec, mleitner, kuznet, jmorris, wensong, horms, ja, edumazet,
pshelar, jasowang, alexander.h.duyck, coreteam
In-Reply-To: <20131106145104.GA18406@macbook.localnet>
Wed, Nov 06, 2013 at 03:51:04PM CET, kaber@trash.net wrote:
>On Wed, Nov 06, 2013 at 03:44:53PM +0100, Jiri Pirko wrote:
>> Wed, Nov 06, 2013 at 03:33:49PM CET, fw@strlen.de wrote:
>> >Jiri Pirko <jiri@resnulli.us> wrote:
>> >> >> >So if someone wants to change this, simply *only* pass the reassembled
>> >> >> >packet through the netfilter hooks and drop the fragments, as in IPv4.
>> >> >>
>> >> >> This is unfortunatelly not possible because in forwarding use case, the
>> >> >> fragments have to be send out as they come in.
>> >> >
>> >> >No, the IPv6 NAT patches fixed that, we still do proper refragmentation
>> >> >and we still respect the original fragment sizes, thus are not responsible
>> >> >for potentially exceeding the PMTU on the following path.
>> >>
>> >> Can you please point where this is done. Where the original fragment
>> >> sizes are stored and in which code are they restored? Thanks.
>> >
>> >Patrick is probably talking about
>> >
>> >commit 4cdd34084d539c758d00c5dc7bf95db2e4f2bc70
>> >(netfilter: nf_conntrack_ipv6: improve fragmentation handling)
>> >which introduces 'frag_max_size' in inet6_skb_parm struct.
>
>Indeed.
>
>> Thanks for the pointer. Interestingly though, according to my testing,
>> if reassembled packet would fit into outdev mtu, it is not fragmented
>> to the original frag size and it is send as single big packet. That is
>> I believe not correct.
>
>Hmm right, that wasn't the intention. We currently only use frag_max_size
>to send PKT_TOOBIG messages if the size exceeds the MTU, but not to trigger
>fragmentation itself.
>
>We probably need to handle this in ip6_finish_output(). Would you mind
>fixing this up as well in your patches?
I'm working on it atm.
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-06 15:06 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131106143927.GA21604@gondor.apana.org.au>
On Wed, 2013-11-06 at 22:39 +0800, Herbert Xu wrote:
> On Wed, Nov 06, 2013 at 09:30:45PM +0800, Herbert Xu wrote:
> >
> > In order to handle malicious GSO packets that is now possible with
> > the use of frag_list in virtio_net, we need to remove the BUG_ONs.
>
> OK Eric was right and I am a dumb ass. This has no chance in hell
> of handling the new virtio_net frag_list since we won't have any
> headers in the frag_list skbs.
>
> In fact, we never relied on the frag_list having headers anyway so
> it's not hard to fix this.
>
> Still totally untested but at least this has a chance of handling
> the new virtio_net.
OK I'll take a look at this and make tests today, thanks !
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-06 15:05 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton, mst
In-Reply-To: <20131106081638.GA17665@gondor.apana.org.au>
On Wed, 2013-11-06 at 16:16 +0800, Herbert Xu wrote:
> Instead of using perfectly sane 4K pages per frag to store guest to
> guest traffic, we now end up using 1.5K frags, which that's why you
> end up having to use the frag_list, WTF?
Sure, if your host has infinite amount of memory, we can remove all the
silly and expensive and bore some checks we added in the stack against
skb->truesize.
And yes, network stack will be faster.
But in real life, we have memory constraints.
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-06 15:01 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton, mst
In-Reply-To: <20131106131252.GA20680@gondor.apana.org.au>
On Wed, 2013-11-06 at 21:12 +0800, Herbert Xu wrote:
> I take that back. While the original patch was seriously broken,
> it has since been fixed by the coalescing patch that Jason Wang
> wrote.
>
Patch was not 'broken', but a step into the right direction. I am very
sorry if you think otherwise.
> It's still pretty weird to be dividing page frags into 1500-byte
> chunks and then merging back up to 4K but at least it should do the
> right thing now.
Have you thought about arches having PAGE_SIZE=65536, and how bad it is
to use a full page per network frame ? It is lazy and x86 centered.
So after our patches, we now have an optimal situation, even on these
arches.
On x86, a full 64KB GSO packet will now fit in 2 (or 3) frags allocated
in 32KB pages (order-3) in fast path (ie if there is not high memory
pressure)
According to our tests, performance is better on x86, and virtio_net now
is usable on arches with PAGE_SIZE=65536
^ permalink raw reply
* Re: [patch net-next 2/3] netfilter: ip6_tables: use reasm skb for matching
From: Patrick McHardy @ 2013-11-06 14:51 UTC (permalink / raw)
To: Jiri Pirko
Cc: Florian Westphal, netdev, davem, pablo, netfilter-devel, yoshfuji,
kadlec, mleitner, kuznet, jmorris, wensong, horms, ja, edumazet,
pshelar, jasowang, alexander.h.duyck, coreteam
In-Reply-To: <20131106144453.GD2458@minipsycho.orion>
On Wed, Nov 06, 2013 at 03:44:53PM +0100, Jiri Pirko wrote:
> Wed, Nov 06, 2013 at 03:33:49PM CET, fw@strlen.de wrote:
> >Jiri Pirko <jiri@resnulli.us> wrote:
> >> >> >So if someone wants to change this, simply *only* pass the reassembled
> >> >> >packet through the netfilter hooks and drop the fragments, as in IPv4.
> >> >>
> >> >> This is unfortunatelly not possible because in forwarding use case, the
> >> >> fragments have to be send out as they come in.
> >> >
> >> >No, the IPv6 NAT patches fixed that, we still do proper refragmentation
> >> >and we still respect the original fragment sizes, thus are not responsible
> >> >for potentially exceeding the PMTU on the following path.
> >>
> >> Can you please point where this is done. Where the original fragment
> >> sizes are stored and in which code are they restored? Thanks.
> >
> >Patrick is probably talking about
> >
> >commit 4cdd34084d539c758d00c5dc7bf95db2e4f2bc70
> >(netfilter: nf_conntrack_ipv6: improve fragmentation handling)
> >which introduces 'frag_max_size' in inet6_skb_parm struct.
Indeed.
> Thanks for the pointer. Interestingly though, according to my testing,
> if reassembled packet would fit into outdev mtu, it is not fragmented
> to the original frag size and it is send as single big packet. That is
> I believe not correct.
Hmm right, that wasn't the intention. We currently only use frag_max_size
to send PKT_TOOBIG messages if the size exceeds the MTU, but not to trigger
fragmentation itself.
We probably need to handle this in ip6_finish_output(). Would you mind
fixing this up as well in your patches?
^ permalink raw reply
* Re: [patch net-next 2/3] netfilter: ip6_tables: use reasm skb for matching
From: Jiri Pirko @ 2013-11-06 14:44 UTC (permalink / raw)
To: Florian Westphal
Cc: Patrick McHardy, netdev, davem, pablo, netfilter-devel, yoshfuji,
kadlec, mleitner, kuznet, jmorris, wensong, horms, ja, edumazet,
pshelar, jasowang, alexander.h.duyck, coreteam
In-Reply-To: <20131106143349.GF15370@breakpoint.cc>
Wed, Nov 06, 2013 at 03:33:49PM CET, fw@strlen.de wrote:
>Jiri Pirko <jiri@resnulli.us> wrote:
>> >> >So if someone wants to change this, simply *only* pass the reassembled
>> >> >packet through the netfilter hooks and drop the fragments, as in IPv4.
>> >>
>> >> This is unfortunatelly not possible because in forwarding use case, the
>> >> fragments have to be send out as they come in.
>> >
>> >No, the IPv6 NAT patches fixed that, we still do proper refragmentation
>> >and we still respect the original fragment sizes, thus are not responsible
>> >for potentially exceeding the PMTU on the following path.
>>
>> Can you please point where this is done. Where the original fragment
>> sizes are stored and in which code are they restored? Thanks.
>
>Patrick is probably talking about
>
>commit 4cdd34084d539c758d00c5dc7bf95db2e4f2bc70
>(netfilter: nf_conntrack_ipv6: improve fragmentation handling)
>which introduces 'frag_max_size' in inet6_skb_parm struct.
Thanks for the pointer. Interestingly though, according to my testing,
if reassembled packet would fit into outdev mtu, it is not fragmented
to the original frag size and it is send as single big packet. That is
I believe not correct.
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-06 14:39 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131106133045.GA20931@gondor.apana.org.au>
On Wed, Nov 06, 2013 at 09:30:45PM +0800, Herbert Xu wrote:
>
> In order to handle malicious GSO packets that is now possible with
> the use of frag_list in virtio_net, we need to remove the BUG_ONs.
OK Eric was right and I am a dumb ass. This has no chance in hell
of handling the new virtio_net frag_list since we won't have any
headers in the frag_list skbs.
In fact, we never relied on the frag_list having headers anyway so
it's not hard to fix this.
Still totally untested but at least this has a chance of handling
the new virtio_net.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3735fad..3e8819c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2816,8 +2816,6 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
hsize = len;
if (!hsize && i >= nfrags) {
- BUG_ON(fskb->len != len);
-
pos += len;
nskb = skb_clone(fskb, GFP_ATOMIC);
fskb = fskb->next;
@@ -2846,12 +2844,6 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
__skb_put(nskb, doffset);
}
- if (segs)
- tail->next = nskb;
- else
- segs = nskb;
- tail = nskb;
-
__copy_skb_header(nskb, skb);
nskb->mac_len = skb->mac_len;
@@ -2861,15 +2853,62 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
nskb->data - tnl_hlen,
doffset + tnl_hlen);
- if (fskb != skb_shinfo(skb)->frag_list)
- goto perform_csum_check;
+ if (fskb != skb_shinfo(skb)->frag_list) {
+ struct sk_buff *nsegs;
+
+ if (nskb->len == len + doffset)
+ goto perform_csum_check;
+
+ if (skb_has_frag_list(nskb)) {
+ net_warn_ratelimited(
+ "skb_segment: "
+ "nested frag_list detected");
+ kfree(nskb);
+ err = -EINVAL;
+ goto err;
+ }
+
+ __skb_pull(nskb, doffset);
+ skb_shinfo(nskb)->gso_size = mss;
+ nsegs = skb_segment(nskb, features);
+
+ err = PTR_ERR(nsegs);
+ if (IS_ERR(nsegs)) {
+ kfree(nskb);
+ goto err;
+ }
+ err = -ENOMEM;
+
+ if (segs)
+ tail->next = nsegs;
+ else
+ segs = nsegs;
+
+ tail = nsegs;
+ while (tail->next)
+ tail = tail->next;
+
+ if (fskb && tail->len != len) {
+ net_warn_ratelimited(
+ "skb_segment: "
+ "illegal GSO fragment: %u %u",
+ tail->len, len);
+ kfree(nskb);
+ err = -EINVAL;
+ goto err;
+ }
+
+ len = nskb->len;
+ kfree(nskb);
+ continue;
+ }
if (!sg) {
nskb->ip_summed = CHECKSUM_NONE;
nskb->csum = skb_copy_and_csum_bits(skb, offset,
skb_put(nskb, len),
len, 0);
- continue;
+ goto add_to_segs;
}
frag = skb_shinfo(nskb)->frags;
@@ -2905,15 +2944,25 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
if (pos < offset + len) {
struct sk_buff *fskb2 = fskb;
- BUG_ON(pos + fskb->len != offset + len);
+ if (pos + fskb->len != offset + len) {
+ net_warn_ratelimited(
+ "skb_segment: "
+ "illegal GSO trailer: %u %u",
+ pos + fskb->len, offset + len);
+ kfree(nskb);
+ err = -EINVAL;
+ goto err;
+ }
pos += fskb->len;
fskb = fskb->next;
if (fskb2->next) {
fskb2 = skb_clone(fskb2, GFP_ATOMIC);
- if (!fskb2)
+ if (!fskb2) {
+ kfree(nskb);
goto err;
+ }
} else
skb_get(fskb2);
@@ -2932,6 +2981,13 @@ perform_csum_check:
nskb->len - doffset, 0);
nskb->ip_summed = CHECKSUM_NONE;
}
+
+add_to_segs:
+ if (segs)
+ tail->next = nskb;
+ else
+ segs = nskb;
+ tail = nskb;
} while ((offset += len) < skb->len);
return segs;
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [patch net-next 2/3] netfilter: ip6_tables: use reasm skb for matching
From: Florian Westphal @ 2013-11-06 14:33 UTC (permalink / raw)
To: Jiri Pirko
Cc: Patrick McHardy, Florian Westphal, netdev, davem, pablo,
netfilter-devel, yoshfuji, kadlec, mleitner, kuznet, jmorris,
wensong, horms, ja, edumazet, pshelar, jasowang,
alexander.h.duyck, coreteam
In-Reply-To: <20131106141845.GC2458@minipsycho.orion>
Jiri Pirko <jiri@resnulli.us> wrote:
> >> >So if someone wants to change this, simply *only* pass the reassembled
> >> >packet through the netfilter hooks and drop the fragments, as in IPv4.
> >>
> >> This is unfortunatelly not possible because in forwarding use case, the
> >> fragments have to be send out as they come in.
> >
> >No, the IPv6 NAT patches fixed that, we still do proper refragmentation
> >and we still respect the original fragment sizes, thus are not responsible
> >for potentially exceeding the PMTU on the following path.
>
> Can you please point where this is done. Where the original fragment
> sizes are stored and in which code are they restored? Thanks.
Patrick is probably talking about
commit 4cdd34084d539c758d00c5dc7bf95db2e4f2bc70
(netfilter: nf_conntrack_ipv6: improve fragmentation handling)
which introduces 'frag_max_size' in inet6_skb_parm struct.
^ permalink raw reply
* Re: [patch net-next 2/3] netfilter: ip6_tables: use reasm skb for matching
From: Jiri Pirko @ 2013-11-06 14:18 UTC (permalink / raw)
To: Patrick McHardy
Cc: Florian Westphal, netdev, davem, pablo, netfilter-devel, yoshfuji,
kadlec, mleitner, kuznet, jmorris, wensong, horms, ja, edumazet,
pshelar, jasowang, alexander.h.duyck, coreteam
In-Reply-To: <20131105181633.GA7435@macbook.localnet>
>> >So if someone wants to change this, simply *only* pass the reassembled
>> >packet through the netfilter hooks and drop the fragments, as in IPv4.
>>
>> This is unfortunatelly not possible because in forwarding use case, the
>> fragments have to be send out as they come in.
>
>No, the IPv6 NAT patches fixed that, we still do proper refragmentation
>and we still respect the original fragment sizes, thus are not responsible
>for potentially exceeding the PMTU on the following path.
Can you please point where this is done. Where the original fragment
sizes are stored and in which code are they restored? Thanks.
^ permalink raw reply
* Re: Bluetooth 6LoWPAN and routing
From: Claudio Takahasi @ 2013-11-06 14:17 UTC (permalink / raw)
To: Alexander Aring; +Cc: Jukka Rissanen, netdev
In-Reply-To: <20131105085555.GA32080@omega>
Hi Jukka/Alexander:
On Tue, Nov 5, 2013 at 5:55 AM, Alexander Aring <alex.aring@gmail.com> wrote:
> On Tue, Nov 05, 2013 at 10:36:29AM +0200, Jukka Rissanen wrote:
>> Hi Claudio,
>>
>> On ma, 2013-11-04 at 19:46 -0200, Claudio Takahasi wrote:
>> > Hi Jukka,
>> >
>> > On Thu, Oct 24, 2013 at 9:48 AM, Jukka Rissanen
>> > <jukka.rissanen@linux.intel.com> wrote:
>> > > Hi Alexander,
>> > >
>> > >
>> > > On 24.10.2013 15:25, Alexander Aring wrote:
>> > >>
>> > >> Hi Jukka,
>> > >>
>> > >> On Thu, Oct 24, 2013 at 09:45:40AM +0300, Jukka Rissanen wrote:
>> > >>>
>> > >>> Hi,
>> > >>>
>> > >>> I have been prototyping with BT 6LoWPAN support (using this draft
>> > >>> http://tools.ietf.org/html/draft-ietf-6lowpan-btle-12 as a
>> > >>> reference). I sent first version yesterday to linux-bluetooth ml
>> > >>> http://thread.gmane.org/gmane.linux.bluez.kernel/39394
>> > >>>
>> > >> I see you take many code from the 6lowpan ieee802154 implementation.
>> > >> (Just notice you drop the original authors from there)
>> > >
>> > >
>> > > Hmm, those got dropped, I am sorry about that. I will add the original
>> > > authors information of course.
>> > >
>> > >
>> > >>
>> > >> I have a couple of patches to fix a lot of bugs in the current 6LoWPAN
>> > >> ieee802154 implementation.
>> > >>
>> > >> Some bugs which I found:
>> > >>
>> > >> - Fix race conditions in fragmentation handling
>> > >> - Fix UDP compression/uncompressionm, which is completly broken
>> > >> - Fragmentation handling isn't rfc4944 compatible
>> > >>
>> > >> And some other improvements. I see your rfc has the same issues (e.g.
>> > >> fragmentation race conditions).
>> > >>
>> > >> Currently I preparing these patches for mainlining.
>> > >
>> > >
>> > > Excellent news!
>> >
>> >
>> > Is it necessary to implement 6loWPAN fragmentation/reassembling? For
>> > Bluetooth, I thought L2CAP FAR (Fragmentation and Reassembling) could
>> > handle the transfer of IPv6 packets that doesn't fit in one single
>> > BTLE PDU.
>>
>> Yes, the Bluetooth 6lowpan code I sent handles the L2CAP FAR already.
>> The question is who handles the IPv6 packets that are larger than
>> IPV6_MIN_MTU (1280 bytes), or is that automatically done by other parts
> ahh I understand now, yes that's handled by the normal IPv6 Fragmentation.
> But you don't want a high payload like this. ;)
>
> - Alex
I still not convinced that we need to worry about 6loWPAN
fragmentation now. Use fixed channel is only a way to implement the
initial 6loWPAN support, (if necessary) you can push the fragmentation
code later, it will make your upstreaming work easier.
Maybe you read this Bluetooth SIG draft already: IPSS (Internet
Protocol Support Service). It contains some infos about fixed vs
dynamic channels, MTU and flow control.
Regards,
Claudio
^ permalink raw reply
* Re: [net-next PATCH v3 2/2] ixgbe: enable l2 forwarding acceleration for macvlans
From: Neil Horman @ 2013-11-06 13:59 UTC (permalink / raw)
To: John Fastabend
Cc: alexander.h.duyck, andy, davem, netdev, jeffrey.t.kirsher,
vyasevich
In-Reply-To: <20131105233017.5400.91554.stgit@jf-dev1-dcblab>
On Tue, Nov 05, 2013 at 03:30:17PM -0800, John Fastabend wrote:
> Now that l2 acceleration ops are in place from the prior patch,
> enable ixgbe to take advantage of these operations. Allow it to
> allocate queues for a macvlan so that when we transmit a frame,
> we can do the switching in hardware inside the ixgbe card, rather
> than in software.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: "David S. Miller" <davem@davemloft.net>
> ---
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH net-next 1/2] bonding: extend round-robin mode with packets_per_slave
From: Veaceslav Falico @ 2013-11-06 13:47 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, davem, andy, fubar
In-Reply-To: <1383655902-18744-2-git-send-email-nikolay@redhat.com>
On Tue, Nov 05, 2013 at 01:51:41PM +0100, Nikolay Aleksandrov wrote:
>This patch aims to extend round-robin mode with a new option called
>packets_per_slave which can have the following values and effects:
>0 - choose a random slave
>1 (default) - standard round-robin, 1 packet per slave
> >1 - round-robin when >1 packets have been transmitted per slave
>The allowed values are between 0 and 65535.
>This patch also fixes the comment style in bond_xmit_roundrobin().
>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Clean and simple, thanks a lot!
Acked-by: Veaceslav Falico <vfalico@redhat.com>
>---
> drivers/net/bonding/bond_main.c | 55 ++++++++++++++++++++++++++++++++++++----
> drivers/net/bonding/bond_sysfs.c | 49 +++++++++++++++++++++++++++++++++++
> drivers/net/bonding/bonding.h | 3 ++-
> 3 files changed, 101 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index a141f40..4dd5ee2 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -79,6 +79,7 @@
> #include <net/pkt_sched.h>
> #include <linux/rculist.h>
> #include <net/flow_keys.h>
>+#include <linux/reciprocal_div.h>
> #include "bonding.h"
> #include "bond_3ad.h"
> #include "bond_alb.h"
>@@ -111,6 +112,7 @@ static char *fail_over_mac;
> static int all_slaves_active;
> static struct bond_params bonding_defaults;
> static int resend_igmp = BOND_DEFAULT_RESEND_IGMP;
>+static int packets_per_slave = 1;
>
> module_param(max_bonds, int, 0);
> MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
>@@ -183,6 +185,10 @@ MODULE_PARM_DESC(all_slaves_active, "Keep all frames received on an interface"
> module_param(resend_igmp, int, 0);
> MODULE_PARM_DESC(resend_igmp, "Number of IGMP membership reports to send on "
> "link failure");
>+module_param(packets_per_slave, int, 0);
>+MODULE_PARM_DESC(packets_per_slave, "Packets to send per slave in balance-rr "
>+ "mode; 0 for a random slave, 1 packet per "
>+ "slave (default), >1 packets per slave.");
>
> /*----------------------------- Global variables ----------------------------*/
>
>@@ -3574,14 +3580,44 @@ void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id)
> kfree_skb(skb);
> }
>
>+/**
>+ * bond_rr_gen_slave_id - generate slave id based on packets_per_slave
>+ * @bond: bonding device to use
>+ *
>+ * Based on the value of the bonding device's packets_per_slave parameter
>+ * this function generates a slave id, which is usually used as the next
>+ * slave to transmit through.
>+ */
>+static u32 bond_rr_gen_slave_id(struct bonding *bond)
>+{
>+ int packets_per_slave = bond->params.packets_per_slave;
>+ u32 slave_id;
>+
>+ switch (packets_per_slave) {
>+ case 0:
>+ slave_id = prandom_u32();
>+ break;
>+ case 1:
>+ slave_id = bond->rr_tx_counter;
>+ break;
>+ default:
>+ slave_id = reciprocal_divide(bond->rr_tx_counter,
>+ packets_per_slave);
>+ break;
>+ }
>+ bond->rr_tx_counter++;
>+
>+ return slave_id;
>+}
>+
> static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev)
> {
> struct bonding *bond = netdev_priv(bond_dev);
> struct iphdr *iph = ip_hdr(skb);
> struct slave *slave;
>+ u32 slave_id;
>
>- /*
>- * Start with the curr_active_slave that joined the bond as the
>+ /* Start with the curr_active_slave that joined the bond as the
> * default for sending IGMP traffic. For failover purposes one
> * needs to maintain some consistency for the interface that will
> * send the join/membership reports. The curr_active_slave found
>@@ -3594,8 +3630,8 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
> else
> bond_xmit_slave_id(bond, skb, 0);
> } else {
>- bond_xmit_slave_id(bond, skb,
>- bond->rr_tx_counter++ % bond->slave_cnt);
>+ slave_id = bond_rr_gen_slave_id(bond);
>+ bond_xmit_slave_id(bond, skb, slave_id % bond->slave_cnt);
> }
>
> return NETDEV_TX_OK;
>@@ -4099,6 +4135,12 @@ static int bond_check_params(struct bond_params *params)
> resend_igmp = BOND_DEFAULT_RESEND_IGMP;
> }
>
>+ if (packets_per_slave < 0 || packets_per_slave > USHRT_MAX) {
>+ pr_warn("Warning: packets_per_slave (%d) should be between 0 and %u resetting to 1\n",
>+ packets_per_slave, USHRT_MAX);
>+ packets_per_slave = 1;
>+ }
>+
> /* reset values for TLB/ALB */
> if ((bond_mode == BOND_MODE_TLB) ||
> (bond_mode == BOND_MODE_ALB)) {
>@@ -4288,7 +4330,10 @@ static int bond_check_params(struct bond_params *params)
> params->resend_igmp = resend_igmp;
> params->min_links = min_links;
> params->lp_interval = BOND_ALB_DEFAULT_LP_INTERVAL;
>-
>+ if (packets_per_slave > 1)
>+ params->packets_per_slave = reciprocal_value(packets_per_slave);
>+ else
>+ params->packets_per_slave = packets_per_slave;
> if (primary) {
> strncpy(params->primary, primary, IFNAMSIZ);
> params->primary[IFNAMSIZ - 1] = 0;
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 47749c9..75dc4d0 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -40,6 +40,7 @@
> #include <net/net_namespace.h>
> #include <net/netns/generic.h>
> #include <linux/nsproxy.h>
>+#include <linux/reciprocal_div.h>
>
> #include "bonding.h"
>
>@@ -1640,6 +1641,53 @@ out:
> static DEVICE_ATTR(lp_interval, S_IRUGO | S_IWUSR,
> bonding_show_lp_interval, bonding_store_lp_interval);
>
>+static ssize_t bonding_show_packets_per_slave(struct device *d,
>+ struct device_attribute *attr,
>+ char *buf)
>+{
>+ struct bonding *bond = to_bond(d);
>+ int packets_per_slave = bond->params.packets_per_slave;
>+
>+ if (packets_per_slave > 1)
>+ packets_per_slave = reciprocal_value(packets_per_slave);
>+
>+ return sprintf(buf, "%d\n", packets_per_slave);
>+}
>+
>+static ssize_t bonding_store_packets_per_slave(struct device *d,
>+ struct device_attribute *attr,
>+ const char *buf, size_t count)
>+{
>+ struct bonding *bond = to_bond(d);
>+ int new_value, ret = count;
>+
>+ if (sscanf(buf, "%d", &new_value) != 1) {
>+ pr_err("%s: no packets_per_slave value specified.\n",
>+ bond->dev->name);
>+ ret = -EINVAL;
>+ goto out;
>+ }
>+ if (new_value < 0 || new_value > USHRT_MAX) {
>+ pr_err("%s: packets_per_slave must be between 0 and %u\n",
>+ bond->dev->name, USHRT_MAX);
>+ ret = -EINVAL;
>+ goto out;
>+ }
>+ if (bond->params.mode != BOND_MODE_ROUNDROBIN)
>+ pr_warn("%s: Warning: packets_per_slave has effect only in balance-rr mode\n",
>+ bond->dev->name);
>+ if (new_value > 1)
>+ bond->params.packets_per_slave = reciprocal_value(new_value);
>+ else
>+ bond->params.packets_per_slave = new_value;
>+out:
>+ return ret;
>+}
>+
>+static DEVICE_ATTR(packets_per_slave, S_IRUGO | S_IWUSR,
>+ bonding_show_packets_per_slave,
>+ bonding_store_packets_per_slave);
>+
> static struct attribute *per_bond_attrs[] = {
> &dev_attr_slaves.attr,
> &dev_attr_mode.attr,
>@@ -1671,6 +1719,7 @@ static struct attribute *per_bond_attrs[] = {
> &dev_attr_resend_igmp.attr,
> &dev_attr_min_links.attr,
> &dev_attr_lp_interval.attr,
>+ &dev_attr_packets_per_slave.attr,
> NULL,
> };
>
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 046a605..77a07a1 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -156,6 +156,7 @@ struct bond_params {
> int all_slaves_active;
> int resend_igmp;
> int lp_interval;
>+ int packets_per_slave;
> };
>
> struct bond_parm_tbl {
>@@ -222,7 +223,7 @@ struct bonding {
> char proc_file_name[IFNAMSIZ];
> #endif /* CONFIG_PROC_FS */
> struct list_head bond_list;
>- u16 rr_tx_counter;
>+ u32 rr_tx_counter;
> struct ad_bond_info ad_info;
> struct alb_bond_info alb_info;
> struct bond_params params;
>--
>1.8.1.4
>
^ permalink raw reply
* [PATCH net-next 2/2] RDMA/cma: Set IBoE SL (user-priority) by egress map when using vlans
From: Or Gerlitz @ 2013-11-06 13:37 UTC (permalink / raw)
To: davem; +Cc: netdev, amirv, sean.hefty, eyalpe, Or Gerlitz
In-Reply-To: <1383745044-32575-1-git-send-email-ogerlitz@mellanox.com>
From: Eyal Perry <eyalpe@mellanox.com>
On top of commit 366cddb40 "IB/rdma_cm: TOS <=> UP mapping for IBoE", add
support for case vlan egress map is used.
When the IBoE session is being set over a vlan, inherit the socket priority
to vlan priority mapping which was configured for the vlan device egress map.
Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/infiniband/core/cma.c | 26 +++++++++++++++++++++-----
1 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index a082fd9..d2172e7 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1848,6 +1848,26 @@ static int cma_resolve_iw_route(struct rdma_id_private *id_priv, int timeout_ms)
return 0;
}
+static int iboe_tos_to_sl(struct net_device *ndev, int tos)
+{
+ int prio;
+ struct net_device *dev;
+
+ prio = rt_tos2priority(tos);
+ dev = ndev->priv_flags & IFF_802_1Q_VLAN ?
+ vlan_dev_real_dev(ndev) : ndev;
+
+ if (dev->num_tc)
+ return netdev_get_prio_tc_map(dev, prio);
+
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
+ if (ndev->priv_flags & IFF_802_1Q_VLAN)
+ return (vlan_dev_get_egress_qos_mask(ndev, prio) &
+ VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+#endif
+ return 0;
+}
+
static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
{
struct rdma_route *route = &id_priv->id.route;
@@ -1888,11 +1908,7 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
route->path_rec->reversible = 1;
route->path_rec->pkey = cpu_to_be16(0xffff);
route->path_rec->mtu_selector = IB_SA_EQ;
- route->path_rec->sl = netdev_get_prio_tc_map(
- ndev->priv_flags & IFF_802_1Q_VLAN ?
- vlan_dev_real_dev(ndev) : ndev,
- rt_tos2priority(id_priv->tos));
-
+ route->path_rec->sl = iboe_tos_to_sl(ndev, id_priv->tos);
route->path_rec->mtu = iboe_get_mtu(ndev->mtu);
route->path_rec->rate_selector = IB_SA_EQ;
route->path_rec->rate = iboe_get_rate(ndev);
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 1/2] net/vlan: Provide read access to the vlan egress map
From: Or Gerlitz @ 2013-11-06 13:37 UTC (permalink / raw)
To: davem; +Cc: netdev, amirv, sean.hefty, eyalpe, Hadar Hen Zion
In-Reply-To: <1383745044-32575-1-git-send-email-ogerlitz@mellanox.com>
From: Eyal Perry <eyalpe@mellanox.com>
Provide a method for read-only access to the vlan device egress mapping.
Do this by refactoring vlan_dev_get_egress_qos_mask() such that now it
receives as an argument the skb priority instead of pointer to the skb.
Such an access is needed for the IBoE stack where the control plane
goes through the network stack. This is an add-on step on top of commit
d4a968658c "net/route: export symbol ip_tos2prio" which allowed the RDMA-CM
to use ip_tos2prio.
Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
---
include/linux/if_vlan.h | 9 ++++++++-
net/8021q/vlan_dev.c | 18 ++++++++++++------
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 715c343..f3088a0 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -88,7 +88,8 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
__be16 vlan_proto, u16 vlan_id);
extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
extern u16 vlan_dev_vlan_id(const struct net_device *dev);
-
+extern u16 vlan_dev_get_egress_qos_mask(struct net_device *dev,
+ u32 skprio);
extern bool vlan_do_receive(struct sk_buff **skb);
extern struct sk_buff *vlan_untag(struct sk_buff *skb);
@@ -121,6 +122,12 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
return 0;
}
+static inline u16 vlan_dev_get_egress_qos_mask(struct net_device *dev,
+ u32 skprio)
+{
+ return 0;
+}
+
static inline bool vlan_do_receive(struct sk_buff **skb)
{
return false;
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 09bf1c3..13904a4 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -69,15 +69,15 @@ static int vlan_dev_rebuild_header(struct sk_buff *skb)
}
static inline u16
-vlan_dev_get_egress_qos_mask(struct net_device *dev, struct sk_buff *skb)
+__vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
{
struct vlan_priority_tci_mapping *mp;
smp_rmb(); /* coupled with smp_wmb() in vlan_dev_set_egress_priority() */
- mp = vlan_dev_priv(dev)->egress_priority_map[(skb->priority & 0xF)];
+ mp = vlan_dev_priv(dev)->egress_priority_map[(skprio & 0xF)];
while (mp) {
- if (mp->priority == skb->priority) {
+ if (mp->priority == skprio) {
return mp->vlan_qos; /* This should already be shifted
* to mask correctly with the
* VLAN's TCI */
@@ -87,6 +87,12 @@ vlan_dev_get_egress_qos_mask(struct net_device *dev, struct sk_buff *skb)
return 0;
}
+u16 vlan_dev_get_egress_qos_mask(struct net_device *dev, u32 skprio)
+{
+ return __vlan_dev_get_egress_qos_mask(dev, skprio);
+}
+EXPORT_SYMBOL(vlan_dev_get_egress_qos_mask);
+
/*
* Create the VLAN header for an arbitrary protocol layer
*
@@ -111,7 +117,7 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
vhdr = (struct vlan_hdr *) skb_push(skb, VLAN_HLEN);
vlan_tci = vlan->vlan_id;
- vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb);
+ vlan_tci |= __vlan_dev_get_egress_qos_mask(dev, skb->priority);
vhdr->h_vlan_TCI = htons(vlan_tci);
/*
@@ -168,7 +174,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
vlan->flags & VLAN_FLAG_REORDER_HDR) {
u16 vlan_tci;
vlan_tci = vlan->vlan_id;
- vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb);
+ vlan_tci |= __vlan_dev_get_egress_qos_mask(dev, skb->priority);
skb = __vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
}
@@ -253,7 +259,7 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
np->vlan_qos = vlan_qos;
/* Before inserting this element in hash table, make sure all its fields
* are committed to memory.
- * coupled with smp_rmb() in vlan_dev_get_egress_qos_mask()
+ * coupled with smp_rmb() in __vlan_dev_get_egress_qos_mask()
*/
smp_wmb();
vlan->egress_priority_map[skb_prio & 0xF] = np;
--
1.7.1
^ permalink raw reply related
* [PATCH] net: calxedaxgmac: Fix panic caused by MTU change of active interface
From: Andreas Herrmann @ 2013-11-06 13:31 UTC (permalink / raw)
To: Rob Herring; +Cc: Grant Likely, David S. Miller, netdev
Changing MTU size of an xgmac network interface while it is active can
cause a panic like
skbuff: skb_over_panic: text:c03bc62c len:1090 put:1090 head:edfb6900 data:edfb6942 tail:0xedfb6d84 end:0xedfb6bc0 dev:eth0
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:126!
Internal error: Oops - BUG: 0 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 762 Comm: python Tainted: G W 3.10.0-00015-g3e33cd7 #309
task: edcfe000 ti: ed67e000 task.ti: ed67e000
PC is at skb_panic+0x64/0x70
LR is at wake_up_klogd+0x5c/0x68
This happens because xgmac_change_mtu modifies dev->mtu before the
network interface is quiesced. And thus there still might be buffers
in use which have a buffer size based on the old MTU.
To fix this I moved the change of dev->mtu after the call to
xgmac_stop.
Another modification is required (in xgmac_stop) to ensure that
xgmac_xmit is really not called anymore (xgmac_tx_complete might wake
up the queue again).
I've tested the fix by switching MTU size every second between 600 and
1500 while network traffic was going on. The test box survived a test
of several hours (until I've stopped it) whereas w/o this fix above
panic occurs after several minutes (at most).
Signed-off-by: Andreas Herrmann <andreas.herrmann@calxeda.com>
---
drivers/net/ethernet/calxeda/xgmac.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
Patch is against v3.12-48-gbe408cd.
Regards,
Andreas
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 48f5288..8eb422a 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -1067,6 +1067,10 @@ static int xgmac_stop(struct net_device *dev)
writel(0, priv->base + XGMAC_DMA_INTR_ENA);
+ netif_tx_lock_bh(dev);
+ netif_stop_queue(dev);
+ netif_tx_unlock_bh(dev);
+
/* Disable the MAC core */
xgmac_mac_disable(priv->base);
@@ -1370,11 +1374,8 @@ static int xgmac_change_mtu(struct net_device *dev, int new_mtu)
}
old_mtu = dev->mtu;
- dev->mtu = new_mtu;
/* return early if the buffer sizes will not change */
- if (old_mtu <= ETH_DATA_LEN && new_mtu <= ETH_DATA_LEN)
- return 0;
if (old_mtu == new_mtu)
return 0;
@@ -1382,8 +1383,9 @@ static int xgmac_change_mtu(struct net_device *dev, int new_mtu)
if (!netif_running(dev))
return 0;
- /* Bring the interface down and then back up */
+ /* Bring interface down, change mtu and bring interface back up */
xgmac_stop(dev);
+ dev->mtu = new_mtu;
return xgmac_open(dev);
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next 0/2] Add accessor to vlan egress map
From: Or Gerlitz @ 2013-11-06 13:37 UTC (permalink / raw)
To: davem; +Cc: netdev, amirv, sean.hefty, eyalpe, Or Gerlitz
Hi Dave, Sean,
This series adds on the work done through commits d4a968658c "net/route: export
symbol ip_tos2prio" and 366cddb40 "IB/rdma_cm: TOS <=> UP mapping for IBoE".
When vlans are used, the IBoE stack needs to inherit the egress map
which is configured on the vlan device. For that end, we provide a
read-only access method and call it from the RDMA-CM.
Or.
Eyal Perry (2):
net/vlan: Provide access to the vlan egress map
RDMA/cma: Set IBoE SL (user-priority) by egress map when using vlans
drivers/infiniband/core/cma.c | 26 +++++++++++++++++++++-----
include/linux/if_vlan.h | 9 ++++++++-
net/8021q/vlan_dev.c | 18 ++++++++++++------
3 files changed, 41 insertions(+), 12 deletions(-)
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-06 13:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131106123900.GA20259@gondor.apana.org.au>
On Wed, Nov 06, 2013 at 08:39:00PM +0800, Herbert Xu wrote:
>
> That patch obviously didn't have a chance of working since I missed
> a continue.
>
> Here is a better version.
In order to handle malicious GSO packets that is now possible with
the use of frag_list in virtio_net, we need to remove the BUG_ONs.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3735fad..f336e5c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2816,7 +2816,44 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
hsize = len;
if (!hsize && i >= nfrags) {
- BUG_ON(fskb->len != len);
+ if (fskb->len != len) {
+ if (skb_has_frag_list(fskb)) {
+ net_warn_ratelimited(
+ "skb_segment: "
+ "nested frag_list detected");
+ err = -EINVAL;
+ goto err;
+ }
+
+ nskb = skb_segment(fskb, features);
+
+ err = PTR_ERR(nskb);
+ if (IS_ERR(nskb))
+ goto err;
+ err = -ENOMEM;
+
+ if (segs)
+ tail->next = nskb;
+ else
+ segs = nskb;
+
+ tail = nskb;
+ while (tail->next)
+ tail = tail->next;
+
+ if (fskb->next && tail->len != len) {
+ net_warn_ratelimited(
+ "skb_segment: "
+ "illegal GSO fragment: %u %u",
+ tail->len, len);
+ err = -EINVAL;
+ goto err;
+ }
+
+ len = fskb->len;
+ fskb = fskb->next;
+ continue;
+ }
pos += len;
nskb = skb_clone(fskb, GFP_ATOMIC);
@@ -2905,7 +2942,14 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
if (pos < offset + len) {
struct sk_buff *fskb2 = fskb;
- BUG_ON(pos + fskb->len != offset + len);
+ if (pos + fskb->len != offset + len) {
+ net_warn_ratelimited(
+ "skb_segment: "
+ "illegal GSO trailer: %u %u",
+ pos + fskb->len, offset + len);
+ err = -EINVAL;
+ goto err;
+ }
pos += fskb->len;
fskb = fskb->next;
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-06 13:12 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton, mst
In-Reply-To: <20131106081638.GA17665@gondor.apana.org.au>
On Wed, Nov 06, 2013 at 04:16:38PM +0800, Herbert Xu wrote:
>
> I just looked at the aformentioned patch again and it is seriously
> broken! How on earth did it get merged?
>
> Instead of using perfectly sane 4K pages per frag to store guest to
> guest traffic, we now end up using 1.5K frags, which that's why you
> end up having to use the frag_list, WTF?
>
> Dave, please revert the above commit as it is seriously broken.
I take that back. While the original patch was seriously broken,
it has since been fixed by the coalescing patch that Jason Wang
wrote.
It's still pretty weird to be dividing page frags into 1500-byte
chunks and then merging back up to 4K but at least it should do the
right thing now.
With regards to the impact on my skb_segment patch, there should be
none because those packets ultimately are still originating from
either GRO or the TCP stack. In which case the same assumptions
on frag_list still holds.
However, we do have to guard against evil hosts/guests injecting
bogus packets into our stack, so either we'll have to lose the
BUG_ON in skb_segment or we'll need some sort of a filter in
virtio_net.
Perhaps a rate-limited printk might be the go.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH net] tg3: avoid double-freeing of rx data memory
From: Ivan Vecera @ 2013-11-06 13:02 UTC (permalink / raw)
To: netdev; +Cc: davem, Nithin Nayak Sujir, Michael Chan
If build_skb fails the memory associated with the ring buffer is freed but
the ri->data member is not zeroed in this case. This causes a double-free
of this memory in tg3_free_rings->... path. The patch moves this block after
setting ri->data to NULL.
It would be nice to fix this bug also in stable >= v3.4 trees.
Cc: Nithin Nayak Sujir <nsujir@broadcom.com>
Cc: Michael Chan <mchan@broadcom.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/net/ethernet/broadcom/tg3.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 12d961c..cd76d2a 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -6848,12 +6848,6 @@ static int tg3_rx(struct tg3_napi *tnapi, int budget)
pci_unmap_single(tp->pdev, dma_addr, skb_size,
PCI_DMA_FROMDEVICE);
- skb = build_skb(data, frag_size);
- if (!skb) {
- tg3_frag_free(frag_size != 0, data);
- goto drop_it_no_recycle;
- }
- skb_reserve(skb, TG3_RX_OFFSET(tp));
/* Ensure that the update to the data happens
* after the usage of the old DMA mapping.
*/
@@ -6861,6 +6855,12 @@ static int tg3_rx(struct tg3_napi *tnapi, int budget)
ri->data = NULL;
+ skb = build_skb(data, frag_size);
+ if (!skb) {
+ tg3_frag_free(frag_size != 0, data);
+ goto drop_it_no_recycle;
+ }
+ skb_reserve(skb, TG3_RX_OFFSET(tp));
} else {
tg3_recycle_rx(tnapi, tpr, opaque_key,
desc_idx, *post_ptr);
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH net-next 3/3] net: allow to leave the buffer fragmented in skb_cow_data()
From: Herbert Xu @ 2013-11-06 12:48 UTC (permalink / raw)
To: Mathias Krause
Cc: David S. Miller, Steffen Klassert, Dmitry Tarnyagin, netdev
In-Reply-To: <527A391B.4050907@secunet.com>
On Wed, Nov 06, 2013 at 01:42:03PM +0100, Mathias Krause wrote:
>
> Well, skb_cow_data() will only copy, i.e. call __pskb_pull_tail(), in
> case the skb is either cloned or fragmented. As you already said it
> won't be cloned in your case. Does it contain fragments, i.e. is
> skb_shinfo(skb)->nr_frags != 0? If not, we won't copy with the current
> code either.
Whenever we say page it means nr_frags != 0. So currently as
long as we have pages in our skb we will copy. With your patch
we will no longer copy in the case where we have pages but the
skb isn't cloned. In fact that is the whole point of your patch.
> Can you please explain why this would be needed? I still don't get the
> reasoning behind "pages are considered not writable at the moment even
> if they are anonymous".
As I said you don't know where the page in the skb came from. It
may point to read-only memory or memory that's shared with another
task that isn't expecting things to change underneath it.
It may well turn out to most if not all cases of pages are safe to
be written to if skb_cloned == 0. However, we'd need to do a full
audit of every source of page frags to be sure. For example, you'd
need to look at net drivers and splice.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: allow to leave the buffer fragmented in skb_cow_data()
From: Mathias Krause @ 2013-11-06 12:42 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, Steffen Klassert, Dmitry Tarnyagin, netdev
In-Reply-To: <20131106095217.GA18851@gondor.apana.org.au>
On 06.11.2013 10:52, Herbert Xu wrote:
> On Wed, Nov 06, 2013 at 10:49:18AM +0100, Mathias Krause wrote:
>> On 06.11.2013 10:30, Herbert Xu wrote:
>>
>>> Hang on, you haven't explained why it is OK to write to pages.
>> Why wouldn't it if the skb isn't cloned?
>
> Because if it's owned by an entity outside our stack (e.g., virt
> host or app) then the skb itself won't be cloned.
Ok.
>>> What if said page is owned by the virt host or some app?
>> How would one detect such a case. I could image not by testing
>> skb_shinfo(skb)->nr_frags as it is right now?
>
> You can't. That's why we always copy.
Well, skb_cow_data() will only copy, i.e. call __pskb_pull_tail(), in
case the skb is either cloned or fragmented. As you already said it
won't be cloned in your case. Does it contain fragments, i.e. is
skb_shinfo(skb)->nr_frags != 0? If not, we won't copy with the current
code either.
We *will* copy, though, if we're expected to expand the tailroom. That
won't change in my patch as we'll linearize the skb in that case -- even
if there would enough room in the skb. That's needed to not hit the
SKB_LINEAR_ASSERT(skb) in skb_put().
> If you want to do this
> properly then we'll need to add at least a bit to indicate whether
> the page originated from within the network stack and we have
> full ownership.
Can you please explain why this would be needed? I still don't get the
reasoning behind "pages are considered not writable at the moment even
if they are anonymous".
skb_cow_data() is only used in those places:
- net/caif/cfpkt_skbuff.c
- net/ipv4/ah4.c
- net/ipv4/esp4.c
- net/ipv6/ah6.c
- net/ipv6/esp6.c
- net/rxrpc/rxkad.c
Can you explain how this change would be a problem for them?
Thanks,
Mathias
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-06 12:39 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131106013038.GA14894@gondor.apana.org.au>
On Wed, Nov 06, 2013 at 09:30:38AM +0800, Herbert Xu wrote:
> Here is a totally untested patch that tries to trivially process
> these new frags + frag_list skbs. It should actually be trivial
> to make this generate TSO packets by just adding a gso_ok check
> and short-circuit.
That patch obviously didn't have a chance of working since I missed
a continue.
Here is a better version.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3735fad..409bd9b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2816,7 +2816,31 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
hsize = len;
if (!hsize && i >= nfrags) {
- BUG_ON(fskb->len != len);
+ if (fskb->len != len) {
+ SKB_FRAG_ASSERT(fskb);
+
+ nskb = skb_segment(fskb, features);
+
+ err = PTR_ERR(nskb);
+ if (IS_ERR(nskb))
+ goto err;
+ err = -ENOMEM;
+
+ if (segs)
+ tail->next = nskb;
+ else
+ segs = nskb;
+
+ tail = nskb;
+ while (tail->next)
+ tail = tail->next;
+
+ BUG_ON(fskb->next && tail->len != len);
+
+ len = fskb->len;
+ fskb = fskb->next;
+ continue;
+ }
pos += len;
nskb = skb_clone(fskb, GFP_ATOMIC);
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Generating fragmented skb
From: David Laight @ 2013-11-06 12:31 UTC (permalink / raw)
To: netdev
What is the easiest way to generate fragmented skb?
I'm trying to test the xhci USB bulk code using a couple
of USB ethernet devices (smsc95xx and ax88179_178a).
I think the latter should support TCP segmentation offload.
FWIW some tests that seemed to imply the ax88179_178a needed
the ZLP avoidance code may have been caused because the
xhci driver completely ignored USB_ZETO_PACKET.
David
^ permalink raw reply
* Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag
From: Hannes Frederic Sowa @ 2013-11-06 12:03 UTC (permalink / raw)
To: David Laight
Cc: nicolas.dichtel, David Miller, jiri, vyasevich, netdev, kuznet,
jmorris, yoshfuji, kaber, thaller, stephen
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73E3@saturn3.aculab.com>
On Wed, Nov 06, 2013 at 09:23:05AM -0000, David Laight wrote:
> > By the way:
> > Although dhclient uses an AF_PACKET socket, it never does request
> > additional multicast addresses or promisc mode. So the only overhead is
> > that we have to run the socket filter on each incoming packet.
>
> Just creating the AF_PACKET socket requires that every receive
> frame be duplicated.
> On Linux this is a reference counted but the cost is still non-zero.
> On some other OS (I first saw this with VxWorks) it is a full data copy.
Yes, I saw that, so my guess was that running the filter on every incoming UDP
packet would be the highes cost factor (it still does get filtered by protocol
number early in the stack).
^ permalink raw reply
* Re: [PATCH] netfilter: ipvs: ip_vs_sync: Remove unused variable
From: Fabio Estevam @ 2013-11-06 11:05 UTC (permalink / raw)
To: Simon Horman; +Cc: ja, netdev@vger.kernel.org, Fabio Estevam
In-Reply-To: <20131028010418.GJ849@verge.net.au>
Hi Simon,
On Sun, Oct 27, 2013 at 11:04 PM, Simon Horman <horms@verge.net.au> wrote:
> Hi Fabio,
>
> this patch does not seem to apply against ipvs-next.
> Could you please rework it and repost if you believe it is still needed.
Yes, ipvs-next does not have this issue, but linux-next still shows
this problem though.
Won't ipvs-next get synced with linux-next?
Regards,
Fabio Estevam
^ 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