Netdev List
 help / color / mirror / Atom feed
* Re: pull-request: bpf 2018-11-25
From: David Miller @ 2018-11-26  4:13 UTC (permalink / raw)
  To: daniel; +Cc: ast, netdev
In-Reply-To: <20181126001651.16528-1-daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Mon, 26 Nov 2018 01:16:51 +0100

> The following pull-request contains BPF updates for your *net* tree.

Pulled, thanks.

^ permalink raw reply

* Re: [PATCH v2 1/4] net: macb: Check MDIO state before read/write and use timeouts
From: Andrew Lunn @ 2018-11-26 14:52 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: harini.katakam, Nicolas.Ferre, davem, netdev, linux-kernel,
	michal.simek, harinikatakamlinux, harinik, shubhrajyoti.datta,
	sai.pavan.boddu
In-Reply-To: <5de882e3-65ac-7ff1-bb55-7537666dfc77@microchip.com>

On Mon, Nov 26, 2018 at 02:46:01PM +0000, Claudiu.Beznea@microchip.com wrote:
> Hi Harini,
> 
> On 26.11.2018 09:07, Harini Katakam wrote:
> > From: Harini Katakam <harinik@xilinx.com>
> > 
> > Replace the while loop in MDIO read/write functions with a timeout.
> > In addition, add a check for MDIO bus busy before initiating a new
> > operation as well to make sure there is no ongoing MDIO operation.
> 
> Is this MDIO bus busy check necessary? The caller of
> macb_mdio_read/macb_mdio_write locks the mdio bus mutex before calling it
> (e.g. mdiobus_read).

Hi Claudiu

It depends on the implementation. A write operation you could just
launch, but not wait for it to complete, allowing you to do other
stuff while the hardware is busy. For the next operation you then do
need to check the previous operation has completed.

I've not checked it is actually implemented this way.

     Andrew

^ permalink raw reply

* Re: [PATCH perf,bpf 0/5] reveal invisible bpf programs
From: Peter Zijlstra @ 2018-11-26 14:50 UTC (permalink / raw)
  To: Song Liu
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	ast@kernel.org, daniel@iogearbox.net, acme@kernel.org,
	Kernel Team
In-Reply-To: <71189F83-A09F-4A03-95EC-694D37FD7675@fb.com>

On Thu, Nov 22, 2018 at 06:13:32PM +0000, Song Liu wrote:
> Hi Peter,
> 
> > On Nov 22, 2018, at 1:32 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > On Wed, Nov 21, 2018 at 11:54:57AM -0800, Song Liu wrote:
> >> Changes RFC -> PATCH v1:
> >> 
> >> 1. In perf-record, poll vip events in a separate thread;
> >> 2. Add tag to bpf prog name;
> >> 3. Small refactorings.
> >> 
> >> Original cover letter (with minor revisions):
> >> 
> >> This is to follow up Alexei's early effort to show bpf programs

> >> In this version, PERF_RECORD_BPF_EVENT is introduced to send real time BPF
> >> load/unload events to user space. In user space, perf-record is modified
> >> to listen to these events (through a dedicated ring buffer) and generate
> >> detailed information about the program (struct bpf_prog_info_event). Then,
> >> perf-report translates these events into proper symbols.
> >> 
> >> With this set, perf-report will show bpf program as:
> >> 
> >>   18.49%     0.16%  test  [kernel.vmlinux]    [k] ksys_write
> >>   18.01%     0.47%  test  [kernel.vmlinux]    [k] vfs_write
> >>   17.02%     0.40%  test  bpf_prog            [k] bpf_prog_07367f7ba80df72b_
> >>   16.97%     0.10%  test  [kernel.vmlinux]    [k] __vfs_write
> >>   16.86%     0.12%  test  [kernel.vmlinux]    [k] comm_write
> >>   16.67%     0.39%  test  [kernel.vmlinux]    [k] bpf_probe_read
> >> 
> >> Note that, the program name is still work in progress, it will be cleaner
> >> with function types in BTF.
> >> 
> >> Please share your comments on this.
> > 
> > So I see:
> > 
> >  kernel/bpf/core.c:void bpf_prog_kallsyms_add(struct bpf_prog *fp)
> > 
> > which should already provide basic symbol information for extant eBPF
> > programs, right?
> 
> Right, if the BPF program is still loaded when perf-report runs, symbols 
> are available. 

Good, that is not something that was clear. The Changelog seems to imply
we need this new stuff in order to observe symbols.

> > And (AFAIK) perf uses /proc/kcore for annotate on the current running
> > kernel (if not, it really should, given alternatives, jump_labels and
> > all other other self-modifying code).
> > 
> > So this fancy new stuff is only for the case where your profile spans
> > eBPF load/unload events (which should be relatively rare in the normal
> > case, right), or when you want source annotated asm output (I normally
> > don't bother with that).
> 
> This patch set adds two pieces of information:
> 1. At the beginning of perf-record, save info of existing BPF programs;
> 2. Gather information of BPF programs load/unload during perf-record. 
> 
> (1) is all in user space. It is necessary to show symbols of BPF program
> that are unloaded _after_ perf-record. (2) needs PERF_RECORD_BPF_EVENT 
> from the ring buffer. It covers BPF program loaded during perf-record 
> (perf record -- bpf_test). 

I'm saying that if you given them symbols; most people won't need any of
that ever.

And just tracking kallsyms is _much_ cheaper than either 1 or 2. Alexei
was talking fairly big amounts of data per BPF prog. Dumping and saving
that sounds like pointless overhead for 99% of the users.

> > That is; I would really like this fancy stuff to be an optional extra
> > that is typically not needed.
> > 
> > Does that make sense?
> 
> (1) above is always enabled with this set. I added option no-bpf-events 
> to disable (2). I guess you prefer the (2) is disabled by default, and 
> enabled with an option?

I'm saying neither should be default enabled. Instead we should do
recording and tracking by default.

That gets people symbol information on BPF stuff, which is likely all
they ever need.

If they then decide they need more, then and only then can they enable
the fancy pants bpf-synchronous-syscall nonsense thingy. And like I said
before; I don't bother with source annotated asm output for
perf-annotate.

And if the bpf prog is still loaded, kcore should contain the raw jitted
asm just fine; you again don't need the bpf syscall stuff.


Now, I'm not saying this patch set is useless; but I'm saying most
people should not need this, and it is massive overkill for the needs of
most people.

^ permalink raw reply

* Re: [PATCH v2 3/4] net: macb: Add pm runtime support
From: Andrew Lunn @ 2018-11-26 14:47 UTC (permalink / raw)
  To: Harini Katakam
  Cc: nicolas.ferre, davem, claudiu.beznea, netdev, linux-kernel,
	michal.simek, harinikatakamlinux, Harini Katakam,
	Shubhrajyoti Datta
In-Reply-To: <1543216072-9623-4-git-send-email-harini.katakam@xilinx.com>

> @@ -335,6 +338,10 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>  	int value;
>  	int err;
>  
> +	err = pm_runtime_get_sync(&bp->pdev->dev);
> +	if (err < 0)
> +		return err;
> +
>  	err = macb_mdio_wait_for_idle(bp);
>  	if (err < 0)
>  		return err;

Hi Harini

Thanks for adding runtime PM support to the MDIO bus.

It looks like on the error paths you are not always undoing the
pm_runtime_get_sync(). You probably need some sort of goto err;
construct. macb_mdio_write() has the same issue.

	Andrew

^ permalink raw reply

* Re: [PATCH v2 4/4] net: macb: Add support for suspend/resume with full power down
From: Claudiu.Beznea @ 2018-11-26 14:46 UTC (permalink / raw)
  To: harini.katakam, Nicolas.Ferre, davem
  Cc: netdev, linux-kernel, michal.simek, harinikatakamlinux, appanad
In-Reply-To: <1543216072-9623-5-git-send-email-harini.katakam@xilinx.com>



On 26.11.2018 09:07, Harini Katakam wrote:
> When macb device is suspended and system is powered down, the clocks
> are removed and hence macb should be closed gracefully and restored
> upon resume. This patch does the same by switching off the net device,
> suspending phy and performing necessary cleanup of interrupts and BDs.
> Upon resume, all these are reinitialized again.
> 
> Reset of macb device is done only when GEM is not a wake device.
> Even when gem is a wake device, tx queues can be stopped and ptp device
> can be closed (tsu clock will be disabled in pm_runtime_suspend) as
> wake event detection has no dependency on this.
> 
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
> ---
> v2 changes:
> Fixed parameter passed to phy calls.
> 
>  drivers/net/ethernet/cadence/macb_main.c | 38 ++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 4b85ad7..dcb0194 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4249,16 +4249,33 @@ static int __maybe_unused macb_suspend(struct device *dev)
>  {
>  	struct net_device *netdev = dev_get_drvdata(dev);
>  	struct macb *bp = netdev_priv(netdev);
> +	struct macb_queue *queue = bp->queues;
> +	unsigned long flags;
> +	unsigned int q;
> +
> +	if (!netif_running(netdev))
> +		return 0;
>  
> -	netif_carrier_off(netdev);
> -	netif_device_detach(netdev);

Is it necessary to remove this from here and have it on every if branch?

>  
>  	if (bp->wol & MACB_WOL_ENABLED) {
>  		macb_writel(bp, IER, MACB_BIT(WOL));
>  		macb_writel(bp, WOL, MACB_BIT(MAG));
>  		enable_irq_wake(bp->queues[0].irq);
> +		netif_device_detach(netdev);
> +	} else {
> +		netif_device_detach(netdev);
> +		for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)
> +			napi_disable(&queue->napi);
> +		phy_stop(netdev->phydev);
> +		phy_suspend(netdev->phydev);
> +		spin_lock_irqsave(&bp->lock, flags);
> +		macb_reset_hw(bp);
> +		spin_unlock_irqrestore(&bp->lock, flags);

In the previous version you said you encountered some crashes while
stressing this part if macb_open()/macb_close() was used in here. Could you
share the tests so that I can debug it on my side?

>  	}
>  
> +	netif_carrier_off(netdev);
> +	if (bp->ptp_info)
> +		bp->ptp_info->ptp_remove(netdev);
>  	pm_runtime_force_suspend(dev);
>  
>  	return 0;
> @@ -4268,6 +4285,11 @@ static int __maybe_unused macb_resume(struct device *dev)
>  {
>  	struct net_device *netdev = dev_get_drvdata(dev);
>  	struct macb *bp = netdev_priv(netdev);
> +	struct macb_queue *queue = bp->queues;
> +	unsigned int q;
> +
> +	if (!netif_running(netdev))
> +		return 0;
>  
>  	pm_runtime_force_resume(dev);
>  
> @@ -4275,9 +4297,21 @@ static int __maybe_unused macb_resume(struct device *dev)
>  		macb_writel(bp, IDR, MACB_BIT(WOL));
>  		macb_writel(bp, WOL, 0);
>  		disable_irq_wake(bp->queues[0].irq);
> +	} else {
> +		macb_writel(bp, NCR, MACB_BIT(MPE));

Just asking... shouldn't other registers be restored here after SoC power
is cut off?

Thank you,
Claudiu Beznea

> +		for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)
> +			napi_enable(&queue->napi);
> +		phy_resume(netdev->phydev);
> +		phy_init_hw(netdev->phydev);
> +		phy_start(netdev->phydev);
>  	}
>  
> +	bp->macbgem_ops.mog_init_rings(bp);
> +	macb_init_hw(bp);
> +	macb_set_rx_mode(netdev);
>  	netif_device_attach(netdev);
> +	if (bp->ptp_info)
> +		bp->ptp_info->ptp_init(netdev);
>  
>  	return 0;
>  }
> 

