Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net] sk_buff: drop all skb extensions on free and skb scrubbing
From: Florian Westphal @ 2019-09-26 18:37 UTC (permalink / raw)
  To: netdev; +Cc: steffen.klassert, paulb, vladbu, Florian Westphal

Now that we have a 3rd extension, add a new helper that drops the
extension space and use it when we need to scrub an sk_buff.

At this time, scrubbing clears secpath and bridge netfilter data, but
retains the tc skb extension, after this patch all three get cleared.

NAPI reuse/free assumes we can only have a secpath attached to skb, but
it seems better to clear all extensions there as well.

v2: add unlikely hint (Eric Dumazet)

Fixes: 95a7233c452a ("net: openvswitch: Set OvS recirc_id from tc chain index")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/skbuff.h | 9 +++++++++
 net/core/dev.c         | 4 ++--
 net/core/skbuff.c      | 2 +-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 907209c0794e..e7d3b1a513ef 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4144,8 +4144,17 @@ static inline void *skb_ext_find(const struct sk_buff *skb, enum skb_ext_id id)
 
 	return NULL;
 }
+
+static inline void skb_ext_reset(struct sk_buff *skb)
+{
+	if (unlikely(skb->active_extensions)) {
+		__skb_ext_put(skb->extensions);
+		skb->active_extensions = 0;
+	}
+}
 #else
 static inline void skb_ext_put(struct sk_buff *skb) {}
+static inline void skb_ext_reset(struct sk_buff *skb) {}
 static inline void skb_ext_del(struct sk_buff *skb, int unused) {}
 static inline void __skb_ext_copy(struct sk_buff *d, const struct sk_buff *s) {}
 static inline void skb_ext_copy(struct sk_buff *dst, const struct sk_buff *s) {}
diff --git a/net/core/dev.c b/net/core/dev.c
index 71b18e80389f..bf3ed413abaf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5666,7 +5666,7 @@ EXPORT_SYMBOL(gro_find_complete_by_type);
 static void napi_skb_free_stolen_head(struct sk_buff *skb)
 {
 	skb_dst_drop(skb);
-	secpath_reset(skb);
+	skb_ext_put(skb);
 	kmem_cache_free(skbuff_head_cache, skb);
 }
 
@@ -5733,7 +5733,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
 	skb->encapsulation = 0;
 	skb_shinfo(skb)->gso_type = 0;
 	skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
-	secpath_reset(skb);
+	skb_ext_reset(skb);
 
 	napi->skb = skb;
 }
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f12e8a050edb..01d65206f4fb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5119,7 +5119,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
 	skb->skb_iif = 0;
 	skb->ignore_df = 0;
 	skb_dst_drop(skb);
-	secpath_reset(skb);
+	skb_ext_reset(skb);
 	nf_reset(skb);
 	nf_reset_trace(skb);
 
-- 
2.21.0


^ permalink raw reply related

* Re: Question on LACP Bypass feature
From: Jay Vosburgh @ 2019-09-26 18:35 UTC (permalink / raw)
  To: Madhavi Joshi; +Cc: netdev@vger.kernel.org, lalit-arrcus
In-Reply-To: <6390BF88-7D04-41CC-8D57-844FC6A742FE@arrcus.com>

Madhavi Joshi <madhavi@arrcus.com> wrote:

>  We have a question regarding LACP Bypass feature . It appears that by default, this feature is enabled in the kernel (we are on 4.1.1274 version kernel). 
>Having said that, we do not see any sysctl or directory on the lines of /sys/class/net/<bond>/bonding/lacp_bypass.
> 
>Really appreciate your help with this.

	I do not recall having ever heard of a "lacp_bypass" feature in
bonding, and do not see it as present in the kernel sources I have
handy.  I presume this is a feature added by a third party, but I don't
know who, or what it does.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* Re: [1/2] net/phy/mdio-mscc-miim: Use devm_platform_ioremap_resource() in mscc_miim_probe()
From: Markus Elfring @ 2019-09-26 18:52 UTC (permalink / raw)
  To: Alexandre Belloni, Andrew Lunn, netdev
  Cc: Florian Fainelli, David S. Miller, Heiner Kallweit, LKML,
	kernel-janitors
In-Reply-To: <20190926161825.GB6825@piout.net>

>> Does this feedback indicate also an agreement for the detail
>> if the mapping of internal phy registers would be a required operation?
>> (Would such a resource allocation eventually be optional?)
>
> It is optional.

Would you like to integrate an other patch variant then?

Regards,
Markus

^ permalink raw reply

* Re: [PATCH v2 net] sk_buff: drop all skb extensions on free and skb scrubbing
From: Eric Dumazet @ 2019-09-26 19:05 UTC (permalink / raw)
  To: Florian Westphal, netdev; +Cc: steffen.klassert, paulb, vladbu
In-Reply-To: <20190926183705.16951-1-fw@strlen.de>



On 9/26/19 11:37 AM, Florian Westphal wrote:
> Now that we have a 3rd extension, add a new helper that drops the
> extension space and use it when we need to scrub an sk_buff.
> 

>  }
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index f12e8a050edb..01d65206f4fb 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -5119,7 +5119,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
>  	skb->skb_iif = 0;
>  	skb->ignore_df = 0;
>  	skb_dst_drop(skb);
> -	secpath_reset(skb);
> +	skb_ext_reset(skb);
>  	nf_reset(skb);
>  	nf_reset_trace(skb);


It is unfortunate nf_reset(skb) will call skb_ext_del(skb, SKB_EXT_BRIDGE_NF),
which is useless after skb_ext_reset(skb) 

Maybe time for a nf_ct_reset() helper only dealing with nfct.



^ permalink raw reply

* Re: [PATCH v2 net] sk_buff: drop all skb extensions on free and skb scrubbing
From: Florian Westphal @ 2019-09-26 19:09 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Florian Westphal, netdev, steffen.klassert, paulb, vladbu
In-Reply-To: <1ad4b9f0-c9d4-954b-eafe-8652ea6ce409@gmail.com>

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > -	secpath_reset(skb);
> > +	skb_ext_reset(skb);
> >  	nf_reset(skb);
> >  	nf_reset_trace(skb);
> 
> 
> It is unfortunate nf_reset(skb) will call skb_ext_del(skb, SKB_EXT_BRIDGE_NF),
> which is useless after skb_ext_reset(skb) 
> 
> Maybe time for a nf_ct_reset() helper only dealing with nfct.

Agree, but that seems more like -next material?

^ permalink raw reply

* Re: [PATCH v2 net] sk_buff: drop all skb extensions on free and skb scrubbing
From: Eric Dumazet @ 2019-09-26 19:16 UTC (permalink / raw)
  To: Florian Westphal, Eric Dumazet; +Cc: netdev, steffen.klassert, paulb, vladbu
In-Reply-To: <20190926190920.GC9938@breakpoint.cc>



On 9/26/19 12:09 PM, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> -	secpath_reset(skb);
>>> +	skb_ext_reset(skb);
>>>  	nf_reset(skb);
>>>  	nf_reset_trace(skb);
>>
>>
>> It is unfortunate nf_reset(skb) will call skb_ext_del(skb, SKB_EXT_BRIDGE_NF),
>> which is useless after skb_ext_reset(skb) 
>>
>> Maybe time for a nf_ct_reset() helper only dealing with nfct.
> 
> Agree, but that seems more like -next material?

Sure.



^ permalink raw reply

* Re: [1/2] net/phy/mdio-mscc-miim: Use devm_platform_ioremap_resource() in mscc_miim_probe()
From: Alexandre Belloni @ 2019-09-26 19:32 UTC (permalink / raw)
  To: Markus Elfring
  Cc: Andrew Lunn, netdev, Florian Fainelli, David S. Miller,
	Heiner Kallweit, LKML, kernel-janitors
In-Reply-To: <0a1f4dbf-4cc6-8530-a38e-31c3369e6db6@web.de>

On 26/09/2019 20:52:38+0200, Markus Elfring wrote:
> >> Does this feedback indicate also an agreement for the detail
> >> if the mapping of internal phy registers would be a required operation?
> >> (Would such a resource allocation eventually be optional?)
> >
> > It is optional.
> 
> Would you like to integrate an other patch variant then?
> 

You have to ensure it stays optional. Also, adjust the subject so it
uses the correct prefix.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
From: Eric Dumazet @ 2019-09-26 19:33 UTC (permalink / raw)
  To: Matias Ezequiel Vara Larsen, stefanha
  Cc: davem, kvm, virtualization, netdev, linux-kernel, sgarzare
