Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] ipvs: replace atomic_add_return()
From: Pablo Neira Ayuso @ 2020-11-22 12:46 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: Yejune Deng, wensong, horms, kadlec, fw, davem, kuba, netdev,
	lvs-devel, netfilter-devel, linux-kernel
In-Reply-To: <9cd77e1e-1c52-d647-9443-485510b4a9b1@ssi.bg>

On Tue, Nov 17, 2020 at 10:57:52PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 16 Nov 2020, Yejune Deng wrote:
> 
> > atomic_inc_return() looks better
> > 
> > Signed-off-by: Yejune Deng <yejune.deng@gmail.com>
> 
> 	Looks good to me for -next, thanks!
> 
> Acked-by: Julian Anastasov <ja@ssi.bg>

Applied, thanks.

^ permalink raw reply

* [PATCH] drivers: Fix the Raspberry Pi debug version compile
From: hby @ 2020-11-22 10:06 UTC (permalink / raw)
  To: kvalo, davem, kuba; +Cc: linux-wireless, netdev, linux-kernel, hby

enable the DEBUG in source code, and it will compile fail,
modify the DEBUG macro, to adapt the compile

Signed-off-by: hby <hby2003@163.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
index 4146faeed..c2eb3aa67 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -60,7 +60,7 @@ void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...);
 				  ##__VA_ARGS__);			\
 	} while (0)
 
-#if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
+#if defined(CONFIG_BRCM_TRACING) || defined(CONFIG_BRCMDBG)
 
 /* For debug/tracing purposes treat info messages as errors */
 #define brcmf_info brcmf_err
@@ -114,7 +114,7 @@ extern int brcmf_msg_level;
 
 struct brcmf_bus;
 struct brcmf_pub;
-#ifdef DEBUG
+#if defined(CONFIG_BRCMDBG)
 struct dentry *brcmf_debugfs_get_devdir(struct brcmf_pub *drvr);
 void brcmf_debugfs_add_entry(struct brcmf_pub *drvr, const char *fn,
 			     int (*read_fn)(struct seq_file *seq, void *data));
-- 
2.17.1



^ permalink raw reply related

* Re: [PATCH net-next,v5 0/9] netfilter: flowtable bridge and vlan enhancements
From: Pablo Neira Ayuso @ 2020-11-22 11:42 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: netfilter-devel, davem, netdev, kuba, fw, razor, jeremy, tobias,
	linux-kernel
In-Reply-To: <20201122102605.2342-1-alobakin@pm.me>

On Sun, Nov 22, 2020 at 10:26:16AM +0000, Alexander Lobakin wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Fri, 20 Nov 2020 13:49:12 +0100
[...]
> > Something like this:
> > 
> >                        fast path
> >                 .------------------------.
> >                /                          \
> >                |           IP forwarding   |
> >                |          /             \  .
> >                |       br0               eth0
> >                .       / \
> >                -- veth1  veth2
> >                    .
> >                    .
> >                    .
> >                  eth0
> >            ab:cd:ef:ab:cd:ef
> >                   VM
> 
> I'm concerned about bypassing vlan and bridge's .ndo_start_xmit() in
> case of this shortcut. We'll have incomplete netdevice Tx stats for
> these two, as it gets updated inside this callbacks.

TX device stats are being updated accordingly.

# ip netns exec nsr1 ip -s link
1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    RX: bytes  packets  errors  dropped overrun mcast   
    0          0        0       0       0       0       
    TX: bytes  packets  errors  dropped carrier collsns 
    0          0        0       0       0       0       
2: veth0@if2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether 82:0d:f3:b5:59:5d brd ff:ff:ff:ff:ff:ff link-netns ns1
    RX: bytes  packets  errors  dropped overrun mcast   
    213290848248 4869765  0       0       0       0       
    TX: bytes  packets  errors  dropped carrier collsns 
    315346667  4777953  0       0       0       0       
3: veth1@if2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether 4a:81:2d:9a:02:88 brd ff:ff:ff:ff:ff:ff link-netns ns2
    RX: bytes  packets  errors  dropped overrun mcast   
    315337919  4777833  0       0       0       0       
    TX: bytes  packets  errors  dropped carrier collsns 
    213290844826 4869708  0       0       0       0       
4: br0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether 82:0d:f3:b5:59:5d brd ff:ff:ff:ff:ff:ff
    RX: bytes  packets  errors  dropped overrun mcast   
    4101       73       0       0       0       0       
    TX: bytes  packets  errors  dropped carrier collsns 
    5256       74       0       0       0       0       
5: veth0.10@veth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue master br0 state UP mode DEFAULT group default qlen 1000
    link/ether 82:0d:f3:b5:59:5d brd ff:ff:ff:ff:ff:ff
    RX: bytes  packets  errors  dropped overrun mcast   
    4101       73       0       0       0       62      
    TX: bytes  packets  errors  dropped carrier collsns 
    315342363  4777893  0       0       0       0       


^ permalink raw reply

* Re: [PATCH net-next,v3 0/9] netfilter: flowtable bridge and vlan enhancements
From: Pablo Neira Ayuso @ 2020-11-22 11:33 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Tobias Waldekranz, netfilter-devel, davem, netdev, razor, jeremy
In-Reply-To: <20201121112348.0e25afa3@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

Hi Jakub,

On Sat, Nov 21, 2020 at 11:23:48AM -0800, Jakub Kicinski wrote:
> On Sat, 21 Nov 2020 19:56:21 +0100 Pablo Neira Ayuso wrote:
> > > Please gather some review tags from senior netdev developers. I don't
> > > feel confident enough to apply this as 100% my own decision.  
> > 
> > Fair enough.
> > 
> > This requirement for very specific Netfilter infrastructure which does
> > not affect any other Networking subsystem sounds new to me.
> 
> You mean me asking for reviews from other senior folks when I don't
> feel good about some code? I've asked others the same thing in the
> past, e.g. Paolo for his RPS thing.

No, I'm perfectly fine with peer review.

Note that I am sending this to net-next as a patchset (not as a PR)
_only_ because this is adding a new .ndo_fill_forward_path to
netdev_ops.

That's the only thing that is relevant to the Netdev core
infrastructure IMO, and this new .ndo that is private, not exposed to
userspace.

Let's have a look at the diffstats again:

 include/linux/netdevice.h                     |  35 +++
 include/net/netfilter/nf_flow_table.h         |  43 +++-
 net/8021q/vlan_dev.c                          |  15 ++
 net/bridge/br_device.c                        |  27 +++
 net/core/dev.c                                |  46 ++++
 net/netfilter/nf_flow_table_core.c            |  51 +++--
 net/netfilter/nf_flow_table_ip.c              | 200 ++++++++++++++----
 net/netfilter/nft_flow_offload.c              | 159 +++++++++++++-
 .../selftests/netfilter/nft_flowtable.sh      |  82 +++++++
 9 files changed, 598 insertions(+), 60 deletions(-)