^ permalink raw reply

* Re: [PATCH v2 1/4] net: macb: Check MDIO state before read/write and use timeouts
From: Claudiu.Beznea @ 2018-11-26 14:46 UTC (permalink / raw)
  To: harini.katakam, Nicolas.Ferre, davem
  Cc: netdev, linux-kernel, michal.simek, harinikatakamlinux, harinik,
	shubhrajyoti.datta, sai.pavan.boddu
In-Reply-To: <1543216072-9623-2-git-send-email-harini.katakam@xilinx.com>

Hi Harini,

On 26.11.2018 09:07, Harini Katakam wrote:
> From: Harini Katakam <harinik@xilinx.com>
> 
> Replace the while loop in MDIO read/write functions with a timeout.
> In addition, add a check for MDIO bus busy before initiating a new
> operation as well to make sure there is no ongoing MDIO operation.

Is this MDIO bus busy check necessary? The caller of
macb_mdio_read/macb_mdio_write locks the mdio bus mutex before calling it
(e.g. mdiobus_read).

> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> Signed-off-by: Harini Katakam <harinik@xilinx.com>
> ---
> v2 changes:
> Use readx_poll_timeout
> 
> Changes form RFC:
> Cleaned up timeout implementation and moved it to a helper.
> 
>  drivers/net/ethernet/cadence/macb.h      |  3 +++
>  drivers/net/ethernet/cadence/macb_main.c | 33 ++++++++++++++++++++++++++------
>  2 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 3d45f4c..df7bee1 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -714,6 +714,9 @@
>  		__v; \
>  	})
>  
> +#define MACB_IDLE_MASK		(1 << MACB_IDLE_OFFSET)

You could use MACB_BIT(IDLE) instead.

> +#define MACB_READ_NSR(bp)	macb_readl(bp, NSR)

Is this necessary?


> +
>  /* struct macb_dma_desc - Hardware DMA descriptor
>   * @addr: DMA address of data buffer
>   * @ctrl: Control and status bits
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 1d86b4d..fd86ece 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -36,6 +36,7 @@
>  #include <linux/ip.h>
>  #include <linux/udp.h>
>  #include <linux/tcp.h>
> +#include <linux/iopoll.h>
>  #include "macb.h"
>  
>  #define MACB_RX_BUFFER_SIZE	128
> @@ -79,6 +80,8 @@
>   */
>  #define MACB_HALT_TIMEOUT	1230
>  
> +#define MACB_MDIO_TIMEOUT	1000000 /* in usecs */
> +
>  /* DMA buffer descriptor might be different size
>   * depends on hardware configuration:
>   *
> @@ -318,10 +321,23 @@ static void macb_get_hwaddr(struct macb *bp)
>  	eth_hw_addr_random(bp->dev);
>  }
>  
> +static int macb_mdio_wait_for_idle(struct macb *bp)
> +{
> +	u32 val;
> +
> +	return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_IDLE_MASK,
> +				  1, MACB_MDIO_TIMEOUT);
> +}
> +
>  static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>  {
>  	struct macb *bp = bus->priv;
>  	int value;
> +	int err;
> +
> +	err = macb_mdio_wait_for_idle(bp);
> +	if (err < 0)
> +		return err;
>  
>  	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
>  			      | MACB_BF(RW, MACB_MAN_READ)
> @@ -329,9 +345,9 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>  			      | MACB_BF(REGA, regnum)
>  			      | MACB_BF(CODE, MACB_MAN_CODE)));
>  
> -	/* wait for end of transfer */
> -	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> -		cpu_relax();
> +	err = macb_mdio_wait_for_idle(bp);
> +	if (err < 0)
> +		return err;
>  
>  	value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
>  
> @@ -342,6 +358,11 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
>  			   u16 value)
>  {
>  	struct macb *bp = bus->priv;
> +	int err;
> +
> +	err = macb_mdio_wait_for_idle(bp);
> +	if (err < 0)
> +		return err;
>  
>  	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
>  			      | MACB_BF(RW, MACB_MAN_WRITE)
> @@ -350,9 +371,9 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
>  			      | MACB_BF(CODE, MACB_MAN_CODE)
>  			      | MACB_BF(DATA, value)));
>  
> -	/* wait for end of transfer */
> -	while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> -		cpu_relax();
> +	err = macb_mdio_wait_for_idle(bp);
> +	if (err < 0)
> +		return err;
>  
>  	return 0;
>  }
> 

^ permalink raw reply

* Re: [PATCH net-next] net: remove unsafe skb_insert()
From: David Miller @ 2018-11-26  3:52 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, eric.dumazet, faisal.latif, dledford, jgg, linux-rdma
In-Reply-To: <CANn89i+mEYyLWt74jK-0ruUjHafgAPMdFKJGgM37B-iuZzk12g@mail.gmail.com>

From: Eric Dumazet <edumazet@google.com>
Date: Sun, 25 Nov 2018 15:37:43 -0800