In-Reply-To: <1569522214-28223-1-git-send-email-matiasevara@gmail.com>



On 9/26/19 11:23 AM, Matias Ezequiel Vara Larsen wrote:
> This patch adds support for MSG_PEEK. In such a case, packets are not
> removed from the rx_queue and credit updates are not sent.
> 
> Signed-off-by: Matias Ezequiel Vara Larsen <matiasevara@gmail.com>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 50 +++++++++++++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 94cc0fa..938f2ed 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -264,6 +264,50 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
>  }
>  
>  static ssize_t
> +virtio_transport_stream_do_peek(struct vsock_sock *vsk,
> +				struct msghdr *msg,
> +				size_t len)
> +{
> +	struct virtio_vsock_sock *vvs = vsk->trans;
> +	struct virtio_vsock_pkt *pkt;
> +	size_t bytes, total = 0;
> +	int err = -EFAULT;
> +
> +	spin_lock_bh(&vvs->rx_lock);
> +
> +	list_for_each_entry(pkt, &vvs->rx_queue, list) {
> +		if (total == len)
> +			break;
> +
> +		bytes = len - total;
> +		if (bytes > pkt->len - pkt->off)
> +			bytes = pkt->len - pkt->off;
> +
> +		/* sk_lock is held by caller so no one else can dequeue.
> +		 * Unlock rx_lock since memcpy_to_msg() may sleep.
> +		 */
> +		spin_unlock_bh(&vvs->rx_lock);
> +
> +		err = memcpy_to_msg(msg, pkt->buf + pkt->off, bytes);
> +		if (err)
> +			goto out;
> +
> +		spin_lock_bh(&vvs->rx_lock);
> +
> +		total += bytes;
> +	}
> +
> +	spin_unlock_bh(&vvs->rx_lock);
> +
> +	return total;
> +
> +out:
> +	if (total)
> +		err = total;
> +	return err;
> +}
>

This seems buggy to me.

virtio_transport_recv_enqueue() seems to be able to add payload to the last packet in the queue.

The loop you wrote here would miss newly added chunks while the vvs->rx_lock spinlock has been released.

virtio_transport_stream_do_dequeue() is ok, because it makes sure pkt->off == pkt->len
before cleaning the packet from the queue.




^ permalink raw reply

* RE: [RFC 15/20] RDMA/irdma: Add miscellaneous utility definitions
From: Saleem, Shiraz @ 2019-09-26 19:49 UTC (permalink / raw)
  To: Leon Romanovsky, Kirsher, Jeffrey T
  Cc: dledford@redhat.com, jgg@mellanox.com, gregkh@linuxfoundation.org,
	Ismail, Mustafa, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org
In-Reply-To: <20190926174948.GE14368@unreal>

> Subject: Re: [RFC 15/20] RDMA/irdma: Add miscellaneous utility definitions
> 
> On Thu, Sep 26, 2019 at 09:45:14AM -0700, Jeff Kirsher wrote:
> > From: Mustafa Ismail <mustafa.ismail@intel.com>
> >
> > Add miscellaneous utility functions and headers.
> >
> > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > ---
> >  drivers/infiniband/hw/irdma/osdep.h  |  108 ++
> >  drivers/infiniband/hw/irdma/protos.h |   96 ++
> >  drivers/infiniband/hw/irdma/status.h |   70 +
> >  drivers/infiniband/hw/irdma/utils.c  | 2333
> > ++++++++++++++++++++++++++
> >  4 files changed, 2607 insertions(+)
> >  create mode 100644 drivers/infiniband/hw/irdma/osdep.h
> >  create mode 100644 drivers/infiniband/hw/irdma/protos.h
> >  create mode 100644 drivers/infiniband/hw/irdma/status.h
> >  create mode 100644 drivers/infiniband/hw/irdma/utils.c
> >
> > diff --git a/drivers/infiniband/hw/irdma/osdep.h
> > b/drivers/infiniband/hw/irdma/osdep.h
> > new file mode 100644
> > index 000000000000..5885b6fa413d
> > --- /dev/null
> > +++ b/drivers/infiniband/hw/irdma/osdep.h
> > @@ -0,0 +1,108 @@
> > +/* SPDX-License-Identifier: GPL-2.0 or Linux-OpenIB */
> > +/* Copyright (c) 2019, Intel Corporation. */
> > +
> > +#ifndef IRDMA_OSDEP_H
> > +#define IRDMA_OSDEP_H
> > +
> > +#include <linux/version.h>
> > +#include <linux/kernel.h>
> > +#include <linux/vmalloc.h>
> > +#include <linux/string.h>
> > +#include <linux/bitops.h>
> > +#include <linux/pci.h>
> > +#include <net/tcp.h>
> > +#include <crypto/hash.h>
> > +/* get readq/writeq support for 32 bit kernels, use the low-first
> > +version */ #include <linux/io-64-nonatomic-lo-hi.h>
> > +
> > +#define MAKEMASK(m, s) ((m) << (s))
> 
> It is a little bit over-macro.
> 

Why is this a problem?
We are not translating any basic kernel construct here.

^ permalink raw reply

* RE: [RFC 20/20] RDMA/i40iw: Mark i40iw as deprecated
From: Saleem, Shiraz @ 2019-09-26 19:49 UTC (permalink / raw)
  To: Leon Romanovsky, Kirsher, Jeffrey T
  Cc: dledford@redhat.com, jgg@mellanox.com, gregkh@linuxfoundation.org,
	netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190926174009.GD14368@unreal>

> Subject: Re: [RFC 20/20] RDMA/i40iw: Mark i40iw as deprecated
> 
> On Thu, Sep 26, 2019 at 09:45:19AM -0700, Jeff Kirsher wrote:
> > From: Shiraz Saleem <shiraz.saleem@intel.com>
> >
> > Mark i40iw as deprecated/obsolete.
> >
> > irdma is the replacement driver that supports X722.
> 
> Can you simply delete old one and add MODULE_ALIAS() in new driver?
> 

Yes, but we thought typically driver has to be deprecated for a few cycles before removing it.

^ permalink raw reply

* RE: [RFC 12/20] RDMA/irdma: Implement device supported verb APIs
From: Saleem, Shiraz @ 2019-09-26 19:49 UTC (permalink / raw)
  To: Leon Romanovsky, Kirsher, Jeffrey T
  Cc: dledford@redhat.com, jgg@mellanox.com, gregkh@linuxfoundation.org,
	Ismail, Mustafa, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org
In-Reply-To: <20190926173710.GC14368@unreal>

> Subject: Re: [RFC 12/20] RDMA/irdma: Implement device supported verb APIs
> 
> On Thu, Sep 26, 2019 at 09:45:11AM -0700, Jeff Kirsher wrote:
> > From: Mustafa Ismail <mustafa.ismail@intel.com>
> >
> > Implement device supported verb APIs. The supported APIs vary based on
> > the underlying transport the ibdev is registered as (i.e. iWARP or
> > RoCEv2).
> >
> > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > ---
> >  drivers/infiniband/hw/irdma/verbs.c      | 4346 ++++++++++++++++++++++
> >  drivers/infiniband/hw/irdma/verbs.h      |  199 +
> >  include/uapi/rdma/rdma_user_ioctl_cmds.h |    1 +
> >  3 files changed, 4546 insertions(+)
> >  create mode 100644 drivers/infiniband/hw/irdma/verbs.c
> >  create mode 100644 drivers/infiniband/hw/irdma/verbs.h
> >
> > diff --git a/drivers/infiniband/hw/irdma/verbs.c
> > b/drivers/infiniband/hw/irdma/verbs.c
> > new file mode 100644
> > index 000000000000..025c21c722e2
> > --- /dev/null
> > +++ b/drivers/infiniband/hw/irdma/verbs.c
> > @@ -0,0 +1,4346 @@
> > +// SPDX-License-Identifier: GPL-2.0 or Linux-OpenIB
> > +/* Copyright (c) 2019, Intel Corporation. */
> 
> <...>
> 
> > +
> > +	size = sqdepth * sizeof(struct irdma_sq_uk_wr_trk_info) +
> > +	       (rqdepth << 3);
> > +	iwqp->kqp.wrid_mem = kzalloc(size, GFP_KERNEL);
> > +	if (!iwqp->kqp.wrid_mem)
> > +		return -ENOMEM;
> > +
> > +	ukinfo->sq_wrtrk_array = (struct irdma_sq_uk_wr_trk_info *)
> > +				 iwqp->kqp.wrid_mem;
> > +	if (!ukinfo->sq_wrtrk_array)
> > +		return -ENOMEM;
> 
> You are leaking resources here, forgot to do proper error unwinding.
> 