So this is adding _minimal_ changes to the NetDev infrastructure. Most
of the code is an extension to the flowtable Netfilter infrastructure.
And the flowtable is a cache since its conception.

I am adding the .ndo indirection to avoid the dependencies with
Netfilter modules, e.g. Netfilter could use direct reference to bridge
function, but that would pull in bridge modules.

> > What senior developers specifically you would like I should poke to
> > get an acknowledgement on this to get this accepted of your
> > preference?
> 
> I don't want to make a list. Maybe netconf attendees are a safe bet?

I have no idea who to ask to, traditionally it's the NetDev maintainer
(AFAIK it's only you at this stage) that have the last word on
something to get this merged.

I consider all developers that have reviewed this patchset to be
senior developers.

^ permalink raw reply

* Re: [PATCH net-next v2] compat: always include linux/compat.h from net/compat.h
From: Arnd Bergmann @ 2020-11-22 11:23 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: David Miller, Networking, Christoph Hellwig, Arnd Bergmann
In-Reply-To: <20201121214844.1488283-1-kuba@kernel.org>

On Sat, Nov 21, 2020 at 10:49 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> We're about to do reshuffling in networking headers and
> eliminate some implicit includes. This results in:
>
> In file included from ../net/ipv4/netfilter/arp_tables.c:26:
> include/net/compat.h:60:40: error: unknown type name ‘compat_uptr_t’; did you mean ‘compat_ptr_ioctl’?
>     struct sockaddr __user **save_addr, compat_uptr_t *ptr,
>                                         ^~~~~~~~~~~~~
>                                         compat_ptr_ioctl
> include/net/compat.h:61:4: error: unknown type name ‘compat_size_t’; did you mean ‘compat_sigset_t’?
>     compat_size_t *len);
>     ^~~~~~~~~~~~~
>     compat_sigset_t
>
> Currently net/compat.h depends on linux/compat.h being included
> first. After the upcoming changes this would break the 32bit build.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* RE: [PATCH net] vsock/virtio: discard packets only when socket is really closed
From: Justin He @ 2020-11-22 11:10 UTC (permalink / raw)
  To: Stefano Garzarella, netdev@vger.kernel.org
  Cc: Sergio Lopez, David S. Miller, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, Stefan Hajnoczi,
	virtualization@lists.linux-foundation.org, Jakub Kicinski
In-Reply-To: <20201120104736.73749-1-sgarzare@redhat.com>