> On Sun, Nov 25, 2018 at 10:29 AM David Miller <davem@davemloft.net> wrote:
>>
>> From: Eric Dumazet <edumazet@google.com>
>> Date: Sun, 25 Nov 2018 08:26:23 -0800
>>
>> > I do not see how one can effectively use skb_insert() without holding
>> > some kind of lock. Otherwise other cpus could have changed the list
>> > right before we have a chance of acquiring list->lock.
>> >
>> > Only existing user is in drivers/infiniband/hw/nes/nes_mgt.c and this
>> > one probably meant to use __skb_insert() since it appears nesqp->pau_list
>> > is protected by nesqp->pau_lock. This looks like nesqp->pau_lock
>> > could be removed, since nesqp->pau_list.lock could be used instead.
>> >
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>>
>> Good find.
>>
>> Indeed, any of the queue SKB manipulation functions that take two SKBs
>> as an argument are suspect in this manner.
>>
>> Applied, thanks Eric.
> 
> Oh well, this does not build.
> 
> Since you have not pushed your tree yet, maybe we can replace this
> with a version that actually compiles.
> 
> Please let me know if a relative patch or a v2 is needed, thanks.

I fixed up the build in your original patch and am about to push that
out.

^ permalink raw reply

* Re: [PATCH v2 1/4] net: macb: Check MDIO state before read/write and use timeouts
From: Andrew Lunn @ 2018-11-26 14:39 UTC (permalink / raw)
  To: Harini Katakam
  Cc: nicolas.ferre, davem, claudiu.beznea, netdev, linux-kernel,
	michal.simek, harinikatakamlinux, Harini Katakam,
	Shubhrajyoti Datta, Sai Pavan Boddu
In-Reply-To: <1543216072-9623-2-git-send-email-harini.katakam@xilinx.com>

On Mon, Nov 26, 2018 at 12:37:49PM +0530, Harini Katakam wrote:
> From: Harini Katakam <harinik@xilinx.com>
> 
> Replace the while loop in MDIO read/write functions with a timeout.
> In addition, add a check for MDIO bus busy before initiating a new
> operation as well to make sure there is no ongoing MDIO operation.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> Signed-off-by: Harini Katakam <harinik@xilinx.com>
> ---
> v2 changes:
> Use readx_poll_timeout
> 
> Changes form RFC:
> Cleaned up timeout implementation and moved it to a helper.
> 
>  drivers/net/ethernet/cadence/macb.h      |  3 +++
>  drivers/net/ethernet/cadence/macb_main.c | 33 ++++++++++++++++++++++++++------
>  2 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 3d45f4c..df7bee1 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -714,6 +714,9 @@
>  		__v; \
>  	})
>  
> +#define MACB_IDLE_MASK		(1 << MACB_IDLE_OFFSET)

I think you could use the MACB_BIT() macro here.

But otherwise,

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH V2] mm: Replace all open encodings for NUMA_NO_NODE
From: Anshuman Khandual @ 2018-11-26 14:02 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm, linux-kernel
  Cc: ocfs2-devel, linux-fbdev, dri-devel, netdev, intel-wired-lan,
	linux-media, iommu, linux-rdma, dmaengine, linux-block,
	sparclinux, linuxppc-dev, linux-ia64, linux-alpha, akpm,
	jiangqi903, hverkuil, vkoul
In-Reply-To: <bcf609de-c60a-d6ad-7acb-6c59c412adbc@redhat.com>



On 11/26/2018 06:18 PM, David Hildenbrand wrote:
> On 26.11.18 13:26, Anshuman Khandual wrote:
>> At present there are multiple places where invalid node number is encoded
>> as -1. Even though implicitly understood it is always better to have macros
>> in there. Replace these open encodings for an invalid node number with the
>> global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like
>> 'invalid node' from various places redirecting them to a common definition.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> Changes in V2:
>>
>> - Added inclusion of 'numa.h' header at various places per Andrew
>> - Updated 'dev_to_node' to use NUMA_NO_NODE instead per Vinod
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks David. My bad, forgot to add your review tag from the earlier version.

^ permalink raw reply

* Re: [PATCH net-next] net: phy: fix two issues with linkmode bitmaps
From: Andrew Lunn @ 2018-11-26  2:10 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <3917cdd2-ae04-7392-4b08-cf2d15082dcd@gmail.com>

> Because function naming is the same I'm afraid they easily can be used
> incorrectly (the bugs we just discuss are good examples). Maybe it
> could be an option to reflect the semantics in the name like this
> (better suited proposals welcome):
> 
> case 1: mii_xxx_to_linkmode_yyy
> case 2: mii_xxx_or_linkmode_yyy
> case 3: mii_xxx_mod_linkmode_yyy
 
Hi Heiner

I started a patchset using this idea, and reworks your fix. Lets work
on that, rather than merge this patch.

   Andrew

^ permalink raw reply

* Re: [PATCH V2] mm: Replace all open encodings for NUMA_NO_NODE
From: David Hildenbrand @ 2018-11-26 12:48 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, linux-kernel
  Cc: ocfs2-devel, linux-fbdev, dri-devel, netdev, intel-wired-lan,
	linux-media, iommu, linux-rdma, dmaengine, linux-block,
	sparclinux, linuxppc-dev, linux-ia64, linux-alpha, akpm,
	jiangqi903, hverkuil, vkoul
In-Reply-To: <1543235202-9075-1-git-send-email-anshuman.khandual@arm.com>

On 26.11.18 13:26, Anshuman Khandual wrote:
> At present there are multiple places where invalid node number is encoded
> as -1. Even though implicitly understood it is always better to have macros
> in there. Replace these open encodings for an invalid node number with the
> global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like
> 'invalid node' from various places redirecting them to a common definition.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> Changes in V2:
> 
> - Added inclusion of 'numa.h' header at various places per Andrew
> - Updated 'dev_to_node' to use NUMA_NO_NODE instead per Vinod

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

^ permalink raw reply

* Re: consistency for statistics with XDP mode
From: Toshiaki Makita @ 2018-11-26  1:37 UTC (permalink / raw)
  To: David Ahern, netdev@vger.kernel.org
  Cc: Paweł Staszewski, Jesper Dangaard Brouer, Saeed Mahameed,
	Jason Wang, Michael S. Tsirkin, David Miller
In-Reply-To: <05db5e18-acd5-772b-3620-820fe82be5a9@gmail.com>

On 2018/11/23 1:43, David Ahern wrote:
> On 11/21/18 5:53 PM, Toshiaki Makita wrote:
>>> We really need consistency in the counters and at a minimum, users
>>> should be able to track packet and byte counters for both Rx and Tx
>>> including XDP.
>>>
>>> It seems to me the Rx and Tx packet, byte and dropped counters returned
>>> for the standard device stats (/proc/net/dev, ip -s li show, ...) should
>>> include all packets managed by the driver regardless of whether they are
>>> forwarded / dropped in XDP or go up the Linux stack. This also aligns
>>
>> Agreed. When I introduced virtio_net XDP counters, I just forgot to
>> update tx packets/bytes counters on ndo_xdp_xmit. Probably I thought it
>> is handled by free_old_xmit_skbs.
> 
> Do you have some time to look at adding the Tx counters to virtio_net?

hoping I can make some time within a couple of days.

-- 
Toshiaki Makita

^ permalink raw reply

* Re: [PATCH bpf-next 1/3] bpf: helper to pop data from messages
From: Daniel Borkmann @ 2018-11-26  1:05 UTC (permalink / raw)
  To: John Fastabend, ast; +Cc: netdev
In-Reply-To: <1542937130-4860-2-git-send-email-john.fastabend@gmail.com>