irdma_free_qp_rsrc() will free up that memory in case of an error.

^ permalink raw reply

* RE: [RFC 12/20] RDMA/irdma: Implement device supported verb APIs
From: Saleem, Shiraz @ 2019-09-26 19:50 UTC (permalink / raw)
  To: Jason Gunthorpe, Leon Romanovsky
  Cc: Kirsher, Jeffrey T, dledford@redhat.com,
	gregkh@linuxfoundation.org, Ismail, Mustafa,
	netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190926174001.GG19509@mellanox.com>

> Subject: Re: [RFC 12/20] RDMA/irdma: Implement device supported verb APIs
> 
> On Thu, Sep 26, 2019 at 08:37:10PM +0300, Leon Romanovsky wrote:
> > On Thu, Sep 26, 2019 at 09:45:11AM -0700, Jeff Kirsher wrote:
> > > From: Mustafa Ismail <mustafa.ismail@intel.com>
> > >
> > > Implement device supported verb APIs. The supported APIs vary based
> > > on the underlying transport the ibdev is registered as (i.e. iWARP
> > > or RoCEv2).
> > >
> > > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> > > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > >  drivers/infiniband/hw/irdma/verbs.c      | 4346 ++++++++++++++++++++++
> > >  drivers/infiniband/hw/irdma/verbs.h      |  199 +
> > >  include/uapi/rdma/rdma_user_ioctl_cmds.h |    1 +
> > >  3 files changed, 4546 insertions(+)  create mode 100644
> > > drivers/infiniband/hw/irdma/verbs.c
> > >  create mode 100644 drivers/infiniband/hw/irdma/verbs.h
> > >
> > > diff --git a/drivers/infiniband/hw/irdma/verbs.c
> > > b/drivers/infiniband/hw/irdma/verbs.c
> > > new file mode 100644
> > > index 000000000000..025c21c722e2
> > > +++ b/drivers/infiniband/hw/irdma/verbs.c
> > > @@ -0,0 +1,4346 @@
> > > +// SPDX-License-Identifier: GPL-2.0 or Linux-OpenIB
> > > +/* Copyright (c) 2019, Intel Corporation. */
> >
> > <...>
> >
> > > +
> > > +	size = sqdepth * sizeof(struct irdma_sq_uk_wr_trk_info) +
> > > +	       (rqdepth << 3);
> > > +	iwqp->kqp.wrid_mem = kzalloc(size, GFP_KERNEL);
> 
> This weird allocation math also looks sketchy, should it be using one of the
> various anti-overflow helpers, or maybe a flex array?
> 

OK. We ll review this.

^ permalink raw reply

* RE: [RFC 04/20] RDMA/irdma: Add driver framework definitions
From: Saleem, Shiraz @ 2019-09-26 19:51 UTC (permalink / raw)
  To: Leon Romanovsky, Kirsher, Jeffrey T
  Cc: dledford@redhat.com, jgg@mellanox.com, gregkh@linuxfoundation.org,
	Ismail, Mustafa, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org
In-Reply-To: <20190926173046.GB14368@unreal>

> Subject: Re: [RFC 04/20] RDMA/irdma: Add driver framework definitions
> 

<...............>

