* [PATCH next] netdevice: shrink size of struct netdev_queue
From: Florian Westphal @ 2016-05-03 1:29 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
- trans_timeout is incremented when tx queue timed out (tx watchdog).
- tx_maxrate is set via sysfs
Moving tx_maxrate to read-mostly part shrinks the struct by 64 bytes.
While at it, also move trans_timeout (it is out-of-place in the
'write-mostly' part).
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/linux/netdevice.h | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 52914a8..f218259 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -569,6 +569,12 @@ struct netdev_queue {
#if defined(CONFIG_XPS) && defined(CONFIG_NUMA)
int numa_node;
#endif
+ unsigned long tx_maxrate;
+ /*
+ * Number of TX timeouts for this queue
+ * (/sys/class/net/DEV/Q/trans_timeout)
+ */
+ unsigned long trans_timeout;
/*
* write-mostly part
*/
@@ -579,18 +585,11 @@ struct netdev_queue {
*/
unsigned long trans_start;
- /*
- * Number of TX timeouts for this queue
- * (/sys/class/net/DEV/Q/trans_timeout)
- */
- unsigned long trans_timeout;
-
unsigned long state;
#ifdef CONFIG_BQL
struct dql dql;
#endif
- unsigned long tx_maxrate;
} ____cacheline_aligned_in_smp;
static inline int netdev_queue_numa_node_read(const struct netdev_queue *q)
--
2.7.3
^ permalink raw reply related
* Re: [PATCH net 2/3] udp_offload: Set encapsulation before inner completes.
From: Alexander Duyck @ 2016-05-03 1:22 UTC (permalink / raw)
To: Tom Herbert; +Cc: Jarno Rajahalme, Netdev, Jesse Gross
In-Reply-To: <CALx6S35QFms9KUE1K4c5odN7QCuvpBaeeL=yjfMBFxqFfcTSWg@mail.gmail.com>
On Mon, May 2, 2016 at 4:18 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, May 2, 2016 at 3:54 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, May 2, 2016 at 2:21 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>>>
>>>> On Apr 29, 2016, at 8:46 PM, Alexander Duyck <alexander.duyck@gmail.com> wrote:
>>>>
>>>> On Fri, Apr 29, 2016 at 3:28 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>>>>> UDP tunnel segmentation code relies on the inner offsets being set for
>>>>> an UDP tunnel GSO packet. The inner *_complete() functions will set
>>>>> the inner offsets only if the encapsulation is set before calling
>>>>> them, but udp_gro_complete() set it only after the inner *_complete()
>>>>> functions were done.
>>>>>
>>>>> Also, remove the setting of the inner_mac_header in udp_gro_complete()
>>>>> as it was wrongly set to the beginning of the UDP tunnel header rather
>>>>> than the inner MAC header.
>>>>>
>>>>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>>>>> ---
>>>>> net/ipv4/udp_offload.c | 8 +++++---
>>>>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>>>>> index 0ed2daf..e330c0e 100644
>>>>> --- a/net/ipv4/udp_offload.c
>>>>> +++ b/net/ipv4/udp_offload.c
>>>>> @@ -399,6 +399,11 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>>>>>
>>>>> uh->len = newlen;
>>>>>
>>>>> + /* Set encapsulation before calling into inner gro_complete() functions
>>>>> + * to make them set up the inner offsets.
>>>>> + */
>>>>> + skb->encapsulation = 1;
>>>>> +
>>>>> rcu_read_lock();
>>>>>
>>>>> uo_priv = rcu_dereference(udp_offload_base);
>>>>> @@ -421,9 +426,6 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>>>>> if (skb->remcsum_offload)
>>>>> skb_shinfo(skb)->gso_type |= SKB_GSO_TUNNEL_REMCSUM;
>>>>>
>>>>> - skb->encapsulation = 1;
>>>>> - skb_set_inner_mac_header(skb, nhoff + sizeof(struct udphdr));
>>>>> -
>>>>> return err;
>>>>> }
>>>>>
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>
>>>> So looking over this I think this patch is just a band-aid and it
>>>> isn't really fixing much of anything. For example I cannot see where
>>>
>>> It fixes the immediate bug/problem that segmentation of GRO UDP tunnel packet fails due to incorrectly set inner MAC offset and/or missing inner network offset.
>>
>> The problem is I think this will introduce other issues as there are
>> now situations where the inner MAC offset never gets set. For example
>> did you try testing with GUE carrying either an IPIP or SIT tunnel?
>>
>> From what I can tell it looks like you are trying to fix VXLAN and
>> similar offloads, and Tom is focused on GUE. What you will probably
>> need to do is add some code to ipip_gro_complete and sit_gro_complete
>> so that if skb->encapsulation is set before you call the function it
>> will set the inner MAC offset to the same offset as the inner network
>> header. Otherwise you run the risk of screwing up segmentation
>> because the UDP tunnel segment code will use the inner mac header
>> offset when computing the tunnel header length and get it completely
>> wrong.
>>
>>>> the inner transport offset is ever set. From what I can tell it never
>>>> is.
>>>>
>>>
>>> inner transport offset is set in validate_xmit_skb(), but you are right, it is not set by the GRO complete code path. AFAIK no-one uses it before reaching validate_xmit_skb(), though. The documentation in (net-next) Documentation/networking/segmentation-offloads.txt could be fixed by removing the inner transport header from UDP/GRE tunnel case, as it is not used by either the GRE or UDP tunnel code.
>>
>> Actually that is an interesting point. Right now we need to keep the
>> inner transport header around because that is getting set. If we were
>> to enforce the use of csum_start as the inner transport header offset,
>> then we could assume that transport_header is always the outer-most
>> transport header. We might be able to drop a bit of space from the
>> skbuff by getting rid of it. If nothing else it means one less header
>> offset we would have to copy.
>>
> Or can we just eliminate the inner headers altogether? IIRC the only
> sticking point for GSO needing this was ipid, all the inner headers
> are just copied per packet assuming segment lengths are the same. Is
> this possible yet?
Actually the big two we need are the inner MAC offset and the inner
network offset. We end up needing the MAC header in order to be able
to UDP checksum offloads because that is how we are currently
measuring the length of the header itself. As you said we end up
needing the inner network header offset in order to handle the inner
IP ID field and checksum.
I was doing a bit of digging and found that the whole reason for
csum_start is essentially for the same use cases as the
inner_transport_header. It was a means of storing off the pointer to
the transport header in the event that you have a packet that goes
through some receive processing that we then have to transmit. On top
of that the only two spots where we actually set it are in
skb_reset_inner_headers which should only be called if we are
performing an offload of some sort on the inner headers, and
validate_xmit_skb when we are supposed to be performing a checksum and
in that case we just overwrite the inner_transport_header offset
anyway.
- Alex
^ permalink raw reply
* Re: [PATCH net-next] fq_codel: add batch ability to fq_codel_drop()
From: Dave Taht @ 2016-05-03 1:07 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Eric Dumazet, David Miller, netdev, Jonathan Morton
In-Reply-To: <20160502191543.170fc046@redhat.com>
I have duplicated the issue on my own hardware. I would like to
explore also upping the codel count in this scenario at some point,
but:
Acked-by: Dave Taht
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: hns: update DT properties according to Rob's comments
From: Yisen Zhuang @ 2016-05-03 0:54 UTC (permalink / raw)
To: David Miller
Cc: devicetree, netdev, linux-arm-kernel, robh+dt, pawel.moll,
mark.rutland, ijc+devicetree, galak, will.deacon, catalin.marinas,
yankejian, huangdaode, salil.mehta, lipeng321, liguozhu,
xieqianqian, xuwei5, linuxarm
In-Reply-To: <20160429.133949.1719521222260369497.davem@davemloft.net>
Hi David,
Thanks for applying the patches.
The series which i reposted is my misoperation. Please ignore them.
Thanks,
Yisen
在 2016/4/30 1:39, David Miller 写道:
> From: Yisen Zhuang <Yisen.Zhuang@huawei.com>
> Date: Thu, 28 Apr 2016 15:09:00 +0800
>
>> There are some inappropriate properties definition in hns DT. We update the definition
>> according to Rob's review comments and fix some typos in binding.
>>
>> For more details, please see individual patches.
>
> Series applied, thanks.
>
> BTW, you reposted your 10 part series for adding the dsaf-debug device, but I
> already applied the series you posted for that nearly a week ago.
>
> .
>
^ permalink raw reply
* Re: [iproute2 1/1] man: tc-ife.8: man page for ife action
From: Jamal Hadi Salim @ 2016-05-02 23:40 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, phil, lucasb
In-Reply-To: <20160502151032.5701009e@xeon-e3>
On 16-05-02 06:10 PM, Stephen Hemminger wrote:
> On Sat, 30 Apr 2016 06:58:04 -0400
> Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
>> From: Lucas Bates <lucasb@mojatatu.com>
>>
>> Signed-off-by: Lucas Bates <lucasb@mojatatu.com>
>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Still waiting for a you to respond to my earlier suggestions about tc-ife.
>
I did respond.
> Last version I saw was expecting header in include/linux which is not in
> kernel source.
>
> ~/kernel/net-next$ ls include/uapi/linux/tc_ife.h
> ls: cannot access include/uapi/linux/tc_ife.h: No such file or directory
>
What version of net-next are you looking at? This is what i see:
linux-gits/net-next$ ls include/uapi/linux/tc_act/
Kbuild tc_csum.h tc_ife.h tc_nat.h tc_vlan.h
tc_bpf.h tc_defact.h tc_ipt.h tc_pedit.h
tc_connmark.h tc_gact.h tc_mirred.h tc_skbedit.h
> You need to get that header in right place, and get it exported correctly.
And I saw you sending a patch which tried to export everything. What
did i miss? ;-> If that didnt make i will send one just for IFE.
cheers,
jamal
PS:- I have other patches which fix a small issue with some actions
(which include ife) - but they are depending on this other one going
in.
^ permalink raw reply
* Re: [PATCH next-next 0/7] net: Cleanup IPv6 ip tunnels
From: David Miller @ 2016-05-02 23:23 UTC (permalink / raw)
To: tom; +Cc: netdev, kernel-team
In-Reply-To: <1461975141-954269-1-git-send-email-tom@herbertland.com>
From: Tom Herbert <tom@herbertland.com>
Date: Fri, 29 Apr 2016 17:12:14 -0700
> The IPv6 tunnel code is very different from IPv4 code. There is a lot
> of redundancy with the IPv4 code, particularly in the GRE tunneling.
>
> This patch set cleans up the tunnel code to make the IPv6 code look
> more like the IPv4 code and use common functions between the two
> stacks where possible.
Series applied, nice work Tom.
^ permalink raw reply
* Re: [PATCH net 2/3] udp_offload: Set encapsulation before inner completes.
From: Tom Herbert @ 2016-05-02 23:18 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Jarno Rajahalme, Netdev, Jesse Gross
In-Reply-To: <CAKgT0Uf-dvDj2KDnt4GZoMJONMC6LgLBtEUcsO0JJ1jqiFDfaw@mail.gmail.com>
On Mon, May 2, 2016 at 3:54 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, May 2, 2016 at 2:21 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>>
>>> On Apr 29, 2016, at 8:46 PM, Alexander Duyck <alexander.duyck@gmail.com> wrote:
>>>
>>> On Fri, Apr 29, 2016 at 3:28 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>>>> UDP tunnel segmentation code relies on the inner offsets being set for
>>>> an UDP tunnel GSO packet. The inner *_complete() functions will set
>>>> the inner offsets only if the encapsulation is set before calling
>>>> them, but udp_gro_complete() set it only after the inner *_complete()
>>>> functions were done.
>>>>
>>>> Also, remove the setting of the inner_mac_header in udp_gro_complete()
>>>> as it was wrongly set to the beginning of the UDP tunnel header rather
>>>> than the inner MAC header.
>>>>
>>>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>>>> ---
>>>> net/ipv4/udp_offload.c | 8 +++++---
>>>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>>>> index 0ed2daf..e330c0e 100644
>>>> --- a/net/ipv4/udp_offload.c
>>>> +++ b/net/ipv4/udp_offload.c
>>>> @@ -399,6 +399,11 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>>>>
>>>> uh->len = newlen;
>>>>
>>>> + /* Set encapsulation before calling into inner gro_complete() functions
>>>> + * to make them set up the inner offsets.
>>>> + */
>>>> + skb->encapsulation = 1;
>>>> +
>>>> rcu_read_lock();
>>>>
>>>> uo_priv = rcu_dereference(udp_offload_base);
>>>> @@ -421,9 +426,6 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>>>> if (skb->remcsum_offload)
>>>> skb_shinfo(skb)->gso_type |= SKB_GSO_TUNNEL_REMCSUM;
>>>>
>>>> - skb->encapsulation = 1;
>>>> - skb_set_inner_mac_header(skb, nhoff + sizeof(struct udphdr));
>>>> -
>>>> return err;
>>>> }
>>>>
>>>> --
>>>> 2.7.4
>>>>
>>>
>>> So looking over this I think this patch is just a band-aid and it
>>> isn't really fixing much of anything. For example I cannot see where
>>
>> It fixes the immediate bug/problem that segmentation of GRO UDP tunnel packet fails due to incorrectly set inner MAC offset and/or missing inner network offset.
>
> The problem is I think this will introduce other issues as there are
> now situations where the inner MAC offset never gets set. For example
> did you try testing with GUE carrying either an IPIP or SIT tunnel?
>
> From what I can tell it looks like you are trying to fix VXLAN and
> similar offloads, and Tom is focused on GUE. What you will probably
> need to do is add some code to ipip_gro_complete and sit_gro_complete
> so that if skb->encapsulation is set before you call the function it
> will set the inner MAC offset to the same offset as the inner network
> header. Otherwise you run the risk of screwing up segmentation
> because the UDP tunnel segment code will use the inner mac header
> offset when computing the tunnel header length and get it completely
> wrong.
>
>>> the inner transport offset is ever set. From what I can tell it never
>>> is.
>>>
>>
>> inner transport offset is set in validate_xmit_skb(), but you are right, it is not set by the GRO complete code path. AFAIK no-one uses it before reaching validate_xmit_skb(), though. The documentation in (net-next) Documentation/networking/segmentation-offloads.txt could be fixed by removing the inner transport header from UDP/GRE tunnel case, as it is not used by either the GRE or UDP tunnel code.
>
> Actually that is an interesting point. Right now we need to keep the
> inner transport header around because that is getting set. If we were
> to enforce the use of csum_start as the inner transport header offset,
> then we could assume that transport_header is always the outer-most
> transport header. We might be able to drop a bit of space from the
> skbuff by getting rid of it. If nothing else it means one less header
> offset we would have to copy.
>
Or can we just eliminate the inner headers altogether? IIRC the only
sticking point for GSO needing this was ipid, all the inner headers
are just copied per packet assuming segment lengths are the same. Is
this possible yet?
Tom
> - Alex
^ permalink raw reply
* Re: [PATCHv2 bluetooth-next 00/10] 6lowpan: introduce basic 6lowpan-nd
From: Marcel Holtmann @ 2016-05-02 23:17 UTC (permalink / raw)
To: Hannes Frederic Sowa, David S. Miller
Cc: Alexander Aring, linux-wpan-u79uwXL29TY76Z2rM5mHXA,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Jukka Rissanen, Stefan Schmidt,
mcr-SWp7JaYWvAQV+D8aMU/kSg, Werner Almesberger, Linux Bluetooth,
Network Development, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <870fdf5f-82ed-b621-a8d5-625bf9d81d0d-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org>
Hi Hannes,
>>> this patch series introduces a layer for IPv6 neighbour discovery. At first
>>> it introduce the "ndisc_ops" to run a different handling for recv/send of
>>> NA/NS messages. The use case for such ndisc operation is RFC 6775 [0].
>>> Which describes a different neighbour discovery handling for 6LoWPAN networks.
>>>
>>> I didn't implement RFC 6775 in this patch series, but introduce callback
>>> structure for replace different functions in ndisc implementation might be
>>> the right direction.
>>>
>>> Another use case would be RFC 7400 [1] which describes a new option field to
>>> getting capabilities of 6LoWPAN next header compression methods.
>>>
>>> What I implemented is a necessary functionality to handle short address for
>>> 802.15.4 6LoWPAN networks. The L2-Layer "802.15.4" can have two different
>>> link-layer addresses which can be used mixed at the same time inside 802.15.4
>>> networks. To deal with such behaviour in ndisc, it is defined at RFC 4944 [2].
>>> The bad news is, that I saw different handling of such handling. What Linux
>>> will do is to add two source/target address information option fields, each
>>> with different length, if short address is valid (can also not be given).
>>> Example:
>>>
>>> - WPAN interface address settings
>>> - extended addr (must always be there)
>>> - short addr (0xfffe or 0xffff -> invalid)
>>>
>>> Will add an extended addr to source/target address information option field.
>>> If short addr is in some valid range, then both address will be added to
>>> the option fields. Indicated are these different address types by the length
>>> field (extended -> length=2, short -> length=1), according to [1].
>>>
>>> The tested 6LoWPAN implementation (RIOT-OS) allows only one source/target
>>> option field which is short XOR extended, otherwise it will be dropped.
>>> There is some lack of information there [2] and I don't know how do deal with
>>> it right, maybe we need to update the implementation there if it's really
>>> wrong.
>>>
>>> To save such information for each neighbour we use the already implemented
>>> neighbour private data which some casting strategy for 6LoWPAN and 6LoWPAN
>>> link-layer specific data e.g. 802.15.4 short address handling.
>>>
>>> Additional I implemented to add 6CO to the is_useropt callback in case of
>>> 6LoWPAN interface. The 6CO option will currently parsed in userspace which
>>> are placed in RA-Messages.
>>>
>>> The ndisc_ops are not finished yet, of course we need handling for RS messages
>>> to place the 802.15.4 short address there as well and then also processing
>>> of RA messages for the 802.15.4 SLLAO option field.
>>>
>>> - Alex
>>>
>>> [0] https://tools.ietf.org/html/rfc6775
>>> [1] https://tools.ietf.org/html/rfc7400#section-3.3
>>> [2] https://tools.ietf.org/html/rfc4944#section-8
>>>
>>> Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>>> Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
>>> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
>>> Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
>>> Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
>>>
>>> changes since v2:
>>> - replace #ifdef CONFIG_IPV6 to #if IS_ENABLED(...)
>>> - replace #ifdef CONFIG_IEEE802154... to #if IS_ENABLED(...)
>>> - add more #if IS_ENABLED(CONFIG_IPV6) in ndisc.h
>>>
>>> Alexander Aring (10):
>>> 6lowpan: add private neighbour data
>>> 6lowpan: add 802.15.4 short addr slaac
>>> 6lowpan: remove ipv6 module request
>>> ndisc: add addr_len parameter to ndisc_opt_addr_space
>>> ndisc: add addr_len parameter to ndisc_opt_addr_data
>>> ndisc: add addr_len parameter to ndisc_fill_addr_option
>>> ipv6: introduce neighbour discovery ops
>>> ipv6: export ndisc functions
>>> 6lowpan: introduce 6lowpan-nd
>>> 6lowpan: add support for 802.15.4 short addr handling
>>>
>>> include/linux/netdevice.h | 6 +-
>>> include/net/6lowpan.h | 24 ++
>>> include/net/addrconf.h | 3 +
>>> include/net/ndisc.h | 124 ++++++++-
>>> net/6lowpan/6lowpan_i.h | 2 +
>>> net/6lowpan/Makefile | 2 +-
>>> net/6lowpan/core.c | 50 +++-
>>> net/6lowpan/iphc.c | 167 +++++++++--
>>> net/6lowpan/ndisc.c | 633 ++++++++++++++++++++++++++++++++++++++++++
>>> net/bluetooth/6lowpan.c | 2 +
>>> net/ieee802154/6lowpan/core.c | 12 +
>>> net/ieee802154/6lowpan/tx.c | 107 ++++---
>>> net/ipv6/addrconf.c | 7 +-
>>> net/ipv6/ndisc.c | 132 +++++----
>>> net/ipv6/route.c | 4 +-
>>> 15 files changed, 1117 insertions(+), 158 deletions(-)
>>> create mode 100644 net/6lowpan/ndisc.c
>>
>> is there a chance that we get input into this patch set? I wonder also if it would be acceptable to take this through bluetooth-next or should it better go straight into net-next?
>
> My proposal would be that the IPv6 patches go via net-next to reduce
> merge conflicts with maybe upcoming changes. If they are split up, they
> seem very much self contained and easy to review. The rest seems to be
> also very much self contained and can go in via bluetooth-next, then.
> What do you think?
I am actually fine with having this all go via net-next. We only have driver patches pending in bluetooth-next for the next merge window. Which means I can just pull net-next back into bluetooth-next at any time.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 0/2] sctp: Add GSO support
From: David Miller @ 2016-05-02 23:16 UTC (permalink / raw)
To: marcelo.leitner
Cc: netdev, vyasevich, nhorman, linux-sctp, David.Laight,
alexander.duyck
In-Reply-To: <cover.1461965035.git.marcelo.leitner@gmail.com>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Fri, 29 Apr 2016 18:33:31 -0300
> This patchset adds sctp GSO support.
>
> Performance tests indicates that increases throughput by 10% if using
> bigger chunk sizes, specially if bigger than MTU. For small chunks, it
> doesn't help much if not using heavy firewall rules.
>
> For small chunks it will probably be of more use once we get something
> like MSG_MORE as David Laight had suggested.
>
> I believe I could address all comments from the RFC attempt.
Are these packets idempotent?
Ie. if we GRO a bunch of SCTP frames on receive and that GRO frame is
forwarded rather than received locally, is the same exact packet
stream emitted on transmit?
^ permalink raw reply
* Re: [PATCH net 2/3] udp_offload: Set encapsulation before inner completes.
From: Alexander Duyck @ 2016-05-02 22:54 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: Netdev, Jesse Gross
In-Reply-To: <DB78A60B-3899-4767-AD0B-DCDCC65C04E1@ovn.org>
On Mon, May 2, 2016 at 2:21 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>
>> On Apr 29, 2016, at 8:46 PM, Alexander Duyck <alexander.duyck@gmail.com> wrote:
>>
>> On Fri, Apr 29, 2016 at 3:28 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>>> UDP tunnel segmentation code relies on the inner offsets being set for
>>> an UDP tunnel GSO packet. The inner *_complete() functions will set
>>> the inner offsets only if the encapsulation is set before calling
>>> them, but udp_gro_complete() set it only after the inner *_complete()
>>> functions were done.
>>>
>>> Also, remove the setting of the inner_mac_header in udp_gro_complete()
>>> as it was wrongly set to the beginning of the UDP tunnel header rather
>>> than the inner MAC header.
>>>
>>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>>> ---
>>> net/ipv4/udp_offload.c | 8 +++++---
>>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>>> index 0ed2daf..e330c0e 100644
>>> --- a/net/ipv4/udp_offload.c
>>> +++ b/net/ipv4/udp_offload.c
>>> @@ -399,6 +399,11 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>>>
>>> uh->len = newlen;
>>>
>>> + /* Set encapsulation before calling into inner gro_complete() functions
>>> + * to make them set up the inner offsets.
>>> + */
>>> + skb->encapsulation = 1;
>>> +
>>> rcu_read_lock();
>>>
>>> uo_priv = rcu_dereference(udp_offload_base);
>>> @@ -421,9 +426,6 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>>> if (skb->remcsum_offload)
>>> skb_shinfo(skb)->gso_type |= SKB_GSO_TUNNEL_REMCSUM;
>>>
>>> - skb->encapsulation = 1;
>>> - skb_set_inner_mac_header(skb, nhoff + sizeof(struct udphdr));
>>> -
>>> return err;
>>> }
>>>
>>> --
>>> 2.7.4
>>>
>>
>> So looking over this I think this patch is just a band-aid and it
>> isn't really fixing much of anything. For example I cannot see where
>
> It fixes the immediate bug/problem that segmentation of GRO UDP tunnel packet fails due to incorrectly set inner MAC offset and/or missing inner network offset.
The problem is I think this will introduce other issues as there are
now situations where the inner MAC offset never gets set. For example
did you try testing with GUE carrying either an IPIP or SIT tunnel?
>From what I can tell it looks like you are trying to fix VXLAN and
similar offloads, and Tom is focused on GUE. What you will probably
need to do is add some code to ipip_gro_complete and sit_gro_complete
so that if skb->encapsulation is set before you call the function it
will set the inner MAC offset to the same offset as the inner network
header. Otherwise you run the risk of screwing up segmentation
because the UDP tunnel segment code will use the inner mac header
offset when computing the tunnel header length and get it completely
wrong.
>> the inner transport offset is ever set. From what I can tell it never
>> is.
>>
>
> inner transport offset is set in validate_xmit_skb(), but you are right, it is not set by the GRO complete code path. AFAIK no-one uses it before reaching validate_xmit_skb(), though. The documentation in (net-next) Documentation/networking/segmentation-offloads.txt could be fixed by removing the inner transport header from UDP/GRE tunnel case, as it is not used by either the GRE or UDP tunnel code.
Actually that is an interesting point. Right now we need to keep the
inner transport header around because that is getting set. If we were
to enforce the use of csum_start as the inner transport header offset,
then we could assume that transport_header is always the outer-most
transport header. We might be able to drop a bit of space from the
skbuff by getting rid of it. If nothing else it means one less header
offset we would have to copy.
- Alex
^ permalink raw reply
* Performance suggestions for bridging module?
From: Ben Greear @ 2016-05-02 22:50 UTC (permalink / raw)
To: netdev
Hello!
I have a network emulator module that acts a lot like an ethernet bridge.
It is implemented roughly like this:
Hook into the rx logic and steal packets in the rx-all logic, similar to how sniffers
work.
Then, it puts the packet onto a queue for transmit.
A kernel thread services this queue transmitting frames on a different NIC.
I am using spin-locks to protect this queue.
I am disabling LRO/GRO etc on the ixgbe NICs so that I don't have
to deal with linearization when trying to do corruptions and such. Re-enabling
LRO/GRO makes the transmit logic use less CPU, but the RX logic is the bottleneck
anyway it seems.
The code, which is GPL, is here, in case someone wants to take a look:
http://www.candelatech.com/downloads/wanlink/
What I see is that this is very sensitive to which CPU core does what.
If I run the transmitter thread on cpu-0, performance is awful. If I run
it on 1, then it is good. Sometimes, though hard to reproduce, I can run
right at 10Gbps bi-directional throughput. More often, it is stuck at
around 7Gbps bi-directional throughput.
I tried adding some prefetch logic, and that helped when emulating very long
latency (like, 10 seconds worth), but not sure I am really doing that optimally
either.
My basic question is: Any suggestion for an optimal CPU core configuration
(most likely including binding a NIC's irqs to a particular core)??
Any other suggestions for things to look for?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] ip: add udp_csum, udp6_csum_tx, udp6_csum_rx control flags to ip l2tp add tunnel
From: Stephen Hemminger @ 2016-05-02 22:19 UTC (permalink / raw)
To: James Chapman; +Cc: Wang Shanker, netdev
In-Reply-To: <CAEwTi7SyKKh4de6TKYiuypWfOhzOCeD6k2uTLUfCmzQmvsNHWw@mail.gmail.com>
On Thu, 28 Apr 2016 15:50:47 +0100
James Chapman <jchapman@katalix.com> wrote:
> Yes, that looks like the problem.
>
> The comments in l2tp.h which indicate that the csum attributes are u8
> values are wrong. Code in net/l2tp/l2tp_netlink.c accesses these
> attributes using nla_get_flag().
>
> Please submit a patch to fix l2tp_tunnel_sock_create(). Include good
> change notes and your signed-off-by tag so that it gets reviewed. See
> Documentation/SubmittingPatches if you haven't submitted a kernel
> patch here before.
Thank you for fixing this. James is is correct.
Please format the patch according to the submission guidelines.
For example, checkpatch complains about current patch.
ERROR: code indent should use tabs where possible
#156: FILE: net/l2tp/l2tp_core.c:1379:
+ ! cfg->udp6_zero_tx_checksums;$
WARNING: please, no spaces at the start of a line
#156: FILE: net/l2tp/l2tp_core.c:1379:
+ ! cfg->udp6_zero_tx_checksums;$
ERROR: space prohibited after that '!' (ctx:ExW)
#156: FILE: net/l2tp/l2tp_core.c:1379:
+ ! cfg->udp6_zero_tx_checksums;
^
ERROR: code indent should use tabs where possible
#159: FILE: net/l2tp/l2tp_core.c:1381:
+ ! cfg->udp6_zero_rx_checksums;$
WARNING: please, no spaces at the start of a line
#159: FILE: net/l2tp/l2tp_core.c:1381:
+ ! cfg->udp6_zero_rx_checksums;$
ERROR: space prohibited after that '!' (ctx:ExW)
#159: FILE: net/l2tp/l2tp_core.c:1381:
+ ! cfg->udp6_zero_rx_checksums;
^
ERROR: Missing Signed-off-by: line(s)
I am sorry that maintainers may seem like picky teachers in school
always putting redline around spelling errors, but this is how we work to teach
others how to follow the process.
^ permalink raw reply
* Re: [PATCH iproute2] tc: add bash-completion function
From: Stephen Hemminger @ 2016-05-02 22:14 UTC (permalink / raw)
To: Quentin Monnet; +Cc: alexei.starovoitov, hadi, netdev, vincent.jardin
In-Reply-To: <1461918431-6518-1-git-send-email-quentin.monnet@6wind.com>
On Fri, 29 Apr 2016 10:27:11 +0200
Quentin Monnet <quentin.monnet@6wind.com> wrote:
> Add function for command completion for tc in bash, and update Makefile
> to install it:
>
> - Under /usr/share/bash-completion/completions/ (default).
> - Or under /etc/bash_completions.d/, which is the old directory for
> bash-completion, if /usr/share/bash-completion/completions/ is not
> found AND /etc/bash_completions.d/ exists already.
>
> Inside iproute2 repository, the completion code is in a new
> `bash-completion` toplevel directory.
>
> Signed-off-by: Quentin Monnet <quentin.monnet@6wind.com>
Rather than having an if statement in the install; lets follow how
other paths are handles.
* default value should be what current distros use /usr/share
* other distro's can override it with environment variable.
Something like:
Makefile:
PREFIX?=/usr
LIBDIR?=$(PREFIX)/lib
SBINDIR?=/sbin
CONFDIR?=/etc/iproute2
DATADIR?=$(PREFIX)/share
DOCDIR?=$(DATADIR)/doc/iproute2
MANDIR?=$(DATADIR)/man
ARPDDIR?=/var/lib/arpd
KERNEL_INCLUDE?=/usr/include
BASH_COMPDIR?=$(DATADIR)/bash-completion/completions/
^ permalink raw reply
* Re: [iproute2 1/1] man: tc-ife.8: man page for ife action
From: Stephen Hemminger @ 2016-05-02 22:10 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: netdev, phil, lucasb
In-Reply-To: <1462013884-4971-1-git-send-email-jhs@emojatatu.com>
On Sat, 30 Apr 2016 06:58:04 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> From: Lucas Bates <lucasb@mojatatu.com>
>
> Signed-off-by: Lucas Bates <lucasb@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Still waiting for a you to respond to my earlier suggestions about tc-ife.
Last version I saw was expecting header in include/linux which is not in
kernel source.
~/kernel/net-next$ ls include/uapi/linux/tc_ife.h
ls: cannot access include/uapi/linux/tc_ife.h: No such file or directory
You need to get that header in right place, and get it exported correctly.
^ permalink raw reply
* Re: [Bug 117521] New: BUG: unable to handle kernel paging request at 000001a400015ff4
From: Andrew Morton @ 2016-05-02 21:48 UTC (permalink / raw)
To: gbalasun; +Cc: bugzilla-daemon, Jon Maloy, Ying Xue, netdev, tipc-discussion
In-Reply-To: <bug-117521-27@https.bugzilla.kernel.org/>
(switched to email. Please respond via emailed reply-to-all, not via the
bugzilla web interface).
Thanks. It's probably a TIPC issue.
On Mon, 02 May 2016 16:44:18 +0000 bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=117521
>
> Bug ID: 117521
> Summary: BUG: unable to handle kernel paging request at
> 000001a400015ff4
> Product: Memory Management
> Version: 2.5
> Kernel Version: 4.4.0
> Hardware: All
> OS: Linux
> Tree: Mainline
> Status: NEW
> Severity: high
> Priority: P1
> Component: Other
> Assignee: akpm@linux-foundation.org
> Reporter: gbalasun@gmail.com
> Regression: No
>
> [ 65.954959] sm-msp-queue[1279]: unable to qualify my own domain name
> (dcsx5testslot3) -- using short name
> [ 632.098785] perf interrupt took too long (2505 > 2500), lowering
> kernel.perf_event_max_sample_rate to 50000
> [ 5880.428123] perf interrupt took too long (5585 > 5000), lowering
> kernel.perf_event_max_sample_rate to 25000
> [17934.014969] CE: hpet increased min_delta_ns to 20115 nsec
> [38956.721789] CE: hpet4 increased min_delta_ns to 20115 nsec
> [46927.872827] hrtimer: interrupt took 63361 ns
> [101662.241093] CE: hpet2 increased min_delta_ns to 20115 nsec
> [245973.044600] CE: hpet6 increased min_delta_ns to 20115 nsec
> [368639.565040] show_signal_msg: 6 callbacks suppressed
> [375832.498126] BUG: unable to handle kernel paging request at 000001a400015ff4
> [375832.505300] IP: [<ffffffff810c3566>] queued_spin_lock_slowpath+0xe6/0x160
> [375832.512394] PGD 0
> [375832.514657] Oops: 0002 [#1] SMP
> [375832.518306] Modules linked in: nf_log_ipv6 nf_log_ipv4 nf_log_common xt_LOG
> sctp libcrc32c e1000e tipc udp_tunnel ip6_udp_tunnel 8021q garp iTCO_wdt
> xt_physdev br_netfilter bridge stp llc nf_conntrack_ipv4 nf_defrag_ipv4
> ipmiq_drv(O) sio_mmc(O) ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6
> nf_defrag_ipv6 xt_state nf_conntrack lockd ip6table_filter event_drv(O)
> ip6_tables grace pt_timer_info(O) ddi(O) usb_storage ixgbe igb i2c_i801
> iTCO_vendor_support i2c_algo_bit ioatdma intel_ips i2c_core pcspkr sunrpc ptp
> mdio dca pps_core lpc_ich tpm_tis mfd_core tpm [last unloaded: iTCO_wdt]
> [375832.573693] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G O 4.4.0
> #14
> [375832.581385] Hardware name: PT AMC124/Base Board Product Name, BIOS
> LGNAJFIP.PTI.0012.P15 01/15/2014
> [375832.591028] task: ffff880351a89b40 ti: ffff880351a90000 task.ti:
> ffff880351a90000
> [375832.599026] RIP: 0010:[<ffffffff810c3566>] [<ffffffff810c3566>]
> queued_spin_lock_slowpath+0xe6/0x160
> [375832.608964] RSP: 0018:ffff88035fc83d58 EFLAGS: 00010002
> [375832.614825] RAX: 0000000000001447 RBX: 0000000000000292 RCX:
> ffff88035fc95fc0
> [375832.622743] RDX: 000001a400015ff4 RSI: 0000000000140000 RDI:
> ffff880351232f80
> [375832.630567] RBP: ffff88035fc83d58 R08: 0000000000000101 R09:
> 0000000000000004
> [375832.638348] R10: 0000000000000000 R11: 0000000000000000 R12:
> 0000000001001002
> [375832.645919] R13: 0000000000000001 R14: 0000000000000000 R15:
> 0000000000000000
> [375832.653610] FS: 0000000000000000(0000) GS:ffff88035fc80000(0000)
> knlGS:0000000000000000
> [375832.662317] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [375832.668483] CR2: 000001a400015ff4 CR3: 0000000001c0a000 CR4:
> 00000000000006e0
> [375832.676133] Stack:
> [375832.678344] ffff88035fc83d78 ffffffff816de2c1 ffff88034a8bba60
> ffff880351232f80
> [375832.686163] ffff88035fc83db8 ffffffff810bc592 ffff88035fc83dc8
> ffff880351758000
> [375832.694139] 0000000001001002 0000000000000000 00000000b802f4bd
> ffffffffa024e6f0
> [375832.702154] Call Trace:
> [375832.704844] <IRQ>
> [375832.707018] [<ffffffff816de2c1>] _raw_spin_lock_irqsave+0x31/0x40
> [375832.713970] [<ffffffff810bc592>] __wake_up+0x32/0x70
> [375832.719444] [<ffffffffa024e6f0>] ? tipc_recv_stream+0x370/0x370 [tipc]
> [375832.726589] [<ffffffff815bf890>] sock_def_wakeup+0x30/0x40
> [375832.732566] [<ffffffffa024e838>] tipc_sk_timeout+0x148/0x180 [tipc]
> [375832.739388] [<ffffffffa024e6f0>] ? tipc_recv_stream+0x370/0x370 [tipc]
> [375832.746507] [<ffffffff810e1644>] call_timer_fn+0x44/0x110
> [375832.752378] [<ffffffff810e0e6a>] ? cascade+0x4a/0x80
> [375832.757848] [<ffffffffa024e6f0>] ? tipc_recv_stream+0x370/0x370 [tipc]
> [375832.764871] [<ffffffff810e2c4c>] run_timer_softirq+0x22c/0x280
> [375832.771175] [<ffffffff81083d88>] __do_softirq+0xc8/0x260
> [375832.776958] [<ffffffff81084123>] irq_exit+0x83/0xb0
> [375832.782369] [<ffffffff816e0c65>] do_IRQ+0x65/0xf0
> [375832.787607] [<ffffffff816df13f>] common_interrupt+0x7f/0x7f
> [375832.793709] <EOI>
> [375832.795803] [<ffffffff81591bdd>] ? cpuidle_enter_state+0xad/0x200
> [375832.802765] [<ffffffff81591bc1>] ? cpuidle_enter_state+0x91/0x200
> [375832.809338] [<ffffffff81591d67>] cpuidle_enter+0x17/0x20
> [375832.815155] [<ffffffff810bcdc7>] call_cpuidle+0x37/0x60
> [375832.821184] [<ffffffff81591d43>] ? cpuidle_select+0x13/0x20
> [375832.827249] [<ffffffff810bd001>] cpu_startup_entry+0x211/0x2d0
> [375832.833535] [<ffffffff8103b213>] start_secondary+0x103/0x130
> [375832.839759] Code: 87 47 02 c1 e0 10 85 c0 74 38 48 89 c2 c1 e8 12 48 c1 ea
> 0c 83 e8 01 83 e2 30 48 98 48 81 c2 c0 5f 01 00 48 03 14 c5 00 b2 d1 81 <48> 89
> 0a 8b 41 08 85 c0 75 0d f3 90 8b 41 08 85 c0 74 f7 eb 02
> [375832.861151] RIP [<ffffffff810c3566>] queued_spin_lock_slowpath+0xe6/0x160
> [375832.868607] RSP <ffff88035fc83d58>
> [375832.872371] CR2: 000001a400015ff4
> [375832.876408] ---[ end trace f12e0074b180a165 ]---
> [375832.881433] Kernel panic - not syncing: Fatal exception in interrupt
> [375832.888391] Kernel Offset: disabled
> [375832.891968] ---[ end Kernel panic - not syncing: Fatal exception in
> interrupt
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.
^ permalink raw reply
* RE: [Intel-wired-lan] NULL dereference on v4.1.x while enabling VF
From: Skidmore, Donald C @ 2016-05-02 21:33 UTC (permalink / raw)
To: William Dauchy, NETDEV, intel-wired-lan@lists.osuosl.org; +Cc: Alex Duyck
In-Reply-To: <CAJ75kXZV32hLzYY-dynYAj6NHm2xNoq89inmSSbqUyxPvUazEw@mail.gmail.com>
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of William Dauchy
> Sent: Monday, May 02, 2016 1:26 PM
> To: NETDEV <netdev@vger.kernel.org>; intel-wired-lan@lists.osuosl.org
> Cc: Alex Duyck <aduyck@mirantis.com>
> Subject: [Intel-wired-lan] NULL dereference on v4.1.x while enabling VF
>
> Hello,
>
> I am getting a NULL dereference on v4.1.x while enabling VF on a ixgbe (
> Intel Corporation 82599ES 10-Gigabit) card during boot.
> It seems to be related to a race since I don't have the issue if I test it a few
> seconds after boot. I had the same issue while testing on v4.5.x What can I
> do to help debug the issue?
>
> ip link set dev eth0 up
> echo 32 > /sys/class/net/eth0/device/sriov_numvfs
Hey William,
Thanks for reporting the dereference. Could you provide a little more detail on how you created this issue? Are you just running the two commands (ip, sriov_numvfs) in some rc script and if you put a few second sleep in front of it you don't see the failure?
Thanks,
-Don Skidmore <donald.c.skidmore@intel.com>
^ permalink raw reply
* Re: [PATCH net 2/3] udp_offload: Set encapsulation before inner completes.
From: Jarno Rajahalme @ 2016-05-02 21:21 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Netdev, Jesse Gross
In-Reply-To: <CAKgT0UcGCgRt5dpG2Y0a6sBar1KF1DkHxdhMbCOYV6qj15D50A@mail.gmail.com>
> On Apr 29, 2016, at 8:46 PM, Alexander Duyck <alexander.duyck@gmail.com> wrote:
>
> On Fri, Apr 29, 2016 at 3:28 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>> UDP tunnel segmentation code relies on the inner offsets being set for
>> an UDP tunnel GSO packet. The inner *_complete() functions will set
>> the inner offsets only if the encapsulation is set before calling
>> them, but udp_gro_complete() set it only after the inner *_complete()
>> functions were done.
>>
>> Also, remove the setting of the inner_mac_header in udp_gro_complete()
>> as it was wrongly set to the beginning of the UDP tunnel header rather
>> than the inner MAC header.
>>
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>> ---
>> net/ipv4/udp_offload.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>> index 0ed2daf..e330c0e 100644
>> --- a/net/ipv4/udp_offload.c
>> +++ b/net/ipv4/udp_offload.c
>> @@ -399,6 +399,11 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>>
>> uh->len = newlen;
>>
>> + /* Set encapsulation before calling into inner gro_complete() functions
>> + * to make them set up the inner offsets.
>> + */
>> + skb->encapsulation = 1;
>> +
>> rcu_read_lock();
>>
>> uo_priv = rcu_dereference(udp_offload_base);
>> @@ -421,9 +426,6 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>> if (skb->remcsum_offload)
>> skb_shinfo(skb)->gso_type |= SKB_GSO_TUNNEL_REMCSUM;
>>
>> - skb->encapsulation = 1;
>> - skb_set_inner_mac_header(skb, nhoff + sizeof(struct udphdr));
>> -
>> return err;
>> }
>>
>> --
>> 2.7.4
>>
>
> So looking over this I think this patch is just a band-aid and it
> isn't really fixing much of anything. For example I cannot see where
It fixes the immediate bug/problem that segmentation of GRO UDP tunnel packet fails due to incorrectly set inner MAC offset and/or missing inner network offset.
> the inner transport offset is ever set. From what I can tell it never
> is.
>
inner transport offset is set in validate_xmit_skb(), but you are right, it is not set by the GRO complete code path. AFAIK no-one uses it before reaching validate_xmit_skb(), though. The documentation in (net-next) Documentation/networking/segmentation-offloads.txt could be fixed by removing the inner transport header from UDP/GRE tunnel case, as it is not used by either the GRE or UDP tunnel code.
Jarno
> What we probably need to do is the same thing we currently do in the
> transmit path itself. We need to record all the headers on the way up
> so that we end up with the network and transport headers matching the
> inner headers, then when we complete the inner-most tunnel we set the
> inner headers and set skb->encapsulation. Then on the way down we
> start overwriting the outer header offsets when skb->encapsulation is
> set. That way we don't have to worry about screwing things up for
> headers like GRE/GUE because the GRE can write the inner header
> offsets, set skb->encapsulation, and then we only overwrite the outer
> headers all the way down because with skb->encapsulation set we know
> we are only dealing with outer headers because the inner header values
> have been written.
>
> - Alex
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: SOCKWQ_ASYNC_NOSPACE optimizations
From: Eric Dumazet @ 2016-05-02 20:55 UTC (permalink / raw)
To: David Miller; +Cc: jiri, edumazet, netdev, eladr, idosch
In-Reply-To: <20160502.163140.11281931277716285.davem@davemloft.net>
On Mon, 2016-05-02 at 16:31 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 02 May 2016 13:23:27 -0700
>
> > It looks like net/sunrpc/xprtsock.c should set SOCK_FASYNC
> > even if it is not actually using fasync_list
> >
> > Could you try this quick hack to check if this is the right way ?
>
> Indeed, it tests the ASYNC bit without enabling FASYNC.
>
> There are three other places that do this: macvtap, tun, dlm lowcomms.
Yes, although macvtap and tun have a private usage of this bit.
When the flag was moved (commit ceb5d58b217098a657f3850b7a2640f995032e62
"net: fix sock_wake_async() rcu protection"), I did not change the code
in these drivers. And apparently nobody complained (linux-4.4)
drivers/net/macvtap.c:501: !test_and_clear_bit(SOCKWQ_ASYNC_NOSPACE, &sk->sk_socket->flags))
drivers/net/macvtap.c:588: (!test_and_set_bit(SOCKWQ_ASYNC_NOSPACE, &q->sock.flags) &&
drivers/net/tun.c:1111: (!test_and_set_bit(SOCKWQ_ASYNC_NOSPACE, &sk->sk_socket->flags) &&
drivers/net/tun.c:1576: if (!test_and_clear_bit(SOCKWQ_ASYNC_NOSPACE, &sk->sk_socket->flags))
fs/dlm/lowcomms.c probably needs a fix.
^ permalink raw reply
* Re: [PATCH net 2/3] udp_offload: Set encapsulation before inner completes.
From: Jarno Rajahalme @ 2016-05-02 21:00 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers, Jesse Gross
In-Reply-To: <CALx6S35N7PtaERFCs27X5GuqUZKYkPFeG8ZQB=0uGpydw+ABEg@mail.gmail.com>
> On Apr 29, 2016, at 6:42 PM, Tom Herbert <tom@herbertland.com> wrote:
>
> On Fri, Apr 29, 2016 at 3:28 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>> UDP tunnel segmentation code relies on the inner offsets being set for
>> an UDP tunnel GSO packet. The inner *_complete() functions will set
>> the inner offsets only if the encapsulation is set before calling
>> them, but udp_gro_complete() set it only after the inner *_complete()
>> functions were done.
>>
>> Also, remove the setting of the inner_mac_header in udp_gro_complete()
>> as it was wrongly set to the beginning of the UDP tunnel header rather
>> than the inner MAC header.
>>
> How did you test this? Do you have a test case for the problem?
>
I faced this problem when developing UDP tunnel offloads for virtio_net. Basically the GRO of encapsulated packets on the rx side should produce the same inner offsets that the original encapsulation did on the tx side. __skb_udp_tunnel_segment() has this line:
int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
which is intended to capture both the UDP header and the tunneling protocol header (e.g., UDP + variable length GENEVE header). So, if the inner MAC header is incorrectly set the segmentation would not work as expected.
> The general problem is that skb->encapsulation is the indicator that
> the inner offsets are valid, but there are many instances where we set
> skb->encapsulation independently of setting the inner headers. It
> would be nice if setting the flag and setting the inner headers were
> somehow unified.
>
Agree, it was rather tedious to trace the call flow to figure out what is going on.
Jarno
> Thanks,
> Tom
>
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>> ---
>> net/ipv4/udp_offload.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>> index 0ed2daf..e330c0e 100644
>> --- a/net/ipv4/udp_offload.c
>> +++ b/net/ipv4/udp_offload.c
>> @@ -399,6 +399,11 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>>
>> uh->len = newlen;
>>
>> + /* Set encapsulation before calling into inner gro_complete() functions
>> + * to make them set up the inner offsets.
>> + */
>> + skb->encapsulation = 1;
>> +
>> rcu_read_lock();
>>
>> uo_priv = rcu_dereference(udp_offload_base);
>> @@ -421,9 +426,6 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
>> if (skb->remcsum_offload)
>> skb_shinfo(skb)->gso_type |= SKB_GSO_TUNNEL_REMCSUM;
>>
>> - skb->encapsulation = 1;
>> - skb_set_inner_mac_header(skb, nhoff + sizeof(struct udphdr));
>> -
>> return err;
>> }
>>
>> --
>> 2.7.4
>>
^ permalink raw reply
* Re: [PATCH v2 0/3] drivers: net: xgene: fix: Get channel number from device binding
From: David Miller @ 2016-05-02 20:48 UTC (permalink / raw)
To: isubramanian; +Cc: netdev, devicetree, linux-arm-kernel, patches
In-Reply-To: <1461953415-9741-1-git-send-email-isubramanian@apm.com>
From: Iyappan Subramanian <isubramanian@apm.com>
Date: Fri, 29 Apr 2016 11:10:12 -0700
> This patch set adds 'channel' property to get ethernet to CPU channel number,
> thus decoupling the Linux driver from static resource selection.
>
> v2: Address review comments from v1
> - removed irq reference from Linux driver
> - added 'channel' property to get ethernet to CPU channel number
>
> v1:
> - Initial version
>
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: SOCKWQ_ASYNC_NOSPACE optimizations
From: Jiri Pirko @ 2016-05-02 20:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, David S . Miller, netdev, eladr, idosch
In-Reply-To: <1462220607.5535.273.camel@edumazet-glaptop3.roam.corp.google.com>
Mon, May 02, 2016 at 10:23:27PM CEST, eric.dumazet@gmail.com wrote:
>On Mon, 2016-05-02 at 21:12 +0200, Jiri Pirko wrote:
>> Mon, May 02, 2016 at 06:22:18PM CEST, eric.dumazet@gmail.com wrote:
>> >On Mon, 2016-05-02 at 18:16 +0200, Jiri Pirko wrote:
>> >> Mon, Apr 25, 2016 at 07:39:32PM CEST, edumazet@google.com wrote:
>> >> >SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async()
>> >> >so that a SIGIO signal is sent when needed.
>> >> >
>> >> >tcp_sendmsg() clears the bit.
>> >> >tcp_poll() sets the bit when stream is not writeable.
>> >> >
>> >> >We can avoid two atomic operations by first checking if socket
>> >> >is actually interested in the FASYNC business (most sockets in
>> >> >real applications do not use AIO, but select()/poll()/epoll())
>> >> >
>> >> >This also removes one cache line miss to access sk->sk_wq->flags
>> >> >in tcp_sendmsg()
>> >> >
>> >> >Signed-off-by: Eric Dumazet <edumazet@google.com>
>> >>
>> >> I just bisected down to this. This is causing a regression for me when
>> >> my nfs mount becomes stuck. I can easily reproduce this if you need to
>> >> test the fix.
>> >
>> >What do you mean by 'when nfs mount becomes stuck' ?
>> >
>> >Is this patch making nfs not functional , or does it make recovery from
>> >some nfs error bad ?
>>
>> I can mount nfs on the host. But when I do something (compile a kernel
>> module in my case), it gets stuck. Then I cannot even ssh to the machine.
>> No messages in dmesg. I didn't debug it any further. I just bisected and
>> verified that this patch caused this behaviour.
>
>Interesting.
>
>It looks like net/sunrpc/xprtsock.c should set SOCK_FASYNC
>even if it is not actually using fasync_list
>
>Could you try this quick hack to check if this is the right way ?
Yep, works, I do not see the issue with this patch anymore. Thanks.
>
>Thanks !
>
>diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>index a6c68dc086af83233ee315642638f4a1990ee622..b90c5397b5e137c6cc8accad6eebe2b876363d4e 100644
>--- a/net/sunrpc/xprtsock.c
>+++ b/net/sunrpc/xprtsock.c
>@@ -1950,6 +1950,7 @@ static int xs_local_finish_connecting(struct rpc_xprt *xprt,
> sk->sk_user_data = xprt;
> sk->sk_data_ready = xs_data_ready;
> sk->sk_write_space = xs_udp_write_space;
>+ sock_set_flag(sk, SOCK_FASYNC);
> sk->sk_error_report = xs_error_report;
> sk->sk_allocation = GFP_NOIO;
>
>@@ -2136,6 +2137,7 @@ static void xs_udp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
> sk->sk_user_data = xprt;
> sk->sk_data_ready = xs_data_ready;
> sk->sk_write_space = xs_udp_write_space;
>+ sock_set_flag(sk, SOCK_FASYNC);
> sk->sk_allocation = GFP_NOIO;
>
> xprt_set_connected(xprt);
>@@ -2237,6 +2239,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
> sk->sk_data_ready = xs_tcp_data_ready;
> sk->sk_state_change = xs_tcp_state_change;
> sk->sk_write_space = xs_tcp_write_space;
>+ sock_set_flag(sk, SOCK_FASYNC);
> sk->sk_error_report = xs_error_report;
> sk->sk_allocation = GFP_NOIO;
>
>
>
^ permalink raw reply
* [PATCH net v2 1/2] udp_tunnel: Remove redundant udp_tunnel_gro_complete().
From: Jarno Rajahalme @ 2016-05-02 20:39 UTC (permalink / raw)
To: davem, kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel
Cc: jesse, jarno
The setting of the UDP tunnel GSO type is already performed by
udp[46]_gro_complete().
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
drivers/net/geneve.c | 2 --
drivers/net/vxlan.c | 2 --
include/net/udp_tunnel.h | 9 ---------
net/ipv4/fou.c | 2 --
4 files changed, 15 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index bc16889..98f1224 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -504,8 +504,6 @@ static int geneve_gro_complete(struct sk_buff *skb, int nhoff,
int gh_len;
int err = -ENOSYS;
- udp_tunnel_gro_complete(skb, nhoff);
-
gh = (struct genevehdr *)(skb->data + nhoff);
gh_len = geneve_hlen(gh);
type = gh->proto_type;
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1c0fa36..dd2d032 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -616,8 +616,6 @@ out:
static int vxlan_gro_complete(struct sk_buff *skb, int nhoff,
struct udp_offload *uoff)
{
- udp_tunnel_gro_complete(skb, nhoff);
-
return eth_gro_complete(skb, nhoff + sizeof(struct vxlanhdr));
}
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index b831140..a114024 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -106,15 +106,6 @@ static inline struct sk_buff *udp_tunnel_handle_offloads(struct sk_buff *skb,
return iptunnel_handle_offloads(skb, type);
}
-static inline void udp_tunnel_gro_complete(struct sk_buff *skb, int nhoff)
-{
- struct udphdr *uh;
-
- uh = (struct udphdr *)(skb->data + nhoff - sizeof(struct udphdr));
- skb_shinfo(skb)->gso_type |= uh->check ?
- SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
-}
-
static inline void udp_tunnel_encap_enable(struct socket *sock)
{
#if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index a39068b..305d9ac 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -228,8 +228,6 @@ static int fou_gro_complete(struct sk_buff *skb, int nhoff,
int err = -ENOSYS;
const struct net_offload **offloads;
- udp_tunnel_gro_complete(skb, nhoff);
-
rcu_read_lock();
offloads = NAPI_GRO_CB(skb)->is_ipv6 ? inet6_offloads : inet_offloads;
ops = rcu_dereference(offloads[proto]);
--
2.7.4
^ permalink raw reply related
* [PATCH net v2 2/2] udp_offload: Set encapsulation before inner completes.
From: Jarno Rajahalme @ 2016-05-02 20:39 UTC (permalink / raw)
To: davem, kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel
Cc: jesse, jarno
In-Reply-To: <1462221590-14309-1-git-send-email-jarno@ovn.org>
UDP tunnel segmentation code relies on the inner offsets being set for
an UDP tunnel GSO packet, but the inner *_complete() functions will
set the inner offsets only if 'encapsulation' is set before calling
them. Currently, udp_gro_complete() sets 'encapsulation' only after
the inner *_complete() functions are done. This causes the inner
offsets having invalid values after udp_gro_complete() returns, which
in turn will make it impossible to properly segment the packet in case
it needs to be forwarded, which would be visible to the user either as
invalid packets being sent or as packet loss.
This patch fixes this by setting skb's 'encapsulation' before calling
into the inner complete functions, and by removing the setting of the
inner_mac_header in udp_gro_complete(). The inner complete now
functions set the inner_mac_header, so it need not be done here.
Furthermore, it was wrongly set to the beginning of the UDP tunnel
header rather than the inner MAC header.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
net/ipv4/udp_offload.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 0ed2daf..e330c0e 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -399,6 +399,11 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
uh->len = newlen;
+ /* Set encapsulation before calling into inner gro_complete() functions
+ * to make them set up the inner offsets.
+ */
+ skb->encapsulation = 1;
+
rcu_read_lock();
uo_priv = rcu_dereference(udp_offload_base);
@@ -421,9 +426,6 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
if (skb->remcsum_offload)
skb_shinfo(skb)->gso_type |= SKB_GSO_TUNNEL_REMCSUM;
- skb->encapsulation = 1;
- skb_set_inner_mac_header(skb, nhoff + sizeof(struct udphdr));
-
return err;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 1/2] net: SOCKWQ_ASYNC_NOSPACE optimizations
From: David Miller @ 2016-05-02 20:31 UTC (permalink / raw)
To: eric.dumazet; +Cc: jiri, edumazet, netdev, eladr, idosch
In-Reply-To: <1462220607.5535.273.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 May 2016 13:23:27 -0700
> It looks like net/sunrpc/xprtsock.c should set SOCK_FASYNC
> even if it is not actually using fasync_list
>
> Could you try this quick hack to check if this is the right way ?
Indeed, it tests the ASYNC bit without enabling FASYNC.
There are three other places that do this: macvtap, tun, dlm lowcomms.
^ permalink raw reply
* NULL dereference on v4.1.x while enabling VF
From: William Dauchy @ 2016-05-02 20:25 UTC (permalink / raw)
To: NETDEV, intel-wired-lan; +Cc: Emil Tantilov, Jeff Kirsher, Alex Duyck
Hello,
I am getting a NULL dereference on v4.1.x while enabling VF on a ixgbe
( Intel Corporation 82599ES 10-Gigabit) card during boot.
It seems to be related to a race since I don't have the issue if I
test it a few seconds after boot. I had the same issue while testing
on v4.5.x
What can I do to help debug the issue?
ip link set dev eth0 up
echo 32 > /sys/class/net/eth0/device/sriov_numvfs
part of the dmesg:
ixgbe 0000:82:00.1: removed PHC on eth1
ixgbe 0000:82:00.1: Multiqueue Enabled: Rx Queue count = 2, Tx Queue count = 2
ixgbe 0000:82:00.1: registered PHC device on eth1
ixgbe 0000:82:00.1 eth1: detected SFP+: 4
ixgbe 0000:82:00.1 eth1: NIC Link is Up 10 Gbps, Flow Control: RX/TX
ixgbevf: Could not enable Tx Queue 0
ixgbevf: Could not enable Tx Queue 1
8021q: adding VLAN 0 to HW filter on device eth2
ixgbe 0000:82:00.0 eth0: VF Reset msg received from vf 0
ixgbe 0000:82:00.0: VF 0 has no MAC address assigned, you may have to
assign one manually
BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
ixgbevf: Could not enable Tx Queue 0
ixgbevf: Could not enable Tx Queue 1
IP: [<ffffffff813d3873>] ixgbevf_alloc_rx_buffers+0x53/0x170
PGD 0
Oops: 0000 [#1] PREEMPT SMP
Modules linked in:
CPU: 8 PID: 1034 Comm: kworker/8:1 Tainted: G W 4.1.23 #1
Workqueue: events ixgbevf_service_task
task: ffff882021558000 ti: ffff882021554000 task.ti: ffff882021554000
RIP: 0010:[<ffffffff813d3873>] [<ffffffff813d3873>]
ixgbevf_alloc_rx_buffers+0x53/0x170
RSP: 0018:ffff882021557c58 EFLAGS: 00010246
RAX: 0000000000000200 RBX: ffff88201ccc09c0 RCX: 0000000000000000
RDX: ffffc9000e860000 RSI: 00000000000001ff RDI: ffff88201ccc09c0
RBP: ffff882021557c98 R08: 0000000000000001 R09: 0000000000000001
R10: ffff882021558000 R11: 000000000000017e R12: 0000000000000000
R13: 00000000fffffe00 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff88207fc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000008 CR3: 0000000001a08000 CR4: 00000000001406e0
Stack:
ffff88201d630940 01ff00000000000a ffff882021557c78 ffff88201d630c28
ffff88201d630940 0000000000000009 ffff88201ccc09c0 0000000000000000
ffff882021557d18 ffffffff813d54b8 0000102800001028 0000000000000000
Call Trace:
[<ffffffff813d54b8>] ixgbevf_configure+0x50d/0x54e
[<ffffffff813d6a11>] ? ixgbevf_down+0x2ba/0x2cb
[<ffffffff813d6687>] ixgbevf_up+0x13/0x1f
[<ffffffff813d6a85>] ixgbevf_reinit_locked+0x63/0x6c
[<ffffffff813d76c9>] ixgbevf_service_task+0xe5/0x2dc
[<ffffffff810743b7>] process_one_work+0x275/0x4f2
[<ffffffff81074ae1>] worker_thread+0x1f5/0x2a9
[<ffffffff810748ec>] ? rescuer_thread+0x289/0x289
[<ffffffff810748ec>] ? rescuer_thread+0x289/0x289
[<ffffffff81079775>] kthread+0xc5/0xcd
[<ffffffff81577e9f>] ? _raw_spin_unlock_irq+0x2d/0x4c
[<ffffffff810796b0>] ? __kthread_parkme+0x87/0x87
[<ffffffff81578a82>] ret_from_fork+0x42/0x70
[<ffffffff810796b0>] ? __kthread_parkme+0x87/0x87
Code: 0f 84 33 01 00 00 44 0f b7 67 2e 48 89 fb 0f b7 47 2c 4d 89 e5
4d 89 e6 4d 6b e4 18 49 c1 e6 04 4c 03 77 18 41 29 c5 4c 03 67 38 <49>
83 7c 24 08 00 0f 85 9e 00 00 00 65 8b 05 2a dc c3 7e 48 98
RIP [<ffffffff813d3873>] ixgbevf_alloc_rx_buffers+0x53/0x170
RSP <ffff882021557c58>
CR2: 0000000000000008
---[ end trace 6e4d9e7fa97d4fca ]---
BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:21
in_atomic(): 0, irqs_disabled(): 1, pid: 1034, name: kworker/8:1
INFO: lockdep is turned off.
irq event stamp: 43238
hardirqs last enabled at (43237): [<ffffffff81577efd>]
_raw_spin_unlock_irqrestore+0x3f/0x60
hardirqs last disabled at (43238): [ 41.225859] 8021q: adding VLAN 0
to HW filter on device eth3
[<ffffffff8157a4b3>] error_sti+0x5/0x6
Thanks,
--
William
^ 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