On 11/23/2018 02:38 AM, John Fastabend wrote:
> This adds a BPF SK_MSG program helper so that we can pop data from a
> msg. We use this to pop metadata from a previous push data call.
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  include/uapi/linux/bpf.h |  13 +++-
>  net/core/filter.c        | 169 +++++++++++++++++++++++++++++++++++++++++++++++
>  net/ipv4/tcp_bpf.c       |  14 +++-
>  3 files changed, 192 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c1554aa..64681f8 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2268,6 +2268,16 @@ union bpf_attr {
>   *
>   *	Return
>   *		0 on success, or a negative error in case of failure.
> + *
> + * int bpf_msg_pop_data(struct sk_msg_buff *msg, u32 start, u32 pop, u64 flags)
> + *	 Description
> + *		Will remove 'pop' bytes from a msg starting at byte 'start'.
> + *		This result in ENOMEM errors under certain situations where
> + *		a allocation and copy are required due to a full ring buffer.
> + *		However, the helper will try to avoid doing the allocation
> + *		if possible. Other errors can occur if input parameters are
> + *		invalid either do to start byte not being valid part of msg
> + *		payload and/or pop value being to large.
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -2360,7 +2370,8 @@ union bpf_attr {
>  	FN(map_push_elem),		\
>  	FN(map_pop_elem),		\
>  	FN(map_peek_elem),		\
> -	FN(msg_push_data),
> +	FN(msg_push_data),		\
> +	FN(msg_pop_data),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/net/core/filter.c b/net/core/filter.c
> index f6ca38a..c6b35b5 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2428,6 +2428,173 @@ static const struct bpf_func_proto bpf_msg_push_data_proto = {
>  	.arg4_type	= ARG_ANYTHING,
>  };
>  
> +static void sk_msg_shift_left(struct sk_msg *msg, int i)
> +{
> +	int prev;
> +
> +	do {
> +		prev = i;
> +		sk_msg_iter_var_next(i);
> +		msg->sg.data[prev] = msg->sg.data[i];
> +	} while (i != msg->sg.end);
> +
> +	sk_msg_iter_prev(msg, end);
> +}
> +
> +static void sk_msg_shift_right(struct sk_msg *msg, int i)
> +{
> +	struct scatterlist tmp, sge;
> +
> +	sk_msg_iter_next(msg, end);
> +	sge = sk_msg_elem_cpy(msg, i);
> +	sk_msg_iter_var_next(i);
> +	tmp = sk_msg_elem_cpy(msg, i);
> +
> +	while (i != msg->sg.end) {
> +		msg->sg.data[i] = sge;
> +		sk_msg_iter_var_next(i);
> +		sge = tmp;
> +		tmp = sk_msg_elem_cpy(msg, i);
> +	}
> +}
> +
> +BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
> +	   u32, len, u64, flags)
> +{
> +	u32 i = 0, l, space, offset = 0;
> +	u64 last = start + len;
> +	int pop;
> +
> +	if (unlikely(flags))
> +		return -EINVAL;
> +
> +	/* First find the starting scatterlist element */
> +	i = msg->sg.start;
> +	do {
> +		l = sk_msg_elem(msg, i)->length;
> +
> +		if (start < offset + l)
> +			break;
> +		offset += l;
> +		sk_msg_iter_var_next(i);
> +	} while (i != msg->sg.end);
> +
> +	/* Bounds checks: start and pop must be inside message */
> +	if (start >= offset + l || last >= msg->sg.size)
> +		return -EINVAL;
> +
> +	space = MAX_MSG_FRAGS - sk_msg_elem_used(msg);
> +
> +	pop = len;
> +	/* --------------| offset
> +	 * -| start      |------- len ------|
> +	 *
> +	 *  |----- a ----|-------- pop -------|----- b ----|
> +	 *  |______________________________________________| length
> +	 *
> +	 *
> +	 * a:   region at front of scatter element to save
> +	 * b:   region at back of scatter element to save when length > A + pop
> +	 * pop: region to pop from element, same as input 'pop' here will be
> +	 *      decremented below per iteration.
> +	 *
> +	 * Two top-level cases to handle when start != offset, first B is non
> +	 * zero and second B is zero corresponding to when a pop includes more
> +	 * than one element.
> +	 *
> +	 * Then if B is non-zero AND there is no space allocate space and
> +	 * compact A, B regions into page. If there is space shift ring to
> +	 * the rigth free'ing the next element in ring to place B, leaving
> +	 * A untouched except to reduce length.
> +	 */
> +	if (start != offset) {
> +		struct scatterlist *nsge, *sge = sk_msg_elem(msg, i);
> +		int a = start;
> +		int b = sge->length - pop - a;
> +
> +		sk_msg_iter_var_next(i);
> +
> +		if (pop < sge->length - a) {
> +			if (space) {
> +				sge->length = a;
> +				sk_msg_shift_right(msg, i);
> +				nsge = sk_msg_elem(msg, i);
> +				get_page(sg_page(sge));
> +				sg_set_page(nsge,
> +					    sg_page(sge), b, offset + pop);
> +			} else {
> +				struct page *page, *orig;
> +				u8 *to, *from;
> +
> +				page = alloc_pages(__GFP_NOWARN |
> +						   __GFP_COMP   | GFP_ATOMIC,
> +						   get_order(a + b));
> +				if (unlikely(!page))
> +					return -ENOMEM;
> +
> +				sge->length = a;
> +				orig = sg_page(sge);
> +				from = sg_virt(sge);
> +				to = page_address(page);
> +				memcpy(to, from, a);
> +				memcpy(to + a, from + a + pop, b);
> +				sg_set_page(sge, page, a + b, 0);
> +				put_page(orig);
> +			}
> +			pop = 0;
> +		} else if (pop >= sge->length - a) {
> +			sge->length = a;
> +			pop -= (sge->length - a);
> +		}
> +	}
> +
> +	/* From above the current layout _must_ be as follows,
> +	 *
> +	 * -| offset
> +	 * -| start
> +	 *
> +	 *  |---- pop ---|---------------- b ------------|
> +	 *  |____________________________________________| length
> +	 *
> +	 * Offset and start of the current msg elem are equal because in the
> +	 * previous case we handled offset != start and either consumed the
> +	 * entire element and advanced to the next element OR pop == 0.
> +	 *
> +	 * Two cases to handle here are first pop is less than the length
> +	 * leaving some remainder b above. Simply adjust the element's layout
> +	 * in this case. Or pop >= length of the element so that b = 0. In this
> +	 * case advance to next element decrementing pop.
> +	 */
> +	while (pop) {
> +		struct scatterlist *sge = sk_msg_elem(msg, i);
> +
> +		if (pop < sge->length) {
> +			sge->length -= pop;
> +			sge->offset += pop;
> +			pop = 0;
> +		} else {
> +			pop -= sge->length;
> +			sk_msg_shift_left(msg, i);
> +		}
> +		sk_msg_iter_var_next(i);
> +	}
> +
> +	sk_mem_uncharge(msg->sk, len - pop);
> +	msg->sg.size -= (len - pop);
> +	sk_msg_compute_data_pointers(msg);
> +	return 0;
> +}

Hmm, had to revert. I noticed this will have a use after free. While you do the
sk_msg_compute_data_pointers() there is nothing from verifier that forces a
reload of the payload pointers that were set up before the helper call, so they
can still be used after the bpf_msg_pop_data() even though we dropped ref from
page etc.

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH v2] media: bpf: add bpf function to report mouse movement
From: Daniel Borkmann @ 2018-11-26  0:55 UTC (permalink / raw)
  To: Sean Young, Alexei Starovoitov, Roman Gushchin, Martin KaFai Lau
  Cc: linux-media, netdev
In-Reply-To: <20181123115040.13725-1-sean@mess.org>

Hi Sean,

On 11/23/2018 12:50 PM, Sean Young wrote:
> Some IR remotes have a directional pad or other pointer-like thing that
> can be used as a mouse. Make it possible to decode these types of IR
> protocols in BPF.
> 
> Cc: netdev@vger.kernel.org
> Signed-off-by: Sean Young <sean@mess.org>

Is this patch targeted at bpf-next tree?