> > +/**
> > + * i40iw_l2param_change - handle qs handles for QoS and MSS change
> > + * @ldev: LAN device information
> > + * @client: client for parameter change
> > + * @params: new parameters from L2
> > + */
> > +static void i40iw_l2param_change(struct i40e_info *ldev,
> > +				 struct i40e_client *client,
> > +				 struct i40e_params *params)
> > +{
> > +	struct irdma_l2params *l2params;
> > +	struct l2params_work *work;
> > +	struct irdma_device *iwdev;
> > +	struct irdma_handler *hdl;
> > +	int i;
> > +
> > +	hdl = irdma_find_handler(ldev->pcidev);
> > +	if (!hdl)
> > +		return;
> > +
> > +	iwdev = (struct irdma_device *)((u8 *)hdl + sizeof(*hdl));
> > +
> > +	if (atomic_read(&iwdev->params_busy))
> > +		return;
> > +	work = kzalloc(sizeof(*work), GFP_KERNEL);
> > +	if (!work)
> > +		return;
> > +
> > +	atomic_inc(&iwdev->params_busy);
> 
> Changing parameters through workqueue and perform locking with atomic_t,
> exciting.
> Please do proper locking scheme and better to avoid workqueue at all.

Hmmm....Yes, this is buggy. Will come up with a better solution.

> 
> <...>
> 
> > +/* client interface functions */
> > +static const struct i40e_client_ops i40e_ops = {
> > +	.open = i40iw_open,
> > +	.close = i40iw_close,
> > +	.l2_param_change = i40iw_l2param_change };
> > +
> > +static struct i40e_client i40iw_client = {
> > +	.name = "irdma",
> > +	.ops = &i40e_ops,
> > +	.version.major = I40E_CLIENT_VERSION_MAJOR,
> > +	.version.minor = I40E_CLIENT_VERSION_MINOR,
> > +	.version.build = I40E_CLIENT_VERSION_BUILD,
> > +	.type = I40E_CLIENT_IWARP,
> > +};
> > +
> > +int i40iw_probe(struct platform_device *pdev) {
> > +	struct i40e_peer_dev_platform_data *pdata =
> > +		dev_get_platdata(&pdev->dev);
> > +	struct i40e_info *ldev;
> > +
> > +	if (!pdata)
> > +		return -EINVAL;
> > +
> > +	ldev = pdata->ldev;
> > +
> > +	if (ldev->version.major != I40E_CLIENT_VERSION_MAJOR ||
> > +	    ldev->version.minor != I40E_CLIENT_VERSION_MINOR) {
> > +		pr_err("version mismatch:\n");
> > +		pr_err("expected major ver %d, caller specified major ver %d\n",
> > +		       I40E_CLIENT_VERSION_MAJOR, ldev->version.major);
> > +		pr_err("expected minor ver %d, caller specified minor ver %d\n",
> > +		       I40E_CLIENT_VERSION_MINOR, ldev->version.minor);
> > +		return -EINVAL;
> > +	}
> 
> This is can't be in upstream code, we don't support out-of-tree modules,
> everything else will have proper versions.
> 

OK.

^ permalink raw reply

* Re: [RFC 20/20] RDMA/i40iw: Mark i40iw as deprecated
From: gregkh @ 2019-09-26 19:55 UTC (permalink / raw)
  To: Saleem, Shiraz
  Cc: Leon Romanovsky, Kirsher, Jeffrey T, dledford@redhat.com,
	jgg@mellanox.com, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org
In-Reply-To: <9DD61F30A802C4429A01CA4200E302A7AC702BDA@fmsmsx123.amr.corp.intel.com>

On Thu, Sep 26, 2019 at 07:49:44PM +0000, Saleem, Shiraz wrote:
> > Subject: Re: [RFC 20/20] RDMA/i40iw: Mark i40iw as deprecated
> > 
> > On Thu, Sep 26, 2019 at 09:45:19AM -0700, Jeff Kirsher wrote:
> > > From: Shiraz Saleem <shiraz.saleem@intel.com>
> > >
> > > Mark i40iw as deprecated/obsolete.
> > >
> > > irdma is the replacement driver that supports X722.
> > 
> > Can you simply delete old one and add MODULE_ALIAS() in new driver?
> > 
> 
> Yes, but we thought typically driver has to be deprecated for a few cycles before removing it.

If you completely replace it with something that works the same, why
keep the old one around at all?

Unless you don't trust your new code?  :)

thanks,

greg k-h

^ permalink raw reply

* Re: Fwd: [PATCH] bonding/802.3ad: fix slave initialization states race
From: Jay Vosburgh @ 2019-09-26 20:01 UTC (permalink / raw)
  To: Aleksei Zakharov; +Cc: netdev, zhangsha (A)
In-Reply-To: <CAJYOGF-84BfK8DvAnam9+tgfo4=oBs04zF-ETWRfhz7CE_9oBA@mail.gmail.com>

Aleksei Zakharov <zaharov@selectel.ru> wrote:

>чт, 26 сент. 2019 г. в 07:38, Jay Vosburgh <jay.vosburgh@canonical.com>:
>>
>> Aleksei Zakharov <zaharov@selectel.ru> wrote:
>>
>> >ср, 25 сент. 2019 г. в 03:31, Jay Vosburgh <jay.vosburgh@canonical.com>:
>> >>
>> >> Алексей Захаров wrote:
>> >> [...]
>> >> >Right after reboot one of the slaves hangs with actor port state 71
>> >> >and partner port state 1.
>> >> >It doesn't send lacpdu and seems to be broken.
>> >> >Setting link down and up again fixes slave state.
>> >> [...]
>> >>
>> >>         I think I see what failed in the first patch, could you test the
>> >> following patch?  This one is for net-next, so you'd need to again swap
>> >> slave_err / netdev_err for the Ubuntu 4.15 kernel.
>> >>
>> >I've tested new patch. It seems to work. I can't reproduce the bug
>> >with this patch.
>> >There are two types of messages when link becomes up:
>> >First:
>> >bond-san: EVENT 1 llu 4294895911 slave eth2
>> >8021q: adding VLAN 0 to HW filter on device eth2
>> >bond-san: link status definitely down for interface eth2, disabling it
>> >mlx4_en: eth2: Link Up
>> >bond-san: EVENT 4 llu 4294895911 slave eth2
>> >bond-san: link status up for interface eth2, enabling it in 500 ms
>> >bond-san: invalid new link 3 on slave eth2
>> >bond-san: link status definitely up for interface eth2, 10000 Mbps full duplex
>> >Second:
>> >bond-san: EVENT 1 llu 4295147594 slave eth2
>> >8021q: adding VLAN 0 to HW filter on device eth2
>> >mlx4_en: eth2: Link Up
>> >bond-san: EVENT 4 llu 4295147594 slave eth2
>> >bond-san: link status up again after 0 ms for interface eth2
>> >bond-san: link status definitely up for interface eth2, 10000 Mbps full duplex
>> > [...]
>>
>>         The "invalid new link" is appearing because bond_miimon_commit
>> is being asked to commit a new state that isn't UP or DOWN (3 is
>> BOND_LINK_BACK).  I looked through the patched code today, and I don't
>> see a way to get to that message with the new link set to 3, so I'll add
>> some instrumentation and send out another patch to figure out what's
>> going on, as that shouldn't happen.
>>
>>         I don't see the "invalid" message testing locally, I think
>> because my network device doesn't transition to carrier up as quickly as
>> yours.  I thought you were getting BOND_LINK_BACK passed through from
>> bond_enslave (which calls bond_set_slave_link_state, which will set
>> link_new_link to BOND_LINK_BACK and leave it there), but the
>> link_new_link is reset first thing in bond_miimon_inspect, so I'm not
>> sure how it gets into bond_miimon_commit (I'm thinking perhaps a
>> concurrent commit triggered by another slave, which then picks up this
>> proposed link state change by happenstance).
>I assume that "invalid new link" happens in this way:
>Interface goes up
>NETDEV_CHANGE event occurs
>bond_update_speed_duplex fails
>and slave->last_link_up returns true
>slave->link becomes BOND_LINK_FAIL
>bond_check_dev_link returns 0
>miimon proposes slave->link_new_state BOND_LINK_DOWN
>NETDEV_UP event occurs
>miimon sets commit++
>miimon proposes slave->link_new_state BOND_LINK_BACK
>miimon sets slave->link to BOND_LINK_BACK

	I removed the "proposes link_new_state BOND_LINK_BACK" from the
second test patch and replaced it with the slave->link = BOND_LINK_BACK.
This particular place in the code also does not do commit++.  If you
have both of those in the code you're running, then perhaps you have a
merge error or some such.

	In the second test patch, the only place that could set
link_new_state to BOND_LINK_BACK is in bond_enslave, which calls
bond_set_slave_link_state if the slave is carrier up and updelay is
configured.  If that were to happen, there should be a "BOND_LINK_BACK
initial state" debug message, and the link_new_state should be replaced
with NOCHANGE at the first pass through bond_miimon_inspect.

	So, I'm unclear how the link_new_state can be BOND_LINK_BACK
from the message log you provided based on the second test patch code.

>we have updelay configured, so it doesn't set BOND_LINK_UP in the next
>case section
>miimon says "Invalid new link" and sets link state UP during next
>inspection(after updelay, i suppose)
>
>For the second type of messages it looks like this:
>Interface goes up
>NETDEV_CHANGE event occurs
>bond_update_speed_duplex fails
>and slave->last_link_up returns true
>slave->link becomes BOND_LINK_FAIL
>NETDEV_UP event occurs
>bond_check_dev_link returns 1
>miimon proposes slave->link_new_state BOND_LINK_UP and says "link
>status up again"
>
>My first patch changed slave->last_link_up check to (slave->link ==
>BOND_LINK_UP).
>This check looks more consistent for me, but I might be wrong here.
>As a result if link was in BOND_LINK_FAIL or BOND_LINK_BACK when
>CHANGE or UP event,
>it became BOND_LINK_DOWN.
>But if it was initially UP and bond_update_speed_duplex was unable to
>get speed/duplex,
>link became BOND_LINK_FAIL.
>
>I don't understand a few things here:
>How could a link be in a different state from time to time during the
>first NETDEV_* event?

	I'm not sure; possibly a race between the events in the kernel
and how long it takes for the hardware to establish Ethernet link up.

>And why slave->last_link_up is set when the first NETDEV event occurs?

	slave->last_link_up can be set at enslave time if the carrier
state of the slave (and thus the initial slave->link) is in a not-down
state.  There are some paths as well for modes that have an "active"
slave, but 802.3ad is not one of those.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* [PATCH net] tcp: better handle TCP_USER_TIMEOUT in SYN_SENT state
From: Eric Dumazet @ 2019-09-26 22:42 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Eric Dumazet, Yuchung Cheng,
	Marek Majkowski, Jon Maxwell

Yuchung Cheng and Marek Majkowski independently reported a weird
behavior of TCP_USER_TIMEOUT option when used at connect() time.

When the TCP_USER_TIMEOUT is reached, tcp_write_timeout()
believes the flow should live, and the following condition
in tcp_clamp_rto_to_user_timeout() programs one jiffie timers :

    remaining = icsk->icsk_user_timeout - elapsed;
    if (remaining <= 0)
        return 1; /* user timeout has passed; fire ASAP */

This silly situation ends when the max syn rtx count is reached.

This patch makes sure we honor both TCP_SYNCNT and TCP_USER_TIMEOUT,
avoiding these spurious SYN packets.

Fixes: b701a99e431d ("tcp: Add tcp_clamp_rto_to_user_timeout() helper to improve accuracy")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Yuchung Cheng <ycheng@google.com>
Reported-by: Marek Majkowski <marek@cloudflare.com>
Cc: Jon Maxwell <jmaxwell37@gmail.com>
Link: https://marc.info/?l=linux-netdev&m=156940118307949&w=2
---
 net/ipv4/tcp_timer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index dbd9d2d0ee63aa46ad2dda417da6ec9409442b77..40de2d2364a1eca14c259d77ebed361d17829eb9 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -210,7 +210,7 @@ static int tcp_write_timeout(struct sock *sk)
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct net *net = sock_net(sk);
-	bool expired, do_reset;
+	bool expired = false, do_reset;
 	int retry_until;
 
 	if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
@@ -242,9 +242,10 @@ static int tcp_write_timeout(struct sock *sk)
 			if (tcp_out_of_resources(sk, do_reset))
 				return 1;
 		}
+	}
+	if (!expired)
 		expired = retransmits_timed_out(sk, retry_until,
 						icsk->icsk_user_timeout);
-	}
 	tcp_fastopen_active_detect_blackhole(sk, expired);
 
 	if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTO_CB_FLAG))
-- 
2.23.0.444.g18eeb5a265-goog


^ permalink raw reply related

* Re: chapoly acceleration hardware [Was: Re: [RFC PATCH 00/18] crypto: wireguard using the existing crypto API]
From: Jakub Kicinski @ 2019-09-26 22:47 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Pascal Van Leeuwen, Ard Biesheuvel, Linux Crypto Mailing List,
	linux-arm-kernel, Herbert Xu, David Miller, Greg KH,
	Linus Torvalds, Samuel Neves, Dan Carpenter, Arnd Bergmann,
	Eric Biggers, Andy Lutomirski, Will Deacon, Marc Zyngier,
	Catalin Marinas, Willy Tarreau, Netdev,
	Toke Høiland-Jørgensen, Dave Taht
