* [PATCH net-next V2 1/3] net: Add GRO support for UDP encapsulating protocols
From: Or Gerlitz @ 2014-01-07 15:29 UTC (permalink / raw)
To: hkchu, edumazet, herbert; +Cc: netdev, davem, yanb, shlomop, Or Gerlitz
In-Reply-To: <1389108594-665-1-git-send-email-ogerlitz@mellanox.com>
Add GRO handlers for protocols that do UDP encapsulation, with the intent of
being able to coalesce packets which encapsulate packets belonging to
the same TCP session.
For GRO purposes, the destination UDP port takes the role of the ether type
field in the ethernet header or the next protocol in the IP header.
The UDP GRO handler will only attempt to coalesce packets whose destination
port is registered to have gro handler.
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
include/net/protocol.h | 6 ++++
net/ipv4/protocol.c | 21 ++++++++++++++
net/ipv4/udp_offload.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 96 insertions(+), 0 deletions(-)
diff --git a/include/net/protocol.h b/include/net/protocol.h
index fbf7676..d776c08 100644
--- a/include/net/protocol.h
+++ b/include/net/protocol.h
@@ -92,6 +92,10 @@ extern const struct net_protocol __rcu *inet_protos[MAX_INET_PROTOS];
extern const struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS];
extern const struct net_offload __rcu *inet6_offloads[MAX_INET_PROTOS];
+
+#define MAX_UDP_PORT (1 << 16)
+extern const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT];
+
#if IS_ENABLED(CONFIG_IPV6)
extern const struct inet6_protocol __rcu *inet6_protos[MAX_INET_PROTOS];
#endif
@@ -102,6 +106,8 @@ int inet_add_offload(const struct net_offload *prot, unsigned char num);
int inet_del_offload(const struct net_offload *prot, unsigned char num);
void inet_register_protosw(struct inet_protosw *p);
void inet_unregister_protosw(struct inet_protosw *p);
+int udp_add_offload(const struct net_offload *prot, __be16 port);
+int udp_del_offload(const struct net_offload *prot, __be16 port);
#if IS_ENABLED(CONFIG_IPV6)
int inet6_add_protocol(const struct inet6_protocol *prot, unsigned char num);
diff --git a/net/ipv4/protocol.c b/net/ipv4/protocol.c
index 46d6a1c..426eae5 100644
--- a/net/ipv4/protocol.c
+++ b/net/ipv4/protocol.c
@@ -30,6 +30,7 @@
const struct net_protocol __rcu *inet_protos[MAX_INET_PROTOS] __read_mostly;
const struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS] __read_mostly;
+const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT] __read_mostly;
int inet_add_protocol(const struct net_protocol *prot, unsigned char protocol)
{
@@ -51,6 +52,13 @@ int inet_add_offload(const struct net_offload *prot, unsigned char protocol)
}
EXPORT_SYMBOL(inet_add_offload);
+int udp_add_offload(const struct net_offload *prot, __be16 port)
+{
+ return !cmpxchg((const struct net_offload **)&udp_offloads[port],
+ NULL, prot) ? 0 : -1;
+}
+EXPORT_SYMBOL(udp_add_offload);
+
int inet_del_protocol(const struct net_protocol *prot, unsigned char protocol)
{
int ret;
@@ -76,3 +84,16 @@ int inet_del_offload(const struct net_offload *prot, unsigned char protocol)
return ret;
}
EXPORT_SYMBOL(inet_del_offload);
+
+int udp_del_offload(const struct net_offload *prot, __be16 port)
+{
+ int ret;
+
+ ret = (cmpxchg((const struct net_offload **)&udp_offloads[port],
+ prot, NULL) == prot) ? 0 : -1;
+
+ synchronize_net();
+
+ return ret;
+}
+EXPORT_SYMBOL(udp_del_offload);
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 79c62bd..0a8fdd6 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -89,10 +89,79 @@ out:
return segs;
}
+
+static struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb)
+{
+ const struct net_offload *ops;
+ struct sk_buff *p, **pp = NULL;
+ struct udphdr *uh, *uh2;
+ unsigned int hlen, off;
+ int flush = 1;
+
+ off = skb_gro_offset(skb);
+ hlen = off + sizeof(*uh);
+ uh = skb_gro_header_fast(skb, off);
+ if (skb_gro_header_hard(skb, hlen)) {
+ uh = skb_gro_header_slow(skb, hlen, off);
+ if (unlikely(!uh))
+ goto out;
+ }
+
+ rcu_read_lock();
+ ops = rcu_dereference(udp_offloads[uh->dest]);
+ if (!ops || !ops->callbacks.gro_receive)
+ goto out_unlock;
+
+ flush = 0;
+
+ for (p = *head; p; p = p->next) {
+ if (!NAPI_GRO_CB(p)->same_flow)
+ continue;
+
+ uh2 = (struct udphdr *)(p->data + off);
+ if ((*(u32 *)&uh->source ^ *(u32 *)&uh2->source)) {
+ NAPI_GRO_CB(p)->same_flow = 0;
+ continue;
+ }
+ goto found;
+ }
+
+found:
+ skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
+ pp = ops->callbacks.gro_receive(head, skb);
+
+out_unlock:
+ rcu_read_unlock();
+out:
+ NAPI_GRO_CB(skb)->flush |= flush;
+
+ return pp;
+}
+
+static int udp_gro_complete(struct sk_buff *skb, int nhoff)
+{
+ const struct net_offload *ops;
+ __be16 newlen = htons(skb->len - nhoff);
+ struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
+ int err = -ENOSYS;
+
+ uh->len = newlen;
+
+ rcu_read_lock();
+ ops = rcu_dereference(udp_offloads[uh->dest]);
+ if (ops && ops->callbacks.gro_complete)
+ err = ops->callbacks.gro_complete(skb, nhoff + sizeof(struct udphdr));
+
+ rcu_read_unlock();
+ return err;
+}
+
static const struct net_offload udpv4_offload = {
.callbacks = {
.gso_send_check = udp4_ufo_send_check,
.gso_segment = udp4_ufo_fragment,
+ .gro_receive = udp_gro_receive,
+ .gro_complete = udp_gro_complete,
},
};
--
1.7.1
^ permalink raw reply related
* [PATCH net-next V2 3/3] net: Add GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-07 15:29 UTC (permalink / raw)
To: hkchu, edumazet, herbert; +Cc: netdev, davem, yanb, shlomop, Or Gerlitz
In-Reply-To: <1389108594-665-1-git-send-email-ogerlitz@mellanox.com>
Add gro handlers for vxlan using the udp gro infrastructure
On my setup, which is net-next (now with the mlx4 vxlan offloads patches) --
for single TCP session that goes through vxlan tunneling I got nice improvement
from 6.8Gbs to 11.5Gbs
--> UDP/VXLAN GRO disabled
$ netperf -H 192.168.52.147 -c -C
$ netperf -t TCP_STREAM -H 192.168.52.147 -c -C
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.52.147 () port 0 AF_INET
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
87380 65536 65536 10.00 6799.75 12.54 24.79 0.604 1.195
--> UDP/VXLAN GRO enabled
$ netperf -t TCP_STREAM -H 192.168.52.147 -c -C
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.52.147 () port 0 AF_INET
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
87380 65536 65536 10.00 11562.72 24.90 20.34 0.706 0.577
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/vxlan.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 481f85d..b51823b 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -40,6 +40,7 @@
#include <net/net_namespace.h>
#include <net/netns/generic.h>
#include <net/vxlan.h>
+#include <net/protocol.h>
#if IS_ENABLED(CONFIG_IPV6)
#include <net/ipv6.h>
#include <net/addrconf.h>
@@ -554,6 +555,98 @@ static int vxlan_fdb_append(struct vxlan_fdb *f,
return 1;
}
+static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff *skb)
+{
+ struct sk_buff *p, **pp = NULL;
+ struct vxlanhdr *vh, *vh2;
+ struct ethhdr *eh;
+ unsigned int hlen, off, off_eth;
+ const struct packet_offload *ptype;
+ __be16 type;
+ int flush = 1;
+
+ off = skb_gro_offset(skb);
+ hlen = off + sizeof(*vh);
+ vh = skb_gro_header_fast(skb, off);
+ if (skb_gro_header_hard(skb, hlen)) {
+ vh = skb_gro_header_slow(skb, hlen, off);
+ if (unlikely(!vh))
+ goto out;
+ }
+
+ flush = 0;
+
+ for (p = *head; p; p = p->next) {
+ if (!NAPI_GRO_CB(p)->same_flow)
+ continue;
+
+ vh2 = (struct vxlanhdr *)(p->data + off);
+ if (vh->vx_vni ^ vh2->vx_vni) {
+ NAPI_GRO_CB(p)->same_flow = 0;
+ continue;
+ }
+ goto found;
+ }
+
+found:
+ skb_gro_pull(skb, sizeof(struct vxlanhdr)); /* pull vxlan header */
+
+ off_eth = skb_gro_offset(skb);
+ hlen = off_eth + sizeof(*eh);
+ eh = skb_gro_header_fast(skb, off_eth);
+ if (skb_gro_header_hard(skb, hlen)) {
+ eh = skb_gro_header_slow(skb, hlen, off_eth);
+ if (unlikely(!eh))
+ goto out;
+ }
+ type = eh->h_proto;
+
+ rcu_read_lock();
+ ptype = gro_find_receive_by_type(type);
+ if (ptype == NULL) {
+ flush = 1;
+ goto out_unlock;
+ }
+
+ skb_gro_pull(skb, sizeof(*eh)); /* pull inner eth header */
+ pp = ptype->callbacks.gro_receive(head, skb);
+
+out_unlock:
+ rcu_read_unlock();
+out:
+ NAPI_GRO_CB(skb)->flush |= flush;
+
+ return pp;
+}
+
+static int vxlan_gro_complete(struct sk_buff *skb, int nhoff)
+{
+ struct ethhdr *eh;
+ struct packet_offload *ptype;
+ __be16 type;
+ /* 22 = 8 bytes for the vlxan header + 14 bytes for the inner eth header */
+ int vxlan_len = 22;
+ int err = -ENOSYS;
+
+ eh = (struct ethhdr *)(skb->data + nhoff + sizeof (struct vxlanhdr));
+ type = eh->h_proto;
+
+ rcu_read_lock();
+ ptype = gro_find_complete_by_type(type);
+ if (ptype != NULL)
+ err = ptype->callbacks.gro_complete(skb, nhoff + vxlan_len);
+
+ rcu_read_unlock();
+ return err;
+}
+
+static const struct net_offload vxlan_offload = {
+ .callbacks = {
+ .gro_receive = vxlan_gro_receive,
+ .gro_complete = vxlan_gro_complete,
+ },
+};
+
/* Notify netdevs that UDP port started listening */
static void vxlan_notify_add_rx_port(struct sock *sk)
{
@@ -568,6 +661,8 @@ static void vxlan_notify_add_rx_port(struct sock *sk)
dev->netdev_ops->ndo_add_vxlan_port(dev, sa_family,
port);
}
+ if (sa_family == AF_INET)
+ udp_add_offload(&vxlan_offload, port);
rcu_read_unlock();
}
@@ -585,6 +680,8 @@ static void vxlan_notify_del_rx_port(struct sock *sk)
dev->netdev_ops->ndo_del_vxlan_port(dev, sa_family,
port);
}
+ if (sa_family == AF_INET)
+ udp_del_offload(&vxlan_offload, port);
rcu_read_unlock();
}
@@ -1125,8 +1222,8 @@ static void vxlan_rcv(struct vxlan_sock *vs,
* leave the CHECKSUM_UNNECESSARY, the device checksummed it
* for us. Otherwise force the upper layers to verify it.
*/
- if (skb->ip_summed != CHECKSUM_UNNECESSARY || !skb->encapsulation ||
- !(vxlan->dev->features & NETIF_F_RXCSUM))
+ if ((skb->ip_summed != CHECKSUM_UNNECESSARY && skb->ip_summed != CHECKSUM_PARTIAL) ||
+ !skb->encapsulation || !(vxlan->dev->features & NETIF_F_RXCSUM))
skb->ip_summed = CHECKSUM_NONE;
skb->encapsulation = 0;
--
1.7.1
^ permalink raw reply related
* Re: single process receives own frames due to PACKET_MMAP
From: Daniel Borkmann @ 2014-01-07 15:26 UTC (permalink / raw)
To: Norbert van Bolhuis; +Cc: Jesper Dangaard Brouer, netdev, David Miller, uaca
In-Reply-To: <52CC1A61.5080205@aimvalley.nl>
On 01/07/2014 04:16 PM, Norbert van Bolhuis wrote:
> On 01/07/14 15:09, Jesper Dangaard Brouer wrote:
>> On Tue, 07 Jan 2014 14:16:03 +0100
>> Norbert van Bolhuis<nvbolhuis@aimvalley.nl> wrote:
>>> On 01/07/14 11:06, Jesper Dangaard Brouer wrote:
>>>> On Tue, 07 Jan 2014 10:32:01 +0100
>>>> Daniel Borkmann<dborkman@redhat.com> wrote:
>>>>
>>>>> On 01/06/2014 11:58 PM, Norbert van Bolhuis wrote:
>>>>>>
>> [...]
>>>>>
>>>>>> I'd say it makes no sense to make the same process receive its
>>>>>> own transmitted frames on that same interface (unless its lo).
>>>>
>>>> Have you setup:
>>>> ring->s_ll.sll_protocol = 0
>>>>
>>>> This is what I did in trafgen to avoid this problem.
>>>>
>>>> See line 55 in netsniff-ng/ring.c:
>>>> https://github.com/borkmann/netsniff-ng/blob/c3602a995b21e8133c7f4fd1fb1e7e21b6a844f1/ring.c#L55
>>>>
>>>> Commit:
>>>> https://github.com/borkmann/netsniff-ng/commit/c3602a995b21e8133c7f4fd1fb1e7e21b6a844f1
>>>>
>>>
>>>
>>> No I did not do that, I was checking my code against netsniff-ng-0.5.8-rc4.
>>>
>>> But I just tried it, I believe I do the same as netsniff-ng-0.5.8-rc5, but it doesn't
>>> work for me. Maybe because I have an old FC14 system (kernel 2.6.35.14-106.fc14.x86_64).
>>>
>>> So I tried to see whether netsniff-ng-0.5.8-rc5/trafgen still makes the
>>> kernel call packet_rcv() on my FC14 system. So I build and run it, but I'm not sure
>>> how to (easily) check that.
>>
>> The easiest way is to:
>> cat /proc/net/ptype
>> And look if someone registered a proto handler/function: packet_rcv (or tpacket_rcv).
>>
>> The more exact method is, to run "perf record -a -g" and then look (at
>> the result with "perf report") for a lock contention, and "expand" the
>> spin_lock and see if packet_rcv() is calling this spin lock.
>>
>
>
> I checked the easy way.
> Even on my old FC14 system the "protocol=0 patch" seems to make a difference
> for trafgen.
> Without the patch I see for each CPU in use by trafgen a "packet_rcv entry" in
> /proc/net/ptype.
> With the patch I see no additional "packet_rcv entry".
Yes, that is expected behaviour. ;-) See more below.
> It could be my Appl is wrong or maybe the "protocol=0 patch" does not help.
> I think the latter, afterall my Appl has, unlike trafgen, another RX
> (AF_PACKET) socket.
>
>
>>
>>> In anyway, Wireshark does capture the trafgen generated
>>> frames, does that say anything ?
>>
>> Be careful not to start a wireshark/tcpdump, at the sametime, as this
>> will slow you down.
>>
>>> In the future, I can at least use PACKET_QDISC_BYPASS as a "workaround".
>>
>> And in the future with PACKET_QDISC_BYPASS, your wireshark will not
>> catch these packets, remember that.
>>
>
>
> Yes, this is why I would love to see the "protocol=0 patch" work for my Appl.
>
> So I will try my Appl with the latest net-next kernel to see if that makes
> it work. Hopefully I can find some time in the next coming days, I will keep
> you informed.
As long as there's at least one single PF_PACKET receive socket open and you
do not make use of PACKET_QDISC_BYPASS on your tx socket, then those packets go
back the dev_queue_xmit_nit() path, even if your tx socket uses protocol=0.
If you make use of PACKET_QDISC_BYPASS [1] for your particular tx socket, then
packets generated by that socket will not hit the dev_queue_xmit_nit() path
back to other possible rx listeners that are present on your system (w/ the
side-effects for tx as described in [1]).
[1] Documentation/networking/packet_mmap.txt +960
> ---
> Norbert
^ permalink raw reply
* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get
From: Florian Westphal @ 2014-01-07 15:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andrey Vagin, netfilter-devel, netfilter, coreteam, netdev,
linux-kernel, vvs, Florian Westphal, Pablo Neira Ayuso,
Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Cyrill Gorcunov
In-Reply-To: <1389107305.26646.20.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> > index 43549eb..7a34bb2 100644
> > --- a/net/netfilter/nf_conntrack_core.c
> > +++ b/net/netfilter/nf_conntrack_core.c
> > @@ -387,8 +387,12 @@ begin:
> > !atomic_inc_not_zero(&ct->ct_general.use)))
> > h = NULL;
> > else {
> > + /* A conntrack can be recreated with the equal tuple,
> > + * so we need to check that the conntrack is initialized
> > + */
> > if (unlikely(!nf_ct_tuple_equal(tuple, &h->tuple) ||
> > - nf_ct_zone(ct) != zone)) {
> > + nf_ct_zone(ct) != zone) ||
> > + !nf_ct_is_confirmed(ct)) {
> > nf_ct_put(ct);
> > goto begin;
> > }
>
> I do not think this is the right way to fix this problem (if said
> problem is confirmed)
>
> Remember the rule about SLAB_DESTROY_BY_RCU :
>
> When a struct is freed, then reused, its important to set the its refcnt
> (from 0 to 1) only when the structure is fully ready for use.
>
> If a lookup finds a structure which is not yet setup, the
> atomic_inc_not_zero() will fail.
Indeed. But, the structure itself might be ready (or rather,
can be ready since the allocation side will set the refcount to one
after doing the initial work, such as zapping old ->status flags and
setting tuple information).
The problem is with nat extension area stored in the ct->ext area.
This extension area is preallocated but the snat/dnat action
information is only set up after the ct (or rather, the skb that grabbed
a reference to the nf_conn entry) traverses nat pre/postrouting.
This will also set up a null-binding when no matching SNAT/DNAT/MASQERUADE
rule existed.
The manipulations of the skb->nfct->ext nat area are performed without
a lock. Concurrent access is supposedly impossible as the conntrack
should not (yet) be in the hash table.
The confirmed bit is set right before we insert the conntrack into
the hash table (after we traversed rules, ct is ready to be
'published').
i.e. when the confirmed bit is NOT set we should not be 'seeing' the nf_conn
struct when we perform the lookup, as it should still be sitting on the
'unconfirmed' list, being invisible to readers.
Does that explanation make sense to you?
Thanks for looking into this.
^ permalink raw reply
* Re: [PATCH net-next v2 6/9] xen-netback: Handle guests with too many frags
From: Zoltan Kiss @ 2014-01-07 15:23 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.campbell, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <20131216180908.GC25969@zion.uk.xensource.com>
On 16/12/13 18:09, Wei Liu wrote:
>>>> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
>>>> index e26cdda..f6ed1c8 100644
>>>> --- a/drivers/net/xen-netback/netback.c
>>>> +++ b/drivers/net/xen-netback/netback.c
>>>> @@ -906,11 +906,15 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
>>>> u16 pending_idx = *((u16 *)skb->data);
>>>> int start;
>>>> pending_ring_idx_t index;
>>>> - unsigned int nr_slots;
>>>> + unsigned int nr_slots, frag_overflow = 0;
>>>>
>>>> /* At this point shinfo->nr_frags is in fact the number of
>>>> * slots, which can be as large as XEN_NETBK_LEGACY_SLOTS_MAX.
>>>> */
>>>> + if (shinfo->nr_frags > MAX_SKB_FRAGS) {
>>>> + frag_overflow = shinfo->nr_frags - MAX_SKB_FRAGS;
>>>> + shinfo->nr_frags = MAX_SKB_FRAGS;
>>>> + }
>>>> nr_slots = shinfo->nr_frags;
>>>>
>>>
>>> It is also probably better to check whether shinfo->nr_frags is too
>>> large which makes frag_overflow > MAX_SKB_FRAGS. I know skb should be
>>> already be valid at this point but it wouldn't hurt to be more careful.
>> Ok, I've added this:
>> /* At this point shinfo->nr_frags is in fact the number of
>> * slots, which can be as large as XEN_NETBK_LEGACY_SLOTS_MAX.
>> */
>> + if (shinfo->nr_frags > MAX_SKB_FRAGS) {
>> + if (shinfo->nr_frags > XEN_NETBK_LEGACY_SLOTS_MAX) return NULL;
>> + frag_overflow = shinfo->nr_frags - MAX_SKB_FRAGS;
>>
>
> What I suggested is
>
> BUG_ON(frag_overflow > MAX_SKB_FRAGS)
Ok, I've changed it.
Zoli
^ permalink raw reply
* Re: single process receives own frames due to PACKET_MMAP
From: Norbert van Bolhuis @ 2014-01-07 15:16 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Daniel Borkmann; +Cc: netdev, David Miller, uaca
In-Reply-To: <20140107150938.1058b358@redhat.com>
On 01/07/14 15:09, Jesper Dangaard Brouer wrote:
>
> On Tue, 07 Jan 2014 14:16:03 +0100
> Norbert van Bolhuis<nvbolhuis@aimvalley.nl> wrote:
>> On 01/07/14 11:06, Jesper Dangaard Brouer wrote:
>>> On Tue, 07 Jan 2014 10:32:01 +0100
>>> Daniel Borkmann<dborkman@redhat.com> wrote:
>>>
>>>> On 01/06/2014 11:58 PM, Norbert van Bolhuis wrote:
>>>>>
> [...]
>>>>
>>>>> I'd say it makes no sense to make the same process receive its
>>>>> own transmitted frames on that same interface (unless its lo).
>>>
>>> Have you setup:
>>> ring->s_ll.sll_protocol = 0
>>>
>>> This is what I did in trafgen to avoid this problem.
>>>
>>> See line 55 in netsniff-ng/ring.c:
>>> https://github.com/borkmann/netsniff-ng/blob/c3602a995b21e8133c7f4fd1fb1e7e21b6a844f1/ring.c#L55
>>>
>>> Commit:
>>> https://github.com/borkmann/netsniff-ng/commit/c3602a995b21e8133c7f4fd1fb1e7e21b6a844f1
>>>
>>
>>
>> No I did not do that, I was checking my code against netsniff-ng-0.5.8-rc4.
>>
>> But I just tried it, I believe I do the same as netsniff-ng-0.5.8-rc5, but it doesn't
>> work for me. Maybe because I have an old FC14 system (kernel 2.6.35.14-106.fc14.x86_64).
>>
>> So I tried to see whether netsniff-ng-0.5.8-rc5/trafgen still makes the
>> kernel call packet_rcv() on my FC14 system. So I build and run it, but I'm not sure
>> how to (easily) check that.
>
> The easiest way is to:
> cat /proc/net/ptype
> And look if someone registered a proto handler/function: packet_rcv (or tpacket_rcv).
>
> The more exact method is, to run "perf record -a -g" and then look (at
> the result with "perf report") for a lock contention, and "expand" the
> spin_lock and see if packet_rcv() is calling this spin lock.
>
I checked the easy way.
Even on my old FC14 system the "protocol=0 patch" seems to make a difference
for trafgen.
Without the patch I see for each CPU in use by trafgen a "packet_rcv entry" in
/proc/net/ptype.
With the patch I see no additional "packet_rcv entry".
It could be my Appl is wrong or maybe the "protocol=0 patch" does not help.
I think the latter, afterall my Appl has, unlike trafgen, another RX
(AF_PACKET) socket.
>
>> In anyway, Wireshark does capture the trafgen generated
>> frames, does that say anything ?
>
> Be careful not to start a wireshark/tcpdump, at the sametime, as this
> will slow you down.
>
>> In the future, I can at least use PACKET_QDISC_BYPASS as a "workaround".
>
> And in the future with PACKET_QDISC_BYPASS, your wireshark will not
> catch these packets, remember that.
>
Yes, this is why I would love to see the "protocol=0 patch" work for my Appl.
So I will try my Appl with the latest net-next kernel to see if that makes
it work. Hopefully I can find some time in the next coming days, I will keep
you informed.
---
Norbert
^ permalink raw reply
* Re: [PATCH 10/15] irda: sh_irda: Enable driver compilation with COMPILE_TEST
From: Laurent Pinchart @ 2014-01-07 15:13 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: linux-sh, linux-arm-kernel, netdev
In-Reply-To: <2434798.8bvsZzgdNb@avalon>
On Wednesday 11 December 2013 13:49:15 Laurent Pinchart wrote:
> Hi Samuel,
>
> Could you please pick this patch up for v3.14 ?
Ping ?
> On Wednesday 27 November 2013 02:18:32 Laurent Pinchart wrote:
> > This helps increasing build testing coverage.
> >
> > Cc: Samuel Ortiz <samuel@sortiz.org>
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > Acked-by: Simon Horman <horms@verge.net.au>
> > ---
> >
> > drivers/net/irda/Kconfig | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/irda/Kconfig b/drivers/net/irda/Kconfig
> > index 2a30193..cae5ee2 100644
> > --- a/drivers/net/irda/Kconfig
> > +++ b/drivers/net/irda/Kconfig
> > @@ -403,7 +403,8 @@ config MCS_FIR
> >
> > config SH_IRDA
> >
> > tristate "SuperH IrDA driver"
> >
> > - depends on IRDA && ARCH_SHMOBILE
> > + depends on IRDA
> > + depends on ARCH_SHMOBILE || COMPILE_TEST
> >
> > help
> >
> > Say Y here if your want to enable SuperH IrDA devices.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [Xen-devel] [PATCH net-next v3] xen-netfront: Add support for IPv6 offloads
From: Ian Campbell @ 2014-01-07 15:12 UTC (permalink / raw)
To: Paul Durrant
Cc: Boris Ostrovsky, xen-devel@lists.xen.org, netdev@vger.kernel.org,
Wei Liu, Annie Li, David Vrabel
In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD01E4F72@AMSPEX01CL01.citrite.net>
On Tue, 2014-01-07 at 15:05 +0000, Paul Durrant wrote:
> > -----Original Message-----
> > From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> > Sent: 07 January 2014 14:54
> > To: Paul Durrant
> > Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Wei Liu; Ian Campbell;
> > Annie Li; David Vrabel
> > Subject: Re: [Xen-devel] [PATCH net-next v3] xen-netfront: Add support for
> > IPv6 offloads
> >
> > On 01/07/2014 05:25 AM, Paul Durrant wrote:
> > >> -----Original Message-----
> > >> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> > >> Sent: 31 December 2013 19:10
> > >> To: Paul Durrant
> > >> Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Konrad Rzeszutek
> > >> Wilk; David Vrabel; Ian Campbell; Wei Liu; Annie Li
> > >> Subject: Re: [PATCH net-next v3] xen-netfront: Add support for IPv6
> > offloads
> > >>
> > >> On 11/26/2013 11:41 AM, Paul Durrant wrote:
> > >>> This patch adds support for IPv6 checksum offload and GSO when those
> > >>> features are available in the backend.
> > >> Sorry for late review. Mostly style comments.
> > >>
> > > Thanks for the review.
> > >
> > > The checksum related code essentially needs to be a duplicate of that in
> > netback and it seems wasteful to have the code in both places. Could this
> > code be moved perhaps to net/core/dev.c? It's not specific to
> > netback/netfront usage.
> >
> > Will any of these routines be called for anything other than Xen
> > networking?
> >
>
> I guess similar logic must be duplicated in other drivers - I can't
> believe that netback and netfront are the only ones to want to know
> where the TCP/UDP checksum field is located.
Me neither. Given that we already have two consumers (albeit both *xen*)
and that the functionality is generic in nature it seems to make sense
to me to have it in a generic place.
> > I don't know about net/core/dev.c but given the large amount of
> > duplicate code between netfront and netback I think factoring out should
> > be done at least for these two. Into xen-netcore.c or some such.
> >
>
> That's probably a pragmatic first step; I'll do that and post a patch series as v4.
I think this makes sense for code which has two consumers (both *xen*)
but which is actually Xen specific. Obviously if the network maintainers
don't think the checksum functionality is plausibly generically useful
then we could put it here instead.
Ian.
^ permalink raw reply
* [PATCH v2] net: Do not enable tx-nocache-copy by default
From: Benjamin Poirier @ 2014-01-07 15:11 UTC (permalink / raw)
To: David S. Miller; +Cc: Eric Dumazet, netdev, linux-kernel, Tom Herbert
In-Reply-To: <20140106.200002.1747627391067832069.davem@davemloft.net>
There are many cases where this feature does not improve performance or even
reduces it.
For example, here are the results from tests that I've run using 3.12.6 on one
Intel Xeon W3565 and one i7 920 connected by ixgbe adapters. The results are
from the Xeon, but they're similar on the i7. All numbers report the
mean±stddev over 10 runs of 10s.
1) latency tests similar to what is described in "c6e1a0d net: Allow no-cache
copy from user on transmit"
There is no statistically significant difference between tx-nocache-copy
on/off.
nic irqs spread out (one queue per cpu)
200x netperf -r 1400,1
tx-nocache-copy off
692000±1000 tps
50/90/95/99% latency (us): 275±2/643.8±0.4/799±1/2474.4±0.3
tx-nocache-copy on
693000±1000 tps
50/90/95/99% latency (us): 274±1/644.1±0.7/800±2/2474.5±0.7
200x netperf -r 14000,14000
tx-nocache-copy off
86450±80 tps
50/90/95/99% latency (us): 334.37±0.02/838±1/2100±20/3990±40
tx-nocache-copy on
86110±60 tps
50/90/95/99% latency (us): 334.28±0.01/837±2/2110±20/3990±20
2) single stream throughput tests
tx-nocache-copy leads to higher service demand
throughput cpu0 cpu1 demand
(Gb/s) (Gcycle) (Gcycle) (cycle/B)
nic irqs and netperf on cpu0 (1x netperf -T0,0 -t omni -- -d send)
tx-nocache-copy off 9402±5 9.4±0.2 0.80±0.01
tx-nocache-copy on 9403±3 9.85±0.04 0.838±0.004
nic irqs on cpu0, netperf on cpu1 (1x netperf -T1,1 -t omni -- -d send)
tx-nocache-copy off 9401±5 5.83±0.03 5.0±0.1 0.923±0.007
tx-nocache-copy on 9404±2 5.74±0.03 5.523±0.009 0.958±0.002
As a second example, here are some results from Eric Dumazet with latest
net-next.
tx-nocache-copy also leads to higher service demand
(cpu is Intel(R) Xeon(R) CPU X5660 @ 2.80GHz)
lpq83:~# ./ethtool -K eth0 tx-nocache-copy on
lpq83:~# perf stat ./netperf -H lpq84 -c
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to lpq84.prod.google.com () port 0 AF_INET
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % U us/KB us/KB
87380 16384 16384 10.00 9407.44 2.50 -1.00 0.522 -1.000
Performance counter stats for './netperf -H lpq84 -c':
4282.648396 task-clock # 0.423 CPUs utilized
9,348 context-switches # 0.002 M/sec
88 CPU-migrations # 0.021 K/sec
355 page-faults # 0.083 K/sec
11,812,797,651 cycles # 2.758 GHz [82.79%]
9,020,522,817 stalled-cycles-frontend # 76.36% frontend cycles idle [82.54%]
4,579,889,681 stalled-cycles-backend # 38.77% backend cycles idle [67.33%]
6,053,172,792 instructions # 0.51 insns per cycle
# 1.49 stalled cycles per insn [83.64%]
597,275,583 branches # 139.464 M/sec [83.70%]
8,960,541 branch-misses # 1.50% of all branches [83.65%]
10.128990264 seconds time elapsed
lpq83:~# ./ethtool -K eth0 tx-nocache-copy off
lpq83:~# perf stat ./netperf -H lpq84 -c
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to lpq84.prod.google.com () port 0 AF_INET
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % U us/KB us/KB
87380 16384 16384 10.00 9412.45 2.15 -1.00 0.449 -1.000
Performance counter stats for './netperf -H lpq84 -c':
2847.375441 task-clock # 0.281 CPUs utilized
11,632 context-switches # 0.004 M/sec
49 CPU-migrations # 0.017 K/sec
354 page-faults # 0.124 K/sec
7,646,889,749 cycles # 2.686 GHz [83.34%]
6,115,050,032 stalled-cycles-frontend # 79.97% frontend cycles idle [83.31%]
1,726,460,071 stalled-cycles-backend # 22.58% backend cycles idle [66.55%]
2,079,702,453 instructions # 0.27 insns per cycle
# 2.94 stalled cycles per insn [83.22%]
363,773,213 branches # 127.757 M/sec [83.29%]
4,242,732 branch-misses # 1.17% of all branches [83.51%]
10.128449949 seconds time elapsed
CC: Tom Herbert <therbert@google.com>
Signed-off-by: Benjamin Poirier <bpoirier@suse.de>
---
net/core/dev.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 4fc1722..0e82e77 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5831,13 +5831,8 @@ int register_netdevice(struct net_device *dev)
dev->features |= NETIF_F_SOFT_FEATURES;
dev->wanted_features = dev->features & dev->hw_features;
- /* Turn on no cache copy if HW is doing checksum */
if (!(dev->flags & IFF_LOOPBACK)) {
dev->hw_features |= NETIF_F_NOCACHE_COPY;
- if (dev->features & NETIF_F_ALL_CSUM) {
- dev->wanted_features |= NETIF_F_NOCACHE_COPY;
- dev->features |= NETIF_F_NOCACHE_COPY;
- }
}
/* Make NETIF_F_HIGHDMA inheritable to VLAN devices.
--
1.8.4
^ permalink raw reply related
* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get
From: Eric Dumazet @ 2014-01-07 15:08 UTC (permalink / raw)
To: Andrey Vagin
Cc: netfilter-devel, netfilter, coreteam, netdev, linux-kernel, vvs,
Florian Westphal, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, Cyrill Gorcunov
In-Reply-To: <1389090711-15843-1-git-send-email-avagin@openvz.org>
On Tue, 2014-01-07 at 14:31 +0400, Andrey Vagin wrote:
> Lets look at destroy_conntrack:
>
> hlist_nulls_del_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode);
> ...
> nf_conntrack_free(ct)
> kmem_cache_free(net->ct.nf_conntrack_cachep, ct);
>
> net->ct.nf_conntrack_cachep is created with SLAB_DESTROY_BY_RCU.
>
> The hash is protected by rcu, so readers look up conntracks without
> locks.
> A conntrack is removed from the hash, but in this moment a few readers
> still can use the conntrack. Then this conntrack is released and another
> thread creates conntrack with the same address and the equal tuple.
> After this a reader starts to validate the conntrack:
> * It's not dying, because a new conntrack was created
> * nf_ct_tuple_equal() returns true.
>
> But this conntrack is not initialized yet, so it can not be used by two
> threads concurrently. In this case BUG_ON may be triggered from
> nf_nat_setup_info().
>
> Florian Westphal suggested to check the confirm bit too. I think it's
> right.
>
> task 1 task 2 task 3
> nf_conntrack_find_get
> ____nf_conntrack_find
> destroy_conntrack
> hlist_nulls_del_rcu
> nf_conntrack_free
> kmem_cache_free
> __nf_conntrack_alloc
> kmem_cache_alloc
> memset(&ct->tuplehash[IP_CT_DIR_MAX],
> if (nf_ct_is_dying(ct))
> if (!nf_ct_tuple_equal()
>
> I'm not sure, that I have ever seen this race condition in a real life.
> Currently we are investigating a bug, which is reproduced on a few node.
> In our case one conntrack is initialized from a few tasks concurrently,
> we don't have any other explanation for this.
>
> Cc: Florian Westphal <fw@strlen.de>
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
> net/netfilter/nf_conntrack_core.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 43549eb..7a34bb2 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -387,8 +387,12 @@ begin:
> !atomic_inc_not_zero(&ct->ct_general.use)))
> h = NULL;
> else {
> + /* A conntrack can be recreated with the equal tuple,
> + * so we need to check that the conntrack is initialized
> + */
> if (unlikely(!nf_ct_tuple_equal(tuple, &h->tuple) ||
> - nf_ct_zone(ct) != zone)) {
> + nf_ct_zone(ct) != zone) ||
> + !nf_ct_is_confirmed(ct)) {
> nf_ct_put(ct);
> goto begin;
> }
I do not think this is the right way to fix this problem (if said
problem is confirmed)
Remember the rule about SLAB_DESTROY_BY_RCU :
When a struct is freed, then reused, its important to set the its refcnt
(from 0 to 1) only when the structure is fully ready for use.
If a lookup finds a structure which is not yet setup, the
atomic_inc_not_zero() will fail.
Take a look at sk_clone_lock() and Documentation/RCU/rculist_nulls.txt
^ permalink raw reply
* RE: [Xen-devel] [PATCH net-next v3] xen-netfront: Add support for IPv6 offloads
From: Paul Durrant @ 2014-01-07 15:05 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: xen-devel@lists.xen.org, netdev@vger.kernel.org, Wei Liu,
Ian Campbell, Annie Li, David Vrabel
In-Reply-To: <52CC1500.8050104@oracle.com>
> -----Original Message-----
> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> Sent: 07 January 2014 14:54
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Wei Liu; Ian Campbell;
> Annie Li; David Vrabel
> Subject: Re: [Xen-devel] [PATCH net-next v3] xen-netfront: Add support for
> IPv6 offloads
>
> On 01/07/2014 05:25 AM, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> >> Sent: 31 December 2013 19:10
> >> To: Paul Durrant
> >> Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Konrad Rzeszutek
> >> Wilk; David Vrabel; Ian Campbell; Wei Liu; Annie Li
> >> Subject: Re: [PATCH net-next v3] xen-netfront: Add support for IPv6
> offloads
> >>
> >> On 11/26/2013 11:41 AM, Paul Durrant wrote:
> >>> This patch adds support for IPv6 checksum offload and GSO when those
> >>> features are available in the backend.
> >> Sorry for late review. Mostly style comments.
> >>
> > Thanks for the review.
> >
> > The checksum related code essentially needs to be a duplicate of that in
> netback and it seems wasteful to have the code in both places. Could this
> code be moved perhaps to net/core/dev.c? It's not specific to
> netback/netfront usage.
>
> Will any of these routines be called for anything other than Xen
> networking?
>
I guess similar logic must be duplicated in other drivers - I can't believe that netback and netfront are the only ones to want to know where the TCP/UDP checksum field is located.
> I don't know about net/core/dev.c but given the large amount of
> duplicate code between netfront and netback I think factoring out should
> be done at least for these two. Into xen-netcore.c or some such.
>
That's probably a pragmatic first step; I'll do that and post a patch series as v4.
Paul
> -boris
>
>
> >
> > Opinions?
> >
> > Paul
> >
> >>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> >>> Cc: David Vrabel <david.vrabel@citrix.com>
> >>> Cc: Ian Campbell <ian.campbell@citrix.com>
> >>> Cc: Wei Liu <wei.liu2@citrix.com>
> >>> Cc: Annie Li <annie.li@oracle.com>
> >>> ---
> >>>
> >>> v3:
> >>> - Addressed comments raised by Annie Li
> >>>
> >>> v2:
> >>> - Addressed comments raised by Ian Campbell
> >>>
> >>> drivers/net/xen-netfront.c | 239
> >> ++++++++++++++++++++++++++++++++++++++++----
> >>> include/linux/ipv6.h | 2 +
> >>> 2 files changed, 224 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> >>> index dd1011e..fe747e4 100644
> >>> --- a/drivers/net/xen-netfront.c
> >>> +++ b/drivers/net/xen-netfront.c
> >>> @@ -616,7 +616,9 @@ static int xennet_start_xmit(struct sk_buff *skb,
> >> struct net_device *dev)
> >>> tx->flags |= XEN_NETTXF_extra_info;
> >>>
> >>> gso->u.gso.size = skb_shinfo(skb)->gso_size;
> >>> - gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4;
> >>> + gso->u.gso.type = (skb_shinfo(skb)->gso_type &
> >> SKB_GSO_TCPV6) ?
> >>> + XEN_NETIF_GSO_TYPE_TCPV6 :
> >>> + XEN_NETIF_GSO_TYPE_TCPV4;
> >>> gso->u.gso.pad = 0;
> >>> gso->u.gso.features = 0;
> >>>
> >>> @@ -808,15 +810,18 @@ static int xennet_set_skb_gso(struct sk_buff
> >> *skb,
> >>> return -EINVAL;
> >>> }
> >>>
> >>> - /* Currently only TCPv4 S.O. is supported. */
> >>> - if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) {
> >>> + if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4 &&
> >>> + gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV6) {
> >>> if (net_ratelimit())
> >>> pr_warn("Bad GSO type %d\n", gso->u.gso.type);
> >>> return -EINVAL;
> >>> }
> >>>
> >>> skb_shinfo(skb)->gso_size = gso->u.gso.size;
> >>> - skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
> >>> + skb_shinfo(skb)->gso_type =
> >>> + (gso->u.gso.type == XEN_NETIF_GSO_TYPE_TCPV4) ?
> >>> + SKB_GSO_TCPV4 :
> >>> + SKB_GSO_TCPV6;
> >>>
> >>> /* Header must be checked, and gso_segs computed. */
> >>> skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
> >>> @@ -856,11 +861,42 @@ static RING_IDX xennet_fill_frags(struct
> >> netfront_info *np,
> >>> return cons;
> >>> }
> >>>
> >>> -static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
> >>> +static inline bool maybe_pull_tail(struct sk_buff *skb, unsigned int min,
> >>> + unsigned int max)
> >> Should this routine return error code instead of a boolean? Otherwise
> >> it's not clear what "false" should mean --- whether it is that it failed
> >> to pull or that the pull wasn't needed.
> >>
> >>> {
> >>> - struct iphdr *iph;
> >>> - int err = -EPROTO;
> >>> + int target;
> >>> +
> >>> + BUG_ON(max < min);
> >>> +
> >>> + if (!skb_is_nonlinear(skb) || skb_headlen(skb) >= min)
> >>> + return true;
> >>> +
> >>> + /* If we need to pullup then pullup to max, so we hopefully
> >>> + * won't need to do it again.
> >>> + */
> >> Comment style.
> >>
> >>> + target = min_t(int, skb->len, max);
> >>> + __pskb_pull_tail(skb, target - skb_headlen(skb));
> >>> +
> >>> + if (skb_headlen(skb) < min) {
> >> Why not explicitly check whether__pskb_pull_tail() returned NULL ?
> >>
> >>> + net_err_ratelimited("Failed to pullup packet header\n");
> >>> + return false;
> >>> + }
> >>> +
> >>> + return true;
> >>> +}
> >>> +
> >>> +/* This value should be large enough to cover a tagged ethernet
> header
> >> plus
> >>> + * maximally sized IP and TCP or UDP headers.
> >>> + */
> >> Comment style.
> >>
> >>> +#define MAX_IP_HEADER 128
> >>> +
> >>> +static int checksum_setup_ip(struct net_device *dev, struct sk_buff
> >> *skb)
> >>> +{
> >>> + struct iphdr *iph = (void *)skb->data;
> >>> + unsigned int header_size;
> >>> + unsigned int off;
> >>> int recalculate_partial_csum = 0;
> >>> + int err = -EPROTO;
> >>>
> >>> /*
> >>> * A GSO SKB must be CHECKSUM_PARTIAL. However some buggy
> >>> @@ -879,40 +915,158 @@ static int checksum_setup(struct net_device
> >> *dev, struct sk_buff *skb)
> >>> if (skb->ip_summed != CHECKSUM_PARTIAL)
> >>> return 0;
> >>>
> >>> - if (skb->protocol != htons(ETH_P_IP))
> >>> + off = sizeof(struct iphdr);
> >>> +
> >>> + header_size = skb->network_header + off;
> >>> + if (!maybe_pull_tail(skb, header_size, MAX_IP_HEADER))
> >>> goto out;
> >>>
> >>> - iph = (void *)skb->data;
> >>> + off = iph->ihl * 4;
> >>>
> >>> switch (iph->protocol) {
> >>> case IPPROTO_TCP:
> >>> - if (!skb_partial_csum_set(skb, 4 * iph->ihl,
> >>> + if (!skb_partial_csum_set(skb, off,
> >>> offsetof(struct tcphdr, check)))
> >>> goto out;
> >>>
> >>> if (recalculate_partial_csum) {
> >>> struct tcphdr *tcph = tcp_hdr(skb);
> >>> +
> >>> + header_size = skb->network_header +
> >>> + off +
> >>> + sizeof(struct tcphdr);
> >> You can put these (off and sizeof) onto the same line.
> >>
> >>> + if (!maybe_pull_tail(skb, header_size,
> >> MAX_IP_HEADER))
> >>> + goto out;
> >>> +
> >>> tcph->check = ~csum_tcpudp_magic(iph->saddr, iph-
> >>> daddr,
> >>> - skb->len - iph->ihl*4,
> >>> + skb->len - off,
> >>> IPPROTO_TCP, 0);
> >>> }
> >>> break;
> >>> case IPPROTO_UDP:
> >>> - if (!skb_partial_csum_set(skb, 4 * iph->ihl,
> >>> + if (!skb_partial_csum_set(skb, off,
> >>> offsetof(struct udphdr, check)))
> >>> goto out;
> >>>
> >>> if (recalculate_partial_csum) {
> >>> struct udphdr *udph = udp_hdr(skb);
> >>> +
> >>> + header_size = skb->network_header +
> >>> + off +
> >>> + sizeof(struct udphdr);
> >>> + if (!maybe_pull_tail(skb, header_size,
> >> MAX_IP_HEADER))
> >>> + goto out;
> >>> +
> >>> udph->check = ~csum_tcpudp_magic(iph->saddr,
> >> iph->daddr,
> >>> - skb->len - iph->ihl*4,
> >>> + skb->len - off,
> >>> IPPROTO_UDP, 0);
> >>> }
> >>> break;
> >>> default:
> >>> - if (net_ratelimit())
> >>> - pr_err("Attempting to checksum a non-TCP/UDP
> >> packet, dropping a protocol %d packet\n",
> >>> - iph->protocol);
> >>> + net_err_ratelimited("Attempting to checksum a non-
> >> TCP/UDP packet, dropping a protocol %d packet\n",
> >>> + iph->protocol);
> >>> + goto out;
> >>> + }
> >>> +
> >>> + err = 0;
> >>> +
> >>> +out:
> >>> + return err;
> >>> +}
> >>> +
> >>> +/* This value should be large enough to cover a tagged ethernet
> header
> >> plus
> >>> + * an IPv6 header, all options, and a maximal TCP or UDP header.
> >>> + */
> >>> +#define MAX_IPV6_HEADER 256
> >>> +
> >>> +static int checksum_setup_ipv6(struct net_device *dev, struct sk_buff
> >> *skb)
> >>> +{
> >>> + struct ipv6hdr *ipv6h = (void *)skb->data;
> >>> + u8 nexthdr;
> >>> + unsigned int header_size;
> >>> + unsigned int off;
> >>> + bool fragment;
> >>> + bool done;
> >>> + int err = -EPROTO;
> >>> +
> >>> + done = false;
> >> This should probably be moved down to the beginning of the while loop.
> >> And you also need to initialize fragment to "false" (and possibly rename
> >> it to is_fragment?)
> >>
> >>> +
> >>> + /* A non-CHECKSUM_PARTIAL SKB does not require setup. */
> >>> + if (skb->ip_summed != CHECKSUM_PARTIAL)
> >>> + return 0;
> >>> +
> >>> + off = sizeof(struct ipv6hdr);
> >>> +
> >>> + header_size = skb->network_header + off;
> >>> + if (!maybe_pull_tail(skb, header_size, MAX_IPV6_HEADER))
> >>> + goto out;
> >>> +
> >>> + nexthdr = ipv6h->nexthdr;
> >>> +
> >>> + while ((off <= sizeof(struct ipv6hdr) + ntohs(ipv6h->payload_len))
> >> &&
> >>> + !done) {
> >>> + switch (nexthdr) {
> >>> + case IPPROTO_DSTOPTS:
> >>> + case IPPROTO_HOPOPTS:
> >>> + case IPPROTO_ROUTING: {
> >>> + struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
> >>> +
> >>> + header_size = skb->network_header +
> >>> + off +
> >>> + sizeof(struct ipv6_opt_hdr);
> >> I'd merge the last two lines.
> >>
> >>> + if (!maybe_pull_tail(skb, header_size,
> >> MAX_IPV6_HEADER))
> >>> + goto out;
> >>> +
> >>> + nexthdr = hp->nexthdr;
> >>> + off += ipv6_optlen(hp);
> >>> + break;
> >>> + }
> >>> + case IPPROTO_AH: {
> >>> + struct ip_auth_hdr *hp = (void *)(skb->data + off);
> >>> +
> >>> + header_size = skb->network_header +
> >>> + off +
> >>> + sizeof(struct ip_auth_hdr);
> >> Here as well.
> >>
> >>> + if (!maybe_pull_tail(skb, header_size,
> >> MAX_IPV6_HEADER))
> >>> + goto out;
> >>> +
> >>> + nexthdr = hp->nexthdr;
> >>> + off += ipv6_ahlen(hp);
> >>> + break;
> >>> + }
> >>> + case IPPROTO_FRAGMENT:
> >>> + fragment = true;
> >>> + /* fall through */
> >>> + default:
> >>> + done = true;
> >>> + break;
> >>> + }
> >>> + }
> >>> +
> >>> + if (!done) {
> >>> + net_err_ratelimited("Failed to parse packet header\n");
> >>> + goto out;
> >>> + }
> >>> +
> >>> + if (fragment) {
> >>> + net_err_ratelimited("Packet is a fragment!\n");
> >>> + goto out;
> >>> + }
> >>> +
> >>> + switch (nexthdr) {
> >>> + case IPPROTO_TCP:
> >>> + if (!skb_partial_csum_set(skb, off,
> >>> + offsetof(struct tcphdr, check)))
> >>> + goto out;
> >>> + break;
> >>> + case IPPROTO_UDP:
> >>> + if (!skb_partial_csum_set(skb, off,
> >>> + offsetof(struct udphdr, check)))
> >>> + goto out;
> >>> + break;
> >>> + default:
> >>> + net_err_ratelimited("Attempting to checksum a non-
> >> TCP/UDP packet, dropping a protocol %d packet\n",
> >>> + nexthdr);
> >>> goto out;
> >>> }
> >>>
> >>> @@ -922,6 +1076,25 @@ out:
> >>> return err;
> >>> }
> >>>
> >>> +static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
> >>> +{
> >>> + int err;
> >> Initialize to -EPROTO (just to keep consistent with the rest of the file)
> >>
> >>> +
> >>> + switch (skb->protocol) {
> >>> + case htons(ETH_P_IP):
> >>> + err = checksum_setup_ip(dev, skb);
> >>> + break;
> >>> + case htons(ETH_P_IPV6):
> >>> + err = checksum_setup_ipv6(dev, skb);
> >>> + break;
> >>> + default:
> >>> + err = -EPROTO;
> >>> + break;
> >>> + }
> >>> +
> >>> + return err;
> >>> +}
> >>> +
> >>> static int handle_incoming_queue(struct net_device *dev,
> >>> struct sk_buff_head *rxq)
> >>> {
> >>> @@ -1232,6 +1405,15 @@ static netdev_features_t
> >> xennet_fix_features(struct net_device *dev,
> >>> features &= ~NETIF_F_SG;
> >>> }
> >>>
> >>> + if (features & NETIF_F_IPV6_CSUM) {
> >>> + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
> >>> + "feature-ipv6-csum-offload", "%d", &val) <
> >> 0)
> >>> + val = 0;
> >>> +
> >>> + if (!val)
> >>> + features &= ~NETIF_F_IPV6_CSUM;
> >>> + }
> >>> +
> >>> if (features & NETIF_F_TSO) {
> >>> if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
> >>> "feature-gso-tcpv4", "%d", &val) < 0)
> >>> @@ -1241,6 +1423,15 @@ static netdev_features_t
> >> xennet_fix_features(struct net_device *dev,
> >>> features &= ~NETIF_F_TSO;
> >>> }
> >>>
> >>> + if (features & NETIF_F_TSO6) {
> >>> + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
> >>> + "feature-gso-tcpv6", "%d", &val) < 0)
> >>> + val = 0;
> >>> +
> >>> + if (!val)
> >>> + features &= ~NETIF_F_TSO6;
> >>> + }
> >>> +
> >>> return features;
> >>> }
> >>>
> >>> @@ -1373,7 +1564,9 @@ static struct net_device
> >> *xennet_create_dev(struct xenbus_device *dev)
> >>> netif_napi_add(netdev, &np->napi, xennet_poll, 64);
> >>> netdev->features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> >>> NETIF_F_GSO_ROBUST;
> >>> - netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
> >> NETIF_F_TSO;
> >>> + netdev->hw_features = NETIF_F_SG |
> >>> + NETIF_F_IPV6_CSUM |
> >>> + NETIF_F_TSO | NETIF_F_TSO6;
> >> Can you merge these three lines and stay under 80? If not, merge either
> >> of the two of them.
> >>
> >>
> >> -boris
> >>
> >>> /*
> >>> * Assume that all hw features are available for now. This set
> >>> @@ -1751,6 +1944,18 @@ again:
> >>> goto abort_transaction;
> >>> }
> >>>
> >>> + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6",
> >> "%d", 1);
> >>> + if (err) {
> >>> + message = "writing feature-gso-tcpv6";
> >>> + goto abort_transaction;
> >>> + }
> >>> +
> >>> + err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum-
> >> offload", "%d", 1);
> >>> + if (err) {
> >>> + message = "writing feature-ipv6-csum-offload";
> >>> + goto abort_transaction;
> >>> + }
> >>> +
> >>> err = xenbus_transaction_end(xbt, 0);
> >>> if (err) {
> >>> if (err == -EAGAIN)
> >>> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> >>> index 5d89d1b..10f1b03 100644
> >>> --- a/include/linux/ipv6.h
> >>> +++ b/include/linux/ipv6.h
> >>> @@ -4,6 +4,8 @@
> >>> #include <uapi/linux/ipv6.h>
> >>>
> >>> #define ipv6_optlen(p) (((p)->hdrlen+1) << 3)
> >>> +#define ipv6_ahlen(p) (((p)->hdrlen+2) << 2);
> >>> +
> >>> /*
> >>> * This structure contains configuration options per IPv6 link.
> >>> */
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
^ permalink raw reply
* [patch net-next] ipv4: loopback device: ignore value changes after device is upped
From: Jiri Pirko @ 2014-01-07 14:55 UTC (permalink / raw)
To: netdev; +Cc: davem, kuznet, jmorris, yoshfuji, kaber, hannes
When lo is brought up, new ifa is created. Then, devconf and neigh values
bitfield should be set so later changes of default values would not
affect lo values.
Note that the same behaviour is in ipv6. Also note that this is likely
not an issue in many distros (for example Fedora 19) because userspace
sets address to lo manually before bringing it up.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
net/ipv4/devinet.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 0feebd5..9809f7b 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1385,6 +1385,8 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
set_ifa_lifetime(ifa, INFINITY_LIFE_TIME,
INFINITY_LIFE_TIME);
+ ipv4_devconf_setall(in_dev);
+ neigh_parms_data_state_setall(in_dev->arp_parms);
inet_insert_ifa(ifa);
}
}
--
1.8.3.1
^ permalink raw reply related
* Re: [Xen-devel] [PATCH net-next v3] xen-netfront: Add support for IPv6 offloads
From: Boris Ostrovsky @ 2014-01-07 14:53 UTC (permalink / raw)
To: Paul Durrant
Cc: xen-devel@lists.xen.org, netdev@vger.kernel.org, Wei Liu,
Ian Campbell, Annie Li, David Vrabel
In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD01E2C05@AMSPEX01CL01.citrite.net>
On 01/07/2014 05:25 AM, Paul Durrant wrote:
>> -----Original Message-----
>> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
>> Sent: 31 December 2013 19:10
>> To: Paul Durrant
>> Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Konrad Rzeszutek
>> Wilk; David Vrabel; Ian Campbell; Wei Liu; Annie Li
>> Subject: Re: [PATCH net-next v3] xen-netfront: Add support for IPv6 offloads
>>
>> On 11/26/2013 11:41 AM, Paul Durrant wrote:
>>> This patch adds support for IPv6 checksum offload and GSO when those
>>> features are available in the backend.
>> Sorry for late review. Mostly style comments.
>>
> Thanks for the review.
>
> The checksum related code essentially needs to be a duplicate of that in netback and it seems wasteful to have the code in both places. Could this code be moved perhaps to net/core/dev.c? It's not specific to netback/netfront usage.
Will any of these routines be called for anything other than Xen
networking?
I don't know about net/core/dev.c but given the large amount of
duplicate code between netfront and netback I think factoring out should
be done at least for these two. Into xen-netcore.c or some such.
-boris
>
> Opinions?
>
> Paul
>
>>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> Cc: David Vrabel <david.vrabel@citrix.com>
>>> Cc: Ian Campbell <ian.campbell@citrix.com>
>>> Cc: Wei Liu <wei.liu2@citrix.com>
>>> Cc: Annie Li <annie.li@oracle.com>
>>> ---
>>>
>>> v3:
>>> - Addressed comments raised by Annie Li
>>>
>>> v2:
>>> - Addressed comments raised by Ian Campbell
>>>
>>> drivers/net/xen-netfront.c | 239
>> ++++++++++++++++++++++++++++++++++++++++----
>>> include/linux/ipv6.h | 2 +
>>> 2 files changed, 224 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>>> index dd1011e..fe747e4 100644
>>> --- a/drivers/net/xen-netfront.c
>>> +++ b/drivers/net/xen-netfront.c
>>> @@ -616,7 +616,9 @@ static int xennet_start_xmit(struct sk_buff *skb,
>> struct net_device *dev)
>>> tx->flags |= XEN_NETTXF_extra_info;
>>>
>>> gso->u.gso.size = skb_shinfo(skb)->gso_size;
>>> - gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4;
>>> + gso->u.gso.type = (skb_shinfo(skb)->gso_type &
>> SKB_GSO_TCPV6) ?
>>> + XEN_NETIF_GSO_TYPE_TCPV6 :
>>> + XEN_NETIF_GSO_TYPE_TCPV4;
>>> gso->u.gso.pad = 0;
>>> gso->u.gso.features = 0;
>>>
>>> @@ -808,15 +810,18 @@ static int xennet_set_skb_gso(struct sk_buff
>> *skb,
>>> return -EINVAL;
>>> }
>>>
>>> - /* Currently only TCPv4 S.O. is supported. */
>>> - if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) {
>>> + if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4 &&
>>> + gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV6) {
>>> if (net_ratelimit())
>>> pr_warn("Bad GSO type %d\n", gso->u.gso.type);
>>> return -EINVAL;
>>> }
>>>
>>> skb_shinfo(skb)->gso_size = gso->u.gso.size;
>>> - skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
>>> + skb_shinfo(skb)->gso_type =
>>> + (gso->u.gso.type == XEN_NETIF_GSO_TYPE_TCPV4) ?
>>> + SKB_GSO_TCPV4 :
>>> + SKB_GSO_TCPV6;
>>>
>>> /* Header must be checked, and gso_segs computed. */
>>> skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
>>> @@ -856,11 +861,42 @@ static RING_IDX xennet_fill_frags(struct
>> netfront_info *np,
>>> return cons;
>>> }
>>>
>>> -static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
>>> +static inline bool maybe_pull_tail(struct sk_buff *skb, unsigned int min,
>>> + unsigned int max)
>> Should this routine return error code instead of a boolean? Otherwise
>> it's not clear what "false" should mean --- whether it is that it failed
>> to pull or that the pull wasn't needed.
>>
>>> {
>>> - struct iphdr *iph;
>>> - int err = -EPROTO;
>>> + int target;
>>> +
>>> + BUG_ON(max < min);
>>> +
>>> + if (!skb_is_nonlinear(skb) || skb_headlen(skb) >= min)
>>> + return true;
>>> +
>>> + /* If we need to pullup then pullup to max, so we hopefully
>>> + * won't need to do it again.
>>> + */
>> Comment style.
>>
>>> + target = min_t(int, skb->len, max);
>>> + __pskb_pull_tail(skb, target - skb_headlen(skb));
>>> +
>>> + if (skb_headlen(skb) < min) {
>> Why not explicitly check whether__pskb_pull_tail() returned NULL ?
>>
>>> + net_err_ratelimited("Failed to pullup packet header\n");
>>> + return false;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +/* This value should be large enough to cover a tagged ethernet header
>> plus
>>> + * maximally sized IP and TCP or UDP headers.
>>> + */
>> Comment style.
>>
>>> +#define MAX_IP_HEADER 128
>>> +
>>> +static int checksum_setup_ip(struct net_device *dev, struct sk_buff
>> *skb)
>>> +{
>>> + struct iphdr *iph = (void *)skb->data;
>>> + unsigned int header_size;
>>> + unsigned int off;
>>> int recalculate_partial_csum = 0;
>>> + int err = -EPROTO;
>>>
>>> /*
>>> * A GSO SKB must be CHECKSUM_PARTIAL. However some buggy
>>> @@ -879,40 +915,158 @@ static int checksum_setup(struct net_device
>> *dev, struct sk_buff *skb)
>>> if (skb->ip_summed != CHECKSUM_PARTIAL)
>>> return 0;
>>>
>>> - if (skb->protocol != htons(ETH_P_IP))
>>> + off = sizeof(struct iphdr);
>>> +
>>> + header_size = skb->network_header + off;
>>> + if (!maybe_pull_tail(skb, header_size, MAX_IP_HEADER))
>>> goto out;
>>>
>>> - iph = (void *)skb->data;
>>> + off = iph->ihl * 4;
>>>
>>> switch (iph->protocol) {
>>> case IPPROTO_TCP:
>>> - if (!skb_partial_csum_set(skb, 4 * iph->ihl,
>>> + if (!skb_partial_csum_set(skb, off,
>>> offsetof(struct tcphdr, check)))
>>> goto out;
>>>
>>> if (recalculate_partial_csum) {
>>> struct tcphdr *tcph = tcp_hdr(skb);
>>> +
>>> + header_size = skb->network_header +
>>> + off +
>>> + sizeof(struct tcphdr);
>> You can put these (off and sizeof) onto the same line.
>>
>>> + if (!maybe_pull_tail(skb, header_size,
>> MAX_IP_HEADER))
>>> + goto out;
>>> +
>>> tcph->check = ~csum_tcpudp_magic(iph->saddr, iph-
>>> daddr,
>>> - skb->len - iph->ihl*4,
>>> + skb->len - off,
>>> IPPROTO_TCP, 0);
>>> }
>>> break;
>>> case IPPROTO_UDP:
>>> - if (!skb_partial_csum_set(skb, 4 * iph->ihl,
>>> + if (!skb_partial_csum_set(skb, off,
>>> offsetof(struct udphdr, check)))
>>> goto out;
>>>
>>> if (recalculate_partial_csum) {
>>> struct udphdr *udph = udp_hdr(skb);
>>> +
>>> + header_size = skb->network_header +
>>> + off +
>>> + sizeof(struct udphdr);
>>> + if (!maybe_pull_tail(skb, header_size,
>> MAX_IP_HEADER))
>>> + goto out;
>>> +
>>> udph->check = ~csum_tcpudp_magic(iph->saddr,
>> iph->daddr,
>>> - skb->len - iph->ihl*4,
>>> + skb->len - off,
>>> IPPROTO_UDP, 0);
>>> }
>>> break;
>>> default:
>>> - if (net_ratelimit())
>>> - pr_err("Attempting to checksum a non-TCP/UDP
>> packet, dropping a protocol %d packet\n",
>>> - iph->protocol);
>>> + net_err_ratelimited("Attempting to checksum a non-
>> TCP/UDP packet, dropping a protocol %d packet\n",
>>> + iph->protocol);
>>> + goto out;
>>> + }
>>> +
>>> + err = 0;
>>> +
>>> +out:
>>> + return err;
>>> +}
>>> +
>>> +/* This value should be large enough to cover a tagged ethernet header
>> plus
>>> + * an IPv6 header, all options, and a maximal TCP or UDP header.
>>> + */
>>> +#define MAX_IPV6_HEADER 256
>>> +
>>> +static int checksum_setup_ipv6(struct net_device *dev, struct sk_buff
>> *skb)
>>> +{
>>> + struct ipv6hdr *ipv6h = (void *)skb->data;
>>> + u8 nexthdr;
>>> + unsigned int header_size;
>>> + unsigned int off;
>>> + bool fragment;
>>> + bool done;
>>> + int err = -EPROTO;
>>> +
>>> + done = false;
>> This should probably be moved down to the beginning of the while loop.
>> And you also need to initialize fragment to "false" (and possibly rename
>> it to is_fragment?)
>>
>>> +
>>> + /* A non-CHECKSUM_PARTIAL SKB does not require setup. */
>>> + if (skb->ip_summed != CHECKSUM_PARTIAL)
>>> + return 0;
>>> +
>>> + off = sizeof(struct ipv6hdr);
>>> +
>>> + header_size = skb->network_header + off;
>>> + if (!maybe_pull_tail(skb, header_size, MAX_IPV6_HEADER))
>>> + goto out;
>>> +
>>> + nexthdr = ipv6h->nexthdr;
>>> +
>>> + while ((off <= sizeof(struct ipv6hdr) + ntohs(ipv6h->payload_len))
>> &&
>>> + !done) {
>>> + switch (nexthdr) {
>>> + case IPPROTO_DSTOPTS:
>>> + case IPPROTO_HOPOPTS:
>>> + case IPPROTO_ROUTING: {
>>> + struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
>>> +
>>> + header_size = skb->network_header +
>>> + off +
>>> + sizeof(struct ipv6_opt_hdr);
>> I'd merge the last two lines.
>>
>>> + if (!maybe_pull_tail(skb, header_size,
>> MAX_IPV6_HEADER))
>>> + goto out;
>>> +
>>> + nexthdr = hp->nexthdr;
>>> + off += ipv6_optlen(hp);
>>> + break;
>>> + }
>>> + case IPPROTO_AH: {
>>> + struct ip_auth_hdr *hp = (void *)(skb->data + off);
>>> +
>>> + header_size = skb->network_header +
>>> + off +
>>> + sizeof(struct ip_auth_hdr);
>> Here as well.
>>
>>> + if (!maybe_pull_tail(skb, header_size,
>> MAX_IPV6_HEADER))
>>> + goto out;
>>> +
>>> + nexthdr = hp->nexthdr;
>>> + off += ipv6_ahlen(hp);
>>> + break;
>>> + }
>>> + case IPPROTO_FRAGMENT:
>>> + fragment = true;
>>> + /* fall through */
>>> + default:
>>> + done = true;
>>> + break;
>>> + }
>>> + }
>>> +
>>> + if (!done) {
>>> + net_err_ratelimited("Failed to parse packet header\n");
>>> + goto out;
>>> + }
>>> +
>>> + if (fragment) {
>>> + net_err_ratelimited("Packet is a fragment!\n");
>>> + goto out;
>>> + }
>>> +
>>> + switch (nexthdr) {
>>> + case IPPROTO_TCP:
>>> + if (!skb_partial_csum_set(skb, off,
>>> + offsetof(struct tcphdr, check)))
>>> + goto out;
>>> + break;
>>> + case IPPROTO_UDP:
>>> + if (!skb_partial_csum_set(skb, off,
>>> + offsetof(struct udphdr, check)))
>>> + goto out;
>>> + break;
>>> + default:
>>> + net_err_ratelimited("Attempting to checksum a non-
>> TCP/UDP packet, dropping a protocol %d packet\n",
>>> + nexthdr);
>>> goto out;
>>> }
>>>
>>> @@ -922,6 +1076,25 @@ out:
>>> return err;
>>> }
>>>
>>> +static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
>>> +{
>>> + int err;
>> Initialize to -EPROTO (just to keep consistent with the rest of the file)
>>
>>> +
>>> + switch (skb->protocol) {
>>> + case htons(ETH_P_IP):
>>> + err = checksum_setup_ip(dev, skb);
>>> + break;
>>> + case htons(ETH_P_IPV6):
>>> + err = checksum_setup_ipv6(dev, skb);
>>> + break;
>>> + default:
>>> + err = -EPROTO;
>>> + break;
>>> + }
>>> +
>>> + return err;
>>> +}
>>> +
>>> static int handle_incoming_queue(struct net_device *dev,
>>> struct sk_buff_head *rxq)
>>> {
>>> @@ -1232,6 +1405,15 @@ static netdev_features_t
>> xennet_fix_features(struct net_device *dev,
>>> features &= ~NETIF_F_SG;
>>> }
>>>
>>> + if (features & NETIF_F_IPV6_CSUM) {
>>> + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>>> + "feature-ipv6-csum-offload", "%d", &val) <
>> 0)
>>> + val = 0;
>>> +
>>> + if (!val)
>>> + features &= ~NETIF_F_IPV6_CSUM;
>>> + }
>>> +
>>> if (features & NETIF_F_TSO) {
>>> if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>>> "feature-gso-tcpv4", "%d", &val) < 0)
>>> @@ -1241,6 +1423,15 @@ static netdev_features_t
>> xennet_fix_features(struct net_device *dev,
>>> features &= ~NETIF_F_TSO;
>>> }
>>>
>>> + if (features & NETIF_F_TSO6) {
>>> + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>>> + "feature-gso-tcpv6", "%d", &val) < 0)
>>> + val = 0;
>>> +
>>> + if (!val)
>>> + features &= ~NETIF_F_TSO6;
>>> + }
>>> +
>>> return features;
>>> }
>>>
>>> @@ -1373,7 +1564,9 @@ static struct net_device
>> *xennet_create_dev(struct xenbus_device *dev)
>>> netif_napi_add(netdev, &np->napi, xennet_poll, 64);
>>> netdev->features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
>>> NETIF_F_GSO_ROBUST;
>>> - netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
>> NETIF_F_TSO;
>>> + netdev->hw_features = NETIF_F_SG |
>>> + NETIF_F_IPV6_CSUM |
>>> + NETIF_F_TSO | NETIF_F_TSO6;
>> Can you merge these three lines and stay under 80? If not, merge either
>> of the two of them.
>>
>>
>> -boris
>>
>>> /*
>>> * Assume that all hw features are available for now. This set
>>> @@ -1751,6 +1944,18 @@ again:
>>> goto abort_transaction;
>>> }
>>>
>>> + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6",
>> "%d", 1);
>>> + if (err) {
>>> + message = "writing feature-gso-tcpv6";
>>> + goto abort_transaction;
>>> + }
>>> +
>>> + err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum-
>> offload", "%d", 1);
>>> + if (err) {
>>> + message = "writing feature-ipv6-csum-offload";
>>> + goto abort_transaction;
>>> + }
>>> +
>>> err = xenbus_transaction_end(xbt, 0);
>>> if (err) {
>>> if (err == -EAGAIN)
>>> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
>>> index 5d89d1b..10f1b03 100644
>>> --- a/include/linux/ipv6.h
>>> +++ b/include/linux/ipv6.h
>>> @@ -4,6 +4,8 @@
>>> #include <uapi/linux/ipv6.h>
>>>
>>> #define ipv6_optlen(p) (((p)->hdrlen+1) << 3)
>>> +#define ipv6_ahlen(p) (((p)->hdrlen+2) << 2);
>>> +
>>> /*
>>> * This structure contains configuration options per IPv6 link.
>>> */
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply
* Re: [PATCH net-next v2 1/9] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2014-01-07 14:50 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.campbell, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <20131216175036.GB25969@zion.uk.xensource.com>
On 16/12/13 17:50, Wei Liu wrote:
> On Mon, Dec 16, 2013 at 03:21:40PM +0000, Zoltan Kiss wrote:
> [...]
>>>>>>>
>>>>>>> Should this be BUG_ON? AIUI this kthread should be the only one doing
>>>>>>> unmap, right?
>>>>> The NAPI instance can do it as well if it is a small packet fits
>>>>> into PKT_PROT_LEN. But still this scenario shouldn't really happen,
>>>>> I was just not sure we have to crash immediately. Maybe handle it as
>>>>> a fatal error and destroy the vif?
>>>>>
>>> It depends. If this is within the trust boundary, i.e. everything at the
>>> stage should have been sanitized then we should BUG_ON because there's
>>> clearly a bug somewhere in the sanitization process, or in the
>>> interaction of various backend routines.
>>
>> My understanding is that crashing should be avoided if we can bail
>> out somehow. At this point there is clearly a bug in netback
>> somewhere, something unmapped that page before it should have
>> happened, or at least that array get corrupted somehow. However
>> there is a chance that xenvif_fatal_tx_err() can contain the issue,
>> and the rest of the system can go unaffected.
>>
>
> That would make debugging much harder if a crash is caused by a previous
> corrupted array and we pretend we can carry on serving IMHO. Now netback
> is having three routines (NAPI, two kthreads) to serve a single vif, the
> interation among them makes bug hard to reproduce.
OK, I'll make this a BUG() in the next series.
Zoli
^ permalink raw reply
* question about ixgbevf/ixgbevf_main.c
From: Julia Lawall @ 2014-01-07 14:46 UTC (permalink / raw)
To: donald.c.skidmore, e1000-devel, netdev
I was wondering why ixgbevf_suspend doesn't call pci_set_power_state? It
is called in the corresponding resume function, and most other PCI drivers
with a suspend functyion also call it in suspend.
thanks,
julia
^ permalink raw reply
* Re: [PATCH net v2 1/9] bridge: Fix the way to find old local fdb entries in br_fdb_changeaddr
From: Vlad Yasevich @ 2014-01-07 14:44 UTC (permalink / raw)
To: Toshiaki Makita
Cc: Toshiaki Makita, David S . Miller, Stephen Hemminger, netdev
In-Reply-To: <1389098578.3768.12.camel@ubuntu-vm-makita>
On 01/07/2014 07:42 AM, Toshiaki Makita wrote:
> On Mon, 2014-01-06 at 06:29 -0500, Vlad Yasevich wrote:
>> On 01/05/2014 10:26 AM, Toshiaki Makita wrote:
>>> On Fri, 2014-01-03 at 15:46 -0500, Vlad Yasevich wrote:
>>>> On 01/03/2014 02:28 PM, Vlad Yasevich wrote:
>>>>> On 12/17/2013 07:03 AM, Toshiaki Makita wrote:
>>>>>> br_fdb_changeaddr() assumes that there is at most one local entry per port
>>>>>> per vlan. It used to be true, but since commit 36fd2b63e3b4 ("bridge: allow
>>>>>> creating/deleting fdb entries via netlink"), it has not been so.
>>>>>> Therefore, the function might fail to search a correct previous address
>>>>>> to be deleted and delete an arbitrary local entry if user has added local
>>>>>> entries manually.
>>>>>>
>>>>>> Example of problematic case:
>>>>>> ip link set eth0 address ee:ff:12:34:56:78
>>>>>> brctl addif br0 eth0
>>>>>> bridge fdb add 12:34:56:78:90:ab dev eth0 master
>>>>>> ip link set eth0 address aa:bb:cc:dd:ee:ff
>>>>>> Then, the address 12:34:56:78:90:ab might be deleted instead of
>>>>>> ee:ff:12:34:56:78, the original mac address of eth0.
>>>>>>
>>>>>> Address this issue by introducing a new flag, added_by_user, to struct
>>>>>> net_bridge_fdb_entry.
>>>>>>
>>>>>> Note that br_fdb_delete_by_port() has to set added_by_user to 0 in case
>>>>>> like:
>>>>>> ip link set eth0 address 12:34:56:78:90:ab
>>>>>> ip link set eth1 address aa:bb:cc:dd:ee:ff
>>>>>> brctl addif br0 eth0
>>>>>> bridge fdb add aa:bb:cc:dd:ee:ff dev eth0 master
>>>>>> brctl addif br0 eth1
>>>>>> brctl delif br0 eth0
>>>>>> In this case, kernel should delete the user-added entry aa:bb:cc:dd:ee:ff,
>>>>>> but it also should have been added by "brctl addif br0 eth1" originally,
>>>>>> so we don't delete it and treat it a new kernel-created entry.
>>>>>>
>>>>>
>>>>> I was looking over my patch series that adds something similar to this
>>>>> and noticed that you are not handing the NTF_USE case. That case was
>>>>> always troublesome for me as it allows for 2 different way to create
>>>>> the same FDB: one through br_fdb_update() and one through fdb_add_entry().
>>>>>
>>>>> It is possible, though I haven't found any users yet, that NTF_USE
>>>>> may be used and in that case, bridge will create a dynamic fdb and
>>>>> disregard all NUD flags. In case case, add_by_user will not be set
>>>>> either.
>>>>>
>>>>> I think that the above is broken and plan to submit a fix shortly.
>>>>
>>>> Just looked again at my NTF_USE patch and while it seems ok, the whole
>>>> NTF_USE usage is racy to begin with and I am really starting to question
>>>> it's validity.
>>>>
>>>> Presently, br_fdb_update() will not update local fdb entries. Instead
>>>> it will log a misleading warning... It will only let you update
>>>> non-local entries. This is fine for user-created entries, but any
>>>> operation on dynamically created entries will only persist until
>>>> the next packet. It also races against the packet, so there is
>>>> absolutely no guarantee that the values of fdb->dst and fdb->updated
>>>> will be consistent..
>>>>
>>>> It seems to me that the update capability of NTF_USE would actually be
>>>> of more value on local or user-created fdb entries.
>>>>
>>>> The fdb creation capability of NTF_USE should be disabled.
>>>>
>>>> Thoughts?
>>>
>>> I ignored NTF_USE in this patch because I regard it as emulating kernel
>>> creating entries after investigating git log.
>>>
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0c5c2d3089068d4aa378f7a40d2b5ad9d4f52ce8
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=292d1398983f3514a0eab13b7606df7f4730b498
>>>
>>> So I think NTF_USE shouldn't set added_by_user.
>>> And to emulate kernel creating entries, simply calling br_fdb_update()
>>> is the right way, isn't it?
>>
>> You can create dynamic entries (emulating the kernel) without NTF_USE.
>> Just set the NUD_REACHABLE. Notice that arp cache only uses NTF_USE
>> to trigger and arp notification. The creation is still triggered via
>> other netlink flags.
>>
>> The more I look at this the more I think NTF_USE should not create
>> an entry all by itself.
>
> I haven't fully understood you yet.
> Currently NTF_USE behaves as if the port receives a frame and it seems
> to work, though the ability to create entries is different from neigh
> subsystem.
> Why do you want to change the behavior?
> Are you worried about inconsistency of NLM-flags/NUD-state with NTF_USE
> between neigh and bridge?
No, it is inconsistent with other NLM/NUD-state within bridge. As
an fdb creation flag NTF_USE is confusing. It will create an entry
without NLM_F_CREATE being set. It will ignore NLM_F_EXCL flag as
well. It will additionally ignore any NUD-state flags that may be set
in the netlink message. So it may not be doing what the user wishes.
It also provides duplicate functionality. The same results are achieved
by setting NLM_F_CREATE flag and NUD_REACHABLE state in the message.
-vlad
>
> Thanks,
> Toshiaki Makita
>
^ permalink raw reply
* [PATCH v2 2/2] ipv6 addrconf: don't cleanup route prefix for IFA_F_NOPREFIXROUTE
From: Thomas Haller @ 2014-01-07 14:39 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <1389105553-21230-1-git-send-email-thaller@redhat.com>
Refactor the deletion/update of route prefixes when removing an
address. Now, consider IFA_F_NOPREFIXROUTE and if there is an address
present with this flag, to not cleanup the prefix. Instead, assume
that userspace is taking care of this prefix.
Also, when adding the NOPREFIXROUTE flag to an already existing address,
check if there there is a prefix that was likly added by the kernel
and delete it.
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
net/ipv6/addrconf.c | 188 +++++++++++++++++++++++++++++++---------------------
1 file changed, 112 insertions(+), 76 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1bc575f..1293a27 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -900,15 +900,106 @@ out:
goto out2;
}
+/*
+ * Check, whether the prefix route without ifp is still valid.
+ * The function returns:
+ * -1 route valid, update lifetimes (returns expires time).
+ * 0 route invalid, delete
+ * 1 route valid, don't update lifetimes.
+ *
+ * 1) we don't purge prefix here if address was not permanent.
+ * prefix is managed by its own lifetime.
+ * 2) we also don't purge, if the address was IFA_F_NOPREFIXROUTE.
+ * 3) if there're no addresses, delete prefix.
+ * 4) if there're still other permanent address(es),
+ * corresponding prefix is still permanent.
+ * 5) if there are still other addresses with IFA_F_NOPREFIXROUTE,
+ * don't purge the prefix, assume user space is
+ * managing it.
+ * 6) otherwise, update prefix lifetime to the
+ * longest valid lifetime among the corresponding
+ * addresses on the device.
+ * Note: subsequent RA will update lifetime.
+ *
+ * --yoshfuji
+ **/
+static int
+check_cleanup_prefix_routes(struct inet6_ifaddr *ifp, u32 ifa_flags, unsigned long *expires)
+{
+ struct inet6_ifaddr *ifa;
+ struct inet6_dev *idev = ifp->idev;
+ unsigned long lifetime;
+ int onlink = 0;
+
+ *expires = jiffies;
+
+
+ if (!(ifa_flags & IFA_F_PERMANENT))
+ return 1;
+ if (ifa_flags & IFA_F_NOPREFIXROUTE)
+ return 1;
+
+ list_for_each_entry(ifa, &idev->addr_list, if_list) {
+ if (ifa == ifp)
+ continue;
+ if (!ipv6_prefix_equal(&ifa->addr, &ifp->addr,
+ ifp->prefix_len))
+ continue;
+ if (ifa->flags & IFA_F_PERMANENT)
+ return 1;
+ if (ifa->flags & IFA_F_NOPREFIXROUTE)
+ return 1; /* user space is managing this prefix. */
+
+ onlink = -1;
+
+ spin_lock(&ifa->lock);
+
+ lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
+ /*
+ * Note: Because this address is
+ * not permanent, lifetime <
+ * LONG_MAX / HZ here.
+ */
+ if (time_before(*expires, ifa->tstamp + lifetime * HZ))
+ *expires = ifa->tstamp + lifetime * HZ;
+ spin_unlock(&ifa->lock);
+ }
+
+ return onlink;
+}
+
+static void
+cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long expires, int onlink)
+{
+ struct rt6_info *rt;
+
+ if (onlink >= 1)
+ return;
+
+ rt = addrconf_get_prefix_route(&ifp->addr,
+ ifp->prefix_len,
+ ifp->idev->dev,
+ 0, RTF_GATEWAY | RTF_DEFAULT);
+
+ if (rt) {
+ if (onlink == 0) {
+ ip6_del_rt(rt);
+ rt = NULL;
+ } else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
+ rt6_set_expires(rt, expires);
+ }
+ }
+ ip6_rt_put(rt);
+}
+
+
/* This function wants to get referenced ifp and releases it before return */
static void ipv6_del_addr(struct inet6_ifaddr *ifp)
{
- struct inet6_ifaddr *ifa, *ifn;
- struct inet6_dev *idev = ifp->idev;
int state;
- int deleted = 0, onlink = 0;
- unsigned long expires = jiffies;
+ int onlink;
+ unsigned long expires;
spin_lock_bh(&ifp->state_lock);
state = ifp->state;
@@ -922,7 +1013,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
hlist_del_init_rcu(&ifp->addr_lst);
spin_unlock_bh(&addrconf_hash_lock);
- write_lock_bh(&idev->lock);
+ write_lock_bh(&ifp->idev->lock);
if (ifp->flags&IFA_F_TEMPORARY) {
list_del(&ifp->tmp_list);
@@ -933,45 +1024,11 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
__in6_ifa_put(ifp);
}
- list_for_each_entry_safe(ifa, ifn, &idev->addr_list, if_list) {
- if (ifa == ifp) {
- list_del_init(&ifp->if_list);
- __in6_ifa_put(ifp);
+ onlink = check_cleanup_prefix_routes(ifp, ifp->flags, &expires);
+ list_del_init(&ifp->if_list);
+ __in6_ifa_put(ifp);
- if (!(ifp->flags & IFA_F_PERMANENT) || onlink > 0)
- break;
- deleted = 1;
- continue;
- } else if (ifp->flags & IFA_F_PERMANENT) {
- if (ipv6_prefix_equal(&ifa->addr, &ifp->addr,
- ifp->prefix_len)) {
- if (ifa->flags & IFA_F_PERMANENT) {
- onlink = 1;
- if (deleted)
- break;
- } else {
- unsigned long lifetime;
-
- if (!onlink)
- onlink = -1;
-
- spin_lock(&ifa->lock);
-
- lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
- /*
- * Note: Because this address is
- * not permanent, lifetime <
- * LONG_MAX / HZ here.
- */
- if (time_before(expires,
- ifa->tstamp + lifetime * HZ))
- expires = ifa->tstamp + lifetime * HZ;
- spin_unlock(&ifa->lock);
- }
- }
- }
- }
- write_unlock_bh(&idev->lock);
+ write_unlock_bh(&ifp->idev->lock);
addrconf_del_dad_timer(ifp);
@@ -979,39 +1036,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);
- /*
- * Purge or update corresponding prefix
- *
- * 1) we don't purge prefix here if address was not permanent.
- * prefix is managed by its own lifetime.
- * 2) if there're no addresses, delete prefix.
- * 3) if there're still other permanent address(es),
- * corresponding prefix is still permanent.
- * 4) otherwise, update prefix lifetime to the
- * longest valid lifetime among the corresponding
- * addresses on the device.
- * Note: subsequent RA will update lifetime.
- *
- * --yoshfuji
- */
- if ((ifp->flags & IFA_F_PERMANENT) && onlink < 1) {
- struct rt6_info *rt;
-
- rt = addrconf_get_prefix_route(&ifp->addr,
- ifp->prefix_len,
- ifp->idev->dev,
- 0, RTF_GATEWAY | RTF_DEFAULT);
-
- if (rt) {
- if (onlink == 0) {
- ip6_del_rt(rt);
- rt = NULL;
- } else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
- rt6_set_expires(rt, expires);
- }
- }
- ip6_rt_put(rt);
- }
+ cleanup_prefix_route(ifp, expires, onlink);
/* clean up prefsrc entries */
rt6_remove_prefsrc(ifp);
@@ -3632,6 +3657,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
clock_t expires;
unsigned long timeout;
bool was_managetempaddr;
+ bool was_noprefixroute;
if (!valid_lft || (prefered_lft > valid_lft))
return -EINVAL;
@@ -3660,6 +3686,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
spin_lock_bh(&ifp->lock);
was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
+ was_noprefixroute = ifp->flags & IFA_F_NOPREFIXROUTE;
ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE);
ifp->flags |= ifa_flags;
@@ -3674,6 +3701,15 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
expires, flags);
+ } else if (was_noprefixroute) {
+ int onlink;
+ unsigned long expires;
+
+ write_lock_bh(&ifp->idev->lock);
+ onlink = check_cleanup_prefix_routes(ifp, ifp->flags & ~IFA_F_NOPREFIXROUTE, &expires);
+ write_unlock_bh(&ifp->idev->lock);
+
+ cleanup_prefix_route(ifp, expires, onlink);
}
if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 1/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes
From: Thomas Haller @ 2014-01-07 14:39 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <1389105553-21230-1-git-send-email-thaller@redhat.com>
When adding/modifying an IPv6 address, the userspace application needs
a way to suppress adding a prefix route. This is for example relevant
together with IFA_F_MANAGERTEMPADDR, where userspace creates autoconf
generated addresses, but depending on on-link, no route for the
prefix should be added.
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
include/uapi/linux/if_addr.h | 1 +
net/ipv6/addrconf.c | 18 ++++++++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index cfed10b..dea10a8 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -49,6 +49,7 @@ enum {
#define IFA_F_TENTATIVE 0x40
#define IFA_F_PERMANENT 0x80
#define IFA_F_MANAGETEMPADDR 0x100
+#define IFA_F_NOPREFIXROUTE 0x200
struct ifa_cacheinfo {
__u32 ifa_prefered;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 31f75ea..1bc575f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2433,8 +2433,11 @@ static int inet6_addr_add(struct net *net, int ifindex,
valid_lft, prefered_lft);
if (!IS_ERR(ifp)) {
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
- expires, flags);
+ if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
+ expires, flags);
+ }
+
/*
* Note that section 3.1 of RFC 4429 indicates
* that the Optimistic flag should not be set for
@@ -3658,7 +3661,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
spin_lock_bh(&ifp->lock);
was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
- IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
+ IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE);
ifp->flags |= ifa_flags;
ifp->tstamp = jiffies;
ifp->valid_lft = valid_lft;
@@ -3668,8 +3671,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
if (!(ifp->flags&IFA_F_TENTATIVE))
ipv6_ifa_notify(0, ifp);
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
- expires, flags);
+ if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
+ expires, flags);
+ }
if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR))
@@ -3723,7 +3728,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
/* We ignore other flags so far. */
- ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR;
+ ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
+ IFA_F_NOPREFIXROUTE;
ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
if (ifa == NULL) {
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes
From: Thomas Haller @ 2014-01-07 14:39 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <1389029375-17698-1-git-send-email-thaller@redhat.com>
Now, the IFA_F_NOPREFIXROUTE flag is saved in ifp->flags.
The second patch reworks the deletion of addresses/cleanup of prefix
routes and considers IFA_F_NOPREFIXROUTE.
Thomas Haller (2):
ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
IP6 routes
ipv6 addrconf: don't cleanup route prefix for IFA_F_NOPREFIXROUTE
include/uapi/linux/if_addr.h | 1 +
net/ipv6/addrconf.c | 206 ++++++++++++++++++++++++++-----------------
2 files changed, 125 insertions(+), 82 deletions(-)
--
1.8.4.2
^ permalink raw reply
* Re: linux-next: manual merge of the bluetooth tree with the net/net-next trees
From: Daniel Borkmann @ 2014-01-07 14:37 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Gustavo Padovan, David Miller, netdev, linux-next, linux-kernel,
Jukka Rissanen
In-Reply-To: <20140107125418.986ae20298ddc55e5e657447@canb.auug.org.au>
On 01/07/2014 02:54 AM, Stephen Rothwell wrote:
> Hi Gustavo,
>
> Today's linux-next merge of the bluetooth tree got a conflict in
> net/ieee802154/6lowpan.c between commit 965801e1eb62 ("net: 6lowpan: fix
> lowpan_header_create non-compression memcpy call") from the tree and
> commit 8df8c56a5abc ("6lowpan: Moving generic compression code into
> 6lowpan_iphc.c") from the bluetooth tree.
>
> I fixed it up (I applied the following patch to move the fix into
> 6lowpan_iphc.c) and can carry the fix as necessary (no action is
> required).
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 7 Jan 2014 12:52:43 +1100
> Subject: [PATCH] net: 6lowpan: fixup for code movement
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Looks good to me, thanks!
> ---
> net/ieee802154/6lowpan_iphc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
> index 11840f9e46da..e14fe8b2c054 100644
> --- a/net/ieee802154/6lowpan_iphc.c
> +++ b/net/ieee802154/6lowpan_iphc.c
> @@ -677,7 +677,7 @@ int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
> hc06_ptr += 3;
> } else {
> /* compress nothing */
> - memcpy(hc06_ptr, &hdr, 4);
> + memcpy(hc06_ptr, hdr, 4);
> /* replace the top byte with new ECN | DSCP format */
> *hc06_ptr = tmp;
> hc06_ptr += 4;
>
^ permalink raw reply
* Re: [PATCH 0/2] Display adjacent switch port's attributes
From: Stefan Raspl @ 2014-01-07 14:28 UTC (permalink / raw)
To: Stephen Hemminger, David Miller
Cc: John Fastabend, Florian Fainelli, David Miller, Ben Hutchings,
blaschka, netdev, linux-s390
In-Reply-To: <52AF1CFF.7040004@linux.vnet.ibm.com>
On 12/16/2013 04:32 PM, Stefan Raspl wrote:
> Am 12.12.2013 22:47, schrieb John Fastabend:
>> [...]
>>
>>>> Just to elaborate...
>>>>
>>>> Any application using lldp information will want to get events when
>>>> TLVs change. Maybe you can contrive ethtool to do this but its
>>>> going to
>>>> be ugly. Netlink can support multicast events and applications can
>>>> register for them. Also netlink's TLV format matches nicely with
>>>> LLDPs
>>>> TLV format.
>>>
>>> ethtool and netlink usually intersect for a few bits of information
>>> such as link status for instance. It is useful to have this
>>> information twice, with ethtool as a debugging aid, and via
>>> netlink to
>>> take appropriate actions.
>>>
>>> Maybe we just need to be clear on what needs to be present in ethtool
>>> only (configuration, static information) and see on a case-by-case
>>> what needs to be present in both ethtool and netlink?
>>>
>>
>> OK if there is an enable/disable bit in ethtool that might make some
>> sense. Or an error flag that is helpful to have.
>>
>> In this case we are dealing with peer attributes which are dynamic and
>> in my opinion should go into netlink and duplicating them in ethtool
>> although possible doesn't seem very useful to me.
>
> I think what most folks in the discussion so far assume is that we
> have a full LLDP implementation with respective hooks and events.
> This is not true for our device: We can only poll it for the
> adjacent link port's state, and that's it. I.e. we don't receive any
> events for changes on the port's state.
> lldpad seems to handle the entire LLDP layer in software. And I'm
> not sure if our device integrates with that so well, as it handles
> LLDP on its own. We'd have to disable pretty much all functionality
> that lldpad offers, and limit support to display of a few parameters
> which we would have to poll on demand.
> Likewise with netlink: If one of the arguments for netlink is that
> it supports notifications, then we can't take any advantage of that
> either, for the reasons stated above.
> By its nature, what we can offer and support with our device is
> simple debugging functionality, displaying the current state of the
> switch port at a given moment - just like ethtool will display the
> current port speed and media type. Hence the original idea to add
> respective functionality to ethtool in a generic manner, so others
> with similar constraints could use it as well.
> If ethtool is not acceptable, and if lldpad and netlink are no good
> fits either, would respective sysfs attributes for our device type work?
Since I didn't receive any further feedback, please let me summarize:
* As elaborated in my most recent reply (cited above), our device
does not provide for any notifications regarding LLDP-related
events - we can only query the current state. Hence we could not
take advantage of a netlink interface. Plus even if we still went
with netlink, we'd have to introduce yet another userspace tool
just for querying the current state.
* For the same reason, lldpad would be hard to integrate with,
since all we can do is to query a limited amount of information,
where lldpad seems to be targeted at devices that can provide
events.
* Integration with ethtool was our attempt at providing a common
interface for our and other devices with similar characteristics
regarding LLDP, since ethtool is semantically a good fit.
However, it was indicated that this is not desirable.
* It seems that the only choice left is to implement sysfs
attributes to query LLDP-related attributes. Any other device
with similar characteristics would probably need to re-do the
same functionality independently. Is this really what we want to
do?
Please let me know what you think.
Regards,
Stefan
^ permalink raw reply
* [PATCH net-next v5] IPv6: add the option to use anycast addresses as source addresses in echo reply
From: Francois-Xavier Le Bail @ 2014-01-07 13:57 UTC (permalink / raw)
To: netdev
Cc: Hannes Frederic Sowa, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki Yoshifuji, Patrick McHardy,
Francois-Xavier Le Bail
This change allows to follow a recommandation of RFC4942.
- Add "anycast_src_echo_reply" sysctl to control the use of anycast addresses
as source addresses for ICMPv6 echo reply. This sysctl is false by default
to preserve existing behavior.
- Add inline check ipv6_anycast_destination().
- Use them in icmpv6_echo_reply().
Reference:
RFC4942 - IPv6 Transition/Coexistence Security Considerations
(http://tools.ietf.org/html/rfc4942#section-2.1.6)
2.1.6. Anycast Traffic Identification and Security
[...]
To avoid exposing knowledge about the internal structure of the
network, it is recommended that anycast servers now take advantage of
the ability to return responses with the anycast address as the
source address if possible.
Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
---
v4: update Subject and Documentation, this work also with anycast addresses
created via API, not just with Subnet-Router anycast addresses.
v5: alternative way, replace ipv6_chk_acast_addr() test by
ipv6_anycast_destination() test.
Documentation/networking/ip-sysctl.txt | 7 +++++++
include/net/ip6_route.h | 7 +++++++
include/net/netns/ipv6.h | 1 +
net/ipv6/icmp.c | 4 +++-
net/ipv6/sysctl_net_ipv6.c | 8 ++++++++
5 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index d71afa8..7373115 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1094,6 +1094,13 @@ bindv6only - BOOLEAN
Default: FALSE (as specified in RFC3493)
+anycast_src_echo_reply - BOOLEAN
+ Controls the use of anycast addresses as source addresses for ICMPv6
+ echo reply
+ TRUE: enabled
+ FALSE: disabled
+ Default: FALSE
+
IPv6 Fragmentation:
ip6frag_high_thresh - INTEGER
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 1fb6cdd..017badb 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -152,6 +152,13 @@ static inline bool ipv6_unicast_destination(const struct sk_buff *skb)
return rt->rt6i_flags & RTF_LOCAL;
}
+static inline bool ipv6_anycast_destination(const struct sk_buff *skb)
+{
+ struct rt6_info *rt = (struct rt6_info *) skb_dst(skb);
+
+ return rt->rt6i_flags & RTF_ANYCAST;
+}
+
int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *));
static inline int ip6_skb_dst_mtu(struct sk_buff *skb)
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 0fb2401..76fc7d1 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -73,6 +73,7 @@ struct netns_ipv6 {
#endif
atomic_t dev_addr_genid;
atomic_t rt_genid;
+ int anycast_src_echo_reply;
};
#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 5d42009..9a809a4 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
saddr = &ipv6_hdr(skb)->daddr;
- if (!ipv6_unicast_destination(skb))
+ if (!ipv6_unicast_destination(skb) &&
+ !(net->ipv6.anycast_src_echo_reply &&
+ ipv6_anycast_destination(skb)))
saddr = NULL;
memcpy(&tmp_hdr, icmph, sizeof(tmp_hdr));
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 107b2f1..6b6a2c8 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -24,6 +24,13 @@ static struct ctl_table ipv6_table_template[] = {
.mode = 0644,
.proc_handler = proc_dointvec
},
+ {
+ .procname = "anycast_src_echo_reply",
+ .data = &init_net.ipv6.anycast_src_echo_reply,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec
+ },
{ }
};
@@ -51,6 +58,7 @@ static int __net_init ipv6_sysctl_net_init(struct net *net)
if (!ipv6_table)
goto out;
ipv6_table[0].data = &net->ipv6.sysctl.bindv6only;
+ ipv6_table[1].data = &net->ipv6.anycast_src_echo_reply;
ipv6_route_table = ipv6_route_sysctl_init(net);
if (!ipv6_route_table)
^ permalink raw reply related
* Re: single process receives own frames due to PACKET_MMAP
From: Jesper Dangaard Brouer @ 2014-01-07 14:09 UTC (permalink / raw)
To: Norbert van Bolhuis
Cc: Jesper Dangaard Brouer, Daniel Borkmann, netdev, David Miller,
uaca
In-Reply-To: <52CBFE13.8@aimvalley.nl>
On Tue, 07 Jan 2014 14:16:03 +0100
Norbert van Bolhuis <nvbolhuis@aimvalley.nl> wrote:
> On 01/07/14 11:06, Jesper Dangaard Brouer wrote:
> > On Tue, 07 Jan 2014 10:32:01 +0100
> > Daniel Borkmann<dborkman@redhat.com> wrote:
> >
> >> On 01/06/2014 11:58 PM, Norbert van Bolhuis wrote:
> >>>
[...]
> >>
> >>> I'd say it makes no sense to make the same process receive its
> >>> own transmitted frames on that same interface (unless its lo).
> >
> > Have you setup:
> > ring->s_ll.sll_protocol = 0
> >
> > This is what I did in trafgen to avoid this problem.
> >
> > See line 55 in netsniff-ng/ring.c:
> > https://github.com/borkmann/netsniff-ng/blob/c3602a995b21e8133c7f4fd1fb1e7e21b6a844f1/ring.c#L55
> >
> > Commit:
> > https://github.com/borkmann/netsniff-ng/commit/c3602a995b21e8133c7f4fd1fb1e7e21b6a844f1
> >
>
>
> No I did not do that, I was checking my code against netsniff-ng-0.5.8-rc4.
>
> But I just tried it, I believe I do the same as netsniff-ng-0.5.8-rc5, but it doesn't
> work for me. Maybe because I have an old FC14 system (kernel 2.6.35.14-106.fc14.x86_64).
>
> So I tried to see whether netsniff-ng-0.5.8-rc5/trafgen still makes the
> kernel call packet_rcv() on my FC14 system. So I build and run it, but I'm not sure
> how to (easily) check that.
The easiest way is to:
cat /proc/net/ptype
And look if someone registered a proto handler/function: packet_rcv (or tpacket_rcv).
The more exact method is, to run "perf record -a -g" and then look (at
the result with "perf report") for a lock contention, and "expand" the
spin_lock and see if packet_rcv() is calling this spin lock.
> In anyway, Wireshark does capture the trafgen generated
> frames, does that say anything ?
Be careful not to start a wireshark/tcpdump, at the sametime, as this
will slow you down.
> In the future, I can at least use PACKET_QDISC_BYPASS as a "workaround".
And in the future with PACKET_QDISC_BYPASS, your wireshark will not
catch these packets, remember that.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
From: François-Xavier Le Bail @ 2014-01-07 14:03 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David Miller, netdev, kuznet, jmorris, yoshfuji, kaber
In-Reply-To: <20140107130101.GD24730@order.stressinduktion.org>
On Tue, 1/7/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
>> Anyway, although I think that this solution is valid, I
>> am testing another way to do this change.
> Ok, thanks for explaining, I see now, of course.
> Maybe we could just do
> (ipv6_addr_type(addr) & IPV6_ADDR_LINKLOCAL) ? skb->dev : NULL?
> I guess the NULL solution would be ok now, too. You can
> decide. I just think we can be a bit more defensive here with no additional
> cost. Routing table behaviour is pretty complicated and maybe can change
> in future.
I see.
first solution : I add this "defensive" update.
second solution : the v5 patch I will send soon.
Thanks for the review.
François-Xavier
^ permalink raw reply
* Re: [PATCH v3 13/19] media: dvb_core: slight optimization of addr compare
From: Mauro Carvalho Chehab @ 2014-01-07 13:54 UTC (permalink / raw)
To: Ding Tianhong
Cc: Sergei Shtylyov, linux-media, linux-kernel@vger.kernel.org,
Netdev
In-Reply-To: <52BC0E56.80003@huawei.com>
Em Thu, 26 Dec 2013 19:09:10 +0800
Ding Tianhong <dingtianhong@huawei.com> escreveu:
> On 2013/12/25 18:57, Sergei Shtylyov wrote:
> > Hello.
> >
> > On 25-12-2013 7:29, Ding Tianhong wrote:
> >
> >> Use possibly more efficient ether_addr_equal
> >> instead of memcmp.
> >
> >> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> >> Cc: linux-media@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> >> ---
> >> drivers/media/dvb-core/dvb_net.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> >> diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
> >> index f91c80c..3dfc33b 100644
> >> --- a/drivers/media/dvb-core/dvb_net.c
> >> +++ b/drivers/media/dvb-core/dvb_net.c
> >> @@ -179,7 +179,7 @@ static __be16 dvb_net_eth_type_trans(struct sk_buff *skb,
> >> eth = eth_hdr(skb);
> >>
> >> if (*eth->h_dest & 1) {
> >> - if(memcmp(eth->h_dest,dev->broadcast, ETH_ALEN)==0)
> >> + if(ether_addr_equal(eth->h_dest,dev->broadcast))
> >
> > There should be space after comma.
> >
> >> @@ -674,11 +674,11 @@ static void dvb_net_ule( struct net_device *dev, const u8 *buf, size_t buf_len )
> >> if (priv->rx_mode != RX_MODE_PROMISC) {
> >> if (priv->ule_skb->data[0] & 0x01) {
> >> /* multicast or broadcast */
> >> - if (memcmp(priv->ule_skb->data, bc_addr, ETH_ALEN)) {
> >> + if (!ether_addr_equal(priv->ule_skb->data, bc_addr)) {
> >> /* multicast */
> >> if (priv->rx_mode == RX_MODE_MULTI) {
> >> int i;
> >> - for(i = 0; i < priv->multi_num && memcmp(priv->ule_skb->data, priv->multi_macs[i], ETH_ALEN); i++)
> >> + for(i = 0; i < priv->multi_num && !ether_addr_equal(priv->ule_skb->data, priv->multi_macs[i]); i++)
> >
> > Shouldn't this line be broken?
> >
>
> ok, thanks.
Also, since you're touching on those lines, could you please add an space
after 'if' (on the first hunk) and after 'for' (on the second one)?
>
> Regards
> >> ;
> >> if (i == priv->multi_num)
> >> drop = 1;
> >
> > WBR, Sergei
> >
> >
> >
> >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks,
Mauro
^ 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