> ---
>  drivers/media/rc/bpf-lirc.c               | 24 +++++++++++++++++++++++
>  include/uapi/linux/bpf.h                  | 17 +++++++++++++++-
>  tools/testing/selftests/bpf/bpf_helpers.h |  2 ++
>  3 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/bpf-lirc.c b/drivers/media/rc/bpf-lirc.c
> index 8b97fd1f0cea..390a722e6211 100644
> --- a/drivers/media/rc/bpf-lirc.c
> +++ b/drivers/media/rc/bpf-lirc.c
> @@ -59,6 +59,28 @@ static const struct bpf_func_proto rc_keydown_proto = {
>  	.arg4_type = ARG_ANYTHING,
>  };
>  
> +BPF_CALL_3(bpf_rc_pointer_rel, u32*, sample, s32, rel_x, s32, rel_y)
> +{
> +	struct ir_raw_event_ctrl *ctrl;
> +
> +	ctrl = container_of(sample, struct ir_raw_event_ctrl, bpf_sample);
> +
> +	input_report_rel(ctrl->dev->input_dev, REL_X, rel_x);
> +	input_report_rel(ctrl->dev->input_dev, REL_Y, rel_y);
> +	input_sync(ctrl->dev->input_dev);
> +
> +	return 0;
> +}
> +
> +static const struct bpf_func_proto rc_pointer_rel_proto = {
> +	.func	   = bpf_rc_pointer_rel,
> +	.gpl_only  = true,
> +	.ret_type  = RET_INTEGER,
> +	.arg1_type = ARG_PTR_TO_CTX,
> +	.arg2_type = ARG_ANYTHING,
> +	.arg3_type = ARG_ANYTHING,
> +};
> +
>  static const struct bpf_func_proto *
>  lirc_mode2_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  {
> @@ -67,6 +89,8 @@ lirc_mode2_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &rc_repeat_proto;
>  	case BPF_FUNC_rc_keydown:
>  		return &rc_keydown_proto;
> +	case BPF_FUNC_rc_pointer_rel:
> +		return &rc_pointer_rel_proto;
>  	case BPF_FUNC_map_lookup_elem:
>  		return &bpf_map_lookup_elem_proto;
>  	case BPF_FUNC_map_update_elem:
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 23e2031a43d4..3499a1555bbf 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2268,6 +2268,20 @@ union bpf_attr {
>   *
>   *	Return
>   *		0 on success, or a negative error in case of failure.
> + *
> + * int bpf_rc_pointer_rel(void *ctx, s32 rel_x, s32 rel_y)
> + *	Description
> + *		This helper is used in programs implementing IR decoding, to
> + *		report a successfully decoded pointer movement.
> + *
> + *		The *ctx* should point to the lirc sample as passed into
> + *		the program.
> + *
> + *		This helper is only available is the kernel was compiled with
> + *		the **CONFIG_BPF_LIRC_MODE2** configuration option set to
> + *		"**y**".
> + *	Return
> + *		0
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -2360,7 +2374,8 @@ union bpf_attr {
>  	FN(map_push_elem),		\
>  	FN(map_pop_elem),		\
>  	FN(map_peek_elem),		\
> -	FN(msg_push_data),
> +	FN(msg_push_data),		\
> +	FN(rc_pointer_rel),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 686e57ce40f4..5c900a917fa4 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -168,6 +168,8 @@ static int (*bpf_skb_vlan_push)(void *ctx, __be16 vlan_proto, __u16 vlan_tci) =
>  	(void *) BPF_FUNC_skb_vlan_push;
>  static int (*bpf_skb_vlan_pop)(void *ctx) =
>  	(void *) BPF_FUNC_skb_vlan_pop;
> +static int (*bpf_rc_pointer_rel)(void *ctx, int rel_x, int rel_y) =
> +	(void *) BPF_FUNC_rc_pointer_rel;
>  
>  /* llvm builtin functions that eBPF C program may use to
>   * emit BPF_LD_ABS and BPF_LD_IND instructions
> 

tools/include/uapi/linux/bpf.h would have to be synced as well here.

Probably would be good to extend the lirc2 BPF kselftest as well?

Thanks,
Daniel

^ permalink raw reply

* Re: WARNING in csum_and_copy_to_iter
From: Slavomir Kaslev @ 2018-11-26 11:46 UTC (permalink / raw)
  To: Al Viro
  Cc: syzbot, davem@davemloft.net, gregkh@linuxfoundation.org,
	kgraul@linux.ibm.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, stranche@codeaurora.org,
	syzkaller-bugs@googlegroups.com
In-Reply-To: <20181125015104.GE2217@ZenIV.linux.org.uk>

On Sun, Nov 25, 2018 at 3:52 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sat, Nov 24, 2018 at 09:44:36PM +0000, Al Viro wrote:
>
> > No point, IMO - the fix isn't hard and bisect hazard created by the whole thing
> > is both mild (spurious WARN() in case that used to fail anyway) _and_ won't
> > disappear from reverting, obviously.  I'll post a fix later tonight...
>
> FWIW, I think the following ought to work; it's obviously a pair of commits
> (introduction of convenience helper/switch to its use + csum_and_copy_to_iter()
> for ITER_PIPE), as well as commit message, etc., but I would really appreciate
> if folks gave it a look _and_ a beating.

Tested the patch in qemu, splice reading from udp and vsock sockets (with
https://github.com/skaslev/thru), and it seems to work great.

No warnings or suspicious messages in dmesg with kernel config similar to what
syzbot is using
https://github.com/google/syzkaller/blob/master/docs/linux/kernel_configs.md

> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 7ebccb5c1637..621984743268 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -560,6 +560,44 @@ static size_t copy_pipe_to_iter(const void *addr, size_t bytes,
>         return bytes;
>  }
>
> +static __wsum csum_and_memcpy(void *to, const void *from, size_t len,
> +                             __wsum sum, size_t off)
> +{
> +       __wsum next = csum_partial_copy_nocheck(from, to, len, 0);
> +       return csum_block_add(sum, next, off);
> +}
> +
> +static size_t csum_and_copy_to_pipe_iter(const void *addr, size_t bytes,
> +                               __wsum *csum, struct iov_iter *i)
> +{
> +       struct pipe_inode_info *pipe = i->pipe;
> +       size_t n, r;
> +       size_t off = 0;
> +       __wsum sum = *csum;
> +       int idx;
> +
> +       if (!sanity(i))
> +               return 0;
> +
> +       bytes = n = push_pipe(i, bytes, &idx, &r);
> +       if (unlikely(!n))
> +               return 0;
> +       for ( ; n; idx = next_idx(idx, pipe), r = 0) {
> +               size_t chunk = min_t(size_t, n, PAGE_SIZE - r);
> +               char *p = kmap_atomic(pipe->bufs[idx].page);
> +               sum = csum_and_memcpy(p + r, addr, chunk, sum, off);
> +               kunmap_atomic(p);
> +               i->idx = idx;
> +               i->iov_offset = r + chunk;
> +               n -= chunk;
> +               off += chunk;
> +               addr += chunk;
> +       }
> +       i->count -= bytes;
> +       *csum = sum;
> +       return bytes;
> +}
> +
>  size_t _copy_to_iter(const void *addr, size_t bytes, struct iov_iter *i)
>  {
>         const char *from = addr;
> @@ -1368,17 +1406,15 @@ size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum,
>                 err ? v.iov_len : 0;
>         }), ({
>                 char *p = kmap_atomic(v.bv_page);
> -               next = csum_partial_copy_nocheck(p + v.bv_offset,
> -                                                (to += v.bv_len) - v.bv_len,
> -                                                v.bv_len, 0);
> +               sum = csum_and_memcpy((to += v.bv_len) - v.bv_len,
> +                                     p + v.bv_offset, v.bv_len,
> +                                     sum, off);
>                 kunmap_atomic(p);
> -               sum = csum_block_add(sum, next, off);
>                 off += v.bv_len;
>         }),({
> -               next = csum_partial_copy_nocheck(v.iov_base,
> -                                                (to += v.iov_len) - v.iov_len,
> -                                                v.iov_len, 0);
> -               sum = csum_block_add(sum, next, off);
> +               sum = csum_and_memcpy((to += v.iov_len) - v.iov_len,
> +                                     v.iov_base, v.iov_len,
> +                                     sum, off);
>                 off += v.iov_len;
>         })
>         )
> @@ -1412,17 +1448,15 @@ bool csum_and_copy_from_iter_full(void *addr, size_t bytes, __wsum *csum,
>                 0;
>         }), ({
>                 char *p = kmap_atomic(v.bv_page);
> -               next = csum_partial_copy_nocheck(p + v.bv_offset,
> -                                                (to += v.bv_len) - v.bv_len,
> -                                                v.bv_len, 0);
> +               sum = csum_and_memcpy((to += v.bv_len) - v.bv_len,
> +                                     p + v.bv_offset, v.bv_len,
> +                                     sum, off);
>                 kunmap_atomic(p);
> -               sum = csum_block_add(sum, next, off);
>                 off += v.bv_len;
>         }),({
> -               next = csum_partial_copy_nocheck(v.iov_base,
> -                                                (to += v.iov_len) - v.iov_len,
> -                                                v.iov_len, 0);
> -               sum = csum_block_add(sum, next, off);
> +               sum = csum_and_memcpy((to += v.iov_len) - v.iov_len,
> +                                     v.iov_base, v.iov_len,
> +                                     sum, off);
>                 off += v.iov_len;
>         })
>         )
> @@ -1438,8 +1472,12 @@ size_t csum_and_copy_to_iter(const void *addr, size_t bytes, __wsum *csum,
>         const char *from = addr;
>         __wsum sum, next;
>         size_t off = 0;
> +
> +       if (unlikely(iov_iter_is_pipe(i)))
> +               return csum_and_copy_to_pipe_iter(addr, bytes, csum, i);
> +
>         sum = *csum;
> -       if (unlikely(iov_iter_is_pipe(i) || iov_iter_is_discard(i))) {
> +       if (unlikely(iov_iter_is_discard(i))) {
>                 WARN_ON(1);     /* for now */
>                 return 0;
>         }
> @@ -1455,17 +1493,15 @@ size_t csum_and_copy_to_iter(const void *addr, size_t bytes, __wsum *csum,
>                 err ? v.iov_len : 0;
>         }), ({
>                 char *p = kmap_atomic(v.bv_page);
> -               next = csum_partial_copy_nocheck((from += v.bv_len) - v.bv_len,
> -                                                p + v.bv_offset,
> -                                                v.bv_len, 0);
> +               sum = csum_and_memcpy(p + v.bv_offset,
> +                                     (from += v.bv_len) - v.bv_len,
> +                                     v.bv_len, sum, off);
>                 kunmap_atomic(p);
> -               sum = csum_block_add(sum, next, off);
>                 off += v.bv_len;
>         }),({
> -               next = csum_partial_copy_nocheck((from += v.iov_len) - v.iov_len,
> -                                                v.iov_base,
> -                                                v.iov_len, 0);
> -               sum = csum_block_add(sum, next, off);
> +               sum = csum_and_memcpy(v.iov_base,
> +                                    (from += v.iov_len) - v.iov_len,
> +                                    v.iov_len, sum, off);
>                 off += v.iov_len;
>         })
>         )