In-Reply-To: <CAHmME9r5m7D-oMU6Lv_ZhEyWmrNscMr5HokzdK0wg2Ayzzbeow@mail.gmail.com>

On Thu, 26 Sep 2019 13:06:51 +0200, Jason A. Donenfeld wrote:
> On Thu, Sep 26, 2019 at 12:19 PM Pascal Van Leeuwen wrote:
> > Actually, that assumption is factually wrong. I don't know if anything
> > is *publicly* available, but I can assure you the silicon is running in
> > labs already. And something will be publicly available early next year
> > at the latest. Which could nicely coincide with having Wireguard support
> > in the kernel (which I would also like to see happen BTW) ...
> >
> > Not "at some point". It will. Very soon. Maybe not in consumer or server
> > CPUs, but definitely in the embedded (networking) space.
> > And it *will* be much faster than the embedded CPU next to it, so it will
> > be worth using it for something like bulk packet encryption.  
> 
> Super! I was wondering if you could speak a bit more about the
> interface. My biggest questions surround latency. Will it be
> synchronous or asynchronous? If the latter, why? What will its
> latencies be? How deep will its buffers be? The reason I ask is that a
> lot of crypto acceleration hardware of the past has been fast and
> having very deep buffers, but at great expense of latency. In the
> networking context, keeping latency low is pretty important.

FWIW are you familiar with existing kTLS, and IPsec offloads in the
networking stack? They offload the crypto into the NIC, inline, which
helps with the latency, and processing overhead.

There are also NIC silicon which can do some ChaCha/Poly, although 
I'm not familiar enough with WireGuard to know if offload to existing
silicon will be possible.

^ permalink raw reply

* Re: [PATCH] net: use unlikely for dql_avail case
From: Eric Dumazet @ 2019-09-26 22:52 UTC (permalink / raw)
  To: Daniel Borkmann, xiaolinkui; +Cc: davem, netdev
In-Reply-To: <20190925122501.GA27720@pc-66.home>



On 9/25/19 5:25 AM, Daniel Borkmann wrote:
> On Wed, Sep 25, 2019 at 10:40:43AM +0800, xiaolinkui wrote:
>> This is an unlikely case, use unlikely() on it seems logical.
>>
>> Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn>
> 
> It's already here [0], but should probably rather get reverted instead
> due to lack of a more elaborate reasoning on why it needs to be done
> this way instead of letting compiler do it's job in this case. "Seems
> logical" is never a good technical explanation. Do you have any better
> analysis you performed prior to submitting the patch (twice by now)?
>

Yes, we need more details here.

We could probably save more cpu cycles checking if we can move
the smb_mb() after the dql_avail() check :)


 
> Thanks,
> Daniel
> 
>   [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f3acd33d840d3ea3e1233d234605c85cbbf26054
> 
>> ---
>>  include/linux/netdevice.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 88292953aa6f..005f3da1b13d 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -3270,7 +3270,7 @@ static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
>>  	 */
>>  	smp_mb();
>>  
>> -	if (dql_avail(&dev_queue->dql) < 0)
>> +	if (unlikely(dql_avail(&dev_queue->dql) < 0))
>>  		return;
>>  
>>  	if (test_and_clear_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state))
>> -- 
>> 2.17.1
>>
>>
>>

^ permalink raw reply

* [PATCH] ath9k_hw: fix uninitialized variable data
From: Denis Efremov @ 2019-09-26 22:56 UTC (permalink / raw)
  Cc: Denis Efremov, ath9k-devel, linux-wireless, netdev, linux-kernel,
	Rajkumar Manoharan, John W . Linville, Kalle Valo,
	David S. Miller, stable

Currently, data variable in ar9003_hw_thermo_cal_apply() could be
uninitialized if ar9300_otp_read_word() will fail to read the value.
Initialize data variable with 0 to prevent an undefined behavior. This
will be enough to handle error case when ar9300_otp_read_word() fails.

