* Re: [PATCH v2] qmi_wwan: add support for Quectel EG91
From: David Miller @ 2018-07-05 11:22 UTC (permalink / raw)
To: vucnikm; +Cc: bjorn, netdev, linux-usb, linux-kernel
In-Reply-To: <1530720768-18349-1-git-send-email-vucnikm@gmail.com>
From: Matevz Vucnik <vucnikm@gmail.com>
Date: Wed, 4 Jul 2018 18:12:48 +0200
> This adds the USB id of LTE modem Quectel EG91. It requires the
> same quirk as other Quectel modems to make it work.
>
> Signed-off-by: Matevz Vucnik <vucnikm@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH] atm: Preserve value of skb->truesize when accounting to vcc
From: David Miller @ 2018-07-05 11:23 UTC (permalink / raw)
To: dwmw2; +Cc: eric.dumazet, netdev, ldir
In-Reply-To: <1530780261.13595.126.camel@infradead.org>
From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 05 Jul 2018 09:44:22 +0100
> On Sat, 2018-06-16 at 16:27 -0700, David Miller wrote:
>> From: "David Woodhouse" <dwmw2@infradead.org>
>> Date: Sat, 16 Jun 2018 20:52:33 -0000
>>
>> >> This Fixes tag shoots the messenger really.
>> >>
>> >> I suggest to instead use :
>> >>
>> >> Fixes: 158f323b9868 ("net: adjust skb->truesize in pskb_expand_head()")
>> >
>> >
>> > Oh, I hadn't realised how recent that was. Sure, let's blame you instead :)
>>
>> Patch applied with adjusted Fixes: tag, and queued up for -stable.
>
> Thanks.... gentle prod about the "stable" part of that. OpenWRT is
> lining up for a release it'd be good to ingest the patch properly if
> possible.
>
> I periodically whine at them about the number of outstanding patches
> not in upstream. It helps if one of them doesn't have my name on :)
It's in my next batch of -stable submissions. :)
^ permalink raw reply
* Re: [PATCH] tipc: mark expected switch fall-throughs
From: David Miller @ 2018-07-05 11:24 UTC (permalink / raw)
To: gustavo; +Cc: jon.maloy, ying.xue, netdev, tipc-discussion, linux-kernel
In-Reply-To: <20180704211359.GA9836@embeddedor.com>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Wed, 4 Jul 2018 16:13:59 -0500
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Warning level 2 was used: -Wimplicit-fallthrough=2
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: decnet: dn_nsp_in: mark expected switch fall-through
From: David Miller @ 2018-07-05 11:27 UTC (permalink / raw)
To: gustavo; +Cc: linux-decnet-user, netdev, linux-kernel
In-Reply-To: <20180704220500.GA11901@embeddedor.com>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Wed, 4 Jul 2018 17:05:00 -0500
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: core: filter: mark expected switch fall-through
From: David Miller @ 2018-07-05 11:27 UTC (permalink / raw)
To: gustavo; +Cc: ast, daniel, netdev, linux-kernel
In-Reply-To: <20180704223437.GA14486@embeddedor.com>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Wed, 4 Jul 2018 17:34:37 -0500
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Warning level 2 was used: -Wimplicit-fallthrough=2
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [PATCH][next] net: dsa: fix spelling mistake "waitting" -> "waiting"
From: David Miller @ 2018-07-05 11:27 UTC (permalink / raw)
To: colin.king
Cc: andrew, vivien.didelot, f.fainelli, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20180705083004.13735-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Thu, 5 Jul 2018 09:30:04 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in dev_err error message.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 6/8] virtio: introduce packed ring defines
From: Jason Wang @ 2018-07-05 11:30 UTC (permalink / raw)
To: Maxime Coquelin, mst, kvm, virtualization, netdev, linux-kernel; +Cc: wexu
In-Reply-To: <0f293989-a0ca-e7ad-f907-83a8d8445335@redhat.com>
On 2018年07月05日 04:15, Maxime Coquelin wrote:
>
>
> On 07/03/2018 07:38 AM, Jason Wang wrote:
>> diff --git a/include/uapi/linux/virtio_ring.h
>> b/include/uapi/linux/virtio_ring.h
>> index 6d5d5fa..71c7a46 100644
>> --- a/include/uapi/linux/virtio_ring.h
>> +++ b/include/uapi/linux/virtio_ring.h
>> @@ -43,6 +43,8 @@
>> #define VRING_DESC_F_WRITE 2
>> /* This means the buffer contains a list of buffer descriptors. */
>> #define VRING_DESC_F_INDIRECT 4
>> +#define VRING_DESC_F_AVAIL 7
>> +#define VRING_DESC_F_USED 15
>
> For consistency, you may want to make VRING_DESC_F_AVAIL and
> VRING_DESC_F_USED to represent the bit mask and not the bit position,
> as done for VRING_DESC_F_WRITE and VRING_DESC_F_INDIRECT.
>
> Regards,
> Maxime
Yes, Tiwei has the same comment.
Will fix this.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] 6lowpan: iphc: reset mac_header after decompress to fix panic
From: Stefan Schmidt @ 2018-07-05 11:34 UTC (permalink / raw)
To: Michael Scott
Cc: Alexander Aring, Jukka Rissanen, David S. Miller, linux-bluetooth,
linux-wpan, netdev, linux-kernel
In-Reply-To: <20180619234406.8217-1-michael@opensourcefoundries.com>
Hello Jukka.
On 20.06.2018 01:44, Michael Scott wrote:
> After decompression of 6lowpan socket data, an IPv6 header is inserted
> before the existing socket payload. After this, we reset the
> network_header value of the skb to account for the difference in payload
> size from prior to decompression + the addition of the IPv6 header.
>
> However, we fail to reset the mac_header value.
>
> Leaving the mac_header value untouched here, can cause a calculation
> error in net/packet/af_packet.c packet_rcv() function when an
> AF_PACKET socket is opened in SOCK_RAW mode for use on a 6lowpan
> interface.
>
> On line 2088, the data pointer is moved backward by the value returned
> from skb_mac_header(). If skb->data is adjusted so that it is before
> the skb->head pointer (which can happen when an old value of mac_header
> is left in place) the kernel generates a panic in net/core/skbuff.c
> line 1717.
>
> This panic can be generated by BLE 6lowpan interfaces (such as bt0) and
> 802.15.4 interfaces (such as lowpan0) as they both use the same 6lowpan
> sources for compression and decompression.
>
> Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
> ---
> net/6lowpan/iphc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 6b1042e21656..52fad5dad9f7 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -770,6 +770,7 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
> hdr.hop_limit, &hdr.daddr);
>
> skb_push(skb, sizeof(hdr));
> + skb_reset_mac_header(skb);
> skb_reset_network_header(skb);
> skb_copy_to_linear_data(skb, &hdr, sizeof(hdr));
>
>
Alex acked this patch for the ieee802154 side. Could you test it for the
bluetooth 6lowpan side and ack or nack as well?
So far plain 6lowpan subsystem patches would still go through the
bluetooth tree after Alex and you acked them. I guess Marcel or Johan
just waiting for your review.
regards
Stefan Schmidt
^ permalink raw reply
* Re: [PATCH] 6lowpan: iphc: reset mac_header after decompress to fix panic
From: Jukka Rissanen @ 2018-07-05 11:42 UTC (permalink / raw)
To: Michael Scott
Cc: Alexander Aring, David S. Miller, linux-bluetooth, linux-wpan,
netdev, linux-kernel
In-Reply-To: <20180619234406.8217-1-michael@opensourcefoundries.com>
On Tue, 2018-06-19 at 16:44 -0700, Michael Scott wrote:
> After decompression of 6lowpan socket data, an IPv6 header is
> inserted
> before the existing socket payload. After this, we reset the
> network_header value of the skb to account for the difference in
> payload
> size from prior to decompression + the addition of the IPv6 header.
>
>
> Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
> ---
> net/6lowpan/iphc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 6b1042e21656..52fad5dad9f7 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -770,6 +770,7 @@ int lowpan_header_decompress(struct sk_buff *skb,
> const struct net_device *dev,
> hdr.hop_limit, &hdr.daddr);
>
> skb_push(skb, sizeof(hdr));
> + skb_reset_mac_header(skb);
> skb_reset_network_header(skb);
> skb_copy_to_linear_data(skb, &hdr, sizeof(hdr));
>
Acked-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
Cheers,
Jukka
^ permalink raw reply
* [PATCH net] MAINTAINERS: update my email address
From: Stefan Schmidt @ 2018-07-05 11:56 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-wpan, aring, Stefan Schmidt
The mail server hosting the old address is going to fade out.
Time to update to an address I control directly.
Signed-off-by: Stefan Schmidt <stefan@datenfreihafen.org>
---
MAINTAINERS | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index b40d702337f2..ca3e75ec9308 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2523,7 +2523,7 @@ S: Supported
F: drivers/scsi/esas2r
ATUSB IEEE 802.15.4 RADIO DRIVER
-M: Stefan Schmidt <stefan@osg.samsung.com>
+M: Stefan Schmidt <stefan@datenfreihafen.org>
L: linux-wpan@vger.kernel.org
S: Maintained
F: drivers/net/ieee802154/atusb.c
@@ -6908,7 +6908,7 @@ F: drivers/clk/clk-versaclock5.c
IEEE 802.15.4 SUBSYSTEM
M: Alexander Aring <alex.aring@gmail.com>
-M: Stefan Schmidt <stefan@osg.samsung.com>
+M: Stefan Schmidt <stefan@datenfreihafen.org>
L: linux-wpan@vger.kernel.org
W: http://wpan.cakelab.org/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan.git
--
2.14.4
^ permalink raw reply related
* Re: [PATCH] 6lowpan: iphc: reset mac_header after decompress to fix panic
From: Stefan Schmidt @ 2018-07-05 11:59 UTC (permalink / raw)
To: Michael Scott
Cc: Alexander Aring, Jukka Rissanen, David S. Miller, linux-bluetooth,
linux-wpan, netdev, linux-kernel
In-Reply-To: <20180619234406.8217-1-michael@opensourcefoundries.com>
Hello.
On 20.06.2018 01:44, Michael Scott wrote:
> After decompression of 6lowpan socket data, an IPv6 header is inserted
> before the existing socket payload. After this, we reset the
> network_header value of the skb to account for the difference in payload
> size from prior to decompression + the addition of the IPv6 header.
>
> However, we fail to reset the mac_header value.
>
> Leaving the mac_header value untouched here, can cause a calculation
> error in net/packet/af_packet.c packet_rcv() function when an
> AF_PACKET socket is opened in SOCK_RAW mode for use on a 6lowpan
> interface.
>
> On line 2088, the data pointer is moved backward by the value returned
> from skb_mac_header(). If skb->data is adjusted so that it is before
> the skb->head pointer (which can happen when an old value of mac_header
> is left in place) the kernel generates a panic in net/core/skbuff.c
> line 1717.
>
> This panic can be generated by BLE 6lowpan interfaces (such as bt0) and
> 802.15.4 interfaces (such as lowpan0) as they both use the same 6lowpan
> sources for compression and decompression.
>
> Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
> ---
> net/6lowpan/iphc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 6b1042e21656..52fad5dad9f7 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -770,6 +770,7 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
> hdr.hop_limit, &hdr.daddr);
>
> skb_push(skb, sizeof(hdr));
> + skb_reset_mac_header(skb);
> skb_reset_network_header(skb);
> skb_copy_to_linear_data(skb, &hdr, sizeof(hdr));
This showed no problems in my ieee802154 + 6lowpan tests.
Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
regards
Stefan Schmidt
^ permalink raw reply
* Re: [RFC PATCH 00/11] OVS eBPF datapath.
From: Paul Chaignon @ 2018-07-05 12:12 UTC (permalink / raw)
To: William Tu
Cc: Alexei Starovoitov,
<dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org>,
Tom Herbert via iovisor-dev, Linux Kernel Network Developers
In-Reply-To: <CALDO+SYUPgCtQqGkRepK5YTKZegAg3-DKWcssoqXwGEqgsodgw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Jul 04, 2018 at 07:25:50PM -0700, William Tu wrote:
> On Tue, Jul 3, 2018 at 10:56 AM, Alexei Starovoitov
> <alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Thu, Jun 28, 2018 at 07:19:35AM -0700, William Tu wrote:
> >> Hi Alexei,
> >>
> >> Thanks a lot for the feedback!
> >>
> >> On Wed, Jun 27, 2018 at 8:00 PM, Alexei Starovoitov
> >> <alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> > On Sat, Jun 23, 2018 at 05:16:32AM -0700, William Tu wrote:
> >> >>
> >> >> Discussion
> >> >> ==========
> >> >> We are still actively working on finishing the feature, currently
> >> >> the basic forwarding and tunnel feature work, but still under
> >> >> heavy debugging and development. The purpose of this RFC is to
> >> >> get some early feedbacks and direction for finishing the complete
> >> >> features in existing kernel's OVS datapath (the net/openvswitch/*).
> >> >
> >> > Thank you for sharing the patches.
> >> >
> >> >> Three major issues we are worried:
> >> >> a. Megaflow support in BPF.
> >> >> b. Connection Tracking support in BPF.
> >> >
> >> > my opinion on the above two didn't change.
> >> > To recap:
> >> > A. Non scalable megaflow map is no go. I'd like to see packet classification
> >> > algorithm like hicuts or efficuts to be implemented instead, since it can be
> >> > shared by generic bpf, bpftiler, ovs and likely others.
> >>
> >> We did try the decision tree approach using dpdk's acl lib. The lookup
> >> speed is 6 times faster than the magaflow using tuple space.
> >> However, the update/insertion requires rebuilding/re-balancing the decision
> >> tree so it's way too slow. I think hicuts or efficuts suffers the same issue.
> >> So decision tree algos are scalable only for lookup operation due to its
> >> optimization over tree depth, but not scalable under
> >> update/insert/delete operations.
> >>
> >> On customer's system we see megaflow update/insert rate around 10 rules/sec,
> >> this makes decision tree unusable, unless we invent something to optimize the
> >> update/insert time or incremental update of these decision tree algo.
> >
> > is this a typo? you probably meant 10K rule updates a second ?
> I mean "new" rules being added at 10 rules/sec.
> Update rate might be much higher.
>
> > Last time I've dealt with these algorithms we had 100K acl updates a second.
> > It was an important metric that we were optimizing for.
> > I'm pretty sure '*cuts' algos do many thousands per second non optimized.
>
> When adding a new rule, do these algorithms require rebuilding the
> entire tree?
>
> In our evaluation, updating an existing entry in the decision tree
> performs OK, because it is equal to lookup and replace, and lookup
> is fast, update is just atomic swap. But inserting a new rule is slow,
> because it requires re-building the tree using all existing rules.
> And we see new rule being added at rate 10 rules per second.
> So we are constantly rebuilding the entire tree.
>
> If the entire tree has 100k of rules, it takes around 2 seconds to rebuild,
> based on the dpdk acl library. And without an incremental algorithm,
> adding 1 new rule will trigger rebuilding the 100k of rules, and it is too slow.
>
> Reading through HyperCuts and EffiCuts, I'm not sure how it supports
> incrementally adding a new rule, without rebuilding the entire tree.
> http://ccr.sigcomm.org/online/files/p207.pdf
> http://cseweb.ucsd.edu/~susingh/papers/hyp-sigcomm03.pdf
>
> The HyperCuts papers says
> "A fast update algorithm can also be implemented; however we do not
> go into the details of incremental update in this paper"
>
> >
> >> >> c. Verifier limitation.
> >> >
> >> > Not sure what limitations you're concerned about.
> >> >
> >>
> >> Mostly related to stack. The flow key OVS uses (struct sw_flow_key)
> >> is 464 byte. We trim a lot, now around 300 byte, but still huge, considering
> >> the BPF's stack limit is 512 byte.
> >
> > have you tried using per-cpu array of one element with large value
> > instead of stack?
>
> yes, now we store the flow key in percpu array with 1 element.
>
> > In the latest verifier most of the operations that can be done with the stack
> > pointer can be done with pointer to map value too.
> >
> Once the flow key is stored in map, another eBPF program
> needs to use that key to lookup flow table (another map).
> So we have to store the flow key on stack first, in order to
> use it as key to lookup the flow table map.
>
> Is there a way to work around it?
d71962f ("bpf: allow map helpers access to map values directly") removes
that limitation from the verifier and should allow you to use map values
as map keys directly. 4.18-rc1 has it.
> Thanks
> William
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1360): https://lists.iovisor.org/g/iovisor-dev/message/1360
Mute This Topic: https://lists.iovisor.org/mt/22656941/1132507
Group Owner: iovisor-dev+owner-9jONkmmOlFHEE9lA1F8Ukti2O/JbrIOy@public.gmane.org
Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub [glki-iovisor-dev@m.gmane.org]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply
* Re: [PATCH] [v2] infiniband: i40iw, nes: don't use wall time for TCP sequence numbers
From: Arnd Bergmann @ 2018-07-05 13:15 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Faisal Latif, Shiraz Saleem, Doug Ledford,
Jason Gunthorpe, David S. Miller, Henry Orosco, Tatyana Nikolova,
Mustafa Ismail, Bart Van Assche, Yuval Shaia, linux-rdma,
Linux Kernel Mailing List, Networking
In-Reply-To: <201807010157.h1EBGb0w%fengguang.wu@intel.com>
On Sat, Jun 30, 2018 at 9:00 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Arnd,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.18-rc2 next-20180629]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Arnd-Bergmann/infiniband-i40iw-nes-don-t-use-wall-time-for-TCP-sequence-numbers/20180627-221142
> config: x86_64-randconfig-s2-06302231 (attached as .config)
> compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> drivers/infiniband/hw/i40iw/i40iw_cm.o: In function `i40iw_make_cm_node':
>>> drivers/infiniband/hw/i40iw/i40iw_cm.c:2232: undefined reference to `secure_tcpv6_seq'
>
Fixed this now by added a
depends on IPV6 || !IPV6
dependency in Kconfig. This ensures that with IPV6=m, i40iw cannot be built-in.
Will send v3 after a little more build testing.
Arnd
^ permalink raw reply
* Re: [PATCHv2 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
From: David Ahern @ 2018-07-05 13:18 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
In-Reply-To: <CADvbK_fsFHuW4gvtpSZ-A8F12PXaMX8YGGKmhsMy3L_KTZt-8Q@mail.gmail.com>
On 7/5/18 1:57 AM, Xin Long wrote:
> On Thu, Jul 5, 2018 at 2:36 AM, David Ahern <dsahern@gmail.com> wrote:
>> On 7/4/18 11:56 AM, Xin Long wrote:
>>
>>>> your commands are not a proper test. The test should succeed and fail
>>>> based on the routing lookup, not iptables rules.
>>> A proper test can be done easily with netns, as vrf can't isolate much.
>>> I don't want to bother forwarding/ directory with netns, so I will probably
>>> just drop this selftest, and let the feature patch go first.
>>>
>>
>> BTW, VRF isolates at the routing layer and this is a routing change. We
>> need to understand why it does not work with VRF. Perhaps another tweak
>> is needed for VRF.
> One problem was that the peer may not use the address on the dev
> that echo_request comes from as the src IP of echo_reply when the
> echo_request's dst IP is broadcast, but try to get another one by
> looking up a route without ".flowi4_oif" set. See:
>
> icmp_reply()->fib_compute_spec_dst():
> struct flowi4 fl4 = {
> .flowi4_iif = LOOPBACK_IFINDEX,
> .daddr = ip_hdr(skb)->saddr,
> .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
> .flowi4_scope = scope,
> .flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
> };
> if (!fib_lookup(net, &fl4, &res, 0))
> return FIB_RES_PREFSRC(net, res);
>
>
> Without ".flowi4_oif" set, it won't match the vrf route. That's why
> I had to make h2 NOT into a vrf so that h1 can get the echo_reply.
> But it can't tell if this echo_reply is from h2 or r1, as r1's echo_reply
> will also use the same src IP which is actually got from main route
> space as ".flowi4_oif" is not set.
> (hope I this description is clear to you) :)
>
> So i'm not sure if we can do any tweak for VRF.
>
Try this:
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index b21833651394..e46cdd310e5f 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -300,6 +300,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
if (!ipv4_is_zeronet(ip_hdr(skb)->saddr)) {
struct flowi4 fl4 = {
.flowi4_iif = LOOPBACK_IFINDEX,
+ .flowi4_oif = l3mdev_master_ifindex_rcu(dev),
.daddr = ip_hdr(skb)->saddr,
.flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
.flowi4_scope = scope,
^ permalink raw reply related
* Re: [PATCH] net: ethernet: sun: remove redundant variables adv and lpa
From: Andrew Lunn @ 2018-07-05 13:54 UTC (permalink / raw)
To: Colin King; +Cc: David S . Miller, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180705093732.21650-1-colin.king@canonical.com>
On Thu, Jul 05, 2018 at 10:37:32AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Variables adv and lpa are being assigned but are never used hence they
> are redundant and can be removed.
>
> Cleans up clang warnings:
> warning: variable 'lpa' set but not used [-Wunused-but-set-variable]
> warning: variable 'adv' set but not used [-Wunused-but-set-variable]
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/sun/niu.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
> index 88c12474a0c3..2d6b62c6d9ab 100644
> --- a/drivers/net/ethernet/sun/niu.c
> +++ b/drivers/net/ethernet/sun/niu.c
> @@ -1225,17 +1225,13 @@ static int link_status_1g_rgmii(struct niu *np, int *link_up_p)
>
> bmsr = err;
> if (bmsr & BMSR_LSTATUS) {
> - u16 adv, lpa;
> -
> err = mii_read(np, np->phy_addr, MII_ADVERTISE);
> if (err < 0)
> goto out;
> - adv = err;
>
> err = mii_read(np, np->phy_addr, MII_LPA);
> if (err < 0)
> goto out;
> - lpa = err;
>
> err = mii_read(np, np->phy_addr, MII_ESTATUS);
> if (err < 0)
What should really happen is something like:
common_adv = lpa & adv;
lp->active_speed = SPEED_10;
lp->active_duplex = DUPLEX_HALF;
if (common_adv_gb & (LPA_1000FULL | LPA_1000HALF)) {
lp->active_speed = SPEED_1000;
if (common_adv_gb & LPA_1000FULL)
lp->active_duplex = DUPLEX_FULL;
} else if (common_adv & (LPA_100FULL | LPA_100HALF)) {
lp->active_speed = SPEED_100;
if (common_adv & LPA_100FULL)
lp->active_duplex = DUPLEX_FULL;
} else
if (common_adv & LPA_10FULL)
lp->active_duplex = DUPLEX_FULL;
i.e. making use of the results of the autoneg to determine the link
speed and duplex, rather than just assuming it is 1G.
If i turn this into a real patch, does anybody have the hardware to
test it?
Andrew
^ permalink raw reply
* [PATCH net] net: aquantia: vlan unicast address list correct handling
From: Igor Russkikh @ 2018-07-05 14:01 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, David Arcari, Pavel Belous, Igor Russkikh
In-Reply-To: <cover.1530714429.git.igor.russkikh@aquantia.com>
Setting up macvlan/macvtap networks over atlantic NIC results
in no traffic over these networks because ndo_set_rx_mode did
not listed UC MACs as registered in unicast filter.
Here we fix that taking into account maximum number of UC
filters supported by hardware. If more than MAX addresses were
registered, we just enable promisc and/or allmulti to pass
the traffic in.
We also remove MULTICAST_ADDRESS_MAX constant from aq_cfg since
thats not a configurable parameter at all.
Fixes: b21f502 ("net:ethernet:aquantia: Fix for multicast filter handling.")
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_cfg.h | 2 -
drivers/net/ethernet/aquantia/atlantic/aq_hw.h | 4 +-
drivers/net/ethernet/aquantia/atlantic/aq_main.c | 11 +----
drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 47 +++++++++++++---------
drivers/net/ethernet/aquantia/atlantic/aq_nic.h | 2 +-
.../ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c | 2 +-
.../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c | 4 +-
7 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_cfg.h b/drivers/net/ethernet/aquantia/atlantic/aq_cfg.h
index fc73831..91eb891 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_cfg.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_cfg.h
@@ -63,8 +63,6 @@
#define AQ_CFG_NAPI_WEIGHT 64U
-#define AQ_CFG_MULTICAST_ADDRESS_MAX 32U
-
/*#define AQ_CFG_MAC_ADDR_PERMANENT {0x30, 0x0E, 0xE3, 0x12, 0x34, 0x56}*/
#define AQ_NIC_FC_OFF 0U
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
index a2d416b..2c6ebd9 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
@@ -98,6 +98,8 @@ struct aq_stats_s {
#define AQ_HW_MEDIA_TYPE_TP 1U
#define AQ_HW_MEDIA_TYPE_FIBRE 2U
+#define AQ_HW_MULTICAST_ADDRESS_MAX 32U
+
struct aq_hw_s {
atomic_t flags;
u8 rbl_enabled:1;
@@ -177,7 +179,7 @@ struct aq_hw_ops {
unsigned int packet_filter);
int (*hw_multicast_list_set)(struct aq_hw_s *self,
- u8 ar_mac[AQ_CFG_MULTICAST_ADDRESS_MAX]
+ u8 ar_mac[AQ_HW_MULTICAST_ADDRESS_MAX]
[ETH_ALEN],
u32 count);
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index ba5fe8c..e3ae29e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -135,17 +135,10 @@ static int aq_ndev_set_mac_address(struct net_device *ndev, void *addr)
static void aq_ndev_set_multicast_settings(struct net_device *ndev)
{
struct aq_nic_s *aq_nic = netdev_priv(ndev);
- int err = 0;
- err = aq_nic_set_packet_filter(aq_nic, ndev->flags);
- if (err < 0)
- return;
+ aq_nic_set_packet_filter(aq_nic, ndev->flags);
- if (netdev_mc_count(ndev)) {
- err = aq_nic_set_multicast_list(aq_nic, ndev);
- if (err < 0)
- return;
- }
+ aq_nic_set_multicast_list(aq_nic, ndev);
}
static const struct net_device_ops aq_ndev_ops = {
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 1a1a638..7a22d02 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -563,34 +563,41 @@ int aq_nic_set_packet_filter(struct aq_nic_s *self, unsigned int flags)
int aq_nic_set_multicast_list(struct aq_nic_s *self, struct net_device *ndev)
{
+ unsigned int packet_filter = self->packet_filter;
struct netdev_hw_addr *ha = NULL;
unsigned int i = 0U;
- self->mc_list.count = 0U;
-
- netdev_for_each_mc_addr(ha, ndev) {
- ether_addr_copy(self->mc_list.ar[i++], ha->addr);
- ++self->mc_list.count;
+ self->mc_list.count = 0;
+ if (netdev_uc_count(ndev) > AQ_HW_MULTICAST_ADDRESS_MAX) {
+ packet_filter |= IFF_PROMISC;
+ } else {
+ netdev_for_each_uc_addr(ha, ndev) {
+ ether_addr_copy(self->mc_list.ar[i++], ha->addr);
- if (i >= AQ_CFG_MULTICAST_ADDRESS_MAX)
- break;
+ if (i >= AQ_HW_MULTICAST_ADDRESS_MAX)
+ break;
+ }
}
- if (i >= AQ_CFG_MULTICAST_ADDRESS_MAX) {
- /* Number of filters is too big: atlantic does not support this.
- * Force all multi filter to support this.
- * With this we disable all UC filters and setup "all pass"
- * multicast mask
- */
- self->packet_filter |= IFF_ALLMULTI;
- self->aq_nic_cfg.mc_list_count = 0;
- return self->aq_hw_ops->hw_packet_filter_set(self->aq_hw,
- self->packet_filter);
+ if (i + netdev_mc_count(ndev) > AQ_HW_MULTICAST_ADDRESS_MAX) {
+ packet_filter |= IFF_ALLMULTI;
} else {
- return self->aq_hw_ops->hw_multicast_list_set(self->aq_hw,
- self->mc_list.ar,
- self->mc_list.count);
+ netdev_for_each_mc_addr(ha, ndev) {
+ ether_addr_copy(self->mc_list.ar[i++], ha->addr);
+
+ if (i >= AQ_HW_MULTICAST_ADDRESS_MAX)
+ break;
+ }
+ }
+
+ if (i > 0 && i < AQ_HW_MULTICAST_ADDRESS_MAX) {
+ packet_filter |= IFF_MULTICAST;
+ self->mc_list.count = i;
+ self->aq_hw_ops->hw_multicast_list_set(self->aq_hw,
+ self->mc_list.ar,
+ self->mc_list.count);
}
+ return aq_nic_set_packet_filter(self, packet_filter);
}
int aq_nic_set_mtu(struct aq_nic_s *self, int new_mtu)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
index faa533a..fecfc40 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
@@ -75,7 +75,7 @@ struct aq_nic_s {
struct aq_hw_link_status_s link_status;
struct {
u32 count;
- u8 ar[AQ_CFG_MULTICAST_ADDRESS_MAX][ETH_ALEN];
+ u8 ar[AQ_HW_MULTICAST_ADDRESS_MAX][ETH_ALEN];
} mc_list;
struct pci_dev *pdev;
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
index 67e2f9f..8cc6aba 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
@@ -765,7 +765,7 @@ static int hw_atl_a0_hw_packet_filter_set(struct aq_hw_s *self,
static int hw_atl_a0_hw_multicast_list_set(struct aq_hw_s *self,
u8 ar_mac
- [AQ_CFG_MULTICAST_ADDRESS_MAX]
+ [AQ_HW_MULTICAST_ADDRESS_MAX]
[ETH_ALEN],
u32 count)
{
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 819f6bc..956860a 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -784,7 +784,7 @@ static int hw_atl_b0_hw_packet_filter_set(struct aq_hw_s *self,
static int hw_atl_b0_hw_multicast_list_set(struct aq_hw_s *self,
u8 ar_mac
- [AQ_CFG_MULTICAST_ADDRESS_MAX]
+ [AQ_HW_MULTICAST_ADDRESS_MAX]
[ETH_ALEN],
u32 count)
{
@@ -812,7 +812,7 @@ static int hw_atl_b0_hw_multicast_list_set(struct aq_hw_s *self,
hw_atl_rpfl2_uc_flr_en_set(self,
(self->aq_nic_cfg->is_mc_list_enabled),
- HW_ATL_B0_MAC_MIN + i);
+ HW_ATL_B0_MAC_MIN + i);
}
err = aq_hw_err_from_flags(self);
--
2.7.4
^ permalink raw reply related
* Re: [PATCHv2 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
From: Xin Long @ 2018-07-05 14:07 UTC (permalink / raw)
To: David Ahern; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
In-Reply-To: <df1c3df6-bd68-8a97-df36-ab3fa14d2bcb@gmail.com>
On Thu, Jul 5, 2018 at 9:18 PM, David Ahern <dsahern@gmail.com> wrote:
> On 7/5/18 1:57 AM, Xin Long wrote:
>> On Thu, Jul 5, 2018 at 2:36 AM, David Ahern <dsahern@gmail.com> wrote:
>>> On 7/4/18 11:56 AM, Xin Long wrote:
>>>
>>>>> your commands are not a proper test. The test should succeed and fail
>>>>> based on the routing lookup, not iptables rules.
>>>> A proper test can be done easily with netns, as vrf can't isolate much.
>>>> I don't want to bother forwarding/ directory with netns, so I will probably
>>>> just drop this selftest, and let the feature patch go first.
>>>>
>>>
>>> BTW, VRF isolates at the routing layer and this is a routing change. We
>>> need to understand why it does not work with VRF. Perhaps another tweak
>>> is needed for VRF.
>> One problem was that the peer may not use the address on the dev
>> that echo_request comes from as the src IP of echo_reply when the
>> echo_request's dst IP is broadcast, but try to get another one by
>> looking up a route without ".flowi4_oif" set. See:
>>
>> icmp_reply()->fib_compute_spec_dst():
>> struct flowi4 fl4 = {
>> .flowi4_iif = LOOPBACK_IFINDEX,
>> .daddr = ip_hdr(skb)->saddr,
>> .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
>> .flowi4_scope = scope,
>> .flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
>> };
>> if (!fib_lookup(net, &fl4, &res, 0))
>> return FIB_RES_PREFSRC(net, res);
>>
>>
>> Without ".flowi4_oif" set, it won't match the vrf route. That's why
>> I had to make h2 NOT into a vrf so that h1 can get the echo_reply.
>> But it can't tell if this echo_reply is from h2 or r1, as r1's echo_reply
>> will also use the same src IP which is actually got from main route
>> space as ".flowi4_oif" is not set.
>> (hope I this description is clear to you) :)
>>
>> So i'm not sure if we can do any tweak for VRF.
>>
>
> Try this:
>
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index b21833651394..e46cdd310e5f 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -300,6 +300,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
> if (!ipv4_is_zeronet(ip_hdr(skb)->saddr)) {
> struct flowi4 fl4 = {
> .flowi4_iif = LOOPBACK_IFINDEX,
> + .flowi4_oif = l3mdev_master_ifindex_rcu(dev),
> .daddr = ip_hdr(skb)->saddr,
> .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
> .flowi4_scope = scope,
Great, with your fix, I can extend more for this selftest.
but I hope no side effects would be caused.
Thank you.
^ permalink raw reply
* [PATCH net 1/1] net/smc: reduce sock_put() for fallback sockets
From: Ursula Braun @ 2018-07-05 14:15 UTC (permalink / raw)
To: davem
Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel, ebiggers3
smc_release() calls a sock_put() for smc fallback sockets to cover
the passive closing sock_hold() in __smc_connect() and
smc_tcp_listen_work(). This does not make sense for sockets in state
SMC_LISTEN and SMC_INIT.
An SMC socket stays in state SMC_INIT if connect fails. The sock_put
in smc_connect_abort() does not cover all failures. Move it into
smc_connect_decline_fallback().
Fixes: ee9dfbef02d18 ("net/smc: handle sockopts forcing fallback")
Reported-by: syzbot+3a0748c8f2f210c0ef9b@syzkaller.appspotmail.com
Reported-by: syzbot+9e60d2428a42049a592a@syzkaller.appspotmail.com
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 15 ++++++++++-----
net/smc/smc_close.c | 2 ++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index e017b6a4452b..5334157f5065 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -147,7 +147,8 @@ static int smc_release(struct socket *sock)
smc->clcsock = NULL;
}
if (smc->use_fallback) {
- sock_put(sk); /* passive closing */
+ if (sk->sk_state != SMC_LISTEN && sk->sk_state != SMC_INIT)
+ sock_put(sk); /* passive closing */
sk->sk_state = SMC_CLOSED;
sk->sk_state_change(sk);
}
@@ -417,12 +418,18 @@ static int smc_connect_decline_fallback(struct smc_sock *smc, int reason_code)
{
int rc;
- if (reason_code < 0) /* error, fallback is not possible */
+ if (reason_code < 0) { /* error, fallback is not possible */
+ if (smc->sk.sk_state == SMC_INIT)
+ sock_put(&smc->sk); /* passive closing */
return reason_code;
+ }
if (reason_code != SMC_CLC_DECL_REPLY) {
rc = smc_clc_send_decline(smc, reason_code);
- if (rc < 0)
+ if (rc < 0) {
+ if (smc->sk.sk_state == SMC_INIT)
+ sock_put(&smc->sk); /* passive closing */
return rc;
+ }
}
return smc_connect_fallback(smc);
}
@@ -435,8 +442,6 @@ static int smc_connect_abort(struct smc_sock *smc, int reason_code,
smc_lgr_forget(smc->conn.lgr);
mutex_unlock(&smc_create_lgr_pending);
smc_conn_free(&smc->conn);
- if (reason_code < 0 && smc->sk.sk_state == SMC_INIT)
- sock_put(&smc->sk); /* passive closing */
return reason_code;
}
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index fa41d9881741..ac961dfb1ea1 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -107,6 +107,8 @@ static void smc_close_active_abort(struct smc_sock *smc)
}
switch (sk->sk_state) {
case SMC_INIT:
+ sk->sk_state = SMC_PEERABORTWAIT;
+ break;
case SMC_ACTIVE:
sk->sk_state = SMC_PEERABORTWAIT;
release_sock(sk);
--
2.16.4
^ permalink raw reply related
* Re: WARNING in smc_unhash_sk
From: Ursula Braun @ 2018-07-05 14:17 UTC (permalink / raw)
To: Eric Biggers, linux-s390
Cc: syzbot, davem, linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <20180704200139.GB725@sol.localdomain>
On 07/04/2018 10:01 PM, Eric Biggers wrote:
> Hi Ursula,
>
> On Fri, Feb 23, 2018 at 07:59:01AM -0800, syzbot wrote:
>> Hello,
>>
>> syzbot hit the following crash on upstream commit
>> af3e79d29555b97dd096e2f8e36a0f50213808a8 (Tue Feb 20 18:05:02 2018 +0000)
>> Merge tag 'leds_for-4.16-rc3' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds
>>
>> So far this crash happened 27 times on
>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/master,
>> net-next, upstream.
>> C reproducer is attached.
>> syzkaller reproducer is attached.
>> Raw console output is attached.
>> compiler: gcc (GCC) 7.1.1 20170620
>> .config is attached.
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+3a0748c8f2f210c0ef9b@syzkaller.appspotmail.com
>> It will help syzbot understand when the bug is fixed. See footer for
>> details.
>> If you forward the report, please keep this part and the footer.
>>
>> WARNING: CPU: 1 PID: 9921 at ./include/net/sock.h:638 sk_del_node_init
>> include/net/sock.h:638 [inline]
>> WARNING: CPU: 1 PID: 9921 at ./include/net/sock.h:638
>> smc_unhash_sk+0x335/0x450 net/smc/af_smc.c:90
>> Kernel panic - not syncing: panic_on_warn set ...
>
> This is still happening and it can be easily reproduced with:
>
> #include <sys/socket.h>
>
> int main()
> {
> char buf[64] = { 0 };
> struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf) };
> struct msghdr msg = { .msg_iov = &iov, .msg_iovlen = 1 };
> int fd;
>
> fd = socket(AF_SMC, SOCK_STREAM, 0);
> sendmsg(fd, &msg, MSG_FASTOPEN);
> }
>
> It seems the following sock_put() in smc_release() is done without any previous
> sock_hold(), causing a use-after-free:
>
> if (smc->use_fallback) {
> sock_put(sk); /* passive closing */
> sk->sk_state = SMC_CLOSED;
> sk->sk_state_change(sk);
> }
>
Sorry for the delay. Now patch is submitted for the net-tree.
^ permalink raw reply
* Re: [PATCH iproute2] man: Fix typos on tc-cbs
From: Simon Horman @ 2018-07-05 14:18 UTC (permalink / raw)
To: Jesus Sanchez-Palencia; +Cc: netdev
In-Reply-To: <20180627175051.29417-1-jesus.sanchez-palencia@intel.com>
Some changelog text should go here.
On Wed, Jun 27, 2018 at 10:50:51AM -0700, Jesus Sanchez-Palencia wrote:
> Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
Otherwise, this seems fine to me.
Reviewed-by: Simon Horman <simon.horman@netronome.com>
> ---
> man/man8/tc-cbs.8 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/man/man8/tc-cbs.8 b/man/man8/tc-cbs.8
> index 32e1e0d4..ad1d8821 100644
> --- a/man/man8/tc-cbs.8
> +++ b/man/man8/tc-cbs.8
> @@ -28,7 +28,7 @@ defined rate limiting method to the traffic.
> This queueing discipline is intended to be used by TSN (Time Sensitive
> Networking) applications, the CBS parameters are derived directly by
> what is described by the Annex L of the IEEE 802.1Q-2014
> -Sepcification. The algorithm and how it affects the latency are
> +Specification. The algorithm and how it affects the latency are
> detailed there.
>
> CBS is meant to be installed under another qdisc that maps packet
> @@ -60,7 +60,7 @@ packet size, which is then used for calculating the idleslope.
> sendslope
> Sendslope is the rate of credits that is depleted (it should be a
> negative number of kilobits per second) when a transmission is
> -ocurring. It can be calculated as follows, (IEEE 802.1Q-2014 Section
> +occurring. It can be calculated as follows, (IEEE 802.1Q-2014 Section
> 8.6.8.2 item g):
>
> sendslope = idleslope - port_transmit_rate
> --
> 2.17.1
>
^ permalink raw reply
* opening tap devices that are created in a container
From: Jason Baron @ 2018-07-05 14:20 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: David S. Miller, libvir-list, rmohr, fabiand, berrange,
Eric W. Biederman
Hi,
Opening tap devices, such as macvtap, that are created in containers is
problematic because the interface for opening tap devices is via
/dev/tapNN and devtmpfs is not typically mounted inside a container as
its not namespace aware. It is possible to do a mknod() in the
container, once the tap devices are created, however, since the tap
devices are created dynamically its not possible to apriori allow access
to certain major/minor numbers, since we don't know what these are going
to be. In addition, its desirable to not allow the mknod capability in
containers. This behavior, I think is somewhat inconsistent with the
tuntap driver where one can create tuntap devices inside a container by
first opening /dev/net/tun and then using them by supplying the tuntap
device name via the ioctl(TUNSETIFF). And since TUNSETIFF validates the
network namespace, one is limited to opening network devices that belong
to your current network namespace.
Here are some options to this issue, that I wanted to get feedback
about, and just wondering if anybody else has run into this.
1)
Don't create the tap device, such as macvtap in the container. Instead,
create the tap device outside of the container and then move it into the
desired container network namespace. In addition, do a mknod() for the
corresponding /dev/tapNN device from outside the container before doing
chroot().
This solution still doesn't allow tap devices to be created inside the
container. Thus, in the case of kubevirt, which runs libvirtd inside of
a container, it would mean changing libvirtd to open existing tap
devices (as opposed to the current behavior of creating new ones). This
would not require any kernel changes, but as mentioned seems
inconsistent with the tuntap interface.
2)
Add a new kernel interface for tap devices similar to how /dev/net/tun
currently works. It might be nice to use TUNSETIFF for tap devices, but
because tap devices have different fops they can't be easily switched
after open(). So the suggestion is a new ioctl (TUNGETFDBYNAME?), where
the tap device name is supplied and a new fd (distinct from the fd
returned by the open of /dev/net/tun) is returned as an output field as
part of the new ioctl parameter.
It may not make sense to have this new ioctl call for /dev/net/tun since
its really about opening a tap device, so it may make sense to introduce
it as part of a new device, such as /dev/net/tap. This new ioctl could
be used for macvtap and ipvtap (or any tap device). I think it might
also improve performance for tuntap devices themselves, if they are
opened this way since currently all tun operations such as read() and
write() take a reference count on the underlying tuntap device, since it
can be changed via TUNSETIFF. I tested this interface out, so I can
provide the kernel changes if that's helpful for clarification.
Thanks,
-Jason
^ permalink raw reply
* [PATCH net-next v6 01/11] net: sched: use rcu for action cookie update
From: Vlad Buslov @ 2018-07-05 14:24 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov, Jiri Pirko
In-Reply-To: <1530800673-12280-1-git-send-email-vladbu@mellanox.com>
Implement functions to atomically update and free action cookie
using rcu mechanism.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/act_api.h | 2 +-
include/net/pkt_cls.h | 1 +
net/sched/act_api.c | 44 ++++++++++++++++++++++++++++++--------------
3 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 5ff11adbe2a6..ffc3ef321776 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -37,7 +37,7 @@ struct tc_action {
spinlock_t tcfa_lock;
struct gnet_stats_basic_cpu __percpu *cpu_bstats;
struct gnet_stats_queue __percpu *cpu_qstats;
- struct tc_cookie *act_cookie;
+ struct tc_cookie __rcu *act_cookie;
struct tcf_chain *goto_chain;
};
#define tcf_index common.tcfa_index
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 6641584b27f1..2081e4219f81 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -781,6 +781,7 @@ struct tc_mqprio_qopt_offload {
struct tc_cookie {
u8 *data;
u32 len;
+ struct rcu_head rcu;
};
struct tc_qopt_offload_stats {
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 3f4cf930f809..02670c7489e3 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -55,6 +55,24 @@ static void tcf_action_goto_chain_exec(const struct tc_action *a,
res->goto_tp = rcu_dereference_bh(chain->filter_chain);
}
+static void tcf_free_cookie_rcu(struct rcu_head *p)
+{
+ struct tc_cookie *cookie = container_of(p, struct tc_cookie, rcu);
+
+ kfree(cookie->data);
+ kfree(cookie);
+}
+
+static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
+ struct tc_cookie *new_cookie)
+{
+ struct tc_cookie *old;
+
+ old = xchg(old_cookie, new_cookie);
+ if (old)
+ call_rcu(&old->rcu, tcf_free_cookie_rcu);
+}
+
/* XXX: For standalone actions, we don't need a RCU grace period either, because
* actions are always connected to filters and filters are already destroyed in
* RCU callbacks, so after a RCU grace period actions are already disconnected
@@ -65,10 +83,7 @@ static void free_tcf(struct tc_action *p)
free_percpu(p->cpu_bstats);
free_percpu(p->cpu_qstats);
- if (p->act_cookie) {
- kfree(p->act_cookie->data);
- kfree(p->act_cookie);
- }
+ tcf_set_action_cookie(&p->act_cookie, NULL);
if (p->goto_chain)
tcf_action_goto_chain_fini(p);
@@ -567,16 +582,22 @@ tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
int err = -EINVAL;
unsigned char *b = skb_tail_pointer(skb);
struct nlattr *nest;
+ struct tc_cookie *cookie;
if (nla_put_string(skb, TCA_KIND, a->ops->kind))
goto nla_put_failure;
if (tcf_action_copy_stats(skb, a, 0))
goto nla_put_failure;
- if (a->act_cookie) {
- if (nla_put(skb, TCA_ACT_COOKIE, a->act_cookie->len,
- a->act_cookie->data))
+
+ rcu_read_lock();
+ cookie = rcu_dereference(a->act_cookie);
+ if (cookie) {
+ if (nla_put(skb, TCA_ACT_COOKIE, cookie->len, cookie->data)) {
+ rcu_read_unlock();
goto nla_put_failure;
+ }
}
+ rcu_read_unlock();
nest = nla_nest_start(skb, TCA_OPTIONS);
if (nest == NULL)
@@ -719,13 +740,8 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
if (err < 0)
goto err_mod;
- if (name == NULL && tb[TCA_ACT_COOKIE]) {
- if (a->act_cookie) {
- kfree(a->act_cookie->data);
- kfree(a->act_cookie);
- }
- a->act_cookie = cookie;
- }
+ if (!name && tb[TCA_ACT_COOKIE])
+ tcf_set_action_cookie(&a->act_cookie, cookie);
/* module count goes up only when brand new policy is created
* if it exists and is only bound to in a_o->init() then
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v6 00/11] Modify action API for implementing lockless actions
From: Vlad Buslov @ 2018-07-05 14:24 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
Currently, all netlink protocol handlers for updating rules, actions and
qdiscs are protected with single global rtnl lock which removes any
possibility for parallelism. This patch set is a first step to remove
rtnl lock dependency from TC rules update path.
Recently, new rtnl registration flag RTNL_FLAG_DOIT_UNLOCKED was added.
Handlers registered with this flag are called without RTNL taken. End
goal is to have rule update handlers(RTM_NEWTFILTER, RTM_DELTFILTER,
etc.) to be registered with UNLOCKED flag to allow parallel execution.
However, there is no intention to completely remove or split rtnl lock
itself. This patch set addresses specific problems in action API that
prevents it from being executed concurrently. This patch set does not
completely unlock rules or actions update path. Additional patch sets
are required to refactor individual actions and filters update for
parallel execution.
As a preparation for executing TC rules update handlers without rtnl
lock, action API code was audited to determine areas that assume
external synchronization with rtnl lock and must be changed to allow
safe concurrent access with following results:
1. Action idr is already protected with spinlock. However, some code
paths assume that idr state is not changes between several
consecutive tcf_idr_* function calls.
2. tc_action reference and bind counters are implemented as plain
integers. They purpose was to allow single actions to be shared
between multiple filters, not to provide means for concurrent
modification.
3. tc_action 'cookie' pointer field is not protected against
modification.
4. Action API functions, that work with set of actions, use intrusive
linked list, which cannot be used concurrently without additional
synchronization.
5. Action API functions don't take reference to actions while using
them, assuming external synchronization with rtnl lock.
Following solutions to these problems are implemented:
1. To remove assumption that idr state doesn't change between tcf_idr_*
calls, implement new functions that atomically perform several
operations on idr without releasing idr spinlock. (function to
atomically lookup and delete action by index, function to atomically
check if action exists and allocate new one if necessary, etc.)
2. Use atomic operations on counters to make them suitable for
concurrent get/put operations.
3. Data that 'cookie' points to is never modified, so it enough to
refactor it to rcu pointer to prevent concurrent de-allocation.
4. Action API doesn't actually use any linked list specific operations
on actions intrusive linked list, so it can be refactored to array in
straightforward manner.
5. Always take reference to action while accessing it in action API.
tcf_idr_search function modified to take reference to action before
returning it, so there is no way to lookup an action without
incrementing its reference counter. All users of this function are
modified to release the reference, after they done using action. With
all users using reference counting, it is now safe to concurrently
delete actions.
Additionally, actions init function signature was expanded with
'rtnl_held' argument, that allows actions that have internal dependency
on rtnl lock to take/release it when necessary.
Since only shared state in action API module are actions themselves and
action idr, these changes are sufficient to not to rely on global rtnl
lock for protection of internal action API data structures.
Changes from V5 to V6:
- Rebase on current net-next
- When action is deleted, set pointer in actions array to NULL to
prevent double freeing.
Changes from V4 to V5:
- Change action delete API to track actions that were deleted, to
prevent releasing them on error.
Changes from V3 to V4:
- Expand cover letter.
- Reduce actions array size in tcf_action_init_1.
- Rebase on latest net-next.
Changes from V2 to V3:
- Re-send with changelog copied to individual patches.
Changes from V1 to V2:
- Removed redundant actions ops lookup during delete.
- Merge action ops delete definition and implementation.
- Assume all actions have delete implemented and don't check for it
explicitly.
- Resplit action lookup/release code to prevent memory leaks in
individual patches.
- Make __tcf_idr_check function static
- Remove unique idr insertion function. Change original idr insert to do
the same thing.
- Merge changes that take reference to action when performing lookup and
changes that account for this additional reference when dumping action
to user space into single patch.
- Change convoluted commit message.
- Rename "unlocked" to "rtnl_held" for clarity.
- Remove estimator lock add patch.
- Refactor action check-alloc code into standalone function.
- Rename tcf_idr_find_delete to tcf_idr_delete_index.
- Rearrange variable definitions in tc_action_delete.
- Add patch that refactors action API code to use array of pointers to
actions instead of intrusive linked list.
- Expand cover letter.
Vlad Buslov (11):
net: sched: use rcu for action cookie update
net: sched: change type of reference and bind counters
net: sched: implement unlocked action init API
net: sched: always take reference to action
net: sched: implement action API that deletes action by index
net: sched: add 'delete' function to action ops
net: sched: implement reference counted action release
net: sched: don't release reference on action overwrite
net: sched: use reference counting action init
net: sched: atomically check-allocate action
net: sched: change action API to use array of pointers to actions
include/net/act_api.h | 25 ++-
include/net/pkt_cls.h | 1 +
net/sched/act_api.c | 415 +++++++++++++++++++++++++++++++--------------
net/sched/act_bpf.c | 34 ++--
net/sched/act_connmark.c | 29 +++-
net/sched/act_csum.c | 34 ++--
net/sched/act_gact.c | 31 +++-
net/sched/act_ife.c | 31 +++-
net/sched/act_ipt.c | 44 ++++-
net/sched/act_mirred.c | 38 +++--
net/sched/act_nat.c | 30 +++-
net/sched/act_pedit.c | 29 +++-
net/sched/act_police.c | 31 ++--
net/sched/act_sample.c | 34 ++--
net/sched/act_simple.c | 31 +++-
net/sched/act_skbedit.c | 31 +++-
net/sched/act_skbmod.c | 34 ++--
net/sched/act_tunnel_key.c | 35 ++--
net/sched/act_vlan.c | 40 +++--
net/sched/cls_api.c | 25 +--
20 files changed, 707 insertions(+), 295 deletions(-)
--
2.7.5
^ permalink raw reply
* [PATCH net-next v6 04/11] net: sched: always take reference to action
From: Vlad Buslov @ 2018-07-05 14:24 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov, Jiri Pirko
In-Reply-To: <1530800673-12280-1-git-send-email-vladbu@mellanox.com>
Without rtnl lock protection it is no longer safe to use pointer to tc
action without holding reference to it. (it can be destroyed concurrently)
Remove unsafe action idr lookup function. Instead of it, implement safe tcf
idr check function that atomically looks up action in idr and increments
its reference and bind counters. Implement both action search and check
using new safe function
Reference taken by idr check is temporal and should not be accounted by
userspace clients (both logically and to preserver current API behavior).
Subtract temporal reference when dumping action to userspace using existing
tca_get_fill function arguments.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
Changes from V1 to V2:
- Make __tcf_idr_check function static
- Merge changes that take reference to action when performing lookup and
changes that account for this additional reference when dumping action
to user space into single patch.
net/sched/act_api.c | 46 ++++++++++++++++++++--------------------------
1 file changed, 20 insertions(+), 26 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 256b0c93916c..aa304d36fee0 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -284,44 +284,38 @@ int tcf_generic_walker(struct tc_action_net *tn, struct sk_buff *skb,
}
EXPORT_SYMBOL(tcf_generic_walker);
-static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo)
+static bool __tcf_idr_check(struct tc_action_net *tn, u32 index,
+ struct tc_action **a, int bind)
{
- struct tc_action *p = NULL;
+ struct tcf_idrinfo *idrinfo = tn->idrinfo;
+ struct tc_action *p;
spin_lock(&idrinfo->lock);
p = idr_find(&idrinfo->action_idr, index);
+ if (p) {
+ refcount_inc(&p->tcfa_refcnt);
+ if (bind)
+ atomic_inc(&p->tcfa_bindcnt);
+ }
spin_unlock(&idrinfo->lock);
- return p;
+ if (p) {
+ *a = p;
+ return true;
+ }
+ return false;
}
int tcf_idr_search(struct tc_action_net *tn, struct tc_action **a, u32 index)
{
- struct tcf_idrinfo *idrinfo = tn->idrinfo;
- struct tc_action *p = tcf_idr_lookup(index, idrinfo);
-
- if (p) {
- *a = p;
- return 1;
- }
- return 0;
+ return __tcf_idr_check(tn, index, a, 0);
}
EXPORT_SYMBOL(tcf_idr_search);
bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
int bind)
{
- struct tcf_idrinfo *idrinfo = tn->idrinfo;
- struct tc_action *p = tcf_idr_lookup(index, idrinfo);
-
- if (index && p) {
- if (bind)
- atomic_inc(&p->tcfa_bindcnt);
- refcount_inc(&p->tcfa_refcnt);
- *a = p;
- return true;
- }
- return false;
+ return __tcf_idr_check(tn, index, a, bind);
}
EXPORT_SYMBOL(tcf_idr_check);
@@ -932,7 +926,7 @@ tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
if (!skb)
return -ENOBUFS;
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event,
- 0, 0) <= 0) {
+ 0, 1) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while adding TC action");
kfree_skb(skb);
return -EINVAL;
@@ -1072,7 +1066,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
return -ENOBUFS;
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
- 0, 1) <= 0) {
+ 0, 2) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action attributes");
kfree_skb(skb);
return -EINVAL;
@@ -1131,14 +1125,14 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, &actions, event, extack);
else { /* delete */
+ cleanup_a(&actions, 1); /* lookup took reference */
ret = tcf_del_notify(net, n, &actions, portid, attr_size, extack);
if (ret)
goto err;
return ret;
}
err:
- if (event != RTM_GETACTION)
- tcf_action_destroy(&actions, 0);
+ tcf_action_destroy(&actions, 0);
return ret;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next v6 05/11] net: sched: implement action API that deletes action by index
From: Vlad Buslov @ 2018-07-05 14:24 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov, Jiri Pirko
In-Reply-To: <1530800673-12280-1-git-send-email-vladbu@mellanox.com>
Implement new action API function that atomically finds and deletes action
from idr by index. Intended to be used by lockless actions that do not rely
on rtnl lock.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
Changes from V1 to V2:
- Rename tcf_idr_find_delete to tcf_idr_delete_index.
include/net/act_api.h | 1 +
net/sched/act_api.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 27823f4e24c4..a8eaae67c264 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -153,6 +153,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
int bind, bool cpustats);
void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a);
+int tcf_idr_delete_index(struct tc_action_net *tn, u32 index);
int __tcf_idr_release(struct tc_action *a, bool bind, bool strict);
static inline int tcf_idr_release(struct tc_action *a, bool bind)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index aa304d36fee0..0f31f09946ab 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -319,6 +319,45 @@ bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
}
EXPORT_SYMBOL(tcf_idr_check);
+int tcf_idr_delete_index(struct tc_action_net *tn, u32 index)
+{
+ struct tcf_idrinfo *idrinfo = tn->idrinfo;
+ struct tc_action *p;
+ int ret = 0;
+
+ spin_lock(&idrinfo->lock);
+ p = idr_find(&idrinfo->action_idr, index);
+ if (!p) {
+ spin_unlock(&idrinfo->lock);
+ return -ENOENT;
+ }
+
+ if (!atomic_read(&p->tcfa_bindcnt)) {
+ if (refcount_dec_and_test(&p->tcfa_refcnt)) {
+ struct module *owner = p->ops->owner;
+
+ WARN_ON(p != idr_remove(&idrinfo->action_idr,
+ p->tcfa_index));
+ spin_unlock(&idrinfo->lock);
+
+ if (p->ops->cleanup)
+ p->ops->cleanup(p);
+
+ gen_kill_estimator(&p->tcfa_rate_est);
+ free_tcf(p);
+ module_put(owner);
+ return 0;
+ }
+ ret = 0;
+ } else {
+ ret = -EPERM;
+ }
+
+ spin_unlock(&idrinfo->lock);
+ return ret;
+}
+EXPORT_SYMBOL(tcf_idr_delete_index);
+
int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
struct tc_action **a, const struct tc_action_ops *ops,
int bind, bool cpustats)
--
2.7.5
^ permalink raw reply related
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