^ permalink raw reply

* Re: [PATCH bpf-next 0/3] bpf: add sk_msg helper sk_msg_pop_data
From: Daniel Borkmann @ 2018-11-26  0:45 UTC (permalink / raw)
  To: John Fastabend, ast; +Cc: netdev
In-Reply-To: <1542937130-4860-1-git-send-email-john.fastabend@gmail.com>

On 11/23/2018 02:38 AM, John Fastabend wrote:
> After being able to add metadata to messages with sk_msg_push_data we
> have also found it useful to be able to "pop" this metadata off before
> sending it to applications in some cases. This series adds a new helper
> sk_msg_pop_data() and the associated patches to add tests and tools/lib
> support.
> 
> Thanks!
> 
> John Fastabend (3):
>   bpf: helper to pop data from messages
>   bpf: add msg_pop_data helper to tools
>   bpf: test_sockmap, add options for msg_pop_data() helper usage
> 
>  include/uapi/linux/bpf.h                        |  13 +-
>  net/core/filter.c                               | 169 ++++++++++++++++++++++++
>  net/ipv4/tcp_bpf.c                              |  14 +-
>  tools/include/uapi/linux/bpf.h                  |  13 +-
>  tools/testing/selftests/bpf/bpf_helpers.h       |   2 +
>  tools/testing/selftests/bpf/test_sockmap.c      | 127 +++++++++++++++++-
>  tools/testing/selftests/bpf/test_sockmap_kern.h |  70 ++++++++--
>  7 files changed, 386 insertions(+), 22 deletions(-)
> 

Applied to bpf-next, thanks.

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: align map type names formatting.
From: Daniel Borkmann @ 2018-11-26  0:33 UTC (permalink / raw)
  To: David Calavera, Alexei Starovoitov, netdev
In-Reply-To: <20181123235839.GA8807@c>

On 11/24/2018 12:58 AM, David Calavera wrote:
> Make the formatting for map_type_name array consistent.
> 
> Signed-off-by: David Calavera <david.calavera@gmail.com>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH] tags: Fix DEFINE_PER_CPU expansion
From: Daniel Borkmann @ 2018-11-26  0:29 UTC (permalink / raw)
  To: Rustam Kovhaev, netdev; +Cc: ast
In-Reply-To: <20181123234816.4519-1-rkovhaev@gmail.com>

On 11/24/2018 12:48 AM, Rustam Kovhaev wrote:
> Building tags produces warning:
>  ctags: Warning: kernel/bpf/local_storage.c:10: null expansion of name pattern "\1"
> 
> Let's use the same fix as in commit <25528213fe9f75f4>, even though it
> violates the usual code style.
> 
> Signed-off-by: Rustam Kovhaev <rkovhaev@gmail.com>

Applied to bpf-next, thanks!

^ permalink raw reply

* pull-request: bpf 2018-11-25
From: Daniel Borkmann @ 2018-11-26  0:16 UTC (permalink / raw)
  To: davem; +Cc: daniel, ast, netdev

Hi David,

The following pull-request contains BPF updates for your *net* tree.

The main changes are:

1) Fix an off-by-one bug when adjusting subprog start offsets after
   patching, from Edward.

2) Fix several bugs such as overflow in size allocation in queue /
   stack map creation, from Alexei.

3) Fix wrong IPv6 destination port byte order in bpf_sk_lookup_udp
   helper, from Andrey.

4) Fix several bugs in bpftool such as preventing an infinite loop
   in get_fdinfo, error handling and man page references, from Quentin.

5) Fix a warning in bpf_trace_printk() that wasn't catching an
   invalid format string, from Martynas.

6) Fix a bug in BPF cgroup local storage where non-atomic allocation
   was used in atomic context, from Roman.

7) Fix a NULL pointer dereference bug in bpftool from reallocarray()
   error handling, from Jakub and Wen.

8) Add a copy of pkt_cls.h and tc_bpf.h uapi headers to the tools
   include infrastructure so that bpftool compiles on older RHEL7-like
   user space which does not ship these headers, from Yonghong.

9) Fix BPF kselftests for user space where to get ping test working
   with ping6 and ping -6, from Li.

Please consider pulling these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git

Thanks a lot!

----------------------------------------------------------------

The following changes since commit 85b18b0237ce9986a81a1b9534b5e2ee116f5504:

  net: smsc95xx: Fix MTU range (2018-11-08 19:54:49 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git 

for you to fetch changes up to 1efb6ee3edea57f57f9fb05dba8dcb3f7333f61f:

  bpf: fix check of allowed specifiers in bpf_trace_printk (2018-11-23 21:54:14 +0100)

----------------------------------------------------------------
Alexei Starovoitov (1):
      bpf: fix integer overflow in queue_stack_map

Andrey Ignatov (1):
      bpf: Fix IPv6 dport byte order in bpf_sk_lookup_udp

Edward Cree (1):
      bpf: fix off-by-one error in adjust_subprog_starts

Jakub Kicinski (1):
      tools: bpftool: fix potential NULL pointer dereference in do_load

Li Zhijian (1):
      kselftests/bpf: use ping6 as the default ipv6 ping binary when it exists

Martynas Pumputis (1):
      bpf: fix check of allowed specifiers in bpf_trace_printk

Quentin Monnet (4):
      tools: bpftool: prevent infinite loop in get_fdinfo()
      tools: bpftool: fix plain output and doc for --bpffs option
      tools: bpftool: pass an argument to silence open_obj_pinned()
      tools: bpftool: update references to other man pages in documentation

Roman Gushchin (1):
      bpf: allocate local storage buffers using GFP_ATOMIC

Yonghong Song (1):
      tools/bpftool: copy a few net uapi headers to tools directory

 kernel/bpf/local_storage.c                         |   3 +-
 kernel/bpf/queue_stack_maps.c                      |  16 +-
 kernel/bpf/verifier.c                              |   2 +-
 kernel/trace/bpf_trace.c                           |   8 +-
 net/core/filter.c                                  |   5 +-
 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst |   8 +-
 tools/bpf/bpftool/Documentation/bpftool-map.rst    |   8 +-
 tools/bpf/bpftool/Documentation/bpftool-net.rst    |   8 +-
 tools/bpf/bpftool/Documentation/bpftool-perf.rst   |   8 +-
 tools/bpf/bpftool/Documentation/bpftool-prog.rst   |  11 +-
 tools/bpf/bpftool/Documentation/bpftool.rst        |   9 +-
 tools/bpf/bpftool/common.c                         |  17 +-
 tools/bpf/bpftool/main.h                           |   2 +-
 tools/bpf/bpftool/prog.c                           |  13 +-
 tools/include/uapi/linux/pkt_cls.h                 | 612 +++++++++++++++++++++
 tools/include/uapi/linux/tc_act/tc_bpf.h           |  37 ++
 tools/testing/selftests/bpf/test_netcnt.c          |   5 +-
 tools/testing/selftests/bpf/test_verifier.c        |  19 +
 18 files changed, 752 insertions(+), 39 deletions(-)
 create mode 100644 tools/include/uapi/linux/pkt_cls.h
 create mode 100644 tools/include/uapi/linux/tc_act/tc_bpf.h

^ permalink raw reply

* [PATCH linux-next 10/10] net: ethernet: ti: cpsw: deprecate cpsw-phy-sel driver
From: Grygorii Strashko @ 2018-11-26  0:15 UTC (permalink / raw)
  To: David S. Miller, Tony Lindgren, Kishon Vijay Abraham I,
	Rob Herring
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel, Andrew Lunn,
	devicetree, linux-omap, Grygorii Strashko
In-Reply-To: <20181126001531.12974-1-grygorii.strashko@ti.com>

Deprecate cpsw-phy-sel driver as it's been replaced with new
TI phy-gmii-sel PHY driver.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/Kconfig | 6 +++---
 drivers/net/ethernet/ti/cpsw.h  | 6 ++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index f932923..96415da 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -49,10 +49,11 @@ config TI_DAVINCI_CPDMA
 	  will be called davinci_cpdma.  This is recommended.
 
 config TI_CPSW_PHY_SEL
-	bool
+	bool "TI CPSW Phy mode Selection (DEPRECATED)"
+	default n
 	---help---
 	  This driver supports configuring of the phy mode connected to
-	  the CPSW.
+	  the CPSW. DEPRECATED: use PHY_TI_GMII_SEL.
 
 config TI_CPSW_ALE
 	tristate "TI CPSW ALE Support"
@@ -64,7 +65,6 @@ config TI_CPSW
 	depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
 	select TI_DAVINCI_CPDMA
 	select TI_DAVINCI_MDIO
-	select TI_CPSW_PHY_SEL
 	select TI_CPSW_ALE
 	select MFD_SYSCON
 	select REGMAP
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index cf111db..907e05fc 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -21,7 +21,13 @@
 			 ((mac)[2] << 16) | ((mac)[3] << 24))
 #define mac_lo(mac)	(((mac)[4] << 0) | ((mac)[5] << 8))
 
+#if IS_ENABLED(CONFIG_TI_CPSW_PHY_SEL)
 void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave);
+#else
+static inline
+void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave)
+{}
+#endif
 int ti_cm_get_macid(struct device *dev, int slave, u8 *mac_addr);
 
 #endif /* __CPSW_H__ */
-- 
2.10.5

^ permalink raw reply related

* [PATCH linux-next 08/10] ARM: dts: am335x: switch to use phy-gmii-sel
From: Grygorii Strashko @ 2018-11-26  0:15 UTC (permalink / raw)
  To: David S. Miller, Tony Lindgren, Kishon Vijay Abraham I,
	Rob Herring
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel, Andrew Lunn,
	devicetree, linux-omap, Grygorii Strashko
In-Reply-To: <20181126001531.12974-1-grygorii.strashko@ti.com>

Switch to use phy-gmii-sel PHY instead of cpsw-phy-sel.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/am335x-baltos-ir2110.dts        |  4 ----
 arch/arm/boot/dts/am335x-baltos-ir3220.dts        |  4 ----
 arch/arm/boot/dts/am335x-baltos-ir5221.dts        |  4 ----
 arch/arm/boot/dts/am335x-chiliboard.dts           |  4 ----
 arch/arm/boot/dts/am335x-icev2.dts                |  4 ----
 arch/arm/boot/dts/am335x-igep0033.dtsi            |  4 ----
 arch/arm/boot/dts/am335x-lxm.dts                  |  4 ----
 arch/arm/boot/dts/am335x-moxa-uc-2100-common.dtsi |  5 -----
 arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts    |  5 -----
 arch/arm/boot/dts/am335x-phycore-som.dtsi         |  4 ----
 arch/arm/boot/dts/am33xx-l4.dtsi                  | 15 ++++++++-------
 11 files changed, 8 insertions(+), 49 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-baltos-ir2110.dts b/arch/arm/boot/dts/am335x-baltos-ir2110.dts
index 75de1e7..50dcf12 100644
--- a/arch/arm/boot/dts/am335x-baltos-ir2110.dts
+++ b/arch/arm/boot/dts/am335x-baltos-ir2110.dts
@@ -72,7 +72,3 @@
 	dual_emac_res_vlan = <2>;
 	phy-handle = <&phy1>;
 };
-
-&phy_sel {
-	rmii-clock-ext = <1>;
-};
diff --git a/arch/arm/boot/dts/am335x-baltos-ir3220.dts b/arch/arm/boot/dts/am335x-baltos-ir3220.dts
index 1b215c4..f3f1abd 100644
--- a/arch/arm/boot/dts/am335x-baltos-ir3220.dts
+++ b/arch/arm/boot/dts/am335x-baltos-ir3220.dts
@@ -114,7 +114,3 @@
 	dual_emac_res_vlan = <2>;
 	phy-handle = <&phy1>;
 };
-
-&phy_sel {
-	rmii-clock-ext = <1>;
-};
diff --git a/arch/arm/boot/dts/am335x-baltos-ir5221.dts b/arch/arm/boot/dts/am335x-baltos-ir5221.dts
index 832ead8..42f473f 100644
--- a/arch/arm/boot/dts/am335x-baltos-ir5221.dts
+++ b/arch/arm/boot/dts/am335x-baltos-ir5221.dts
@@ -133,10 +133,6 @@
 	phy-handle = <&phy1>;
 };
 
-&phy_sel {
-	rmii-clock-ext = <1>;
-};
-
 &dcan1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&dcan1_pins>;