Fixes: 80fe43f2bbd5 ("ath9k_hw: Read and configure thermocal for AR9462")
Cc: Rajkumar Manoharan <rmanohar@qca.qualcomm.com>
Cc: John W. Linville <linville@tuxdriver.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: stable@vger.kernel.org
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index 2b29bf4730f6..b4885a700296 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -4183,7 +4183,7 @@ static void ar9003_hw_thermometer_apply(struct ath_hw *ah)
 
 static void ar9003_hw_thermo_cal_apply(struct ath_hw *ah)
 {
-	u32 data, ko, kg;
+	u32 data = 0, ko, kg;
 
 	if (!AR_SREV_9462_20_OR_LATER(ah))
 		return;
-- 
2.21.0


^ permalink raw reply related

* [PATCH bpf] libbpf: add macro __BUILD_STATIC_LIBBPF__ to guard .symver
From: Yonghong Song @ 2019-09-26 23:02 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev
  Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team

bcc uses libbpf repo as a submodule. It brings in libbpf source
code and builds everything together to produce shared libraries.
With latest libbpf, I got the following errors:
  /bin/ld: libbcc_bpf.so.0.10.0: version node not found for symbol xsk_umem__create@LIBBPF_0.0.2
  /bin/ld: failed to set dynamic section sizes: Bad value
  collect2: error: ld returned 1 exit status
  make[2]: *** [src/cc/libbcc_bpf.so.0.10.0] Error 1

In xsk.c, we have
  asm(".symver xsk_umem__create_v0_0_2, xsk_umem__create@LIBBPF_0.0.2");
  asm(".symver xsk_umem__create_v0_0_4, xsk_umem__create@@LIBBPF_0.0.4");
The linker thinks the built is for LIBBPF but cannot find proper version
LIBBPF_0.0.2/4, so emit errors.

I also confirmed that using libbpf.a to produce a shared library also
has issues:
  -bash-4.4$ cat t.c
  extern void *xsk_umem__create;
  void * test() { return xsk_umem__create; }
  -bash-4.4$ gcc -c t.c
  -bash-4.4$ gcc -shared t.o libbpf.a -o t.so
  /bin/ld: t.so: version node not found for symbol xsk_umem__create@LIBBPF_0.0.2
  /bin/ld: failed to set dynamic section sizes: Bad value
  collect2: error: ld returned 1 exit status
  -bash-4.4$

To fix the problem, I simply added a macro __BUILD_STATIC_LIBBPF__
which will prevent issuing .symver assembly codes when enabled.
The .symver assembly codes are still issued by default.
This will at least give other libbpf users to build libbpf
without these versioned symbols.

I did not touch Makefile to actually use this macro to build
static library as I want to check whether this is desirable or not.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/lib/bpf/xsk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index 24fa313524fb..76c12c4c5c70 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -261,8 +261,11 @@ int xsk_umem__create_v0_0_2(struct xsk_umem **umem_ptr, void *umem_area,
 	return xsk_umem__create_v0_0_4(umem_ptr, umem_area, size, fill, comp,
 					&config);
 }
+
+#ifndef __BUILD_STATIC_LIBBPF__
 asm(".symver xsk_umem__create_v0_0_2, xsk_umem__create@LIBBPF_0.0.2");
 asm(".symver xsk_umem__create_v0_0_4, xsk_umem__create@@LIBBPF_0.0.4");
+#endif
 
 static int xsk_load_xdp_prog(struct xsk_socket *xsk)
 {
-- 
2.17.1


^ permalink raw reply related

* Re: chapoly acceleration hardware [Was: Re: [RFC PATCH 00/18] crypto: wireguard using the existing crypto API]
From: Dave Taht @ 2019-09-26 23:13 UTC (permalink / raw)
  To: Pascal Van Leeuwen
  Cc: Jason A. Donenfeld, Ard Biesheuvel, Linux Crypto Mailing List,
	linux-arm-kernel, Herbert Xu, David Miller, Greg KH,
	Linus Torvalds, Samuel Neves, Dan Carpenter, Arnd Bergmann,
	Eric Biggers, Andy Lutomirski, Will Deacon, Marc Zyngier,
	Catalin Marinas, Willy Tarreau, Netdev,
	Toke Høiland-Jørgensen
In-Reply-To: <MN2PR20MB29733A5E504126B6F4098136CA860@MN2PR20MB2973.namprd20.prod.outlook.com>

On Thu, Sep 26, 2019 at 6:52 AM Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> wrote:
>
> > -----Original Message-----
> > From: Jason A. Donenfeld <Jason@zx2c4.com>
> > Sent: Thursday, September 26, 2019 1:07 PM
> > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Linux Crypto Mailing List <linux-
> > crypto@vger.kernel.org>; linux-arm-kernel <linux-arm-kernel@lists.infradead.org>;
> > Herbert Xu <herbert@gondor.apana.org.au>; David Miller <davem@davemloft.net>; Greg KH
> > <gregkh@linuxfoundation.org>; Linus Torvalds <torvalds@linux-foundation.org>; Samuel
> > Neves <sneves@dei.uc.pt>; Dan Carpenter <dan.carpenter@oracle.com>; Arnd Bergmann
> > <arnd@arndb.de>; Eric Biggers <ebiggers@google.com>; Andy Lutomirski <luto@kernel.org>;
> > Will Deacon <will@kernel.org>; Marc Zyngier <maz@kernel.org>; Catalin Marinas
> > <catalin.marinas@arm.com>; Willy Tarreau <w@1wt.eu>; Netdev <netdev@vger.kernel.org>;
> > Toke Høiland-Jørgensen <toke@toke.dk>; Dave Taht <dave.taht@gmail.com>
> > Subject: chapoly acceleration hardware [Was: Re: [RFC PATCH 00/18] crypto: wireguard
> > using the existing crypto API]
> >
> > [CC +willy, toke, dave, netdev]
> >
> > Hi Pascal
> >
> > On Thu, Sep 26, 2019 at 12:19 PM Pascal Van Leeuwen
> > <pvanleeuwen@verimatrix.com> wrote:
> > > Actually, that assumption is factually wrong. I don't know if anything
> > > is *publicly* available, but I can assure you the silicon is running in
> > > labs already. And something will be publicly available early next year
> > > at the latest. Which could nicely coincide with having Wireguard support
> > > in the kernel (which I would also like to see happen BTW) ...
> > >
> > > Not "at some point". It will. Very soon. Maybe not in consumer or server
> > > CPUs, but definitely in the embedded (networking) space.
> > > And it *will* be much faster than the embedded CPU next to it, so it will
> > > be worth using it for something like bulk packet encryption.
> >
> > Super! I was wondering if you could speak a bit more about the
> > interface. My biggest questions surround latency. Will it be
> > synchronous or asynchronous?
> >
> The hardware being external to the CPU and running in parallel with it,
> obviously asynchronous.
>
> > If the latter, why?
> >
> Because, as you probably already guessed, the round-trip latency is way
> longer than the actual processing time, at least for small packets.
>
> Partly because the only way to communicate between the CPU and the HW
> accelerator (whether that is crypto, a GPU, a NIC, etc.) that doesn't
> keep the CPU busy moving data is through memory, with the HW doing DMA.
> And, as any programmer should now, round trip times to memory are huge
> relative to the processing speed.
>
> And partly because these accelerators are very similar to CPU's in
> terms of architecture, doing pipelined processing and having multiple
> of such pipelines in parallel. Except that these pipelines are not
> working on low-level instructions but on full packets/blocks. So they
> need to have many packets in flight to keep those pipelines fully
> occupied. And packets need to move through the various pipeline stages,
> so they incur the time needed to process them multiple times. (just
> like e.g. a multiply instruction with a throughput of 1 per cycle
> actually may need 4 or more cycles to actually provide its result)
>
> Could you do that from a synchronous interface? In theory, probably,
> if you would spawn a new thread for every new packet arriving and
> rely on the scheduler to preempt the waiting threads. But you'd need
> as many threads as the HW  accelerator can have packets in flight,
> while an async would need only 2 threads: one to handle the input to
> the accelerator and one to handle the output (or at most one thread
> per CPU, if you want to divide the workload)
>
> Such a many-thread approach seems very inefficient to me.
>
> > What will its latencies be?
> >
> Depends very much on the specific integration scenario (i.e. bus
> speed, bus hierarchy, cache hierarchy, memory speed, etc.) but on
> the order of a few thousand CPU clocks is not unheard of.
> Which is an eternity for the CPU, but still only a few uSec in
> human time. Not a problem unless you're a high-frequency trader and
> every ns counts ...
> It's not like the CPU would process those packets in zero time.
>
> > How deep will its buffers be?
> >
> That of course depends on the specific accelerator implementation,
> but possibly dozens of small packets in our case, as you'd need
> at least width x depth packets in there to keep the pipes busy.
> Just like a modern CPU needs hundreds of instructions in flight
> to keep all its resources busy.
>
> > The reason I ask is that a
> > lot of crypto acceleration hardware of the past has been fast and
> > having very deep buffers, but at great expense of latency.
> >
> Define "great expense". Everything is relative. The latency is very
> high compared to per-packet processing time but at the same time it's
> only on the order of a few uSec. Which may not even be significant on
> the total time it takes for the packet to travel from input MAC to
> output MAC, considering the CPU will still need to parse and classify
> it and do pre- and postprocessing on it.
>
> > In the networking context, keeping latency low is pretty important.
> >
> I've been doing this for IPsec for nearly 20 years now and I've never
> heard anyone complain about our latency, so it must be OK.

Well, it depends on where your bottlenecks are. On low-end hardware
you can and do tend to bottleneck on the crypto step, and with
uncontrolled, non-fq'd non-aqm'd buffering you get results like this:

http://blog.cerowrt.org/post/wireguard/

so in terms of "threads" I would prefer to think of flows entering
the tunnel and attempting to multiplex them as best as possible
across the crypto hard/software so that minimal in-hw latencies are experienced
for most packets and that the coupled queue length does not grow out of control,

Adding fq_codel's hashing algo and queuing to ipsec as was done in
commit: 264b87fa617e758966108db48db220571ff3d60e to leverage
the inner hash...

Had some nice results:

before: http://www.taht.net/~d/ipsec_fq_codel/oldqos.png (100ms spikes)
After: http://www.taht.net/~d/ipsec_fq_codel/newqos.png (2ms spikes)

I'd love to see more vpn vendors using the rrul test or something even
nastier to evaluate their results, rather than dragstrip bulk throughput tests,
steering multiple flows over multiple cores.

> We're also doing (fully inline, no CPU involved) MACsec cores, which
> operate at layer 2 and I know it's a concern there for very specific
> use cases (high frequency trading, precision time protocol, ...).
> For "normal" VPN's though, a few uSec more or less should be a non-issue.

Measured buffering is typically 1000 packets in userspace vpns. If you
can put data in, faster than you can get it out, well....

> > Already
> > WireGuard is multi-threaded which isn't super great all the time for
> > latency (improvements are a work in progress). If you're involved with
> > the design of the hardware, perhaps this is something you can help
> > ensure winds up working well? For example, AES-NI is straightforward
> > and good, but Intel can do that because they are the CPU. It sounds
> > like your silicon will be adjacent. How do you envision this working
> > in a low latency environment?
> >
> Depends on how low low-latency is. If you really need minimal latency,
> you need an inline implementation. Which we can also provide, BTW :-)
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com



-- 

Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740

^ permalink raw reply

* Re: [RFC 01/20] ice: Initialize and register multi-function device to provide RDMA
From: Nguyen, Anthony L @ 2019-09-26 23:39 UTC (permalink / raw)
  To: Kirsher, Jeffrey T, gregkh@linuxfoundation.org
  Cc: jgg@mellanox.com, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org, dledford@redhat.com, Ertman, David M
In-Reply-To: <20190926180556.GB1733924@kroah.com>

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

On Thu, 2019-09-26 at 20:05 +0200, Greg KH wrote:
> On Thu, Sep 26, 2019 at 09:45:00AM -0700, Jeff Kirsher wrote:
> > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> > 
> > The RDMA block does not advertise on the PCI bus or any other bus.
> 
> Huh?  How do you "know" where it is then?  Isn't is usually assigned
> to
> a PCI device?

The RDMA block does not have its own PCI function so it must register
and interact with the ice driver.  

> 
> > Thus the ice driver needs to provide access to the RDMA hardware
> > block
> > via a virtual bus; utilize a multi-function device to provide this
> > access.
> > 
> > This patch initializes the driver to support RDMA as well as
> > creates
> > and registers a multi-function device for the RDMA driver to
> > register to.
> > At this point the driver is fully initialized to register a
> > platform
> > driver, however, can not yet register as the ops have not been
> > implemented.
> > 
> > We refer to the interaction of this platform device as Inter-Driver
> > Communication (IDC); where the platform device is referred to as
> > the peer
> > device and the platform driver is referred to as the peer driver.
> 
> Again, no platform devices, unless it REALLY IS a platform device
> (i.e.
> you are using device tree or platform data to find it.)  Is that what
> you are doing here?
> 

Sorry, this was an oversight.  When I was changing the wording in the
commit message away from platform device, I missed this one.

I will rewrite the commit message to make things clearer and correct
the platform device reference.

Thanks,
Tony

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3277 bytes --]