> -----Original Message-----
> From: Stefano Garzarella <sgarzare@redhat.com>
> Sent: Friday, November 20, 2020 6:48 PM
> To: netdev@vger.kernel.org
> Cc: Sergio Lopez <slp@redhat.com>; David S. Miller <davem@davemloft.net>;
> Stefano Garzarella <sgarzare@redhat.com>; Justin He <Justin.He@arm.com>;
> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; Stefan Hajnoczi
> <stefanha@redhat.com>; virtualization@lists.linux-foundation.org; Jakub
> Kicinski <kuba@kernel.org>
> Subject: [PATCH net] vsock/virtio: discard packets only when socket is
> really closed
>
> Starting from commit 8692cefc433f ("virtio_vsock: Fix race condition
> in virtio_transport_recv_pkt"), we discard packets in
> virtio_transport_recv_pkt() if the socket has been released.
>
> When the socket is connected, we schedule a delayed work to wait the
> RST packet from the other peer, also if SHUTDOWN_MASK is set in
> sk->sk_shutdown.
> This is done to complete the virtio-vsock shutdown algorithm, releasing
> the port assigned to the socket definitively only when the other peer
> has consumed all the packets.
>
> If we discard the RST packet received, the socket will be closed only
> when the VSOCK_CLOSE_TIMEOUT is reached.
>
> Sergio discovered the issue while running ab(1) HTTP benchmark using
> libkrun [1] and observing a latency increase with that commit.
>
> To avoid this issue, we discard packet only if the socket is really
> closed (SOCK_DONE flag is set).
> We also set SOCK_DONE in virtio_transport_release() when we don't need
> to wait any packets from the other peer (we didn't schedule the delayed
> work). In this case we remove the socket from the vsock lists, releasing
> the port assigned.
>
> [1] https://github.com/containers/libkrun
>
> Fixes: 8692cefc433f ("virtio_vsock: Fix race condition in
> virtio_transport_recv_pkt")

Acked-by: Jia He <justin.he@arm.com>


--
Cheers,
Justin (Jia He)


> Cc: justin.he@arm.com
> Reported-by: Sergio Lopez <slp@redhat.com>
> Tested-by: Sergio Lopez <slp@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> index 0edda1edf988..5956939eebb7 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -841,8 +841,10 @@ void virtio_transport_release(struct vsock_sock *vsk)
>  virtio_transport_free_pkt(pkt);
>  }
>
> -if (remove_sock)
> +if (remove_sock) {
> +sock_set_flag(sk, SOCK_DONE);
>  vsock_remove_sock(vsk);
> +}
>  }
>  EXPORT_SYMBOL_GPL(virtio_transport_release);
>
> @@ -1132,8 +1134,8 @@ void virtio_transport_recv_pkt(struct
> virtio_transport *t,
>
>  lock_sock(sk);
>
> -/* Check if sk has been released before lock_sock */
> -if (sk->sk_shutdown == SHUTDOWN_MASK) {
> +/* Check if sk has been closed before lock_sock */
> +if (sock_flag(sk, SOCK_DONE)) {
>  (void)virtio_transport_reset_no_sock(t, pkt);
>  release_sock(sk);
>  sock_put(sk);
> --
> 2.26.2

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply

* Re: [PATCH nf-next v3 3/3] netfilter: Introduce egress hook
From: Pablo Neira Ayuso @ 2020-11-22 11:01 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Lukas Wunner, Daniel Borkmann, Laura García Liébana,
	John Fastabend, Jozsef Kadlecsik, Florian Westphal,
	Netfilter Development Mailing list, coreteam, Network Development,
	Alexei Starovoitov, Eric Dumazet, Thomas Graf, David Miller
In-Reply-To: <CAADnVQK8qHwdZrqMzQ+4Q9Cg589xLX5zTve92ZKN_zftJg_WHw@mail.gmail.com>

Hi Alexei,

On Sat, Nov 21, 2020 at 07:24:24PM -0800, Alexei Starovoitov wrote:
> On Sat, Nov 21, 2020 at 10:59 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > We're lately discussing more and more usecases in the NFWS meetings
> > where the egress can get really useful.
> 
> We also discussed in the meeting XYZ that this hook is completely pointless.
> Got the hint?

No need to use irony.

OK, so at this point it's basically a bunch of BPF core developers
that is pushing back on these egress support series.

The BPF project is moving on and making progress. Why don't you just
keep convincing more users to adopt your solution? You can just
provide incentives for them to adopt your software, make more
benchmarks, more documentation and so on. That's all perfectly fine
and you are making a great job on that field.

But why you do not just let us move ahead?

If you, the BPF team and your users, do not want to use Netfilter,
that's perfectly fine. Why don't you let users choose what subsystem
of choice that they like for packet filtering?

I already made my own mistakes in the past when I pushed back for BPF
work, that was wrong. It's time to make peace and take this to an end.

Thank you.

^ permalink raw reply

* Re: [PATCH net 1/4] netfilter: nftables_offload: set address type in control dissector
From: Pablo Neira Ayuso @ 2020-11-22 10:49 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netfilter-devel, davem, netdev
In-Reply-To: <20201121164442.01b39ffb@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>

On Sat, Nov 21, 2020 at 04:44:42PM -0800, Jakub Kicinski wrote:
> On Sat, 21 Nov 2020 13:35:58 +0100 Pablo Neira Ayuso wrote:
> > If the address type is missing through the control dissector, then
> > matching on IPv4 and IPv6 addresses does not work.
> 
> Doesn't work where? Are you talking about a specific driver?

No.

It does not work for any kind of match, the control flow dissector
needs to be set on.

^ permalink raw reply

* Re: [PATCH net-next,v5 0/9] netfilter: flowtable bridge and vlan enhancements
From: Alexander Lobakin @ 2020-11-22 10:26 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Alexander Lobakin, netfilter-devel, davem, netdev, kuba, fw,
	razor, jeremy, tobias, linux-kernel

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 20 Nov 2020 13:49:12 +0100

> Hi,
> 
> The following patchset augments the Netfilter flowtable fastpath to
> support for network topologies that combine IP forwarding, bridge and
> VLAN devices.
> 
> This v5 includes updates for:
> 
> - Patch #2: fix incorrect xmit type in IPv6 path, per Florian Westphal.
> - Patch #3: fix possible off by one in dev_fill_forward_path() stack logic,
>             per Florian Westphal.
> - Patch #7: add a note to patch description to specify that FDB topology
>             updates are not supported at this stage, per Jakub Kicinski.
> 
> A typical scenario that can benefit from this infrastructure is composed
> of several VMs connected to bridge ports where the bridge master device
> 'br0' has an IP address. A DHCP server is also assumed to be running to
> provide connectivity to the VMs. The VMs reach the Internet through
> 'br0' as default gateway, which makes the packet enter the IP forwarding
> path. Then, netfilter is used to NAT the packets before they leave
> through the wan device.
> 
> Something like this:
> 
>                        fast path
>                 .------------------------.
>                /                          \
>                |           IP forwarding   |
>                |          /             \  .
>                |       br0               eth0
>                .       / \
>                -- veth1  veth2
>                    .
>                    .
>                    .
>                  eth0
>            ab:cd:ef:ab:cd:ef
>                   VM

I'm concerned about bypassing vlan and bridge's .ndo_start_xmit() in
case of this shortcut. We'll have incomplete netdevice Tx stats for
these two, as it gets updated inside this callbacks.

> The idea is to accelerate forwarding by building a fast path that takes
> packets from the ingress path of the bridge port and place them in the
> egress path of the wan device (and vice versa). Hence, skipping the
> classic bridge and IP stack paths.
> 
> This patchset is composed of:
> 
> Patch #1 adds a placeholder for the hash calculation, instead of using
>          the dir field.
> 
> Patch #2 adds the transmit path type field to the flow tuple. Two transmit
>          paths are supported so far: the neighbour and the xfrm transmit
>          paths. This patch comes in preparation to add a new direct ethernet
>          transmit path (see patch #7).
> 
> Patch #3 adds dev_fill_forward_path() and .ndo_fill_forward_path() to
>          netdev_ops. This new function describes the list of netdevice hops
>          to reach a given destination MAC address in the local network topology,
>          e.g.
> 
>                            IP forwarding
>                           /             \
>                        br0              eth0
>                        / \
>                    veth1 veth2
>                     .
>                     .
>                     .
>                    eth0
>              ab:cd:ef:ab:cd:ef
> 
>           where veth1 and veth2 are bridge ports and eth0 provides Internet
>           connectivity. eth0 is the interface in the VM which is connected to
>           the veth1 bridge port. Then, for packets going to br0 whose
>           destination MAC address is ab:cd:ef:ab:cd:ef, dev_fill_forward_path()
>           provides the following path: br0 -> veth1.
> 
> Patch #4 adds .ndo_fill_forward_path for VLAN devices, which provides the next
>          device hop via vlan->real_dev. This annotates the VLAN id and protocol.
>          This is useful to know what VLAN headers are expected from the ingress
>          device. This also provides information regarding the VLAN headers
>          to be pushed in the egress path.
> 
> Patch #5 adds .ndo_fill_forward_path for bridge devices, which allows to make
>          lookups to the FDB to locate the next device hop (bridge port) in the
>          forwarding path.
> 
> Patch #6 updates the flowtable to use the dev_fill_forward_path()
>          infrastructure to obtain the ingress device in the fastpath.
> 
> Patch #7 updates the flowtable to use dev_fill_forward_path() to obtain the
>          egress device in the forwarding path. This also adds the direct
>          ethernet transmit path, which pushes the ethernet header to the
>          packet and send it through dev_queue_xmit(). This patch adds
>          support for the bridge, so bridge ports use this direct xmit path.
> 
> Patch #8 adds ingress VLAN support (up to 2 VLAN tags, QinQ). The VLAN
>          information is also provided by dev_fill_forward_path(). Store the
>          VLAN id and protocol in the flow tuple for hash lookups. The VLAN
>          support in the xmit path is achieved by annotating the first vlan
>          device found in the xmit path and by calling dev_hard_header()
>          (previous patch #7) before dev_queue_xmit().
> 
> Patch #9 extends nft_flowtable.sh selftest: This is adding a test to
>          cover bridge and vlan support coming in this patchset.
> 
> = Performance numbers
> 
> My testbed environment consists of three containers:
> 
>   192.168.20.2     .20.1     .10.1   10.141.10.2
>          veth0       veth0 veth1      veth0
>         ns1 <---------> nsr1 <--------> ns2
>                             SNAT
>      iperf -c                          iperf -s
> 
> where nsr1 is used for forwarding. There is a bridge device br0 in nsr1,
> veth0 is a port of br0. SNAT is performed on the veth1 device of nsr1.
> 
> - ns2 runs iperf -s
> - ns1 runs iperf -c 10.141.10.2 -n 100G
> 
> My results are:
> 
> - Baseline (no flowtable, classic forwarding path + netfilter): ~16 Gbit/s
> - Fastpath (with flowtable, this patchset): ~25 Gbit/s
> 
> This is an improvement of ~50% compared to baseline.
> 
> Please, apply. Thank you.
> 
> Pablo Neira Ayuso (9):
>   netfilter: flowtable: add hash offset field to tuple
>   netfilter: flowtable: add xmit path types
>   net: resolve forwarding path from virtual netdevice and HW destination address
>   net: 8021q: resolve forwarding path for vlan devices
>   bridge: resolve forwarding path for bridge devices
>   netfilter: flowtable: use dev_fill_forward_path() to obtain ingress device
>   netfilter: flowtable: use dev_fill_forward_path() to obtain egress device
>   netfilter: flowtable: add vlan support
>   selftests: netfilter: flowtable bridge and VLAN support
> 
>  include/linux/netdevice.h                     |  35 +++
>  include/net/netfilter/nf_flow_table.h         |  43 +++-
>  net/8021q/vlan_dev.c                          |  15 ++
>  net/bridge/br_device.c                        |  27 +++
>  net/core/dev.c                                |  46 ++++
>  net/netfilter/nf_flow_table_core.c            |  51 +++--
>  net/netfilter/nf_flow_table_ip.c              | 200 ++++++++++++++----
>  net/netfilter/nft_flow_offload.c              | 159 +++++++++++++-
>  .../selftests/netfilter/nft_flowtable.sh      |  82 +++++++
>  9 files changed, 598 insertions(+), 60 deletions(-)
> 
> --
> 2.20.1

Al


^ permalink raw reply

* Re: [PATCH net-next 3/3] net: phy: mscc: use new PTP_MSGTYPE_* defines
From: kernel test robot @ 2020-11-22  9:42 UTC (permalink / raw)
  To: Christian Eggers, Richard Cochran, Andrew Lunn, Heiner Kallweit,
	Jakub Kicinski
  Cc: kbuild-all, Vladimir Oltean, Russell King, David S . Miller,
	netdev, linux-kernel, Christian Eggers
In-Reply-To: <20201122082636.12451-4-ceggers@arri.de>

[-- Attachment #1: Type: text/plain, Size: 4357 bytes --]

Hi Christian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f9e425e99b0756c1479042afe761073779df2a30
config: sparc-randconfig-r012-20201122 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/78cc4b0e1739511ed9712c9466a48ddc6885d153
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christian-Eggers/net-ptp-use-common-defines-for-PTP-message-types-in-further-drivers/20201122-163319
        git checkout 78cc4b0e1739511ed9712c9466a48ddc6885d153
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/phy/mscc/mscc_ptp.c: In function 'vsc85xx_ptp_cmp_init':
   drivers/net/phy/mscc/mscc_ptp.c:510:3: error: 'PTP_MSGTYPE_SYNC' undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'?
     510 |   PTP_MSGTYPE_SYNC,
         |   ^~~~~~~~~~~~~~~~
         |   CSD_TYPE_SYNC
   drivers/net/phy/mscc/mscc_ptp.c:510:3: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/phy/mscc/mscc_ptp.c:511:3: error: 'PTP_MSGTYPE_DELAY_REQ' undeclared (first use in this function)
     511 |   PTP_MSGTYPE_DELAY_REQ
         |   ^~~~~~~~~~~~~~~~~~~~~
   drivers/net/phy/mscc/mscc_ptp.c: In function 'vsc85xx_ptp_conf':
   drivers/net/phy/mscc/mscc_ptp.c:851:3: error: 'PTP_MSGTYPE_SYNC' undeclared (first use in this function); did you mean 'CSD_TYPE_SYNC'?
     851 |   PTP_MSGTYPE_SYNC,
         |   ^~~~~~~~~~~~~~~~
         |   CSD_TYPE_SYNC
>> drivers/net/phy/mscc/mscc_ptp.c:851:3: warning: initialization of 'unsigned char' from 'u8 *' {aka 'unsigned char *'} makes integer from pointer without a cast [-Wint-conversion]
   drivers/net/phy/mscc/mscc_ptp.c:851:3: note: (near initialization for 'msgs[0]')
   drivers/net/phy/mscc/mscc_ptp.c:852:3: error: 'PTP_MSGTYPE_DELAY_REQ' undeclared (first use in this function)
     852 |   PTP_MSGTYPE_DELAY_REQ
         |   ^~~~~~~~~~~~~~~~~~~~~
   drivers/net/phy/mscc/mscc_ptp.c:852:3: warning: initialization of 'unsigned char' from 'u8 *' {aka 'unsigned char *'} makes integer from pointer without a cast [-Wint-conversion]
   drivers/net/phy/mscc/mscc_ptp.c:852:3: note: (near initialization for 'msgs[1]')
>> drivers/net/phy/mscc/mscc_ptp.c:861:20: warning: comparison between pointer and integer
     861 |   else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
         |                    ^~

vim +851 drivers/net/phy/mscc/mscc_ptp.c

   846	
   847	static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
   848				    bool one_step, bool enable)
   849	{
   850		u8 msgs[] = {
 > 851			PTP_MSGTYPE_SYNC,
   852			PTP_MSGTYPE_DELAY_REQ
   853		};
   854		u32 val;
   855		u8 i;
   856	
   857		for (i = 0; i < ARRAY_SIZE(msgs); i++) {
   858			if (blk == INGRESS)
   859				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   860							   PTP_WRITE_NS);
 > 861			else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
   862				/* no need to know Sync t when sending in one_step */
   863				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   864							   PTP_WRITE_1588);
   865			else
   866				vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
   867							   PTP_SAVE_IN_TS_FIFO);
   868	
   869			val = vsc85xx_ts_read_csr(phydev, blk,
   870						  MSCC_ANA_PTP_FLOW_ENA(i));
   871			val &= ~PTP_FLOW_ENA;
   872			if (enable)
   873				val |= PTP_FLOW_ENA;
   874			vsc85xx_ts_write_csr(phydev, blk, MSCC_ANA_PTP_FLOW_ENA(i),
   875					     val);
   876		}
   877	
   878		return 0;
   879	}
   880	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29585 bytes --]

^ permalink raw reply

* [PATCH 1/1] xdp: compact the function xsk_map_inc
From: Zhu Yanjun @ 2020-11-22  9:04 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, davem, netdev; +Cc: Zhu Yanjun, Zhu Yanjun

From: Zhu Yanjun <zyjzyj2000@gmail.com>

The function xsk_map_inc always returns zero. As such, changing the
return type to void and removing the test code.

Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
Signed-off-by: Zhu Yanjun <yanjunz@nvidia.com>
---
 net/xdp/xsk.c    |    1 -
 net/xdp/xsk.h    |    2 +-
 net/xdp/xskmap.c |   10 ++--------
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index cfbec39..c1b8a88 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -548,7 +548,6 @@ static void xsk_unbind_dev(struct xdp_sock *xs)
 	node = list_first_entry_or_null(&xs->map_list, struct xsk_map_node,
 					node);
 	if (node) {
-		WARN_ON(xsk_map_inc(node->map));
 		map = node->map;
 		*map_entry = node->map_entry;
 	}
diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h
index b9e896c..766b9e2 100644
--- a/net/xdp/xsk.h
+++ b/net/xdp/xsk.h
@@ -41,7 +41,7 @@ struct xsk_map_node {
 
 void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,
 			     struct xdp_sock **map_entry);
-int xsk_map_inc(struct xsk_map *map);
+void xsk_map_inc(struct xsk_map *map);
 void xsk_map_put(struct xsk_map *map);
 void xsk_clear_pool_at_qid(struct net_device *dev, u16 queue_id);
 int xsk_reg_pool_at_qid(struct net_device *dev, struct xsk_buff_pool *pool,
diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c
index 49da2b8..c7dd94a 100644
--- a/net/xdp/xskmap.c
+++ b/net/xdp/xskmap.c
@@ -11,10 +11,9 @@
 
 #include "xsk.h"
 
-int xsk_map_inc(struct xsk_map *map)
+void xsk_map_inc(struct xsk_map *map)
 {
 	bpf_map_inc(&map->map);
-	return 0;
 }
 
 void xsk_map_put(struct xsk_map *map)
@@ -26,17 +25,12 @@ void xsk_map_put(struct xsk_map *map)
 					       struct xdp_sock **map_entry)
 {
 	struct xsk_map_node *node;
-	int err;
 
 	node = kzalloc(sizeof(*node), GFP_ATOMIC | __GFP_NOWARN);
 	if (!node)
 		return ERR_PTR(-ENOMEM);
 
-	err = xsk_map_inc(map);
-	if (err) {
-		kfree(node);
-		return ERR_PTR(err);
-	}
+	xsk_map_inc(map);
 
 	node->map = map;
 	node->map_entry = map_entry;
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 044/141] net/mlx4: Fix fall-through warnings for Clang
From: Tariq Toukan @ 2020-11-22  8:36 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Tariq Toukan, David S. Miller,
	Jakub Kicinski
  Cc: netdev, linux-rdma, linux-kernel, linux-hardening
In-Reply-To: <84cd69bc9b9768cf3bc032c0205ffe485b80ba03.1605896059.git.gustavoars@kernel.org>



On 11/20/2020 8:31 PM, Gustavo A. R. Silva wrote:
> In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning
> by explicitly adding a break statement instead of just letting the code
> fall through to the next case.
> 
> Link: https://github.com/KSPP/linux/issues/115
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>   drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> index 1187ef1375e2..e6b8b8dc7894 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> @@ -2660,6 +2660,7 @@ int mlx4_FREE_RES_wrapper(struct mlx4_dev *dev, int slave,
>   	case RES_XRCD:
>   		err = xrcdn_free_res(dev, slave, vhcr->op_modifier, alop,
>   				     vhcr->in_param, &vhcr->out_param);
> +		break;
>   
>   	default:
>   		break;
> 

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Thanks for your patch.

^ permalink raw reply

* [PATCH net-next 3/3] net: phy: mscc: use new PTP_MSGTYPE_* defines
From: Christian Eggers @ 2020-11-22  8:26 UTC (permalink / raw)
  To: Richard Cochran, Andrew Lunn, Heiner Kallweit, Jakub Kicinski
  Cc: Vladimir Oltean, Russell King, David S . Miller, netdev,
	linux-kernel, Christian Eggers, Christian Eggers, Quentin Schulz,
	Antoine Tenart, Antoine Tenart
In-Reply-To: <20201122082636.12451-1-ceggers@arri.de>

Use recently introduced PTP_MSGTYPE_SYNC and PTP_MSGTYPE_DELAY_REQ
defines instead of a driver internal enumeration.

Signed-off-by: Christian Eggers <ceggers@gmx.de>
Cc: Quentin Schulz <quentin.schulz@bootlin.com>
Cc: Antoine Tenart <atenart@kernel.org>
Cc: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_ptp.c | 14 +++++++-------
 drivers/net/phy/mscc/mscc_ptp.h |  5 -----
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index d8a61456d1ce..924ed5b034a4 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -506,9 +506,9 @@ static int vsc85xx_ptp_cmp_init(struct phy_device *phydev, enum ts_blk blk)
 {
 	struct vsc8531_private *vsc8531 = phydev->priv;
 	bool base = phydev->mdio.addr == vsc8531->ts_base_addr;
-	enum vsc85xx_ptp_msg_type msgs[] = {
-		PTP_MSG_TYPE_SYNC,
-		PTP_MSG_TYPE_DELAY_REQ
+	u8 msgs[] = {
+		PTP_MSGTYPE_SYNC,
+		PTP_MSGTYPE_DELAY_REQ
 	};
 	u32 val;
 	u8 i;
@@ -847,9 +847,9 @@ static int vsc85xx_ts_ptp_action_flow(struct phy_device *phydev, enum ts_blk blk
 static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
 			    bool one_step, bool enable)
 {
-	enum vsc85xx_ptp_msg_type msgs[] = {
-		PTP_MSG_TYPE_SYNC,
-		PTP_MSG_TYPE_DELAY_REQ
+	u8 msgs[] = {
+		PTP_MSGTYPE_SYNC,
+		PTP_MSGTYPE_DELAY_REQ
 	};
 	u32 val;
 	u8 i;
@@ -858,7 +858,7 @@ static int vsc85xx_ptp_conf(struct phy_device *phydev, enum ts_blk blk,
 		if (blk == INGRESS)
 			vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
 						   PTP_WRITE_NS);
-		else if (msgs[i] == PTP_MSG_TYPE_SYNC && one_step)
+		else if (msgs[i] == PTP_MSGTYPE_SYNC && one_step)
 			/* no need to know Sync t when sending in one_step */
 			vsc85xx_ts_ptp_action_flow(phydev, blk, msgs[i],
 						   PTP_WRITE_1588);
diff --git a/drivers/net/phy/mscc/mscc_ptp.h b/drivers/net/phy/mscc/mscc_ptp.h
index 3ea163af0f4f..da3465360e90 100644
--- a/drivers/net/phy/mscc/mscc_ptp.h
+++ b/drivers/net/phy/mscc/mscc_ptp.h
@@ -436,11 +436,6 @@ enum ptp_cmd {
 	PTP_SAVE_IN_TS_FIFO = 11, /* invalid when writing in reg */
 };
 
-enum vsc85xx_ptp_msg_type {
-	PTP_MSG_TYPE_SYNC,
-	PTP_MSG_TYPE_DELAY_REQ,
-};
-
 struct vsc85xx_ptphdr {
 	u8 tsmt; /* transportSpecific | messageType */
 	u8 ver;  /* reserved0 | versionPTP */
-- 
Christian Eggers
Embedded software developer


^ permalink raw reply related

* [PATCH net-next 2/3] mlxsw: spectrum_ptp: use PTP wide message type definitions
From: Christian Eggers @ 2020-11-22  8:26 UTC (permalink / raw)
  To: Richard Cochran, Andrew Lunn, Heiner Kallweit, Jakub Kicinski
  Cc: Vladimir Oltean, Russell King, David S . Miller, netdev,
	linux-kernel, Christian Eggers, Christian Eggers, Petr Machata,
	Jiri Pirko, Ido Schimmel
In-Reply-To: <20201122082636.12451-1-ceggers@arri.de>

Use recently introduced PTP wide defines instead of a driver internal
enumeration.

Signed-off-by: Christian Eggers <ceggers@gmx.de>
Cc: Petr Machata <petrm@mellanox.com>
Cc: Jiri Pirko <jiri@nvidia.com>
Cc: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c | 8 ++++----
 drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h | 7 -------
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index ca8090a28dec..d6e9ecb14681 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -828,10 +828,10 @@ struct mlxsw_sp_ptp_state *mlxsw_sp1_ptp_init(struct mlxsw_sp *mlxsw_sp)
 		goto err_hashtable_init;
 
 	/* Delive these message types as PTP0. */
-	message_type = BIT(MLXSW_SP_PTP_MESSAGE_TYPE_SYNC) |
-		       BIT(MLXSW_SP_PTP_MESSAGE_TYPE_DELAY_REQ) |
-		       BIT(MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_REQ) |
-		       BIT(MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_RESP);
+	message_type = BIT(PTP_MSGTYPE_SYNC) |
+		       BIT(PTP_MSGTYPE_DELAY_REQ) |
+		       BIT(PTP_MSGTYPE_PDELAY_REQ) |
+		       BIT(PTP_MSGTYPE_PDELAY_RESP);
 	err = mlxsw_sp_ptp_mtptpt_set(mlxsw_sp, MLXSW_REG_MTPTPT_TRAP_ID_PTP0,
 				      message_type);
 	if (err)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
index 8c386571afce..1d43a3755285 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
@@ -11,13 +11,6 @@ struct mlxsw_sp;
 struct mlxsw_sp_port;
 struct mlxsw_sp_ptp_clock;
 
-enum {
-	MLXSW_SP_PTP_MESSAGE_TYPE_SYNC,
-	MLXSW_SP_PTP_MESSAGE_TYPE_DELAY_REQ,
-	MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_REQ,
-	MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_RESP,
-};
-
 static inline int mlxsw_sp_ptp_get_ts_info_noptp(struct ethtool_ts_info *info)
 {
 	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-- 
Christian Eggers
Embedded software developer


^ permalink raw reply related

* [PATCH net-next 1/3] net: phy: dp83640: use new PTP_MSGTYPE_SYNC define
From: Christian Eggers @ 2020-11-22  8:26 UTC (permalink / raw)
  To: Richard Cochran, Andrew Lunn, Heiner Kallweit, Jakub Kicinski
  Cc: Vladimir Oltean, Russell King, David S . Miller, netdev,
	linux-kernel, Christian Eggers, Christian Eggers, Kurt Kanzenbach
In-Reply-To: <20201122082636.12451-1-ceggers@arri.de>

Replace use of magic number with recently introduced define.

Signed-off-by: Christian Eggers <ceggers@gmx.de>
Cc: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/phy/dp83640.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index f2caccaf4408..9757ca0d9633 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -964,15 +964,12 @@ static void decode_status_frame(struct dp83640_private *dp83640,
 static int is_sync(struct sk_buff *skb, int type)
 {
 	struct ptp_header *hdr;
-	u8 msgtype;
 
 	hdr = ptp_parse_header(skb, type);
 	if (!hdr)
 		return 0;
 
-	msgtype = ptp_get_msgtype(hdr, type);
-
-	return (msgtype & 0xf) == 0;
+	return ptp_get_msgtype(hdr, type) == PTP_MSGTYPE_SYNC;
 }
 
 static void dp83640_free_clocks(void)
-- 
Christian Eggers
Embedded software developer


^ permalink raw reply related

* [PATCH net-next 0/3] net: ptp: use common defines for PTP message types in further drivers
From: Christian Eggers @ 2020-11-22  8:26 UTC (permalink / raw)
  To: Richard Cochran, Andrew Lunn, Heiner Kallweit, Jakub Kicinski
  Cc: Vladimir Oltean, Russell King, David S . Miller, netdev,
	linux-kernel, Christian Eggers

This series replaces further driver internal enumeration / uses of magic
numbers with the newly introduced PTP_MSGTYPE_* defines.

On Friday, 20 November 2020, 23:39:10 CET, Vladimir Oltean wrote:
> On Fri, Nov 20, 2020 at 09:41:03AM +0100, Christian Eggers wrote:
> > This series introduces commen defines for PTP event messages. Driver
> > internal defines are removed and some uses of magic numbers are replaced
> > by the new defines.
> > [...]
> 
> I understand that you don't want to spend a lifetime on this, but I see
> that there are more drivers which you did not touch.
> 
> is_sync() in drivers/net/phy/dp83640.c can be made to
> 	return ptp_get_msgtype(hdr, type) == PTP_MSGTYPE_SYNC;
> 
> this can be removed from drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h:
> enum {
> 	MLXSW_SP_PTP_MESSAGE_TYPE_SYNC,
> 	MLXSW_SP_PTP_MESSAGE_TYPE_DELAY_REQ,
> 	MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_REQ,
> 	MLXSW_SP_PTP_MESSAGE_TYPE_PDELAY_RESP,
> };
I think that I have found an addtional one in the Microsemi VSC85xx PHY driver.




^ permalink raw reply

* Re: [PATCH V2 net 4/4] net: ena: return error code from ena_xdp_xmit_buff
From: Shay Agroskin @ 2020-11-22  7:19 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, dwmw, zorik, matua, saeedb, msw, aliguori, nafea, gtzalik,
	netanel, alisaidi, benh, akiyano, sameehj, ndagan
In-Reply-To: <20201121155304.1751d0c6@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

Jakub Kicinski <kuba@kernel.org> writes:

> On Thu, 19 Nov 2020 22:28:51 +0200 Shay Agroskin wrote:
>> The function mistakenly returns NETDEV_TX_OK regardless of the
>> transmission success. This patch fixes this behavior by 
>> returning the
>> error code from the function.
>> 
>> Fixes: 548c4940b9f1 ("net: ena: Implement XDP_TX action")
>> Signed-off-by: Shay Agroskin <shayagr@amazon.com>
>
> Doesn't seem like a legitimate bug fix, since the only caller of 
> this
> function ignores its return value.

Hi,
I plan to use the return value from this function in future patch 
(next-net series), do you think we better send this fix with
this future patch?

Shay

^ permalink raw reply

* Re: [PATCH mlx5-next 11/16] net/mlx5: Add VDPA priority to NIC RX namespace
From: Eli Cohen @ 2020-11-22  6:41 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Saeed Mahameed, Leon Romanovsky, netdev, linux-rdma, Eli Cohen,
	Mark Bloch, Maor Gottlieb
In-Reply-To: <20201121160155.39d84650@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Sat, Nov 21, 2020 at 04:01:55PM -0800, Jakub Kicinski wrote:
> On Fri, 20 Nov 2020 15:03:34 -0800 Saeed Mahameed wrote:
> > From: Eli Cohen <eli@mellanox.com>
> > 
> > Add a new namespace type to the NIC RX root namespace to allow for
> > inserting VDPA rules before regular NIC but after bypass, thus allowing
> > DPDK to have precedence in packet processing.
> 
> How does DPDK and VDPA relate in this context?

mlx5 steering is hierarchical and defines precedence amongst namespaces.
Up till now, the VDPA implementation would insert a rule into the
MLX5_FLOW_NAMESPACE_BYPASS hierarchy which is used by DPDK thus taking
all the incoming traffic.

The MLX5_FLOW_NAMESPACE_VDPA hirerachy comes after
MLX5_FLOW_NAMESPACE_BYPASS.

^ permalink raw reply

* [PATCH net 2/2] devlink: Make sure devlink instance and port are in same net namespace
From: Parav Pandit @ 2020-11-22  6:12 UTC (permalink / raw)
  To: kuba, davem, netdev, jiri; +Cc: Parav Pandit
In-Reply-To: <20201122061257.60425-1-parav@nvidia.com>

When devlink reload operation is not used, netdev of an Ethernet port
may be present in different net namespace than the net namespace of the
devlink instance.

Ensure that both the devlink instance and devlink port netdev are
located in same net namespace.

Fixes: 070c63f20f6c ("net: devlink: allow to change namespaces during reload")
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 net/core/devlink.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6135ef5972ce..2e1b0e82a102 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -778,12 +778,15 @@ devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *por
 static int devlink_nl_port_netdev_fill(struct sk_buff *msg, struct devlink_port *devlink_port)
 {
 	struct net_device *netdev = devlink_port->type_dev;
-	int err;
+	int err = 0;
 
 	ASSERT_RTNL();
 	if (!netdev)
 		return 0;
 
+	if (!net_eq(devlink_net(devlink_port->devlink), dev_net(netdev)))
+		goto done;
+
 	err = nla_put_u32(msg, DEVLINK_ATTR_PORT_NETDEV_IFINDEX, netdev->ifindex);
 	if (err)
 		goto done;
-- 
2.26.2


^ permalink raw reply related

* [PATCH net 1/2] devlink: Hold rtnl lock while reading netdev attributes
From: Parav Pandit @ 2020-11-22  6:12 UTC (permalink / raw)
  To: kuba, davem, netdev, jiri; +Cc: Parav Pandit
In-Reply-To: <20201122061257.60425-1-parav@nvidia.com>

A netdevice of a devlink port can be moved to different
net namespace than its parent devlink instance.
This scenario occurs when devlink reload is not used for
maintaining backward compatibility.

When netdevice is undergoing migration to net namespace,
its ifindex and name may change.

In such use case, devlink port query may read stale netdev
attributes.

Fix it by reading them under rtnl lock.

Fixes: bfcd3a466172 ("Introduce devlink infrastructure")
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 net/core/devlink.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index acc29d5157f4..6135ef5972ce 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -775,6 +775,23 @@ devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *por
 	return err;
 }
 
+static int devlink_nl_port_netdev_fill(struct sk_buff *msg, struct devlink_port *devlink_port)
+{
+	struct net_device *netdev = devlink_port->type_dev;
+	int err;
+
+	ASSERT_RTNL();
+	if (!netdev)
+		return 0;
+
+	err = nla_put_u32(msg, DEVLINK_ATTR_PORT_NETDEV_IFINDEX, netdev->ifindex);
+	if (err)
+		goto done;
+	err = nla_put_string(msg, DEVLINK_ATTR_PORT_NETDEV_NAME, netdev->name);
+done:
+	return err;
+}
+
 static int devlink_nl_port_fill(struct sk_buff *msg, struct devlink *devlink,
 				struct devlink_port *devlink_port,
 				enum devlink_command cmd, u32 portid,
@@ -792,6 +809,8 @@ static int devlink_nl_port_fill(struct sk_buff *msg, struct devlink *devlink,
 	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX, devlink_port->index))
 		goto nla_put_failure;
 
+	/* Hold rtnl lock while accessing port's netdev attributes. */
+	rtnl_lock();
 	spin_lock_bh(&devlink_port->type_lock);
 	if (nla_put_u16(msg, DEVLINK_ATTR_PORT_TYPE, devlink_port->type))
 		goto nla_put_failure_type_locked;
@@ -800,13 +819,10 @@ static int devlink_nl_port_fill(struct sk_buff *msg, struct devlink *devlink,
 			devlink_port->desired_type))
 		goto nla_put_failure_type_locked;
 	if (devlink_port->type == DEVLINK_PORT_TYPE_ETH) {
-		struct net_device *netdev = devlink_port->type_dev;
+		int err;
 
-		if (netdev &&
-		    (nla_put_u32(msg, DEVLINK_ATTR_PORT_NETDEV_IFINDEX,
-				 netdev->ifindex) ||
-		     nla_put_string(msg, DEVLINK_ATTR_PORT_NETDEV_NAME,
-				    netdev->name)))
+		err = devlink_nl_port_netdev_fill(msg, devlink_port);
+		if (err)
 			goto nla_put_failure_type_locked;
 	}
 	if (devlink_port->type == DEVLINK_PORT_TYPE_IB) {
@@ -818,6 +834,7 @@ static int devlink_nl_port_fill(struct sk_buff *msg, struct devlink *devlink,
 			goto nla_put_failure_type_locked;
 	}
 	spin_unlock_bh(&devlink_port->type_lock);
+	rtnl_unlock();
 	if (devlink_nl_port_attrs_put(msg, devlink_port))
 		goto nla_put_failure;
 	if (devlink_nl_port_function_attrs_put(msg, devlink_port, extack))
@@ -828,6 +845,7 @@ static int devlink_nl_port_fill(struct sk_buff *msg, struct devlink *devlink,
 
 nla_put_failure_type_locked:
 	spin_unlock_bh(&devlink_port->type_lock);
+	rtnl_unlock();
 nla_put_failure:
 	genlmsg_cancel(msg, hdr);
 	return -EMSGSIZE;
-- 
2.26.2


^ permalink raw reply related

* [PATCH net 0/2] devlink port attribute fixes
From: Parav Pandit @ 2020-11-22  6:12 UTC (permalink / raw)
  To: kuba, davem, netdev, jiri; +Cc: Parav Pandit

This patchset contains 2 small fixes for devlink port attributes.

Patch summary:
Patch-1 synchronize the devlink port attribute reader
        with net namespace change operation
Patch-2 Ensure to return devlink port's netdevice attributes
        when netdev and devlink instance belong to same net namespace

Parav Pandit (2):
  devlink: Hold rtnl lock while reading netdev attributes
  devlink: Make sure devlink instance and port are in same net namespace

 net/core/devlink.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

-- 
2.26.2


^ permalink raw reply

* Re: [PATCH nf-next v3 3/3] netfilter: Introduce egress hook
From: Alexei Starovoitov @ 2020-11-22  3:24 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Lukas Wunner, Daniel Borkmann, Laura García Liébana,
	John Fastabend, Jozsef Kadlecsik, Florian Westphal,
	Netfilter Development Mailing list, coreteam, Network Development,
	Alexei Starovoitov, Eric Dumazet, Thomas Graf, David Miller
In-Reply-To: <20201121185922.GA23266@salvia>

On Sat, Nov 21, 2020 at 10:59 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> We're lately discussing more and more usecases in the NFWS meetings
> where the egress can get really useful.

We also discussed in the meeting XYZ that this hook is completely pointless.
Got the hint?

^ permalink raw reply

* Re: [RFC] MAINTAINERS tag for cleanup robot
From: Matthew Wilcox @ 2020-11-22  3:23 UTC (permalink / raw)
  To: trix
  Cc: joe, clang-built-linux, linux-hyperv, linux-kernel, xen-devel,
	tboot-devel, kvm, linux-crypto, linux-acpi, devel, amd-gfx,
	dri-devel, intel-gfx, netdev, linux-media, MPT-FusionLinux.pdl,
	linux-scsi, linux-wireless, ibm-acpi-devel, platform-driver-x86,
	linux-usb, linux-omap, linux-fbdev, ecryptfs, linux-fsdevel,
	cluster-devel, linux-mtd, keyrings, netfilter-devel, coreteam,
	alsa-devel, bpf, linux-bluetooth, linux-nfs, patches
In-Reply-To: <20201121165058.1644182-1-trix@redhat.com>

On Sat, Nov 21, 2020 at 08:50:58AM -0800, trix@redhat.com wrote:
> The fixer review is
> https://reviews.llvm.org/D91789
> 
> A run over allyesconfig for x86_64 finds 62 issues, 5 are false positives.
> The false positives are caused by macros passed to other macros and by
> some macro expansions that did not have an extra semicolon.
> 
> This cleans up about 1,000 of the current 10,000 -Wextra-semi-stmt
> warnings in linux-next.

Are any of them not false-positives?  It's all very well to enable
stricter warnings, but if they don't fix any bugs, they're just churn.

^ permalink raw reply

* Re: [PATCH 072/141] can: peak_usb: Fix fall-through warnings for Clang
From: Joe Perches @ 2020-11-22  2:46 UTC (permalink / raw)
  To: Marc Kleine-Budde, Gustavo A. R. Silva, Wolfgang Grandegger,
	David S. Miller, Jakub Kicinski
  Cc: linux-can, netdev, linux-kernel, linux-hardening
In-Reply-To: <d2fc3c0e-54de-3f2a-1434-76a80847965c@pengutronix.de>

On Sun, 2020-11-22 at 00:04 +0100, Marc Kleine-Budde wrote:
> On 11/21/20 8:50 PM, Joe Perches wrote:
> > > What about moving the default to the end if the case, which is more common anyways:
> > > 
> > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> > []
> > > @@ -295,16 +295,16 @@ static void peak_usb_write_bulk_callback(struct urb *urb)
> > >                 netif_trans_update(netdev);
> > >                 break;
> > >  
> > > 
> > > -       default:
> > > -               if (net_ratelimit())
> > > -                       netdev_err(netdev, "Tx urb aborted (%d)\n",
> > > -                                  urb->status);
> > >         case -EPROTO:
> > >         case -ENOENT:
> > >         case -ECONNRESET:
> > >         case -ESHUTDOWN:
> > > -
> > >                 break;
> > > +
> > > +       default:
> > > +               if (net_ratelimit())
> > > +                       netdev_err(netdev, "Tx urb aborted (%d)\n",
> > > +                                  urb->status);
> > 
> > That's fine and is more generally used style but this
> > default: case should IMO also end with a break;
> > 
> > +		break;
> 
> I don't mind.
> 
> process/coding-style.rst is not totally clear about the break after the default,
> if this is the lase one the switch statement.

deprecated.rst has:

All switch/case blocks must end in one of:

* break;
* fallthrough;
* continue;
* goto <label>;
* return [expression];

I suppose that could be moved into coding-style along with
maybe a change to "all switch/case/default blocks"


^ permalink raw reply

* [PATCH net] ipv6: addrlabel: fix possible memory leak in ip6addrlbl_net_init
From: Wang Hai @ 2020-11-22  2:34 UTC (permalink / raw)
  To: davem, kuznet, yoshfuji, kuba; +Cc: netdev, linux-kernel

kmemleak report a memory leak as follows:

unreferenced object 0xffff8880059c6a00 (size 64):
  comm "ip", pid 23696, jiffies 4296590183 (age 1755.384s)
  hex dump (first 32 bytes):
    20 01 00 10 00 00 00 00 00 00 00 00 00 00 00 00   ...............
    1c 00 00 00 00 00 00 00 00 00 00 00 07 00 00 00  ................
  backtrace:
    [<00000000aa4e7a87>] ip6addrlbl_add+0x90/0xbb0
    [<0000000070b8d7f1>] ip6addrlbl_net_init+0x109/0x170
    [<000000006a9ca9d4>] ops_init+0xa8/0x3c0
    [<000000002da57bf2>] setup_net+0x2de/0x7e0
    [<000000004e52d573>] copy_net_ns+0x27d/0x530
    [<00000000b07ae2b4>] create_new_namespaces+0x382/0xa30
    [<000000003b76d36f>] unshare_nsproxy_namespaces+0xa1/0x1d0
    [<0000000030653721>] ksys_unshare+0x3a4/0x780
    [<0000000007e82e40>] __x64_sys_unshare+0x2d/0x40
    [<0000000031a10c08>] do_syscall_64+0x33/0x40
    [<0000000099df30e7>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

We should free all rules when we catch an error in ip6addrlbl_net_init().
otherwise a memory leak will occur.

Fixes: 2a8cc6c89039 ("[IPV6] ADDRCONF: Support RFC3484 configurable address selection policy table.")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Wang Hai <wanghai38@huawei.com>
---
 net/ipv6/addrlabel.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
index 642fc6ac13d2..637e323a0224 100644
--- a/net/ipv6/addrlabel.c
+++ b/net/ipv6/addrlabel.c
@@ -306,6 +306,8 @@ static int ip6addrlbl_del(struct net *net,
 /* add default label */
 static int __net_init ip6addrlbl_net_init(struct net *net)
 {
+	struct ip6addrlbl_entry *p = NULL;
+	struct hlist_node *n;
 	int err = 0;
 	int i;
 
@@ -320,9 +322,17 @@ static int __net_init ip6addrlbl_net_init(struct net *net)
 					 ip6addrlbl_init_table[i].prefixlen,
 					 0,
 					 ip6addrlbl_init_table[i].label, 0);
-		/* XXX: should we free all rules when we catch an error? */
-		if (ret && (!err || err != -ENOMEM))
+		if (ret && (!err || err != -ENOMEM)) {
 			err = ret;
+			goto err_ip6addrlbl_add;
+		}
+	}
+	return err;
+
+err_ip6addrlbl_add:
+	hlist_for_each_entry_safe(p, n, &net->ipv6.ip6addrlbl_table.head, list) {
+		hlist_del_rcu(&p->list);
+		kfree_rcu(p, rcu);
 	}
 	return err;
 }
-- 
2.17.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox