* [PATCH net v2] bridge: netlink: call br_changelink() during br_dev_newlink()
From: Ivan Vecera @ 2017-01-20 16:59 UTC (permalink / raw)
To: netdev; +Cc: stephen, davem, bridge, jiri
Any bridge options specified during link creation (e.g. ip link add)
are ignored as br_dev_newlink() does not process them.
Use br_changelink() to do it.
Fixes: 1332351 bridge: implement rtnl_link_ops->changelink
Signed-off-by: Ivan Vecera <cera@cera.cz>
---
net/bridge/br_netlink.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 71c7453..7109b38 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -781,20 +781,6 @@ static int br_validate(struct nlattr *tb[], struct nlattr *data[])
return 0;
}
-static int br_dev_newlink(struct net *src_net, struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[])
-{
- struct net_bridge *br = netdev_priv(dev);
-
- if (tb[IFLA_ADDRESS]) {
- spin_lock_bh(&br->lock);
- br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS]));
- spin_unlock_bh(&br->lock);
- }
-
- return register_netdevice(dev);
-}
-
static int br_port_slave_changelink(struct net_device *brdev,
struct net_device *dev,
struct nlattr *tb[],
@@ -1115,6 +1101,25 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
return 0;
}
+static int br_dev_newlink(struct net *src_net, struct net_device *dev,
+ struct nlattr *tb[], struct nlattr *data[])
+{
+ struct net_bridge *br = netdev_priv(dev);
+ int err;
+
+ if (tb[IFLA_ADDRESS]) {
+ spin_lock_bh(&br->lock);
+ br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS]));
+ spin_unlock_bh(&br->lock);
+ }
+
+ err = br_changelink(dev, tb, data);
+ if (err)
+ return err;
+
+ return register_netdevice(dev);
+}
+
static size_t br_get_size(const struct net_device *brdev)
{
return nla_total_size(sizeof(u32)) + /* IFLA_BR_FORWARD_DELAY */
--
2.10.2
^ permalink raw reply related
* RE: [net PATCH v5 6/6] virtio_net: XDP support for adjust_head
From: David Laight @ 2017-01-20 16:59 UTC (permalink / raw)
To: 'Michael S. Tsirkin', Jason Wang
Cc: John Fastabend, john.r.fastabend@intel.com,
netdev@vger.kernel.org, alexei.starovoitov@gmail.com,
daniel@iogearbox.net
In-Reply-To: <20170119231027-mutt-send-email-mst@kernel.org>
From: Michael S. Tsirkin
> Sent: 19 January 2017 21:12
> > On 2017?01?18? 23:15, Michael S. Tsirkin wrote:
> > > On Tue, Jan 17, 2017 at 02:22:59PM -0800, John Fastabend wrote:
> > > > Add support for XDP adjust head by allocating a 256B header region
> > > > that XDP programs can grow into. This is only enabled when a XDP
> > > > program is loaded.
> > > >
> > > > In order to ensure that we do not have to unwind queue headroom push
> > > > queue setup below bpf_prog_add. It reads better to do a prog ref
> > > > unwind vs another queue setup call.
> > > >
> > > > At the moment this code must do a full reset to ensure old buffers
> > > > without headroom on program add or with headroom on program removal
> > > > are not used incorrectly in the datapath. Ideally we would only
> > > > have to disable/enable the RX queues being updated but there is no
> > > > API to do this at the moment in virtio so use the big hammer. In
> > > > practice it is likely not that big of a problem as this will only
> > > > happen when XDP is enabled/disabled changing programs does not
> > > > require the reset. There is some risk that the driver may either
> > > > have an allocation failure or for some reason fail to correctly
> > > > negotiate with the underlying backend in this case the driver will
> > > > be left uninitialized. I have not seen this ever happen on my test
> > > > systems and for what its worth this same failure case can occur
> > > > from probe and other contexts in virtio framework.
> > > >
> > > > Signed-off-by: John Fastabend<john.r.fastabend@intel.com>
> > > I've been thinking about it - can't we drop
> > > old buffers without the head room which were posted before
> > > xdp attached?
> > >
> > > Avoiding the reset would be much nicer.
> > >
> > > Thoughts?
> > >
> >
> > As been discussed before, device may use them in the same time so it's not
> > safe. Or do you mean detect them after xdp were set and drop the buffer
> > without head room, this looks sub-optimal.
> >
> > Thanks
>
> Yes, this is what I mean. Why is this suboptimal? It's a single branch
> in code. Yes we might lose some packets but the big hammer of device
> reset will likely lose more.
Why not leave let the hardware receive into the 'small' buffer (without
headroom) and do a copy when a frame is received.
Replace the buffers with 'big' ones for the next receive.
A data copy on a ring full of buffers won't really be noticed.
David
^ permalink raw reply
* Re: [net-next PATCH 0/3] Retrieve number of VFs in a bus-agnostic way
From: Phil Sutter @ 2017-01-20 16:59 UTC (permalink / raw)
To: David Miller; +Cc: bhelgaas, netdev, linux-pci
In-Reply-To: <20170120.114346.786441265338417344.davem@davemloft.net>
On Fri, Jan 20, 2017 at 11:43:46AM -0500, David Miller wrote:
> From: Phil Sutter <phil@nwl.cc>
> Date: Wed, 18 Jan 2017 14:04:36 +0100
>
> > Previously, it was assumed that only PCI NICs would be capable of having
> > virtual functions - with my proposed enhancement of dummy NIC driver
> > implementing (fake) ones for testing purposes, this is no longer true.
> >
> > Discussion of said patch has led to the suggestion of implementing a
> > bus-agnostic method for VF count retrieval so rtnetlink could work with
> > both real VF-capable PCI NICs as well as my dummy modifications without
> > introducing ugly hacks.
> >
> > The following series tries to achieve just that by introducing a bus
> > type callback to retrieve a device's number of VFs, implementing this
> > callback for PCI bus and finally adjusting rtnetlink to make use of the
> > generalized infrastructure.
>
> This is really nice and clean, compare it to your original approach :-)
Yes, indeed! Thanks a lot for pointing me into the right direction. :)
Cheers, Phil
^ permalink raw reply
* Re: [RFC] net: ipv6: return the first matched rt6_info for multicast packets in find_rr_leaf()
From: David Miller @ 2017-01-20 16:58 UTC (permalink / raw)
To: sekraj; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <20170118151337.mcub5vyjszy5ltyn@extreme-Latitude-E4310>
From: Rajasekar Kumar <sekraj@gmail.com>
Date: Wed, 18 Jan 2017 20:43:37 +0530
> There is a performance issue when large number of interfaces are
> enabled with VRRP protocol in 2 router nodes which are connected
> to each other. When VRRP hello is received (which is multicast
> packet with DIP: ff02::18), a rt6_info node is added to fib6_node
> of address ff02::18. This happens for each interface on which
> VRRP is enabled. For 2000 interfaces with VRRP enabled, 2000
> rt6_info nodes are added to the same fib6_node. As of today,
> find_rr_leaf() goes further to find better match, even after first
> successful match based on interface key. In this case, it walks
> 2000 nodes for every incoming packet/outgoing packet, which is
> expensive and not needed. rt6_info match based on supplied
> interface match should be sufficient. The first match occurs
> when there is interface match, and after that there can not be
> another match for multicast packets. So, first match should be
> returned for multicast packets.
>
> find_rr_leaf() tries to find best available gateway, mainly based on
> interface match and gateway's reachablity info.When this is required
> for unicast packets, multicast packets do not need either gateway's
> reachability status or gateway's Layer2 address as it is derived
> from Destination IP (group address). rt6_info match based on supplied
> interface match should be sufficient.
>
> This fix helps in scenario wherein multicast packets arrive in some
> interfaces frequently than other interfaces. rt6_info is added to
> beginning of list for former cases. Verified this case.
>
> Signed-off-by: Rajasekar Kumar <sekraj@gmail.com>
So the only thing different in each rt6_info in the list is the
interface, right?
Well, that's a part of the lookup key, multicast or not. If the user
binds a socket to a specific interface, they want the route lookup to
return the rt6_info node with that device.
So I think your change introduces a regression, therefore another
solution will need to be found for your performance problem.
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: David Miller @ 2017-01-20 16:52 UTC (permalink / raw)
To: marcelo.leitner
Cc: colin.king, vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120164820.GA669@localhost.localdomain>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Fri, 20 Jan 2017 14:48:20 -0200
> Seems you applied it on net tree, but commit that introduced the issue
> (7b9438de0cd4) is still only on net-next.
That explains everything.
> I build-tested it here before acking, it worked, on top of
> 4567d686f5c6d955e57a3afa1741944c1e7f4033.
>
> Colin, please respin the patch.. add the Fixes tag, fix the missing 'o'
> in my name on the changelog :-) and tag the patch as net-next tree too.
Indeed, thanks.
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: marcelo.leitner @ 2017-01-20 16:51 UTC (permalink / raw)
To: David Miller
Cc: colin.king, vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120.113209.1764653226397979876.davem@davemloft.net>
On Fri, Jan 20, 2017 at 11:32:09AM -0500, David Miller wrote:
> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Date: Fri, 20 Jan 2017 11:58:45 -0200
>
> > Not sure I can add the Fixes tag for you here, but:
> > Fixes: 7b9438de0cd4 ("sctp: add stream reconf timer")
> > Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>
> FWIW, patchwork doesn't notice updated Fixes: tags like this but I
> try to do so myself and the effort is definitely appreciated.
Ok, good to know, thanks.
>
> We need a V3 of this change anyways, see the build failure I reported
> for V2, so Colin can add the Fixes: tag there explicitly.
>
Replied this one in there.
Marcelo
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: Marcelo Ricardo Leitner @ 2017-01-20 16:48 UTC (permalink / raw)
To: David Miller
Cc: colin.king, vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120.113117.1592305434267734738.davem@davemloft.net>
On Fri, Jan 20, 2017 at 11:31:17AM -0500, David Miller wrote:
> From: Colin King <colin.king@canonical.com>
> Date: Fri, 20 Jan 2017 13:45:42 +0000
>
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > Table sctp_timer_tbl is missing a TIMEOUT_RECONF string so
> > add this in. Also compare timeout with the size of the array
> > sctp_timer_tbl rather than SCTP_EVENT_TIMEOUT_MAX. Also add
> > a build time check that SCTP_EVENT_TIMEOUT_MAX is correct
> > so we don't ever get this kind of mismatch between the table
> > and SCTP_EVENT_TIMEOUT_MAX in the future.
> >
> > Kudos to Marcel Ricardo Leitner for spotting the missing string
> > and suggesting the build time sanity check.
> >
> > Fixes CoverityScan CID#1397639 ("Out-of-bounds read")
> >
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> Well, my bad... I reverted V1, that's fine.
>
> But this patch doesn't even compile.
>
> In file included from ./include/uapi/linux/stddef.h:1:0,
> from ./include/linux/stddef.h:4,
> from ./include/uapi/linux/posix_types.h:4,
> from ./include/uapi/linux/types.h:13,
> from ./include/linux/types.h:5,
> from ./include/net/sctp/sctp.h:58,
> from net/sctp/debug.c:41:
> net/sctp/debug.c: In function ‘sctp_tname’:
> ./include/linux/compiler.h:518:38: error: call to ‘__compiletime_assert_170’ declared with attribute error: BUILD_BUG_ON failed: SCTP_EVENT_TIMEOUT_MAX + 1 != ARRAY_SIZE(sctp_timer_tbl)
> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
Seems you applied it on net tree, but commit that introduced the issue
(7b9438de0cd4) is still only on net-next.
I build-tested it here before acking, it worked, on top of
4567d686f5c6d955e57a3afa1741944c1e7f4033.
Colin, please respin the patch.. add the Fixes tag, fix the missing 'o'
in my name on the changelog :-) and tag the patch as net-next tree too.
Thanks,
Marcelo
^ permalink raw reply
* [PATCH net] bridge: netlink: call br_changelink() during br_dev_newlink()
From: Ivan Vecera @ 2017-01-20 16:46 UTC (permalink / raw)
To: netdev; +Cc: stephen, davem, bridge, jiri
Any bridge options specified during link creation (e.g. ip link add)
are ignored as br_dev_newlink() does not process them.
Use br_changelink() to do it.
Signed-off-by: Ivan Vecera <cera@cera.cz>
---
net/bridge/br_netlink.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 71c7453..7109b38 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -781,20 +781,6 @@ static int br_validate(struct nlattr *tb[], struct nlattr *data[])
return 0;
}
-static int br_dev_newlink(struct net *src_net, struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[])
-{
- struct net_bridge *br = netdev_priv(dev);
-
- if (tb[IFLA_ADDRESS]) {
- spin_lock_bh(&br->lock);
- br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS]));
- spin_unlock_bh(&br->lock);
- }
-
- return register_netdevice(dev);
-}
-
static int br_port_slave_changelink(struct net_device *brdev,
struct net_device *dev,
struct nlattr *tb[],
@@ -1115,6 +1101,25 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
return 0;
}
+static int br_dev_newlink(struct net *src_net, struct net_device *dev,
+ struct nlattr *tb[], struct nlattr *data[])
+{
+ struct net_bridge *br = netdev_priv(dev);
+ int err;
+
+ if (tb[IFLA_ADDRESS]) {
+ spin_lock_bh(&br->lock);
+ br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS]));
+ spin_unlock_bh(&br->lock);
+ }
+
+ err = br_changelink(dev, tb, data);
+ if (err)
+ return err;
+
+ return register_netdevice(dev);
+}
+
static size_t br_get_size(const struct net_device *brdev)
{
return nla_total_size(sizeof(u32)) + /* IFLA_BR_FORWARD_DELAY */
--
2.10.2
^ permalink raw reply related
* Re: [PATCH 3/3] bcm63xx_enet: avoid uninitialized variable warning
From: David Miller @ 2017-01-20 16:46 UTC (permalink / raw)
To: arnd
Cc: f.fainelli, xypron.glpk, linux-kernel, bcm-kernel-feedback-list,
netdev, linux-arm-kernel, tremyfr
In-Reply-To: <20170118145306.1004008-3-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 18 Jan 2017 15:52:53 +0100
> gcc-7 and probably earlier versions get confused by this function
> and print a harmless warning:
>
> drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open':
> drivers/net/ethernet/broadcom/bcm63xx_enet.c:1130:3: error: 'phydev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This adds an initialization for the 'phydev' variable when it is unused
> and changes the check to test for that NULL pointer to make it clear
> that we always pass a valid pointer here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Also applied, thanks.
^ permalink raw reply
* Re: [PATCH net] virtio-net: restore VIRTIO_HDR_F_DATA_VALID on receiving
From: Michael S. Tsirkin @ 2017-01-20 16:45 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, Rolf Neugebauer, linux-kernel, virtualization
In-Reply-To: <1484893962-7614-1-git-send-email-jasowang@redhat.com>
On Fri, Jan 20, 2017 at 02:32:42PM +0800, Jason Wang wrote:
> Commit 501db511397f ("virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on
> xmit") in fact disables VIRTIO_HDR_F_DATA_VALID on receiving path too,
> fixing this by adding a hint (has_data_valid) and set it only on the
> receiving path.
>
> Cc: Rolf Neugebauer <rolf.neugebauer@docker.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/macvtap.c | 2 +-
> drivers/net/tun.c | 2 +-
> drivers/net/virtio_net.c | 2 +-
> include/linux/virtio_net.h | 6 +++++-
> net/packet/af_packet.c | 4 ++--
> 5 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 5c26653..4026185 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -825,7 +825,7 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
> return -EINVAL;
>
> if (virtio_net_hdr_from_skb(skb, &vnet_hdr,
> - macvtap_is_little_endian(q)))
> + macvtap_is_little_endian(q), true))
> BUG();
>
> if (copy_to_iter(&vnet_hdr, sizeof(vnet_hdr), iter) !=
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index cd8e02c..2cd10b2 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1360,7 +1360,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
> return -EINVAL;
>
> if (virtio_net_hdr_from_skb(skb, &gso,
> - tun_is_little_endian(tun))) {
> + tun_is_little_endian(tun), true)) {
> struct skb_shared_info *sinfo = skb_shinfo(skb);
> pr_err("unexpected GSO type: "
> "0x%x, gso_size %d, hdr_len %d\n",
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 4a10500..3474243 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1104,7 +1104,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
> hdr = skb_vnet_hdr(skb);
>
> if (virtio_net_hdr_from_skb(skb, &hdr->hdr,
> - virtio_is_little_endian(vi->vdev)))
> + virtio_is_little_endian(vi->vdev), false))
> BUG();
>
> if (vi->mergeable_rx_bufs)
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 5643647..5209b5e 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -56,7 +56,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>
> static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> struct virtio_net_hdr *hdr,
> - bool little_endian)
> + bool little_endian,
> + bool has_data_valid)
> {
> memset(hdr, 0, sizeof(*hdr)); /* no info leak */
>
I would prefer naming it is_rx. Callers should not know about
internal details like data valid, the issue we are trying to fix
here is that tx and tx headers are slightly different.
> @@ -91,6 +92,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> skb_checksum_start_offset(skb));
> hdr->csum_offset = __cpu_to_virtio16(little_endian,
> skb->csum_offset);
> + } else if (has_data_valid &&
> + skb->ip_summed == CHECKSUM_UNNECESSARY) {
> + hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
> } /* else everything is zero */
>
> return 0;
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index b9e1a13..3d555c7 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1976,7 +1976,7 @@ static int packet_rcv_vnet(struct msghdr *msg, const struct sk_buff *skb,
> return -EINVAL;
> *len -= sizeof(vnet_hdr);
>
> - if (virtio_net_hdr_from_skb(skb, &vnet_hdr, vio_le()))
> + if (virtio_net_hdr_from_skb(skb, &vnet_hdr, vio_le(), true))
> return -EINVAL;
>
> return memcpy_to_msg(msg, (void *)&vnet_hdr, sizeof(vnet_hdr));
> @@ -2237,7 +2237,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> if (po->has_vnet_hdr) {
> if (virtio_net_hdr_from_skb(skb, h.raw + macoff -
> sizeof(struct virtio_net_hdr),
> - vio_le())) {
> + vio_le(), true)) {
> spin_lock(&sk->sk_receive_queue.lock);
> goto drop_n_account;
> }
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH 2/3] qed: avoid possible stack overflow in qed_ll2_acquire_connection
From: David Miller @ 2017-01-20 16:44 UTC (permalink / raw)
To: arnd
Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, Ram.Amrani,
weiyongjun1, hare, netdev, linux-kernel
In-Reply-To: <20170118145306.1004008-2-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 18 Jan 2017 15:52:52 +0100
> struct qed_ll2_info is rather large, so putting it on the stack
> can cause an overflow, as this warning tries to tell us:
>
> drivers/net/ethernet/qlogic/qed/qed_ll2.c: In function 'qed_ll2_start':
> drivers/net/ethernet/qlogic/qed/qed_ll2.c:2159:1: error: the frame size of 1056 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> qed_ll2_start_ooo() already uses a dynamic allocation for the structure
> to work around that problem, and we could do the same in qed_ll2_start()
> as well as qed_roce_ll2_start(), but since the structure is only
> used to pass a couple of initialization values here, it seems nicer
> to replace it with a different structure.
>
> Lacking any idea for better naming, I'm adding 'struct qed_ll2_conn',
> which now contains all the initialization data, and this now simply
> gets copied into struct qed_ll2_info rather than assigning all members
> one by one.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [net-next PATCH 0/3] Retrieve number of VFs in a bus-agnostic way
From: David Miller @ 2017-01-20 16:43 UTC (permalink / raw)
To: phil; +Cc: bhelgaas, netdev, linux-pci
In-Reply-To: <20170118130439.15023-1-phil@nwl.cc>
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 18 Jan 2017 14:04:36 +0100
> Previously, it was assumed that only PCI NICs would be capable of having
> virtual functions - with my proposed enhancement of dummy NIC driver
> implementing (fake) ones for testing purposes, this is no longer true.
>
> Discussion of said patch has led to the suggestion of implementing a
> bus-agnostic method for VF count retrieval so rtnetlink could work with
> both real VF-capable PCI NICs as well as my dummy modifications without
> introducing ugly hacks.
>
> The following series tries to achieve just that by introducing a bus
> type callback to retrieve a device's number of VFs, implementing this
> callback for PCI bus and finally adjusting rtnetlink to make use of the
> generalized infrastructure.
This is really nice and clean, compare it to your original approach :-)
Series applied, thanks!
^ permalink raw reply
* Re: Getting a handle on all these new NIC features
From: Martin Habets @ 2017-01-20 16:36 UTC (permalink / raw)
To: Tom Herbert, Linux Kernel Network Developers
In-Reply-To: <CALx6S37YLY7-9YUTWBCBuLCWxbQL1=YURzqFZ00S1pb=2W9kFg@mail.gmail.com>
Hi Tom,
On 17/01/17 22:05, Tom Herbert wrote:
> There was some discussion about the problems of dealing with the
> explosion of NIC features in the mlx directory restructuring proposal,
> but I think the is a deeper issue here that should be discussed.
>
> It's hard not to notice that there has been quite a proliferation of
> NIC features in several drivers. This trend had resulted in very
> complex driver code that may or may not segment individual features.
> One visible manifestation of this is number of ndo functions which is
> somewhere around seventy-five now.
>
> I suspect the vast majority of these advances NIC features (e.g.
> bridging, UDP offloads, tc offload, etc.) are only relevant to some of
> the people some of the time. The problem we have, in this case those
> of us that are attempting to deploy and maintain NICs at scale, is
> when we have to deal with the ramifications of these features being
> intertwined with core driver functionality that is relevant to
> everyone. This becomes very obvious when we need to backport drivers
> from later versions of kernel.
>
> I realize that backports of a driver is not a specific concern of the
> Linux kernel, but nevertheless this is a real problem and a fact of
> life for many users. Rebasing the full kernel is still a major effort
> and it seems the best we could ever do is one rebase per year. In the
> interim we need to occasionally backport drivers. Backporting drivers
> is difficult precisely because of new features or API changes to
> existing ones. These sort of changes tend to have a spiderweb of
> dependencies in other parts of the stack so that the number of patches
> we need to cherry-pick goes way beyond those that touch the driver we
> are interested in.
For the sfc driver (Solarflare Adapters) we currently do backports internally for:
- RedHat Enterprise Linux 5.10, 5.11
- RedHat Enterprise Linux 6.5, 6.6, 6.7, 6.8
- Redhat Messaging Realtime and Grid 2.5
- RedHat Enterprise Linux 7.0, 7.1, 7.2
- RedHat Enterprise Linux for Realtime 7.1, 7.2
- SuSE Linux Enterprise Server 11 sp3, sp4
- SuSE Linux Enterprise RealTime Extension 11
- SuSE Linux Enterprise Server 12 base release, sp1
- Canonical Ubuntu Server LTS 14.04, 16.04
- Canonical Ubuntu Server -
- Debian 7 "Wheezy" 7.X
- Debian 8 "Jessie" 8.X
- Linux 2.6.18 to 4.9-rc1
We update this list as needed, and always try to support the latest kernel.
I do not know if that would cover the kernel version you are using.
Best regards,
Martin
> Currently we (FB) need to backport two NIC drivers. I've already gave
> details of backporting mlx5 on the thread to restructure the driver
> directories. The other driver being backporting seems to suffer from
> the same type of feature complexity.
>
> In short, I would like to ask if driver maintainers to start to
> modularize driver features. If something being added is obviously a
> narrow feature that only a subset of users will need can we allow
> config options to #ifdef those out somehow? Furthermore can the file
> and directory structure of drivers reflect that; our lives would be
> _so_ much simpler to maintain drivers in production if we have such
> modularity and the ability to build drivers with the features of our
> choosing.
>
> Thanks,
> Tom
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: David Miller @ 2017-01-20 16:32 UTC (permalink / raw)
To: marcelo.leitner
Cc: colin.king, vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120135845.GT3781@localhost.localdomain>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Fri, 20 Jan 2017 11:58:45 -0200
> Not sure I can add the Fixes tag for you here, but:
> Fixes: 7b9438de0cd4 ("sctp: add stream reconf timer")
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
FWIW, patchwork doesn't notice updated Fixes: tags like this but I
try to do so myself and the effort is definitely appreciated.
We need a V3 of this change anyways, see the build failure I reported
for V2, so Colin can add the Fixes: tag there explicitly.
^ permalink raw reply
* Re: [PATCH net-next V4] tc: flower: Refactor matching flags to be more user friendly
From: Jiri Benc @ 2017-01-20 16:41 UTC (permalink / raw)
To: David Laight
Cc: Paul Blakey, Stephen Hemminger, netdev@vger.kernel.org,
Jiri Pirko, Or Gerlitz, Roi Dayan, Simon Horman
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0268D18@AcuExch.aculab.com>
On Fri, 20 Jan 2017 12:27:42 +0000, David Laight wrote:
> Consider what happened with "no", "nofubar" and "nofubar_baz",
> all ought to be rejected.
Why? "no" translates to "", "nofubar" to "fubar", etc. And those will
be evaluated the same way as if they were supplied without the "no".
I don't see a problem with this.
> Actually using strncmp() is also overkill.
Why? It compares two bytes. There's an extra null at the end of the
"no" string but I wouldn't call that "overkill".
> Nothing wrong with:
> if (token[0] == 'n' && token[1] == 'o' && token[2]) {
Except that strncmp is easier to understand and cleaner.
> no = true;
> token += 2;
> if (token[0] == '_' && token[1])
> token++;
This doesn't make sense. The intent was not to allow both "nofrag" and
"no_frag". The code in the patch treats "no_frag" as invalid and that's
okay.
> ...
>
> or replace the last 3 lines with:
> token += 2 + (token[2] == '_' & token[3]);
That's horribly ugly. Anyone looking at this will spent 2 minutes
trying to untangle the code instead of the 2 seconds with the current
code. We're not trying to win the Obfuscated C Contest here.
Jiri
^ permalink raw reply
* Re: [PATCHv3 net-next 3/4] sctp: add support for generating stream reconf add incoming/outgoing streams request chunk
From: Marcelo Ricardo Leitner @ 2017-01-20 16:39 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, Vlad Yasevich, davem@davemloft.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026900F@AcuExch.aculab.com>
On Fri, Jan 20, 2017 at 02:50:01PM +0000, David Laight wrote:
> From: Xin Long
> > Sent: 19 January 2017 17:19
> > This patch is to define Add Incoming/Outgoing Streams Request
> > Parameter described in rfc6525 section 4.5 and 4.6. They can
> > be in one same chunk trunk as rfc6525 section 3.1-7 describes,
> > so make them in one function.
> ...
> > +struct sctp_strreset_addstrm {
> > + sctp_paramhdr_t param_hdr;
> > + __u32 request_seq;
> > + __u16 number_of_streams;
> > + __u16 reserved;
> > +} __packed;
> ...
> > + addstrm.param_hdr.type = SCTP_PARAM_RESET_ADD_OUT_STREAMS;
> > + addstrm.param_hdr.length = htons(size);
> > + addstrm.number_of_streams = htons(out);
> > + addstrm.request_seq = htonl(asoc->strreset_outseq);
> > + addstrm.reserved = 0;
> > +
> > + sctp_addto_chunk(retval, size, &addstrm);
>
> Since you allocate the sctp_strreset_addstrm structure on stack
> there is no requirement for it to be packed.
It shouldn't matter that it's allocated on stack. Why should it?
We need it to be packed as this is a header that will be sent out to
another peer, so there can't be any padding on it.
Marcelo
^ permalink raw reply
* Re: [PATCH] sock: use hlist_entry_safe
From: David Miller @ 2017-01-20 16:39 UTC (permalink / raw)
To: geliangtang; +Cc: netdev, linux-kernel
In-Reply-To: <697f2692562e48afa03304a7e605e8adbee8437e.1484829069.git.geliangtang@gmail.com>
From: Geliang Tang <geliangtang@gmail.com>
Date: Fri, 20 Jan 2017 22:27:04 +0800
> Use hlist_entry_safe() instead of open-coding it.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] gre6: Clean up unused struct ipv6_tel_txoption definition
From: David Miller @ 2017-01-20 16:37 UTC (permalink / raw)
To: jkbs; +Cc: netdev
In-Reply-To: <1484920386-9058-1-git-send-email-jkbs@redhat.com>
From: Jakub Sitnicki <jkbs@redhat.com>
Date: Fri, 20 Jan 2017 14:53:06 +0100
> Commit b05229f44228 ("gre6: Cleanup GREv6 transmit path, call common GRE
> functions") removed the ip6gre specific transmit function, but left the
> struct ipv6_tel_txoption definition. Clean it up.
>
> Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] uapi: fix signess in ethtool_validate_speed()
From: Michael S. Tsirkin @ 2017-01-20 16:36 UTC (permalink / raw)
To: Volodymyr Bendiuga
Cc: davem, nikolay, netdev, volodymyr.bendiuga, Jonas Johansson
In-Reply-To: <1484918453-5849-1-git-send-email-volodymyr.bendiuga@gmail.com>
On Fri, Jan 20, 2017 at 02:20:53PM +0100, Volodymyr Bendiuga wrote:
> From: Jonas Johansson <jonas.johansson@westermo.se>
>
> There is a comparison of speed variable which
> is unsigned, and SPEED_UNKNOWN which is signed.
So?
>
> Signed-off-by: Jonas Johansson <jonas.johansson@westermo.se>
> Signed-off-by: Volodymyr Bendiuga <volodymyr.bendiuga@gmail.com>
> ---
> include/uapi/linux/ethtool.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index f0db778..1ca4a77 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1500,7 +1500,7 @@ enum ethtool_link_mode_bit_indices {
>
> #define SPEED_UNKNOWN -1
>
> -static inline int ethtool_validate_speed(__u32 speed)
> +static inline int ethtool_validate_speed(__s32 speed)
> {
> return speed <= INT_MAX || speed == SPEED_UNKNOWN;
> }
Then comparison to INT_MAX does not make sense.
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: David Miller @ 2017-01-20 16:31 UTC (permalink / raw)
To: colin.king; +Cc: vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120134542.21104-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Fri, 20 Jan 2017 13:45:42 +0000
> From: Colin Ian King <colin.king@canonical.com>
>
> Table sctp_timer_tbl is missing a TIMEOUT_RECONF string so
> add this in. Also compare timeout with the size of the array
> sctp_timer_tbl rather than SCTP_EVENT_TIMEOUT_MAX. Also add
> a build time check that SCTP_EVENT_TIMEOUT_MAX is correct
> so we don't ever get this kind of mismatch between the table
> and SCTP_EVENT_TIMEOUT_MAX in the future.
>
> Kudos to Marcel Ricardo Leitner for spotting the missing string
> and suggesting the build time sanity check.
>
> Fixes CoverityScan CID#1397639 ("Out-of-bounds read")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Well, my bad... I reverted V1, that's fine.
But this patch doesn't even compile.
In file included from ./include/uapi/linux/stddef.h:1:0,
from ./include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from ./include/uapi/linux/types.h:13,
from ./include/linux/types.h:5,
from ./include/net/sctp/sctp.h:58,
from net/sctp/debug.c:41:
net/sctp/debug.c: In function ‘sctp_tname’:
./include/linux/compiler.h:518:38: error: call to ‘__compiletime_assert_170’ declared with attribute error: BUILD_BUG_ON failed: SCTP_EVENT_TIMEOUT_MAX + 1 != ARRAY_SIZE(sctp_timer_tbl)
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8192x: Enabling and disabling hardware interrupts after enabling local irq flags
From: Larry Finger @ 2017-01-20 16:30 UTC (permalink / raw)
To: Bharat Kumar Gogada, chaoming_li@realsil.com.cn,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: kvalo@codeaurora.org, netdev@vger.kernel.org, Ravikiran Gummaluri
In-Reply-To: <8520D5D51A55D047800579B094147198263CA86D@XAP-PVEXMBX02.xlnx.xilinx.com>
On 01/20/2017 08:14 AM, Bharat Kumar Gogada wrote:
> > On 01/19/2017 04:14 AM, Bharat Kumar Gogada wrote:
>>> -Realtek 8192CE chipset maintains local irq flags after enabling/disabling
>>> hardware interrupts.
>>> -Hardware interrupts are enabled before enabling the local irq
>>> flags(these flags are being checked in interrupt handler),
>>> leading to race condition on some RP, where the irq line between
>>> bridge and GIC goes high at ASSERT_INTx and goes low only
>>> at DEASSERT_INTx. In this kind of RP by the time ASSERT_INTx is seen
>>> irq_enable flag is still set to false, resulting in continuous
>>> interrupts seen by CPU as DEASSERT_INTx cannot be sent since
>>> flag is still false and making CPU stall.
>>> -Changing the sequence of setting these irq flags.
>>>
>>> Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com>
>>> ---
>>
>> This patch should be enhanced with the smb_xx() calls as suggested by by Lino.
>>
>> The subject should be changed. I would suggest something like "rtlwifi:
>> rtl8192ce: Prevent race condition when enabling interrupts", as it explains the
>> condition you are preventing.
>>
>> The other PCI drivers also have the same problem. Do you want to prepare the
>> patches, or should I do it?
>>
> Thanks Larry. Please send out the patches adding the above enhancements suggested by Lino.
I have prepared a patch fixing all the drivers. By the way, what CPU hardware
showed this problem?
Larry
^ permalink raw reply
* Re: [PATCH net-next] net: remove bh disabling around percpu_counter accesses
From: David Miller @ 2017-01-20 16:27 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1484917568.16328.110.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 20 Jan 2017 05:06:08 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> Shaohua Li made percpu_counter irq safe in commit 098faf5805c8
> ("percpu_counter: make APIs irq safe")
>
> We can safely remove BH disable/enable sections around various
> percpu_counter manipulations.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Looks great, applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] net: sctp: fix array overrun read on sctp_timer_tbl
From: David Miller @ 2017-01-20 16:26 UTC (permalink / raw)
To: colin.king; +Cc: vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120130157.20034-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Fri, 20 Jan 2017 13:01:57 +0000
> From: Colin Ian King <colin.king@canonical.com>
>
> The comparison on the timeout can lead to an array overrun
> read on sctp_timer_tbl because of an off-by-one error. Fix
> this by using < instead of <= and also compare to the array
> size rather than SCTP_EVENT_TIMEOUT_MAX.
>
> Fixes CoverityScan CID#1397639 ("Out-of-bounds read")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: Fix unused variable warning
From: David Miller @ 2017-01-20 16:16 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, hariprasad
In-Reply-To: <1484903839-5890-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Fri, 20 Jan 2017 14:47:19 +0530
> Fix unused variable warning when CONFIG_PCI_IOV is not
> defined.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Arnd already submitted a fix for this, and unlike your's his provided
a proper Fixes-by: tag.
Thanks.
^ permalink raw reply
* [PATCH net-next] bnx2x: avoid two atomic ops per page on x86
From: Eric Dumazet @ 2017-01-20 16:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Gabriel Krisman Bertazi, Yuval Mintz, Ariel Elior
From: Eric Dumazet <edumazet@google.com>
Commit 4cace675d687 ("bnx2x: Alloc 4k fragment for each rx ring buffer
element") added extra put_page() and get_page() calls on arches where
PAGE_SIZE=4K like x86
Reorder things to avoid this overhead.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Gabriel Krisman Bertazi <krisman@linux.vnet.ibm.com>
Cc: Yuval Mintz <Yuval.Mintz@cavium.com>
Cc: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 15 ++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 3e199d3e461ef5dd237e3176bcb666f878ac..c0dac0e5696d7d01457a42c2c13f1640c2e2 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -549,14 +549,7 @@ static int bnx2x_alloc_rx_sge(struct bnx2x *bp, struct bnx2x_fastpath *fp,
struct bnx2x_alloc_pool *pool = &fp->page_pool;
dma_addr_t mapping;
- if (!pool->page || (PAGE_SIZE - pool->offset) < SGE_PAGE_SIZE) {
-
- /* put page reference used by the memory pool, since we
- * won't be using this page as the mempool anymore.
- */
- if (pool->page)
- put_page(pool->page);
-
+ if (!pool->page) {
pool->page = alloc_pages(gfp_mask, PAGES_PER_SGE_SHIFT);
if (unlikely(!pool->page))
return -ENOMEM;
@@ -571,7 +564,6 @@ static int bnx2x_alloc_rx_sge(struct bnx2x *bp, struct bnx2x_fastpath *fp,
return -ENOMEM;
}
- get_page(pool->page);
sw_buf->page = pool->page;
sw_buf->offset = pool->offset;
@@ -581,7 +573,10 @@ static int bnx2x_alloc_rx_sge(struct bnx2x *bp, struct bnx2x_fastpath *fp,
sge->addr_lo = cpu_to_le32(U64_LO(mapping));
pool->offset += SGE_PAGE_SIZE;
-
+ if (PAGE_SIZE - pool->offset >= SGE_PAGE_SIZE)
+ get_page(pool->page);
+ else
+ pool->page = NULL;
return 0;
}
^ 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