^ permalink raw reply

* [PATCH] tools: bpf: Use !building_out_of_srctree to determine srctree
From: Shuah Khan @ 2019-09-27  1:13 UTC (permalink / raw)
  To: ast, daniel, kafai, songliubraving, yhs
  Cc: Shuah Khan, netdev, bpf, linux-kernel, linux-kselftest

make TARGETS=bpf kselftest fails with:

Makefile:127: tools/build/Makefile.include: No such file or directory

When the bpf tool make is invoked from tools Makefile, srctree is
cleared and the current logic check for srctree equals to empty
string to determine srctree location from CURDIR.

When the build in invoked from selftests/bpf Makefile, the srctree
is set to "." and the same logic used for srctree equals to empty is
needed to determine srctree.

Check building_out_of_srctree undefined as the condition for both
cases to fix "make TARGETS=bpf kselftest" build failure.

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 tools/bpf/Makefile     | 6 +++++-
 tools/lib/bpf/Makefile | 6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index fbf5e4a0cb9c..5d1995fd369c 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -12,7 +12,11 @@ INSTALL ?= install
 CFLAGS += -Wall -O2
 CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/include/uapi -I$(srctree)/include
 
-ifeq ($(srctree),)
+# This will work when bpf is built in tools env. where srctree
+# isn't set and when invoked from selftests build, where srctree
+# is set to ".". building_out_of_srctree is undefined for in srctree
+# builds
+ifndef building_out_of_srctree
 srctree := $(patsubst %/,%,$(dir $(CURDIR)))
 srctree := $(patsubst %/,%,$(dir $(srctree)))
 endif
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index c6f94cffe06e..20772663d3e1 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -8,7 +8,11 @@ LIBBPF_MAJOR_VERSION := $(firstword $(subst ., ,$(LIBBPF_VERSION)))
 
 MAKEFLAGS += --no-print-directory
 
-ifeq ($(srctree),)
+# This will work when bpf is built in tools env. where srctree
+# isn't set and when invoked from selftests build, where srctree
+# is a ".". building_out_of_srctree is undefined for in srctree
+# builds
+ifndef building_out_of_srctree
 srctree := $(patsubst %/,%,$(dir $(CURDIR)))
 srctree := $(patsubst %/,%,$(dir $(srctree)))
 srctree := $(patsubst %/,%,$(dir $(srctree)))
-- 
2.20.1


^ permalink raw reply related

* memory leak in tls_init
From: syzbot @ 2019-09-27  1:19 UTC (permalink / raw)
  To: aviadye, borisp, daniel, davejwatson, davem, jakub.kicinski,
	john.fastabend, linux-kernel, netdev, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    f41def39 Merge tag 'ceph-for-5.4-rc1' of git://github.com/..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=105b7ff9600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=2e29707d7d1530b3
dashboard link: https://syzkaller.appspot.com/bug?extid=35bc8fe94c9f38db8320
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=145b3419600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+35bc8fe94c9f38db8320@syzkaller.appspotmail.com