diff --git a/arch/arm/boot/dts/am335x-chiliboard.dts b/arch/arm/boot/dts/am335x-chiliboard.dts
index 9c2a947..4a86953 100644
--- a/arch/arm/boot/dts/am335x-chiliboard.dts
+++ b/arch/arm/boot/dts/am335x-chiliboard.dts
@@ -151,10 +151,6 @@
 	phy-mode = "rmii";
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
-
 /* USB */
 &usb {
 	status = "okay";
diff --git a/arch/arm/boot/dts/am335x-icev2.dts b/arch/arm/boot/dts/am335x-icev2.dts
index f2005ec..9ac775c 100644
--- a/arch/arm/boot/dts/am335x-icev2.dts
+++ b/arch/arm/boot/dts/am335x-icev2.dts
@@ -484,10 +484,6 @@
 	dual_emac;
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
-
 &davinci_mdio {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&davinci_mdio_default>;
diff --git a/arch/arm/boot/dts/am335x-igep0033.dtsi b/arch/arm/boot/dts/am335x-igep0033.dtsi
index 55b4c94..cbd22f2 100644
--- a/arch/arm/boot/dts/am335x-igep0033.dtsi
+++ b/arch/arm/boot/dts/am335x-igep0033.dtsi
@@ -123,10 +123,6 @@
 	phy-mode = "rmii";
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
-
 &elm {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/am335x-lxm.dts b/arch/arm/boot/dts/am335x-lxm.dts
index 481edcf..d0e8e72 100644
--- a/arch/arm/boot/dts/am335x-lxm.dts
+++ b/arch/arm/boot/dts/am335x-lxm.dts
@@ -328,10 +328,6 @@
 	dual_emac_res_vlan = <3>;
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
-
 &mac {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&cpsw_default>;
diff --git a/arch/arm/boot/dts/am335x-moxa-uc-2100-common.dtsi b/arch/arm/boot/dts/am335x-moxa-uc-2100-common.dtsi
index 14f7819..cb5913a 100644
--- a/arch/arm/boot/dts/am335x-moxa-uc-2100-common.dtsi
+++ b/arch/arm/boot/dts/am335x-moxa-uc-2100-common.dtsi
@@ -159,11 +159,6 @@
 	status = "okay";
 };
 
-&phy_sel {
-	reg= <0x44e10650 0xf5>;
-	rmii-clock-ext;
-};
-
 &sham {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts b/arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts
index 5a58efc..e562ce4 100644
--- a/arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts
+++ b/arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts
@@ -446,11 +446,6 @@
 	dual_emac_res_vlan = <2>;
 };
 
-&phy_sel {
-	reg= <0x44e10650 0xf5>;
-	rmii-clock-ext;
-};
-
 &sham {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/am335x-phycore-som.dtsi b/arch/arm/boot/dts/am335x-phycore-som.dtsi
index 428a25e..015adb6 100644
--- a/arch/arm/boot/dts/am335x-phycore-som.dtsi
+++ b/arch/arm/boot/dts/am335x-phycore-som.dtsi
@@ -100,10 +100,6 @@
 	status = "okay";
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
-
 /* I2C Busses */
 &am33xx_pinmux {
 	i2c0_pins: pinmux_i2c0 {
diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
index 918bf57..d9aacb5 100644
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -279,12 +279,6 @@
 				#pinctrl-cells = <1>;
 				ranges = <0 0 0x2000>;
 
-				phy_sel: cpsw-phy-sel@650 {
-					compatible = "ti,am3352-cpsw-phy-sel";
-					reg= <0x650 0x4>;
-					reg-names = "gmii-sel";
-				};
-
 				am33xx_pinmux: pinmux@800 {
 					compatible = "pinctrl-single";
 					reg = <0x800 0x238>;
@@ -302,6 +296,12 @@
 					#size-cells = <1>;
 					ranges = <0 0 0x800>;
 
+					phy_gmii_sel: phy-gmii-sel {
+						compatible = "ti,am3352-phy-gmii-sel";
+						reg = <0x650 0x4>;
+						#phy-cells = <2>;
+					};
+
 					scm_clocks: clocks {
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -717,7 +717,6 @@
 				interrupts = <40 41 42 43>;
 				ranges = <0 0 0x8000>;
 				syscon = <&scm_conf>;
-				cpsw-phy-sel = <&phy_sel>;
 				status = "disabled";
 
 				davinci_mdio: mdio@1000 {
@@ -733,11 +732,13 @@
 				cpsw_emac0: slave@200 {
 					/* Filled in by U-Boot */
 					mac-address = [ 00 00 00 00 00 00 ];
+					phys = <&phy_gmii_sel 1 1>;
 				};
 
 				cpsw_emac1: slave@300 {
 					/* Filled in by U-Boot */
 					mac-address = [ 00 00 00 00 00 00 ];
+					phys = <&phy_gmii_sel 2 1>;
 				};
 			};
 		};
-- 
2.10.5

^ permalink raw reply related

* Re: [PATCH -next] pps: using ERR_PTR instead of NULL while pps_register_source fails
From: Rodolfo Giometti @ 2018-11-26 10:43 UTC (permalink / raw)
  To: YueHaibing, davem, richardcochran, sudipm.mukherjee, gregkh
  Cc: linux-kernel, netdev
In-Reply-To: <20181126102422.24028-1-yuehaibing@huawei.com>

On 26/11/2018 11:24, YueHaibing wrote:
> pps_register_source() has keeps error codes in a local variable,
> but it does not make use of the code. This patch let it return
> the errcode in case of failure.
> 
> Suggested-by: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply

* [PATCH v2 net-next] net: remove unsafe skb_insert()
From: Eric Dumazet @ 2018-11-25 23:49 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Eric Dumazet, Faisal Latif, Doug Ledford,
	Jason Gunthorpe, linux-rdma

I do not see how one can effectively use skb_insert() without holding
some kind of lock. Otherwise other cpus could have changed the list
right before we have a chance of acquiring list->lock.

Only existing user is in drivers/infiniband/hw/nes/nes_mgt.c and this
one probably meant to use __skb_insert() since it appears nesqp->pau_list
is protected by nesqp->pau_lock. This looks like nesqp->pau_lock
could be removed, since nesqp->pau_list.lock could be used instead.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Faisal Latif <faisal.latif@intel.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma <linux-rdma@vger.kernel.org>
---
 drivers/infiniband/hw/nes/nes_mgt.c |  4 ++--
 include/linux/skbuff.h              |  2 --
 net/core/skbuff.c                   | 22 ----------------------
 3 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_mgt.c b/drivers/infiniband/hw/nes/nes_mgt.c
index fc0c191014e908eea32d752f3499295ef143aa0a..cc4dce5c3e5f6d99fc44fcde7334e70ac7a33002 100644
--- a/drivers/infiniband/hw/nes/nes_mgt.c
+++ b/drivers/infiniband/hw/nes/nes_mgt.c
@@ -551,14 +551,14 @@ static void queue_fpdus(struct sk_buff *skb, struct nes_vnic *nesvnic, struct ne
 
 	/* Queue skb by sequence number */
 	if (skb_queue_len(&nesqp->pau_list) == 0) {
-		skb_queue_head(&nesqp->pau_list, skb);
+		__skb_queue_head(&nesqp->pau_list, skb);
 	} else {
 		skb_queue_walk(&nesqp->pau_list, tmpskb) {
 			cb = (struct nes_rskb_cb *)&tmpskb->cb[0];
 			if (before(seqnum, cb->seqnum))
 				break;
 		}
-		skb_insert(tmpskb, skb, &nesqp->pau_list);
+		__skb_insert(skb, tmpskb->prev, tmpskb, &nesqp->pau_list);
 	}
 	if (nesqp->pau_state == PAU_READY)
 		process_it = true;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f17a7452ac7bf47ef4bcf89840bba165cee6f50a..73902acf2b71c8800d81b744a936a7420f33b459 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1749,8 +1749,6 @@ static inline void skb_queue_head_init_class(struct sk_buff_head *list,
  *	The "__skb_xxxx()" functions are the non-atomic ones that
  *	can only be called with interrupts disabled.
  */
-void skb_insert(struct sk_buff *old, struct sk_buff *newsk,
-		struct sk_buff_head *list);
 static inline void __skb_insert(struct sk_buff *newsk,
 				struct sk_buff *prev, struct sk_buff *next,
 				struct sk_buff_head *list)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9a8a72cefe9b94d3821b9cc5ba5bba647ae51267..02cd7ae3d0fb26ef0a8b006390154fdefd0d292f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2990,28 +2990,6 @@ void skb_append(struct sk_buff *old, struct sk_buff *newsk, struct sk_buff_head
 }
 EXPORT_SYMBOL(skb_append);
 
-/**
- *	skb_insert	-	insert a buffer
- *	@old: buffer to insert before
- *	@newsk: buffer to insert
- *	@list: list to use
- *
- *	Place a packet before a given packet in a list. The list locks are
- * 	taken and this function is atomic with respect to other list locked
- *	calls.
- *
- *	A buffer cannot be placed on two lists at the same time.
- */
-void skb_insert(struct sk_buff *old, struct sk_buff *newsk, struct sk_buff_head *list)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&list->lock, flags);
-	__skb_insert(newsk, old->prev, old, list);
-	spin_unlock_irqrestore(&list->lock, flags);
-}
-EXPORT_SYMBOL(skb_insert);
-
 static inline void skb_split_inside_header(struct sk_buff *skb,
 					   struct sk_buff* skb1,
 					   const u32 len, const int pos)
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

^ permalink raw reply related

* Re: [PATCH net-next] net: remove unsafe skb_insert()
From: Eric Dumazet @ 2018-11-25 23:37 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Eric Dumazet, faisal.latif, Doug Ledford, jgg, linux-rdma
In-Reply-To: <20181125.102936.907000798531091562.davem@davemloft.net>

On Sun, Nov 25, 2018 at 10:29 AM David Miller <davem@davemloft.net> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
> Date: Sun, 25 Nov 2018 08:26:23 -0800
>
> > I do not see how one can effectively use skb_insert() without holding
> > some kind of lock. Otherwise other cpus could have changed the list
> > right before we have a chance of acquiring list->lock.
> >
> > Only existing user is in drivers/infiniband/hw/nes/nes_mgt.c and this
> > one probably meant to use __skb_insert() since it appears nesqp->pau_list
> > is protected by nesqp->pau_lock. This looks like nesqp->pau_lock
> > could be removed, since nesqp->pau_list.lock could be used instead.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Good find.
>
> Indeed, any of the queue SKB manipulation functions that take two SKBs
> as an argument are suspect in this manner.
>
> Applied, thanks Eric.

Oh well, this does not build.

Since you have not pushed your tree yet, maybe we can replace this
with a version that actually compiles.

Please let me know if a relative patch or a v2 is needed, thanks.

^ permalink raw reply

* [PATCH] bpf: btf: fix spelling mistake "Memmber" -> "Member"
From: Colin King @ 2018-11-25 23:32 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, netdev; +Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

There is a spelling mistake in a btf_verifier_log_member message,
fix it.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 kernel/bpf/btf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 69da9169819a..a09b2f94ab25 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1621,7 +1621,7 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
 
 		if (BITS_ROUNDUP_BYTES(member->offset) > struct_size) {
 			btf_verifier_log_member(env, t, member,
-						"Memmber bits_offset exceeds its struct size");
+						"Member bits_offset exceeds its struct size");
 			return -EINVAL;
 		}
 
-- 
2.19.1

^ permalink raw reply related


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