2019/09/26 13:11:21 executed programs: 23
BUG: memory leak
unreferenced object 0xffff88810e482a00 (size 512):
   comm "syz-executor.4", pid 6874, jiffies 4295090041 (age 14.090s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
   backtrace:
     [<00000000e93f019a>] kmemleak_alloc_recursive  
include/linux/kmemleak.h:43 [inline]
     [<00000000e93f019a>] slab_post_alloc_hook mm/slab.h:586 [inline]
     [<00000000e93f019a>] slab_alloc mm/slab.c:3319 [inline]
     [<00000000e93f019a>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
     [<00000000268637bd>] kmalloc include/linux/slab.h:552 [inline]
     [<00000000268637bd>] kzalloc include/linux/slab.h:686 [inline]
     [<00000000268637bd>] create_ctx net/tls/tls_main.c:611 [inline]
     [<00000000268637bd>] tls_init net/tls/tls_main.c:794 [inline]
     [<00000000268637bd>] tls_init+0xbc/0x200 net/tls/tls_main.c:773
     [<00000000f52c33c5>] __tcp_set_ulp net/ipv4/tcp_ulp.c:139 [inline]
     [<00000000f52c33c5>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:160
     [<0000000009cb49a0>] do_tcp_setsockopt.isra.0+0x1c1/0xe10  
net/ipv4/tcp.c:2825
     [<00000000b9d96429>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3152
     [<0000000038a5546c>] sock_common_setsockopt+0x38/0x50  
net/core/sock.c:3142
     [<00000000d945b2a0>] __sys_setsockopt+0x10f/0x220 net/socket.c:2084
     [<000000003c3afaa0>] __do_sys_setsockopt net/socket.c:2100 [inline]
     [<000000003c3afaa0>] __se_sys_setsockopt net/socket.c:2097 [inline]
     [<000000003c3afaa0>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2097
     [<00000000f7f21cbd>] do_syscall_64+0x73/0x1f0  
arch/x86/entry/common.c:290
     [<00000000d4c003b9>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff88810e71e600 (size 512):
   comm "syz-executor.4", pid 6888, jiffies 4295090060 (age 13.900s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
   backtrace:
     [<00000000e93f019a>] kmemleak_alloc_recursive  
include/linux/kmemleak.h:43 [inline]
     [<00000000e93f019a>] slab_post_alloc_hook mm/slab.h:586 [inline]
     [<00000000e93f019a>] slab_alloc mm/slab.c:3319 [inline]
     [<00000000e93f019a>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
     [<00000000268637bd>] kmalloc include/linux/slab.h:552 [inline]
     [<00000000268637bd>] kzalloc include/linux/slab.h:686 [inline]
     [<00000000268637bd>] create_ctx net/tls/tls_main.c:611 [inline]
     [<00000000268637bd>] tls_init net/tls/tls_main.c:794 [inline]
     [<00000000268637bd>] tls_init+0xbc/0x200 net/tls/tls_main.c:773
     [<00000000f52c33c5>] __tcp_set_ulp net/ipv4/tcp_ulp.c:139 [inline]
     [<00000000f52c33c5>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:160
     [<0000000009cb49a0>] do_tcp_setsockopt.isra.0+0x1c1/0xe10  
net/ipv4/tcp.c:2825
     [<00000000b9d96429>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3152
     [<0000000038a5546c>] sock_common_setsockopt+0x38/0x50  
net/core/sock.c:3142
     [<00000000d945b2a0>] __sys_setsockopt+0x10f/0x220 net/socket.c:2084
     [<000000003c3afaa0>] __do_sys_setsockopt net/socket.c:2100 [inline]
     [<000000003c3afaa0>] __se_sys_setsockopt net/socket.c:2097 [inline]
     [<000000003c3afaa0>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2097
     [<00000000f7f21cbd>] do_syscall_64+0x73/0x1f0  
arch/x86/entry/common.c:290
     [<00000000d4c003b9>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff88810e356800 (size 512):
   comm "syz-executor.0", pid 6926, jiffies 4295090085 (age 13.650s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
   backtrace:
     [<00000000e93f019a>] kmemleak_alloc_recursive  
include/linux/kmemleak.h:43 [inline]
     [<00000000e93f019a>] slab_post_alloc_hook mm/slab.h:586 [inline]
     [<00000000e93f019a>] slab_alloc mm/slab.c:3319 [inline]
     [<00000000e93f019a>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
     [<00000000268637bd>] kmalloc include/linux/slab.h:552 [inline]
     [<00000000268637bd>] kzalloc include/linux/slab.h:686 [inline]
     [<00000000268637bd>] create_ctx net/tls/tls_main.c:611 [inline]
     [<00000000268637bd>] tls_init net/tls/tls_main.c:794 [inline]
     [<00000000268637bd>] tls_init+0xbc/0x200 net/tls/tls_main.c:773
     [<00000000f52c33c5>] __tcp_set_ulp net/ipv4/tcp_ulp.c:139 [inline]
     [<00000000f52c33c5>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:160
     [<0000000009cb49a0>] do_tcp_setsockopt.isra.0+0x1c1/0xe10  
net/ipv4/tcp.c:2825
     [<00000000b9d96429>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3152
     [<0000000038a5546c>] sock_common_setsockopt+0x38/0x50  
net/core/sock.c:3142
     [<00000000d945b2a0>] __sys_setsockopt+0x10f/0x220 net/socket.c:2084
     [<000000003c3afaa0>] __do_sys_setsockopt net/socket.c:2100 [inline]
     [<000000003c3afaa0>] __se_sys_setsockopt net/socket.c:2097 [inline]
     [<000000003c3afaa0>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2097
     [<00000000f7f21cbd>] do_syscall_64+0x73/0x1f0  
arch/x86/entry/common.c:290
     [<00000000d4c003b9>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff88810e3df600 (size 512):
   comm "syz-executor.4", pid 6933, jiffies 4295090088 (age 13.620s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
   backtrace:
     [<00000000e93f019a>] kmemleak_alloc_recursive  
include/linux/kmemleak.h:43 [inline]
     [<00000000e93f019a>] slab_post_alloc_hook mm/slab.h:586 [inline]
     [<00000000e93f019a>] slab_alloc mm/slab.c:3319 [inline]
     [<00000000e93f019a>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
     [<00000000268637bd>] kmalloc include/linux/slab.h:552 [inline]
     [<00000000268637bd>] kzalloc include/linux/slab.h:686 [inline]
     [<00000000268637bd>] create_ctx net/tls/tls_main.c:611 [inline]
     [<00000000268637bd>] tls_init net/tls/tls_main.c:794 [inline]
     [<00000000268637bd>] tls_init+0xbc/0x200 net/tls/tls_main.c:773
     [<00000000f52c33c5>] __tcp_set_ulp net/ipv4/tcp_ulp.c:139 [inline]
     [<00000000f52c33c5>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:160
     [<0000000009cb49a0>] do_tcp_setsockopt.isra.0+0x1c1/0xe10  
net/ipv4/tcp.c:2825
     [<00000000b9d96429>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3152
     [<0000000038a5546c>] sock_common_setsockopt+0x38/0x50  
net/core/sock.c:3142
     [<00000000d945b2a0>] __sys_setsockopt+0x10f/0x220 net/socket.c:2084
     [<000000003c3afaa0>] __do_sys_setsockopt net/socket.c:2100 [inline]
     [<000000003c3afaa0>] __se_sys_setsockopt net/socket.c:2097 [inline]
     [<000000003c3afaa0>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2097
     [<00000000f7f21cbd>] do_syscall_64+0x73/0x1f0  
arch/x86/entry/common.c:290
     [<00000000d4c003b9>] entry_SYSCALL_64_after_hwframe+0x44/0xa9



---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* [PATCH net] sch_cbq: validate TCA_CBQ_WRROPT to avoid crash
From: Eric Dumazet @ 2019-09-27  1:24 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, syzbot

syzbot reported a crash in cbq_normalize_quanta() caused
by an out of range cl->priority.

iproute2 enforces this check, but malicious users do not.

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN PTI
Modules linked in:
CPU: 1 PID: 26447 Comm: syz-executor.1 Not tainted 5.3+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:cbq_normalize_quanta.part.0+0x1fd/0x430 net/sched/sch_cbq.c:902
RSP: 0018:ffff8801a5c333b0 EFLAGS: 00010206
RAX: 0000000020000003 RBX: 00000000fffffff8 RCX: ffffc9000712f000
RDX: 00000000000043bf RSI: ffffffff83be8962 RDI: 0000000100000018
RBP: ffff8801a5c33420 R08: 000000000000003a R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000002ef
R13: ffff88018da95188 R14: dffffc0000000000 R15: 0000000000000015
FS:  00007f37d26b1700(0000) GS:ffff8801dad00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004c7cec CR3: 00000001bcd0a006 CR4: 00000000001626f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 [<ffffffff83be9d57>] cbq_normalize_quanta include/net/pkt_sched.h:27 [inline]
 [<ffffffff83be9d57>] cbq_addprio net/sched/sch_cbq.c:1097 [inline]
 [<ffffffff83be9d57>] cbq_set_wrr+0x2d7/0x450 net/sched/sch_cbq.c:1115
 [<ffffffff83bee8a7>] cbq_change_class+0x987/0x225b net/sched/sch_cbq.c:1537
 [<ffffffff83b96985>] tc_ctl_tclass+0x555/0xcd0 net/sched/sch_api.c:2329
 [<ffffffff83a84655>] rtnetlink_rcv_msg+0x485/0xc10 net/core/rtnetlink.c:5248
 [<ffffffff83cadf0a>] netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2510
 [<ffffffff83a7db6d>] rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5266
 [<ffffffff83cac2c6>] netlink_unicast_kernel net/netlink/af_netlink.c:1324 [inline]
 [<ffffffff83cac2c6>] netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1350
 [<ffffffff83cacd4a>] netlink_sendmsg+0x89a/0xd50 net/netlink/af_netlink.c:1939
 [<ffffffff8399d46e>] sock_sendmsg_nosec net/socket.c:673 [inline]
 [<ffffffff8399d46e>] sock_sendmsg+0x12e/0x170 net/socket.c:684
 [<ffffffff8399f1fd>] ___sys_sendmsg+0x81d/0x960 net/socket.c:2359
 [<ffffffff839a2d05>] __sys_sendmsg+0x105/0x1d0 net/socket.c:2397
 [<ffffffff839a2df9>] SYSC_sendmsg net/socket.c:2406 [inline]
 [<ffffffff839a2df9>] SyS_sendmsg+0x29/0x30 net/socket.c:2404
 [<ffffffff8101ccc8>] do_syscall_64+0x528/0x770 arch/x86/entry/common.c:305
 [<ffffffff84400091>] entry_SYSCALL_64_after_hwframe+0x42/0xb7

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
 net/sched/sch_cbq.c | 43 +++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 06c7a2da21bc20e8f7e6ad02da0b0b3e3d933928..39b427dc751282db7adb2d0803eecccb0457c316 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1127,6 +1127,33 @@ static const struct nla_policy cbq_policy[TCA_CBQ_MAX + 1] = {
 	[TCA_CBQ_POLICE]	= { .len = sizeof(struct tc_cbq_police) },
 };
 
+static int cbq_opt_parse(struct nlattr *tb[TCA_CBQ_MAX + 1],
+			 struct nlattr *opt,
+			 struct netlink_ext_ack *extack)
+{
+	int err;
+
+	if (!opt) {
+		NL_SET_ERR_MSG(extack, "CBQ options are required for this operation");
+		return -EINVAL;
+	}
+
+	err = nla_parse_nested_deprecated(tb, TCA_CBQ_MAX, opt,
+					  cbq_policy, extack);
+	if (err < 0)
+		return err;
+
+	if (tb[TCA_CBQ_WRROPT]) {
+		const struct tc_cbq_wrropt *wrr = nla_data(tb[TCA_CBQ_WRROPT]);
+
+		if (wrr->priority > TC_CBQ_MAXPRIO) {
+			NL_SET_ERR_MSG(extack, "priority is bigger than TC_CBQ_MAXPRIO");
+			err = -EINVAL;
+		}
+	}
+	return err;
+}
+
 static int cbq_init(struct Qdisc *sch, struct nlattr *opt,
 		    struct netlink_ext_ack *extack)
 {
@@ -1139,13 +1166,7 @@ static int cbq_init(struct Qdisc *sch, struct nlattr *opt,
 	hrtimer_init(&q->delay_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_PINNED);
 	q->delay_timer.function = cbq_undelay;
 
-	if (!opt) {
-		NL_SET_ERR_MSG(extack, "CBQ options are required for this operation");
-		return -EINVAL;
-	}
-
-	err = nla_parse_nested_deprecated(tb, TCA_CBQ_MAX, opt, cbq_policy,
-					  extack);
+	err = cbq_opt_parse(tb, opt, extack);
 	if (err < 0)
 		return err;
 
@@ -1464,13 +1485,7 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
 	struct cbq_class *parent;
 	struct qdisc_rate_table *rtab = NULL;
 
-	if (!opt) {
-		NL_SET_ERR_MSG(extack, "Mandatory qdisc options missing");
-		return -EINVAL;
-	}
-
-	err = nla_parse_nested_deprecated(tb, TCA_CBQ_MAX, opt, cbq_policy,
-					  extack);
+	err = cbq_opt_parse(tb, opt, extack);
 	if (err < 0)
 		return err;
 
-- 
2.23.0.444.g18eeb5a265-goog


^ 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