Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH V11 4/5] vsprintf: add printk specifier %px
From: Sergey Senozhatsky @ 2017-12-06  1:36 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Randy Dunlap, David Laight, Kees Cook, Tobin C. Harding,
	kernel-hardening@lists.openwall.com, Jason A. Donenfeld,
	Theodore Ts'o, Paolo Bonzini, Tycho Andersen,
	Roberts, William C, Tejun Heo, Jordan Glover, Greg KH,
	Petr Mladek, Joe Perches, Ian Campbell, Sergey Senozhatsky,
	Catalin Marinas, Will Deacon
In-Reply-To: <CA+55aFw3AZtzf6KHS4desHJ3CajKs46VQxOi6-+0U8A4y0v6ug@mail.gmail.com>

Hello,

On (12/05/17 13:22), Linus Torvalds wrote:
[..]
> It's not like those hex numbers were really helping people anyway.
> We've turned off most of them on x86 oops reports long ago (and
> entirely independently of the pointer hashing). Having stared at a lot
> of oopses in my time, the only hex numbers that tend to be really
> relevant are (a) the register contents (which aren't %p anyway), and
> things like the faulting address (which is not, and never has been, %p
> on x86, but might be on some other architecture).

I see some %p-s being used in _supposedly_ important output,
like arch/x86/mm/fault.c

show_fault_oops(struct pt_regs *regs, unsigned long error_code,
		unsigned long address)
...
	printk(KERN_CONT " at %p\n", (void *) address);
	printk(KERN_ALERT "IP: %pS\n", (void *)regs->ip);


a quick %p grep gives me the following list:

arch/arm/mm/fault.c:    pr_alert("pgd = %p\n", mm->pgd);
arch/arm64/mm/fault.c:  pr_alert("%s pgtable: %luk pages, %u-bit VAs, pgd = %p\n",
arch/arm64/mm/fault.c:          pr_info_ratelimited("%s[%d]: %s exception: pc=%p sp=%p\n",
arch/m68k/mm/fault.c:   pr_debug("send_fault_sig: %p,%d,%d\n", siginfo.si_addr,
arch/m68k/mm/fault.c:           pr_cont(" at virtual address %p\n", siginfo.si_addr);
arch/m68k/mm/fault.c:   pr_debug("do page fault:\nregs->sr=%#x, regs->pc=%#lx, address=%#lx, %ld, %p\n",
arch/microblaze/mm/fault.c:             pr_emerg("Page fault in user mode with faulthandler_disabled(), mm = %p\n",
arch/mn10300/mm/fault.c:        printk(KERN_DEBUG "pgd entry %p: %016Lx\n",
arch/mn10300/mm/fault.c:        printk(KERN_DEBUG "pmd entry %p: %016Lx\n",
arch/mn10300/mm/fault.c:        printk(KERN_DEBUG "pte entry %p: %016Lx\n",
arch/mn10300/mm/fault.c:        printk(KERN_DEBUG "--- do_page_fault(%p,%s:%04lx,%08lx)\n",
arch/powerpc/mm/fault.c:                                           " mm=%p\n",
arch/sh/mm/fault.c:     printk(KERN_ALERT "pgd = %p\n", pgd);
arch/unicore32/mm/fault.c:      printk(KERN_ALERT "pgd = %p\n", mm->pgd);
arch/x86/mm/fault.c:    printk(KERN_CONT " at %p\n", (void *) address);
arch/x86/mm/fault.c:    printk(KERN_ALERT "IP: %pS\n", (void *)regs->ip);
arch/x86/mm/fault.c:    printk("%s%s[%d]: segfault at %lx ip %p sp %p error %lx",


or is it OK to show hashes instead of pgd or pmd pointers?

	-ss

^ permalink raw reply

* Re: [PATCH iproute2] iproute2: Fix undeclared __kernel_long_t type build error in RHEL 6.8
From: Stephen Hemminger @ 2017-12-06  1:33 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: netdev, Leon Romanovsky, Riad Abo Raed, Guy Ergas
In-Reply-To: <20171201110451.25392-1-leon@kernel.org>

On Fri,  1 Dec 2017 13:04:51 +0200
Leon Romanovsky <leon@kernel.org> wrote:

> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Add asm/posix_types.h header file to the list of needed includes,
> because the headers files in RHEL 6.8 are too old and doesn't
> have declaration of __kernel_long_t.
> 
> In file included from ../include/uapi/linux/kernel.h:5,
>                  from ../include/uapi/linux/netfilter/x_tables.h:4,
>                  from ../include/xtables.h:20,
>                  from em_ipset.c:26:
> ../include/uapi/linux/sysinfo.h:9: error: expected specifier-qualifier-list before ‘__kernel_long_t’
> 
> Cc: Riad Abo Raed <riada@mellanox.com>
> Cc: Guy Ergas <guye@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> Stephen,
> I don't know how to properly solve this type of errors and would like to
> hear your guidance on it.
> 
> Should I simply add kernel file? Or maybe I need to add HAVE_xxx checks
> to configure script to check __kernel_long_t existence and declare only
> this type?
> 
> I also have another build error on RHEL 6.8 system and looking for a
> solution.
> 
> In file included from em_ipset.c:26:
> ../include/xtables.h:35:29: error: xtables-version.h: No such file or directory
> make[1]: *** [em_ipset.o] Error 1
> 
> The iptables-devel is iptables-devel-1.4.7-16.el6.x86_64 so check_xt()
> success, but RH headers don't have xtable-version.h and the relevant defines
> are embedded in the main xtables.h header file.
> 
> Thanks
> ---
>  include/uapi/asm/posix_types.h | 97 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 include/uapi/asm/posix_types.h
> 
> diff --git a/include/uapi/asm/posix_types.h b/include/uapi/asm/posix_types.h
> new file mode 100644
> index 00000000..5e6ea22b
> --- /dev/null
> +++ b/include/uapi/asm/posix_types.h
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef __ASM_GENERIC_POSIX_TYPES_H
> +#define __ASM_GENERIC_POSIX_TYPES_H
> +
> +#include <asm/bitsperlong.h>
> +/*
> + * This file is generally used by user-level software, so you need to
> + * be a little careful about namespace pollution etc.
> + *
> + * First the types that are often defined in different ways across
> + * architectures, so that you can override them.
> + */
> +
> +#ifndef __kernel_long_t
> +typedef long		__kernel_long_t;
> +typedef unsigned long	__kernel_ulong_t;
> +#endif
> +
> +#ifndef __kernel_ino_t
> +typedef __kernel_ulong_t __kernel_ino_t;
> +#endif
> +
> +#ifndef __kernel_mode_t
> +typedef unsigned int	__kernel_mode_t;
> +#endif
> +
> +#ifndef __kernel_pid_t
> +typedef int		__kernel_pid_t;
> +#endif
> +
> +#ifndef __kernel_ipc_pid_t
> +typedef int		__kernel_ipc_pid_t;
> +#endif
> +
> +#ifndef __kernel_uid_t
> +typedef unsigned int	__kernel_uid_t;
> +typedef unsigned int	__kernel_gid_t;
> +#endif
> +
> +#ifndef __kernel_suseconds_t
> +typedef __kernel_long_t		__kernel_suseconds_t;
> +#endif
> +
> +#ifndef __kernel_daddr_t
> +typedef int		__kernel_daddr_t;
> +#endif
> +
> +#ifndef __kernel_uid32_t
> +typedef unsigned int	__kernel_uid32_t;
> +typedef unsigned int	__kernel_gid32_t;
> +#endif
> +
> +#ifndef __kernel_old_uid_t
> +typedef __kernel_uid_t	__kernel_old_uid_t;
> +typedef __kernel_gid_t	__kernel_old_gid_t;
> +#endif
> +
> +#ifndef __kernel_old_dev_t
> +typedef unsigned int	__kernel_old_dev_t;
> +#endif
> +
> +/*
> + * Most 32 bit architectures use "unsigned int" size_t,
> + * and all 64 bit architectures use "unsigned long" size_t.
> + */
> +#ifndef __kernel_size_t
> +#if __BITS_PER_LONG != 64
> +typedef unsigned int	__kernel_size_t;
> +typedef int		__kernel_ssize_t;
> +typedef int		__kernel_ptrdiff_t;
> +#else
> +typedef __kernel_ulong_t __kernel_size_t;
> +typedef __kernel_long_t	__kernel_ssize_t;
> +typedef __kernel_long_t	__kernel_ptrdiff_t;
> +#endif
> +#endif
> +
> +#ifndef __kernel_fsid_t
> +typedef struct {
> +	int	val[2];
> +} __kernel_fsid_t;
> +#endif
> +
> +/*
> + * anything below here should be completely generic
> + */
> +typedef __kernel_long_t	__kernel_off_t;
> +typedef long long	__kernel_loff_t;
> +typedef __kernel_long_t	__kernel_time_t;
> +typedef __kernel_long_t	__kernel_clock_t;
> +typedef int		__kernel_timer_t;
> +typedef int		__kernel_clockid_t;
> +typedef char *		__kernel_caddr_t;
> +typedef unsigned short	__kernel_uid16_t;
> +typedef unsigned short	__kernel_gid16_t;
> +
> +#endif /* __ASM_GENERIC_POSIX_TYPES_H */
> --
> 2.15.1
> 

This isn't going to be supportable.  The headers in uapi are updated by
a script from kernel, and this version posix_types.h conflicts with what
the kernel creates by make headers_install.

^ permalink raw reply

* Re: [PATCH 2/2] veth: allow configuring GSO maximums
From: Stephen Hemminger @ 2017-12-06  1:25 UTC (permalink / raw)
  To: Solio Sarabia
  Cc: netdev, davem, eric.dumazet, dsahern, kys, shiny.sebastian,
	Stephen Hemminger
In-Reply-To: <1512522866-5544-3-git-send-email-solio.sarabia@intel.com>

On Tue,  5 Dec 2017 17:14:26 -0800
Solio Sarabia <solio.sarabia@intel.com> wrote:

> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> Veth's can be used in environments (like Azure) where the underlying
> network device is impacted by large GSO packets. This patch allows
> gso maximum values to be passed in when creating the device via
> netlink.
> 
> In theory, other pseudo devices could also use netlink attributes
> to set GSO maximums but for now veth is what has been observed
> to be an issue.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> Signed-off-by: Solio Sarabia <solio.sarabia@intel.com>

I am testing new version with changelink support

^ permalink raw reply

* [PATCH 2/2] veth: allow configuring GSO maximums
From: Solio Sarabia @ 2017-12-06  1:14 UTC (permalink / raw)
  To: netdev, davem, stephen
  Cc: eric.dumazet, dsahern, kys, shiny.sebastian, solio.sarabia,
	Stephen Hemminger
In-Reply-To: <1512522866-5544-1-git-send-email-solio.sarabia@intel.com>

From: Stephen Hemminger <stephen@networkplumber.org>

Veth's can be used in environments (like Azure) where the underlying
network device is impacted by large GSO packets. This patch allows
gso maximum values to be passed in when creating the device via
netlink.

In theory, other pseudo devices could also use netlink attributes
to set GSO maximums but for now veth is what has been observed
to be an issue.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Solio Sarabia <solio.sarabia@intel.com>
---
 drivers/net/veth.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index f5438d0..510c058 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -410,6 +410,26 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 	if (ifmp && (dev->ifindex != 0))
 		peer->ifindex = ifmp->ifi_index;
 
+	if (tbp[IFLA_GSO_MAX_SIZE]) {
+		u32 max_size = nla_get_u32(tbp[IFLA_GSO_MAX_SIZE]);
+
+		if (max_size > GSO_MAX_SIZE)
+			return -EINVAL;
+
+		peer->gso_max_size = max_size;
+		dev->gso_max_size = max_size;
+	}
+
+	if (tbp[IFLA_GSO_MAX_SEGS]) {
+		u32 max_segs = nla_get_u32(tbp[IFLA_GSO_MAX_SEGS]);
+
+		if (max_segs > GSO_MAX_SEGS)
+			return -EINVAL;
+
+		peer->gso_max_segs = max_segs;
+		dev->gso_max_segs = max_segs;
+	}
+
 	err = register_netdevice(peer);
 	put_net(net);
 	net = NULL;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] rtnetlink: allow GSO maximums to be passed to device
From: Solio Sarabia @ 2017-12-06  1:14 UTC (permalink / raw)
  To: netdev, davem, stephen
  Cc: eric.dumazet, dsahern, kys, shiny.sebastian, solio.sarabia,
	Stephen Hemminger
In-Reply-To: <1512522866-5544-1-git-send-email-solio.sarabia@intel.com>

From: Stephen Hemminger <stephen@networkplumber.org>

Allow GSO maximum segments and size as netlink parameters on input,
with 'ip link add' utils for example. Allow also updating these
attributes after rtnetlink devices are created.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Solio Sarabia <solio.sarabia@intel.com>
---
 net/core/rtnetlink.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a4faefd..a1ff2a8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1637,6 +1637,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_PROMISCUITY]	= { .type = NLA_U32 },
 	[IFLA_NUM_TX_QUEUES]	= { .type = NLA_U32 },
 	[IFLA_NUM_RX_QUEUES]	= { .type = NLA_U32 },
+	[IFLA_GSO_MAX_SEGS]	= { .type = NLA_U32 },
+	[IFLA_GSO_MAX_SIZE]	= { .type = NLA_U32 },
 	[IFLA_PHYS_PORT_ID]	= { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN },
 	[IFLA_CARRIER_CHANGES]	= { .type = NLA_U32 },  /* ignored */
 	[IFLA_PHYS_SWITCH_ID]	= { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN },
@@ -2287,6 +2289,34 @@ static int do_setlink(const struct sk_buff *skb,
 		}
 	}
 
+	if (tb[IFLA_GSO_MAX_SIZE]) {
+		u32 max_size = nla_get_u32(tb[IFLA_GSO_MAX_SIZE]);
+
+		if (max_size > GSO_MAX_SIZE) {
+			err = -EINVAL;
+			goto errout;
+		}
+
+		if (dev->gso_max_size ^ max_size) {
+			netif_set_gso_max_size(dev, max_size);
+			status |= DO_SETLINK_MODIFIED;
+		}
+	}
+
+	if (tb[IFLA_GSO_MAX_SEGS]) {
+		u32 max_segs = nla_get_u32(tb[IFLA_GSO_MAX_SEGS]);
+
+		if (max_segs > GSO_MAX_SEGS) {
+			err = -EINVAL;
+			goto errout;
+		}
+
+		if (dev->gso_max_segs ^ max_segs) {
+			dev->gso_max_segs = max_segs;
+			status |= DO_SETLINK_MODIFIED;
+		}
+	}
+
 	if (tb[IFLA_OPERSTATE])
 		set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/2] Allow changing device gso maximums
From: Solio Sarabia @ 2017-12-06  1:14 UTC (permalink / raw)
  To: netdev, davem, stephen
  Cc: eric.dumazet, dsahern, kys, shiny.sebastian, solio.sarabia

Docker uses bridge/veth for its bridged network. veth sends tcp packets
as big as 65536 (its default gso value), even when lower physical or
synthetic devices expose a lower limit. This causes tcp fragmentation in
the host, spinning more cpu cycles.

The proposed solution is to allow user to tune gso settings, via iproute
utils for example. Note: this enables changing gso for all interfaces,
not limited to veth only.

This series rebases Stephen's original patches [1]. It also fixes a
minor issue when validating maximum gso_max_size, which can be in the
range [0,65536]. Changes are validated with and without docker use
cases.

[1] https://marc.info/?l=linux-netdev&m=151217101428494&w=2

Stephen Hemminger (2):
  rtnetlink: allow GSO maximums to be passed to device
  veth: allow configuring GSO maximums

 drivers/net/veth.c   | 20 ++++++++++++++++++++
 net/core/rtnetlink.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH v2 0/3] macb rx filter cleanups
From: David Miller @ 2017-12-06  1:08 UTC (permalink / raw)
  To: julia; +Cc: julia.lawall, rafalo, nicolas.ferre, netdev, linux-kernel,
	kbuild-all
In-Reply-To: <cover.1512518311.git.julia@ni.com>

From: Julia Cartwright <julia@ni.com>
Date: Tue, 5 Dec 2017 18:02:47 -0600

> Here's a proper patchset based on net-next.
> 
> v1 -> v2:
>   - Rebased on net-next
>   - Add Nicolas's Acks
>   - Reorder commits, putting the list_empty() cleanups prior to the
>     others.
>   - Added commit reverting the GFP_ATOMIC change.

Thanks for following up so quickly.

Series applied to net-next, thanks.

^ permalink raw reply

* Re: [PATCH v2 iproute2 net-next] gre6: add collect metadata support
From: Stephen Hemminger @ 2017-12-06  1:07 UTC (permalink / raw)
  To: William Tu; +Cc: netdev
In-Reply-To: <1512515437-20922-1-git-send-email-u9012063@gmail.com>

On Tue,  5 Dec 2017 15:10:37 -0800
William Tu <u9012063@gmail.com> wrote:

> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index a6a10e577b1f..eb04f887c940 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> @@ -755,6 +755,8 @@ the following additional arguments are supported:
>  .BI "dscp inherit"
>  ] [
>  .BI dev " PHYS_DEV "
> +] [
> +.RB external
>  ]
>  
>  .in +8
> @@ -833,6 +835,10 @@ or
>  .IR 00 ".." ff
>  when tunneling non-IP packets. The default value is 00.
>  
> +.sp
> +.RB external
> +- make this tunnel externally controlled (or not, which is the default).
> +
>  .in -8

I don't have any  direct involvement in offload, so would like some feedback
from others that are.

Not a big fan of opaque "metadata" what exactly does it mean?
Also "external" is already used to mean something else on other parts of
the link command. Also the option, and the value in JSON should be the same.

Please reconsider the naming and resubmit

The wording in the man page here could be better

^ permalink raw reply

* Re: [PATCH iproute2] iproute2: Fix undeclared __kernel_long_t type build error in RHEL 6.8
From: Stephen Hemminger @ 2017-12-06  0:59 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: netdev, Riad Abo Raed, Guy Ergas
In-Reply-To: <20171202082833.GF8126@mtr-leonro.local>

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

On Sat, 2 Dec 2017 10:28:33 +0200
Leon Romanovsky <leon@kernel.org> wrote:

> On Fri, Dec 01, 2017 at 08:48:07AM -0800, Stephen Hemminger wrote:
> > On Fri,  1 Dec 2017 13:04:51 +0200
> > Leon Romanovsky <leon@kernel.org> wrote:
> >  
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > >
> > > Add asm/posix_types.h header file to the list of needed includes,
> > > because the headers files in RHEL 6.8 are too old and doesn't
> > > have declaration of __kernel_long_t.
> > >
> > > In file included from ../include/uapi/linux/kernel.h:5,
> > >                  from ../include/uapi/linux/netfilter/x_tables.h:4,
> > >                  from ../include/xtables.h:20,
> > >                  from em_ipset.c:26:
> > > ../include/uapi/linux/sysinfo.h:9: error: expected specifier-qualifier-list before ‘__kernel_long_t’
> > >
> > > Cc: Riad Abo Raed <riada@mellanox.com>
> > > Cc: Guy Ergas <guye@mellanox.com>
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>  
> >
> > I see the problem, but the solution of dragging in posix_types.h
> > would be too much of a long term maintenance issue.
> > All the headers in uapi are regularly generated from upstream
> > kernel headers; I don't want to start making exceptions.
> >
> > Is it just the xtables stuff (which has always been problematic)?  
> 
> Yes, both failures are related to xtables. And this wass my naive approach to
> solve first one, the second mentioned in the original commit log
> (missing xtables-version.h) is more harder to fix.
> 
> Will it work if I test in configure script the existence of __kernel_long_t
> and fallback to xt-internal.h?
> 
> Thanks

Why not just modify the part of the configure script that checks if xtables build will
work. It should fail if header files won't work.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 3/3] ethtool: Add ETHTOOL_RESET support via --reset command
From: Scott Branden @ 2017-12-06  0:45 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: John W. Linville, BCM Kernel Feedback, Steve Lin, Michael Chan,
	netdev, Paul Greenwalt, Stephen Hemminger
In-Reply-To: <20171205222920.3szmrvrhbu72wpf2@unicorn.suse.cz>

Hi Michal,

Thanks - one question below hopefully someone can help with.


On 17-12-05 02:29 PM, Michal Kubecek wrote:
> On Tue, Dec 05, 2017 at 02:06:09PM -0800, Scott Branden wrote:
>> On 17-12-05 01:30 PM, Michal Kubecek wrote:
>>> On Tue, Dec 05, 2017 at 12:53:23PM -0800, Scott Branden wrote:
>>>> Add ETHTOOL_RESET support via --reset command.
>>>>
>>>> ie.  ethtool --reset DEVNAME <flagname(s)>
>>>>
>>>> flagnames currently match the ETH_RESET_xxx names:
>>>> mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
>>>>
>>>> Alternatively, you can specific component bitfield directly using
>>>> ethtool --reset DEVNAME flags %x
>>> IMHO it would be more consistent with e.g. msglvl without the keyword
>>> "flags".
>> I don't see the consistency in ethtool of specifying a number without a
>> keyword in front of it.
>> I can only find --set-dump specify a number?
>> Others have keyword and number.  msglvl is the keyword after specifying -s -
>> same as flags is the keyword I use after specifying --reset.
> What I meant is that you can write
>
>      ethtool -s eth0 msglvl drv on probe off
>      ethtool -s eth0 msglvl 0x7
>
> i.e. either number or names (with on/off in this case) while your patch
> has
>
>      ethtool --reset eth0 mgmg,irq
>      ethtool --reset eth0 flags 0x3
>
> i.e. an extra keyword if a number is used.
>
> But it's not really important, it doesn't seem I would be able to share
> a parser for this with any other subcommand or parameter anyway.
>
>>>    It would be also nice to provide a symbolic way to specify the
>>> shared flags.
>> I'll change to allow -shared to be added to the end of each component
>> specified to use the shared bit.
>>   IE. mgmt-shared, irq-shared, dma-shared ?
> Sounds good to me.
>
>>>> +	resetinfo.cmd = ETHTOOL_RESET;
>>>> +
>>>> +	if (send_ioctl(ctx, &resetinfo)) {
>>>> +		perror("Cannot issue RESET");
>>>> +		return 1;
>>>> +	}
>>>> +	fprintf(stdout, "RESET 0x%x issued\n", resetinfo.data);
>>> According to documentation, driver is supposed to clear the flags
>>> corresponding to components which were reset so that what is left are
>>> those which were _not_ reset.
>> I'll move the print above the send_ioctl.
> It might be even more useful if ethtool informed user what actually
> happened, i.e. either change the message to saying these are bits for
> components not reset (if resetinfo.data is not zero) or save the
> original value of resetinfo.data and show  saved_data & ~resetinfo.data
In making the improvement I found a bug in the bnxt_en kernel driver.
The bnxt_en driver currently doesn't clear any of the component flags on 
success so I need to send in a fix for that.

Although in one case (RESET_ALL) in the driver it doesn't actually 
execute the reset until all necessary drivers are unloaded to prevent 
the PCIe bus from hanging.
So question: should the flags be cleared if the reset is "pending" but 
hasn't actually happened yet, but will reset once all the drivers are 
all properly unloaded?
>
> Michal Kubecek
>

^ permalink raw reply

* [Patch net-next v2] act_mirred: get rid of mirred_list_lock spinlock
From: Cong Wang @ 2017-12-06  0:17 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Eric Dumazet, Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <20171206001727.25731-1-xiyou.wangcong@gmail.com>

TC actions are no longer freed in RCU callbacks and we should
always have RTNL lock, so this spinlock is no longer needed.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/act_mirred.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index ff497909c0ad..cee2d413bf57 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -29,7 +29,6 @@
 #include <net/tc_act/tc_mirred.h>
 
 static LIST_HEAD(mirred_list);
-static DEFINE_SPINLOCK(mirred_list_lock);
 
 static bool tcf_mirred_is_act_redirect(int action)
 {
@@ -55,13 +54,10 @@ static void tcf_mirred_release(struct tc_action *a)
 	struct tcf_mirred *m = to_mirred(a);
 	struct net_device *dev;
 
-	/* We could be called either in a RCU callback or with RTNL lock held. */
-	spin_lock_bh(&mirred_list_lock);
 	list_del(&m->tcfm_list);
-	dev = rcu_dereference_protected(m->tcfm_dev, 1);
+	dev = rtnl_dereference(m->tcfm_dev);
 	if (dev)
 		dev_put(dev);
-	spin_unlock_bh(&mirred_list_lock);
 }
 
 static const struct nla_policy mirred_policy[TCA_MIRRED_MAX + 1] = {
@@ -147,9 +143,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 	}
 
 	if (ret == ACT_P_CREATED) {
-		spin_lock_bh(&mirred_list_lock);
 		list_add(&m->tcfm_list, &mirred_list);
-		spin_unlock_bh(&mirred_list_lock);
 		tcf_idr_insert(tn, *a);
 	}
 
@@ -293,7 +287,6 @@ static int mirred_device_event(struct notifier_block *unused,
 
 	ASSERT_RTNL();
 	if (event == NETDEV_UNREGISTER) {
-		spin_lock_bh(&mirred_list_lock);
 		list_for_each_entry(m, &mirred_list, tcfm_list) {
 			if (rcu_access_pointer(m->tcfm_dev) == dev) {
 				dev_put(dev);
@@ -303,7 +296,6 @@ static int mirred_device_event(struct notifier_block *unused,
 				RCU_INIT_POINTER(m->tcfm_dev, NULL);
 			}
 		}
-		spin_unlock_bh(&mirred_list_lock);
 	}
 
 	return NOTIFY_DONE;
-- 
2.13.0

^ permalink raw reply related

* [Patch net-next v2] act_mirred: get rid of tcfm_ifindex from struct tcf_mirred
From: Cong Wang @ 2017-12-06  0:17 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Jiri Pirko, Jamal Hadi Salim

tcfm_dev always points to the correct netdev and we already
hold a refcnt, so no need to use tcfm_ifindex to lookup again.

If we would support moving target netdev across netns, using
pointer would be better than ifindex.

This also fixes dumping obsolete ifindex, now after the
target device is gone we just dump 0 as ifindex.

Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c          |  6 ++----
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c  | 12 +++++-------
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c     |  8 ++++----
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c         |  6 ++++--
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c       |  5 ++---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c        |  5 +----
 drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c |  3 +--
 drivers/net/ethernet/netronome/nfp/flower/action.c    |  4 +---
 include/net/tc_act/tc_mirred.h                        |  6 ++----
 net/dsa/slave.c                                       |  5 +----
 net/sched/act_mirred.c                                |  7 +++----
 11 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index 3d201d7324bd..b3ff5287aafe 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -54,12 +54,10 @@ static int bnxt_tc_parse_redir(struct bnxt *bp,
 			       struct bnxt_tc_actions *actions,
 			       const struct tc_action *tc_act)
 {
-	int ifindex = tcf_mirred_ifindex(tc_act);
-	struct net_device *dev;
+	struct net_device *dev = tcf_mirred_dev(tc_act);
 
-	dev = __dev_get_by_index(dev_net(bp->dev), ifindex);
 	if (!dev) {
-		netdev_info(bp->dev, "no dev for ifindex=%d", ifindex);
+		netdev_info(bp->dev, "no dev in mirred action");
 		return -EINVAL;
 	}
 
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index d4a548a6a55c..a12b894f135d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -405,9 +405,7 @@ static void cxgb4_process_flow_actions(struct net_device *in,
 		} else if (is_tcf_gact_shot(a)) {
 			fs->action = FILTER_DROP;
 		} else if (is_tcf_mirred_egress_redirect(a)) {
-			int ifindex = tcf_mirred_ifindex(a);
-			struct net_device *out = __dev_get_by_index(dev_net(in),
-								    ifindex);
+			struct net_device *out = tcf_mirred_dev(a);
 			struct port_info *pi = netdev_priv(out);
 
 			fs->action = FILTER_SWITCH;
@@ -582,14 +580,14 @@ static int cxgb4_validate_flow_actions(struct net_device *dev,
 			/* Do nothing */
 		} else if (is_tcf_mirred_egress_redirect(a)) {
 			struct adapter *adap = netdev2adap(dev);
-			struct net_device *n_dev;
-			unsigned int i, ifindex;
+			struct net_device *n_dev, *target_dev;
+			unsigned int i;
 			bool found = false;
 
-			ifindex = tcf_mirred_ifindex(a);
+			target_dev = tcf_mirred_dev(a);
 			for_each_port(adap, i) {
 				n_dev = adap->port[i];
-				if (ifindex == n_dev->ifindex) {
+				if (target_dev == n_dev) {
 					found = true;
 					break;
 				}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
index cd0cd13a964d..ab174bcfbfb0 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
@@ -114,14 +114,14 @@ static int fill_action_fields(struct adapter *adap,
 
 		/* Re-direct to specified port in hardware. */
 		if (is_tcf_mirred_egress_redirect(a)) {
-			struct net_device *n_dev;
-			unsigned int i, index;
+			struct net_device *n_dev, *target_dev;
 			bool found = false;
+			unsigned int i;
 
-			index = tcf_mirred_ifindex(a);
+			target_dev = tcf_mirred_dev(a);
 			for_each_port(adap, i) {
 				n_dev = adap->port[i];
-				if (index == n_dev->ifindex) {
+				if (target_dev == n_dev) {
 					fs->action = FILTER_SWITCH;
 					fs->eport = i;
 					found = true;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 62a18914f00f..7737a05c717c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -9101,9 +9101,11 @@ static int parse_tc_actions(struct ixgbe_adapter *adapter,
 
 		/* Redirect to a VF or a offloaded macvlan */
 		if (is_tcf_mirred_egress_redirect(a)) {
-			int ifindex = tcf_mirred_ifindex(a);
+			struct net_device *dev = tcf_mirred_dev(a);
 
-			err = handle_redirect_action(adapter, ifindex, queue,
+			if (!dev)
+				return -EINVAL;
+			err = handle_redirect_action(adapter, dev->ifindex, queue,
 						     action);
 			if (err == 0)
 				return err;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 55979ec2e88a..3e03d2e8f96a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -1982,11 +1982,10 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
 		}
 
 		if (is_tcf_mirred_egress_redirect(a)) {
-			int ifindex = tcf_mirred_ifindex(a);
 			struct net_device *out_dev;
 			struct mlx5e_priv *out_priv;
 
-			out_dev = __dev_get_by_index(dev_net(priv->netdev), ifindex);
+			out_dev = tcf_mirred_dev(a);
 
 			if (switchdev_port_same_parent_id(priv->netdev,
 							  out_dev)) {
@@ -1996,7 +1995,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
 				rpriv = out_priv->ppriv;
 				attr->out_rep = rpriv->rep;
 			} else if (encap) {
-				parse_attr->mirred_ifindex = ifindex;
+				parse_attr->mirred_ifindex = out_dev->ifindex;
 				parse_attr->tun_info = *info;
 				attr->parse_attr = parse_attr;
 				attr->action |= MLX5_FLOW_CONTEXT_ACTION_ENCAP |
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 2d0897b7d860..d2d945f2fc02 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1571,14 +1571,11 @@ mlxsw_sp_port_add_cls_matchall_mirror(struct mlxsw_sp_port *mlxsw_sp_port,
 				      const struct tc_action *a,
 				      bool ingress)
 {
-	struct net *net = dev_net(mlxsw_sp_port->dev);
 	enum mlxsw_sp_span_type span_type;
 	struct mlxsw_sp_port *to_port;
 	struct net_device *to_dev;
-	int ifindex;
 
-	ifindex = tcf_mirred_ifindex(a);
-	to_dev = __dev_get_by_index(net, ifindex);
+	to_dev = tcf_mirred_dev(a);
 	if (!to_dev) {
 		netdev_err(mlxsw_sp_port->dev, "Could not find requested device\n");
 		return -EINVAL;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 2f0e57857ea4..347e96461273 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -92,7 +92,6 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
 			if (err)
 				return err;
 		} else if (is_tcf_mirred_egress_redirect(a)) {
-			int ifindex = tcf_mirred_ifindex(a);
 			struct net_device *out_dev;
 			struct mlxsw_sp_fid *fid;
 			u16 fid_index;
@@ -104,7 +103,7 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
 			if (err)
 				return err;
 
-			out_dev = __dev_get_by_index(dev_net(dev), ifindex);
+			out_dev = tcf_mirred_dev(a);
 			if (out_dev == dev)
 				out_dev = NULL;
 
diff --git a/drivers/net/ethernet/netronome/nfp/flower/action.c b/drivers/net/ethernet/netronome/nfp/flower/action.c
index c1c595f8bb87..ca74c517f626 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/action.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/action.c
@@ -93,13 +93,11 @@ nfp_fl_output(struct nfp_fl_output *output, const struct tc_action *action,
 	size_t act_size = sizeof(struct nfp_fl_output);
 	struct net_device *out_dev;
 	u16 tmp_flags;
-	int ifindex;
 
 	output->head.jump_id = NFP_FL_ACTION_OPCODE_OUTPUT;
 	output->head.len_lw = act_size >> NFP_FL_LW_SIZ;
 
-	ifindex = tcf_mirred_ifindex(action);
-	out_dev = __dev_get_by_index(dev_net(in_dev), ifindex);
+	out_dev = tcf_mirred_dev(action);
 	if (!out_dev)
 		return -EOPNOTSUPP;
 
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 21d253c9a8c6..a2e9cbca5c9e 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -8,10 +8,8 @@
 struct tcf_mirred {
 	struct tc_action	common;
 	int			tcfm_eaction;
-	int			tcfm_ifindex;
 	bool			tcfm_mac_header_xmit;
 	struct net_device __rcu	*tcfm_dev;
-	struct net		*net;
 	struct list_head	tcfm_list;
 };
 #define to_mirred(a) ((struct tcf_mirred *)a)
@@ -34,9 +32,9 @@ static inline bool is_tcf_mirred_egress_mirror(const struct tc_action *a)
 	return false;
 }
 
-static inline int tcf_mirred_ifindex(const struct tc_action *a)
+static inline struct net_device *tcf_mirred_dev(const struct tc_action *a)
 {
-	return to_mirred(a)->tcfm_ifindex;
+	return rtnl_dereference(to_mirred(a)->tcfm_dev);
 }
 
 #endif /* __NET_TC_MIR_H */
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d6e7a642493b..895db05d8c82 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -709,14 +709,12 @@ static int dsa_slave_add_cls_matchall(struct net_device *dev,
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_mall_tc_entry *mall_tc_entry;
 	__be16 protocol = cls->common.protocol;
-	struct net *net = dev_net(dev);
 	struct dsa_switch *ds = dp->ds;
 	struct net_device *to_dev;
 	const struct tc_action *a;
 	struct dsa_port *to_dp;
 	int err = -EOPNOTSUPP;
 	LIST_HEAD(actions);
-	int ifindex;
 
 	if (!ds->ops->port_mirror_add)
 		return err;
@@ -730,8 +728,7 @@ static int dsa_slave_add_cls_matchall(struct net_device *dev,
 	if (is_tcf_mirred_egress_mirror(a) && protocol == htons(ETH_P_ALL)) {
 		struct dsa_mall_mirror_tc_entry *mirror;
 
-		ifindex = tcf_mirred_ifindex(a);
-		to_dev = __dev_get_by_index(net, ifindex);
+		to_dev = tcf_mirred_dev(a);
 		if (!to_dev)
 			return -EINVAL;
 
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 590f56afb985..ff497909c0ad 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -139,8 +139,6 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 	m->tcf_action = parm->action;
 	m->tcfm_eaction = parm->eaction;
 	if (dev != NULL) {
-		m->tcfm_ifindex = parm->ifindex;
-		m->net = net;
 		if (ret != ACT_P_CREATED)
 			dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
 		dev_hold(dev);
@@ -247,13 +245,14 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_mirred *m = to_mirred(a);
+	struct net_device *dev = rtnl_dereference(m->tcfm_dev);
 	struct tc_mirred opt = {
 		.index   = m->tcf_index,
 		.action  = m->tcf_action,
 		.refcnt  = m->tcf_refcnt - ref,
 		.bindcnt = m->tcf_bindcnt - bind,
 		.eaction = m->tcfm_eaction,
-		.ifindex = m->tcfm_ifindex,
+		.ifindex = dev ? dev->ifindex : 0,
 	};
 	struct tcf_t t;
 
@@ -318,7 +317,7 @@ static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
 {
 	struct tcf_mirred *m = to_mirred(a);
 
-	return __dev_get_by_index(m->net, m->tcfm_ifindex);
+	return rtnl_dereference(m->tcfm_dev);
 }
 
 static struct tc_action_ops act_mirred_ops = {
-- 
2.13.0

^ permalink raw reply related

* [PATCH bpf-next 2/2] bpf, doc: add faq about bpf development process
From: Daniel Borkmann @ 2017-12-06  0:12 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20171206001241.17716-1-daniel@iogearbox.net>

In the same spirit of netdev FAQ, start a BPF FAQ as a collection
of expectations and/or workflow details in the context of BPF patch
processing.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 Documentation/bpf/bpf_devel_QA.txt | 519 +++++++++++++++++++++++++++++++++++++
 1 file changed, 519 insertions(+)
 create mode 100644 Documentation/bpf/bpf_devel_QA.txt

diff --git a/Documentation/bpf/bpf_devel_QA.txt b/Documentation/bpf/bpf_devel_QA.txt
new file mode 100644
index 0000000..b0472a4
--- /dev/null
+++ b/Documentation/bpf/bpf_devel_QA.txt
@@ -0,0 +1,519 @@
+This document provides information for the BPF subsystem about various
+workflows related to reporting bugs, submitting patches, and queueing
+patches for stable kernels.
+
+For general information about submitting patches, please refer to
+Documentation/process/. This document only describes additional specifics
+related to BPF.
+
+Reporting bugs:
+---------------
+
+Q: How do I report bugs for BPF kernel code?
+
+A: Since all BPF kernel development as well as bpftool and iproute2 BPF
+   loader development happens through the netdev kernel mailing list,
+   please report any found issues around BPF to the following mailing
+   list:
+
+     netdev@vger.kernel.org
+
+   This may also include issues related to XDP, BPF tracing, etc.
+
+   Given netdev has a high volume of traffic, please also add the BPF
+   maintainers to Cc (from kernel MAINTAINERS file):
+
+     Alexei Starovoitov <ast@kernel.org>
+     Daniel Borkmann <daniel@iogearbox.net>
+
+   In case a buggy commit has already been identified, make sure to keep
+   the actual commit authors in Cc as well for the report. They can
+   typically be identified through the kernel's git tree.
+
+   Please do *not* report BPF issues to bugzilla.kernel.org since it
+   is a guarantee that the reported issue will be overlooked.
+
+Submitting patches:
+-------------------
+
+Q: To which mailing list do I need to submit my BPF patches?
+
+A: Please submit your BPF patches to the netdev kernel mailing list:
+
+     netdev@vger.kernel.org
+
+   Historically, BPF came out of networking and has always been maintained
+   by the kernel networking community. Although these days BPF touches
+   many other subsystems as well, the patches are still routed mainly
+   through the networking community.
+
+   In case your patch has changes in various different subsystems (e.g.
+   tracing, security, etc), make sure to Cc the related kernel mailing
+   lists and maintainers from there as well, so they are able to review
+   the changes and provide their Acked-by's to the patches.
+
+Q: Where can I find patches currently under discussion for BPF subsystem?
+
+A: All patches that are Cc'ed to netdev are queued for review under netdev
+   patchwork project:
+
+     http://patchwork.ozlabs.org/project/netdev/list/
+
+   Those patches which target BPF, are assigned to a 'bpf' delegate for
+   further processing from BPF maintainers. The current queue with
+   patches under review can be found at:
+
+     https://patchwork.ozlabs.org/project/netdev/list/?delegate=77147
+
+   Once the patches have been reviewed by the BPF community as a whole
+   and approved by the BPF maintainers, their status in patchwork will be
+   changed to 'Accepted' and the submitter will be notified by mail. This
+   means that the patches look good from a BPF perspective and have been
+   applied to one of the two BPF kernel trees.
+
+   In case feedback from the community requires a respin of the patches,
+   their status in patchwork will be set to 'Changes Requested', and purged
+   from the current review queue. Likewise for cases where patches would
+   get rejected or are not applicable to the BPF trees (but assigned to
+   the 'bpf' delegate).
+
+Q: How do the changes make their way into Linux?
+
+A: There are two BPF kernel trees (git repositories). Once patches have
+   been accepted by the BPF maintainers, they will be applied to one
+   of the two BPF trees:
+
+     https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/
+     https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
+
+   The bpf tree itself is for fixes only, whereas bpf-next for features,
+   cleanups or other kind of improvements ("next-like" content). This is
+   analogous to net and net-next trees for networking. Both bpf and
+   bpf-next will only have a master branch in order to simplify against
+   which branch patches should get rebased to.
+
+   Accumulated BPF patches in the bpf tree will regularly get pulled
+   into the net kernel tree. Likewise, accumulated BPF patches accepted
+   into the bpf-next tree will make their way into net-next tree. net and
+   net-next are both run by David S. Miller. From there, they will go
+   into the kernel mainline tree run by Linus Torvalds. To read up on the
+   process of net and net-next being merged into the mainline tree, see
+   the netdev FAQ under:
+
+     Documentation/networking/netdev-FAQ.txt
+
+   Occasionally, to prevent merge conflicts, we might send pull requests
+   to other trees (e.g. tracing) with a small subset of the patches, but
+   net and net-next are always the main trees targeted for integration.
+
+   The pull requests will contain a high-level summary of the accumulated
+   patches and can be searched on netdev kernel mailing list through the
+   following subject lines (yyyy-mm-dd is the date of the pull request):
+
+     pull-request: bpf yyyy-mm-dd
+     pull-request: bpf-next yyyy-mm-dd
+
+Q: How do I indicate which tree (bpf vs. bpf-next) my patch should be
+   applied to?
+
+A: The process is the very same as described in the netdev FAQ, so
+   please read up on it. The subject line must indicate whether the
+   patch is a fix or rather "next-like" content in order to let the
+   maintainers know whether it is targeted at bpf or bpf-next.
+
+   For fixes eventually landing in bpf -> net tree, the subject must
+   look like:
+
+     git format-patch --subject-prefix='PATCH bpf' start..finish
+
+   For features/improvements/etc that should eventually land in
+   bpf-next -> net-next, the subject must look like:
+
+     git format-patch --subject-prefix='PATCH bpf-next' start..finish
+
+   If unsure whether the patch or patch series should go into bpf
+   or net directly, or bpf-next or net-next directly, it is not a
+   problem either if the subject line says net or net-next as target.
+   It is eventually up to the maintainers to do the delegation of
+   the patches.
+
+   If it is clear that patches should go into bpf or bpf-next tree,
+   please make sure to rebase the patches against those trees in
+   order to reduce potential conflicts.
+
+   In case the patch or patch series has to be reworked and sent out
+   again in a second or later revision, it is also required to add a
+   version number (v2, v3, ...) into the subject prefix:
+
+     git format-patch --subject-prefix='PATCH net-next v2' start..finish
+
+   When changes have been requested to the patch series, always send the
+   whole patch series again with the feedback incorporated (never send
+   individual diffs on top of the old series).
+
+Q: What does it mean when a patch gets applied to bpf or bpf-next tree?
+
+A: It means that the patch looks good for mainline inclusion from
+   a BPF point of view.
+
+   Be aware that this is not a final verdict that the patch will
+   automatically get accepted into net or net-next trees eventually:
+
+   On the netdev kernel mailing list reviews can come in at any point
+   in time. If discussions around a patch conclude that they cannot
+   get included as-is, we will either apply a follow-up fix or drop
+   them from the trees entirely. Therefore, we also reserve to rebase
+   the trees when deemed necessary. After all, the purpose of the tree
+   is to i) accumulate and stage BPF patches for integration into trees
+   like net and net-next, and ii) run extensive BPF test suite and
+   workloads on the patches before they make their way any further.
+
+   Once the BPF pull request was accepted by David S. Miller, then
+   the patches end up in net or net-next tree, respectively, and
+   make their way from there further into mainline. Again, see the
+   netdev FAQ for additional information e.g. on how often they are
+   merged to mainline.
+
+Q: How long do I need to wait for feedback on my BPF patches?
+
+A: We try to keep the latency low. The usual time to feedback will
+   be around 2 or 3 business days. It may vary depending on the
+   complexity of changes and current patch load.
+
+Q: How often do you send pull requests to major kernel trees like
+   net or net-next?
+
+A: Pull requests will be sent out rather often in order to not
+   accumulate too many patches in bpf or bpf-next.
+
+   As a rule of thumb, expect pull requests for each tree regularly
+   at the end of the week. In some cases pull requests could additionally
+   come also in the middle of the week depending on the current patch
+   load or urgency.
+
+Q: Are patches applied to bpf-next when the merge window is open?
+
+A: For the time when the merge window is open, bpf-next will not be
+   processed. This is roughly analogous to net-next patch processing,
+   so feel free to read up on the netdev FAQ about further details.
+
+   During those two weeks of merge window, we might ask you to resend
+   your patch series once bpf-next is open again. Once Linus released
+   a v*-rc1 after the merge window, we continue processing of bpf-next.
+
+   For non-subscribers to kernel mailing lists, there is also a status
+   page run by David S. Miller on net-next that provides guidance:
+
+     http://vger.kernel.org/~davem/net-next.html
+
+Q: I made a BPF verifier change, do I need to add test cases for
+   BPF kernel selftests?
+
+A: If the patch has changes to the behavior of the verifier, then yes,
+   it is absolutely necessary to add test cases to the BPF kernel
+   selftests suite. If they are not present and we think they are
+   needed, then we might ask for them before accepting any changes.
+
+   In particular, test_verifier.c is tracking a high number of BPF test
+   cases, including a lot of corner cases that LLVM BPF back end may
+   generate out of the restricted C code. Thus, adding test cases is
+   absolutely crucial to make sure future changes do not accidentally
+   affect prior use-cases. Thus, treat those test cases as: verifier
+   behavior that is not tracked in test_verifier.c could potentially
+   be subject to change.
+
+Q: When should I add code to samples/bpf/ and when to BPF kernel
+   selftests?
+
+A: In general, we prefer additions to BPF kernel selftests rather than
+   samples/bpf/. The rationale is very simple: kernel selftests are
+   regularly run by various bots to test for kernel regressions.
+
+   The more test cases we add to BPF selftests, the better the coverage
+   and the less likely it is that those could accidentally break. It is
+   not that BPF kernel selftests cannot demo how a specific feature can
+   be used.
+
+   That said, samples/bpf/ may be a good place for people to get started,
+   so it might be advisable that simple demos of features could go into
+   samples/bpf/, but advanced functional and corner-case testing rather
+   into kernel selftests.
+
+   If your sample looks like a test case, then go for BPF kernel selftests
+   instead!
+
+Q: When should I add code to the bpftool?
+
+A: The main purpose of bpftool (under tools/bpf/bpftool/) is to provide
+   a central user space tool for debugging and introspection of BPF programs
+   and maps that are active in the kernel. If UAPI changes related to BPF
+   enable for dumping additional information of programs or maps, then
+   bpftool should be extended as well to support dumping them.
+
+Q: When should I add code to iproute2's BPF loader?
+
+A: For UAPI changes related to the XDP or tc layer (e.g. cls_bpf), the
+   convention is that those control-path related changes are added to
+   iproute2's BPF loader as well from user space side. This is not only
+   useful to have UAPI changes properly designed to be usable, but also
+   to make those changes available to a wider user base of major
+   downstream distributions.
+
+Q: Do you accept patches as well for iproute2's BPF loader?
+
+A: Patches for the iproute2's BPF loader have to be sent to:
+
+     netdev@vger.kernel.org
+
+   While those patches are not processed by the BPF kernel maintainers,
+   please keep them in Cc as well, so they can be reviewed.
+
+   The official git repository for iproute2 is run by Stephen Hemminger
+   and can be found at:
+
+     https://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git/
+
+   The patches need to have a subject prefix of '[PATCH iproute2 master]'
+   or '[PATCH iproute2 net-next]'. 'master' or 'net-next' describes the
+   target branch where the patch should be applied to. Meaning, if kernel
+   changes went into the net-next kernel tree, then the related iproute2
+   changes need to go into the iproute2 net-next branch, otherwise they
+   can be targeted at master branch. The iproute2 net-next branch will get
+   merged into the master branch after the current iproute2 version from
+   master has been released.
+
+   Like BPF, the patches end up in patchwork under the netdev project and
+   are delegated to 'shemminger' for further processing:
+
+     http://patchwork.ozlabs.org/project/netdev/list/?delegate=389
+
+Q: What is the minimum requirement before I submit my BPF patches?
+
+A: When submitting patches, always take the time and properly test your
+   patches *prior* to submission. Never rush them! If maintainers find
+   that your patches have not been properly tested, it is a good way to
+   get them grumpy. Testing patch submissions is a hard requirement!
+
+   Note, fixes that go to bpf tree *must* have a Fixes: tag included. The
+   same applies to fixes that target bpf-next, where the affected commit
+   is in net-next (or in some cases bpf-next). The Fixes: tag is crucial
+   in order to identify follow-up commits and tremendously helps for people
+   having to do backporting, so it is a must have!
+
+   We also don't accept patches with an empty commit message. Take your
+   time and properly write up a high quality commit message, it is
+   essential!
+
+   Think about it this way: other developers looking at your code a month
+   from now need to understand *why* a certain change has been done that
+   way, and whether there have been flaws in the analysis or assumptions
+   that the original author did. Thus providing a proper rationale and
+   describing the use-case for the changes is a must.
+
+   Patch submissions with >1 patch must have a cover letter which includes
+   a high level description of the series. This high level summary will
+   then be placed into the merge commit by the BPF maintainers such that
+   it is also accessible from the git log for future reference.
+
+Q: What do I need to consider when adding a new instruction or feature
+   that would require BPF JIT and/or LLVM integration as well?
+
+A: We try hard to keep all BPF JITs up to date such that the same user
+   experience can be guaranteed when running BPF programs on different
+   architectures without having the program punt to the less efficient
+   interpreter in case the in-kernel BPF JIT is enabled.
+
+   If you are unable to implement or test the required JIT changes for
+   certain architectures, please work together with the related BPF JIT
+   developers in order to get the feature implemented in a timely manner.
+   Please refer to the git log (arch/*/net/) to locate the necessary
+   people for helping out.
+
+   Also always make sure to add BPF test cases (e.g. test_bpf.c and
+   test_verifier.c) for new instructions, so that they can receive
+   broad test coverage and help run-time testing the various BPF JITs.
+
+   In case of new BPF instructions, once the changes have been accepted
+   into the Linux kernel, please implement support into LLVM's BPF back
+   end. See LLVM section below for further information.
+
+Stable submission:
+------------------
+
+Q: I need a specific BPF commit in stable kernels. What should I do?
+
+A: In case you need a specific fix in stable kernels, first check whether
+   the commit has already been applied in the related linux-*.y branches:
+
+     https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/
+
+   If not the case, then drop an email to the BPF maintainers with the
+   netdev kernel mailing list in Cc and ask for the fix to be queued up:
+
+     netdev@vger.kernel.org
+
+   The process in general is the same as on netdev itself, see also the
+   netdev FAQ document.
+
+Q: Do you also backport to kernels not currently maintained as stable?
+
+A: No. If you need a specific BPF commit in kernels that are currently not
+   maintained by the stable maintainers, then you are on your own.
+
+   The current stable and longterm stable kernels are all listed here:
+
+     https://www.kernel.org/
+
+Q: The BPF patch I am about to submit needs to go to stable as well. What
+   should I do?
+
+A: The same rules apply as with netdev patch submissions in general, see
+   netdev FAQ under:
+
+     Documentation/networking/netdev-FAQ.txt
+
+   Never add "Cc: stable@vger.kernel.org" to the patch description, but
+   ask the BPF maintainers to queue the patches instead. This can be done
+   with a note, for example, under the "---" part of the patch which does
+   not go into the git log. Alternatively, this can be done as a simple
+   request by mail instead.
+
+Q: Where do I find currently queued BPF patches that will be submitted
+   to stable?
+
+A: Once patches that fix critical bugs got applied into the bpf tree, they
+   are queued up for stable submission under:
+
+     http://patchwork.ozlabs.org/bundle/bpf/stable/?state=*
+
+   They will be on hold there at minimum until the related commit made its
+   way into the mainline kernel tree.
+
+   After having been under broader exposure, the queued patches will be
+   submitted by the BPF maintainers to the stable maintainers.
+
+Testing patches:
+----------------
+
+Q: Which BPF kernel selftests version should I run my kernel against?
+
+A: If you run a kernel xyz, then always run the BPF kernel selftests from
+   that kernel xyz as well. Do not expect that the BPF selftest from the
+   latest mainline tree will pass all the time.
+
+   In particular, test_bpf.c and test_verifier.c have a large number of
+   test cases and are constantly updated with new BPF test sequences, or
+   existing ones are adapted to verifier changes e.g. due to verifier
+   becoming smarter and being able to better track certain things.
+
+LLVM:
+-----
+
+Q: Where do I find LLVM with BPF support?
+
+A: The BPF back end for LLVM is upstream in LLVM since version 3.7.1.
+
+   All major distributions these days ship LLVM with BPF back end enabled,
+   so for the majority of use-cases it is not required to compile LLVM by
+   hand anymore, just install the distribution provided package.
+
+   LLVM's static compiler lists the supported targets through 'llc --version',
+   make sure BPF targets are listed. Example:
+
+     $ llc --version
+     LLVM (http://llvm.org/):
+       LLVM version 6.0.0svn
+       Optimized build.
+       Default target: x86_64-unknown-linux-gnu
+       Host CPU: skylake
+
+       Registered Targets:
+         bpf    - BPF (host endian)
+         bpfeb  - BPF (big endian)
+         bpfel  - BPF (little endian)
+         x86    - 32-bit X86: Pentium-Pro and above
+         x86-64 - 64-bit X86: EM64T and AMD64
+
+   For developers in order to utilize the latest features added to LLVM's
+   BPF back end, it is advisable to run the latest LLVM releases. Support
+   for new BPF kernel features such as additions to the BPF instruction
+   set are often developed together.
+
+   All LLVM releases can be found at: http://releases.llvm.org/
+
+Q: Got it, so how do I build LLVM manually anyway?
+
+A: You need cmake and gcc-c++ as build requisites for LLVM. Once you have
+   that set up, proceed with building the latest LLVM and clang version
+   from the git repositories:
+
+     $ git clone http://llvm.org/git/llvm.git
+     $ cd llvm/tools
+     $ git clone --depth 1 http://llvm.org/git/clang.git
+     $ cd ..; mkdir build; cd build
+     $ cmake .. -DLLVM_TARGETS_TO_BUILD="BPF;X86" \
+                -DBUILD_SHARED_LIBS=OFF           \
+                -DCMAKE_BUILD_TYPE=Release        \
+                -DLLVM_BUILD_RUNTIME=OFF
+     $ make -j $(getconf _NPROCESSORS_ONLN)
+
+   The built binaries can then be found in the build/bin/ directory, where
+   you can point the PATH variable to.
+
+Q: Should I notify BPF kernel maintainers about issues in LLVM's BPF code
+   generation back end or about LLVM generated code that the verifier
+   refuses to accept?
+
+A: Yes, please do! LLVM's BPF back end is a key piece of the whole BPF
+   infrastructure and it ties deeply into verification of programs from the
+   kernel side. Therefore, any issues on either side need to be investigated
+   and fixed whenever necessary.
+
+   Therefore, please make sure to bring them up at netdev kernel mailing
+   list and Cc BPF maintainers for LLVM and kernel bits:
+
+     Yonghong Song <yhs@fb.com>
+     Alexei Starovoitov <ast@kernel.org>
+     Daniel Borkmann <daniel@iogearbox.net>
+
+   LLVM also has an issue tracker where BPF related bugs can be found:
+
+     https://bugs.llvm.org/buglist.cgi?quicksearch=bpf
+
+   However, it is better to reach out through mailing lists with having
+   maintainers in Cc.
+
+Q: I have added a new BPF instruction to the kernel, how can I integrate
+   it into LLVM?
+
+A: LLVM has a -mcpu selector for the BPF back end in order to allow the
+   selection of BPF instruction set extensions. By default the 'generic'
+   processor target is used, which is the base instruction set (v1) of BPF.
+
+   LLVM has an option to select -mcpu=probe where it will probe the host
+   kernel for supported BPF instruction set extensions and selects the
+   optimal set automatically.
+
+   For cross-compilation, a specific version can be select manually as well.
+
+     $ llc -march bpf -mcpu=help
+     Available CPUs for this target:
+
+       generic - Select the generic processor.
+       probe   - Select the probe processor.
+       v1      - Select the v1 processor.
+       v2      - Select the v2 processor.
+     [...]
+
+   Newly added BPF instructions to the Linux kernel need to follow the same
+   scheme, bump the instruction set version and implement probing for the
+   extensions such that -mcpu=probe users can benefit from the optimization
+   transparently when upgrading their kernels.
+
+   If you are unable to implement support for the newly added BPF instruction
+   please reach out to BPF developers for help.
+
+   By the way, the BPF kernel selftests run with -mcpu=probe for better
+   test coverage.
+
+Happy BPF hacking!
-- 
2.9.5

^ permalink raw reply related

* [PATCH bpf-next 0/2] Few BPF doc updates
From: Daniel Borkmann @ 2017-12-06  0:12 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann

Two changes, i) add BPF trees into maintainers file, and ii) add
a BPF doc around the development process, similarly as we have
with netdev FAQ, but just describing BPF specifics. Thanks!

Daniel Borkmann (2):
  bpf, doc: add bpf trees and tps to maintainers entry
  bpf, doc: add faq about bpf development process

 Documentation/bpf/bpf_devel_QA.txt | 519 +++++++++++++++++++++++++++++++++++++
 MAINTAINERS                        |   4 +
 2 files changed, 523 insertions(+)
 create mode 100644 Documentation/bpf/bpf_devel_QA.txt

-- 
2.9.5

^ permalink raw reply

* [PATCH bpf-next 1/2] bpf, doc: add bpf trees and tps to maintainers entry
From: Daniel Borkmann @ 2017-12-06  0:12 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20171206001241.17716-1-daniel@iogearbox.net>

i) Add the bpf and bpf-next trees to the maintainers entry
   so they can be found easily and picked up by test bots
   etc that would integrate all trees from maintainers file.
   Suggested by Stephen while integrating the trees into
   linux-next.

ii) Add the two headers defining BPF/XDP tracepoints to the
    list of files as well.

Suggested-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 MAINTAINERS | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4007fa2..77ad4bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2724,12 +2724,16 @@ M:	Alexei Starovoitov <ast@kernel.org>
 M:	Daniel Borkmann <daniel@iogearbox.net>
 L:	netdev@vger.kernel.org
 L:	linux-kernel@vger.kernel.org
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
 S:	Supported
 F:	arch/x86/net/bpf_jit*
 F:	Documentation/networking/filter.txt
 F:	Documentation/bpf/
 F:	include/linux/bpf*
 F:	include/linux/filter.h
+F:	include/trace/events/bpf.h
+F:	include/trace/events/xdp.h
 F:	include/uapi/linux/bpf*
 F:	include/uapi/linux/filter.h
 F:	kernel/bpf/
-- 
2.9.5

^ permalink raw reply related

* [PATCH net] net: thunderx: Fix TCP/UDP checksum offload for IPv4 pkts
From: Florian Westphal @ 2017-12-06  0:04 UTC (permalink / raw)
  To: netdev; +Cc: Florian Westphal, Sunil Goutham, Aleksey Makarov, Eric Dumazet

Offload IP header checksum to NIC.

This fixes a previous patch which disabled checksum offloading
for both IPv4 and IPv6 packets.  So L3 checksum offload was
getting disabled for IPv4 pkts.  And HW is dropping these pkts
for some reason.

Without this patch, IPv4 TSO appears to be broken:

WIthout this patch I get ~16kbyte/s, with patch close to 2mbyte/s
when copying files via scp from test box to my home workstation.

Looking at tcpdump on sender it looks like hardware drops IPv4 TSO skbs.
This patch restores performance for me, ipv6 looks good too.

Fixes: fa6d7cb5d76c ("net: thunderx: Fix TCP/UDP checksum offload for IPv6 pkts")
Cc: Sunil Goutham <sgoutham@cavium.com>
Cc: Aleksey Makarov <aleksey.makarov@auriga.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 8b2c31e2a2b0..a3d12dbde95b 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -1355,6 +1355,8 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
 
 	/* Offload checksum calculation to HW */
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		if (ip.v4->version == 4)
+			hdr->csum_l3 = 1; /* Enable IP csum calculation */
 		hdr->l3_offset = skb_network_offset(skb);
 		hdr->l4_offset = skb_transport_offset(skb);
 
-- 
2.13.6

^ permalink raw reply related

* [PATCH v2 2/3] net: macb: reduce scope of rx_fs_lock-protected regions
From: Julia Cartwright @ 2017-12-06  0:02 UTC (permalink / raw)
  To: David Miller
  Cc: julia.lawall, rafalo, nicolas.ferre, netdev, linux-kernel,
	kbuild-all
In-Reply-To: <cover.1512518311.git.julia@ni.com>

Commit ae8223de3df5 ("net: macb: Added support for RX filtering")
introduces a lock, rx_fs_lock which is intended to protect the list of
rx_flow items and synchronize access to the hardware rx filtering
registers.

However, the region protected by this lock is overscoped, unnecessarily
including things like slab allocation.  Reduce this lock scope to only
include operations which must be performed atomically: list traversal,
addition, and removal, and hitting the macb filtering registers.

This fixes the use of kmalloc w/ GFP_KERNEL in atomic context.

Fixes: ae8223de3df5 ("net: macb: Added support for RX filtering")
Cc: Rafal Ozieblo <rafalo@cadence.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Julia Cartwright <julia@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b7644836aba1..758e8b3042b2 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2796,6 +2796,7 @@ static int gem_add_flow_filter(struct net_device *netdev,
 	struct macb *bp = netdev_priv(netdev);
 	struct ethtool_rx_flow_spec *fs = &cmd->fs;
 	struct ethtool_rx_fs_item *item, *newfs;
+	unsigned long flags;
 	int ret = -EINVAL;
 	bool added = false;
 
@@ -2811,6 +2812,8 @@ static int gem_add_flow_filter(struct net_device *netdev,
 			htonl(fs->h_u.tcp_ip4_spec.ip4dst),
 			htons(fs->h_u.tcp_ip4_spec.psrc), htons(fs->h_u.tcp_ip4_spec.pdst));
 
+	spin_lock_irqsave(&bp->rx_fs_lock, flags);
+
 	/* find correct place to add in list */
 	list_for_each_entry(item, &bp->rx_fs_list.list, list) {
 		if (item->fs.location > newfs->fs.location) {
@@ -2833,9 +2836,11 @@ static int gem_add_flow_filter(struct net_device *netdev,
 	if (netdev->features & NETIF_F_NTUPLE)
 		gem_enable_flow_filters(bp, 1);
 
+	spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
 	return 0;
 
 err:
+	spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
 	kfree(newfs);
 	return ret;
 }
@@ -2846,6 +2851,9 @@ static int gem_del_flow_filter(struct net_device *netdev,
 	struct macb *bp = netdev_priv(netdev);
 	struct ethtool_rx_fs_item *item;
 	struct ethtool_rx_flow_spec *fs;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bp->rx_fs_lock, flags);
 
 	list_for_each_entry(item, &bp->rx_fs_list.list, list) {
 		if (item->fs.location == cmd->fs.location) {
@@ -2862,12 +2870,14 @@ static int gem_del_flow_filter(struct net_device *netdev,
 			gem_writel_n(bp, SCRT2, fs->location, 0);
 
 			list_del(&item->list);
-			kfree(item);
 			bp->rx_fs_list.count--;
+			spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
+			kfree(item);
 			return 0;
 		}
 	}
 
+	spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
 	return -EINVAL;
 }
 
@@ -2936,11 +2946,8 @@ static int gem_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
 static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
 {
 	struct macb *bp = netdev_priv(netdev);
-	unsigned long flags;
 	int ret;
 
-	spin_lock_irqsave(&bp->rx_fs_lock, flags);
-
 	switch (cmd->cmd) {
 	case ETHTOOL_SRXCLSRLINS:
 		if ((cmd->fs.location >= bp->max_tuples)
@@ -2959,7 +2966,6 @@ static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
 		ret = -EOPNOTSUPP;
 	}
 
-	spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
 	return ret;
 }
 
-- 
2.14.2

^ permalink raw reply related

* [PATCH v2 3/3] net: macb: change GFP_ATOMIC to GFP_KERNEL
From: Julia Cartwright @ 2017-12-06  0:02 UTC (permalink / raw)
  To: David Miller
  Cc: julia.lawall, rafalo, nicolas.ferre, netdev, linux-kernel,
	kbuild-all
In-Reply-To: <cover.1512518311.git.julia@ni.com>

Now that the rx_fs_lock is no longer held across allocation, it's safe
to use GFP_KERNEL for allocating new entries.

This reverts commit 81da3bf6e3f88 ("net: macb: change GFP_KERNEL to
GFP_ATOMIC").

Cc: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Julia Cartwright <julia@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 758e8b3042b2..234667eaaa92 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2800,7 +2800,7 @@ static int gem_add_flow_filter(struct net_device *netdev,
 	int ret = -EINVAL;
 	bool added = false;
 
-	newfs = kmalloc(sizeof(*newfs), GFP_ATOMIC);
+	newfs = kmalloc(sizeof(*newfs), GFP_KERNEL);
 	if (newfs == NULL)
 		return -ENOMEM;
 	memcpy(&newfs->fs, fs, sizeof(newfs->fs));
-- 
2.14.2

^ permalink raw reply related

* [PATCH v2 1/3] net: macb: kill useless use of list_empty()
From: Julia Cartwright @ 2017-12-06  0:02 UTC (permalink / raw)
  To: David Miller
  Cc: julia.lawall, rafalo, nicolas.ferre, netdev, linux-kernel,
	kbuild-all
In-Reply-To: <cover.1512518311.git.julia@ni.com>

The list_for_each_entry() macro already handles the case where the list
is empty (by not executing the loop body).  It's not necessary to handle
this case specially, so stop doing so.

Cc: Rafal Ozieblo <rafalo@cadence.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Julia Cartwright <julia@ni.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ebfeab853bf4..b7644836aba1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2812,24 +2812,20 @@ static int gem_add_flow_filter(struct net_device *netdev,
 			htons(fs->h_u.tcp_ip4_spec.psrc), htons(fs->h_u.tcp_ip4_spec.pdst));
 
 	/* find correct place to add in list */
-	if (list_empty(&bp->rx_fs_list.list))
-		list_add(&newfs->list, &bp->rx_fs_list.list);
-	else {
-		list_for_each_entry(item, &bp->rx_fs_list.list, list) {
-			if (item->fs.location > newfs->fs.location) {
-				list_add_tail(&newfs->list, &item->list);
-				added = true;
-				break;
-			} else if (item->fs.location == fs->location) {
-				netdev_err(netdev, "Rule not added: location %d not free!\n",
-						fs->location);
-				ret = -EBUSY;
-				goto err;
-			}
+	list_for_each_entry(item, &bp->rx_fs_list.list, list) {
+		if (item->fs.location > newfs->fs.location) {
+			list_add_tail(&newfs->list, &item->list);
+			added = true;
+			break;
+		} else if (item->fs.location == fs->location) {
+			netdev_err(netdev, "Rule not added: location %d not free!\n",
+					fs->location);
+			ret = -EBUSY;
+			goto err;
 		}
-		if (!added)
-			list_add_tail(&newfs->list, &bp->rx_fs_list.list);
 	}
+	if (!added)
+		list_add_tail(&newfs->list, &bp->rx_fs_list.list);
 
 	gem_prog_cmp_regs(bp, fs);
 	bp->rx_fs_list.count++;
@@ -2851,9 +2847,6 @@ static int gem_del_flow_filter(struct net_device *netdev,
 	struct ethtool_rx_fs_item *item;
 	struct ethtool_rx_flow_spec *fs;
 
-	if (list_empty(&bp->rx_fs_list.list))
-		return -EINVAL;
-
 	list_for_each_entry(item, &bp->rx_fs_list.list, list) {
 		if (item->fs.location == cmd->fs.location) {
 			/* disable screener regs for the flow entry */
-- 
2.14.2

^ permalink raw reply related

* [PATCH v2 0/3] macb rx filter cleanups
From: Julia Cartwright @ 2017-12-06  0:02 UTC (permalink / raw)
  To: David Miller
  Cc: julia.lawall, rafalo, nicolas.ferre, netdev, linux-kernel,
	kbuild-all
In-Reply-To: <20171205.180031.935589370339834625.davem@davemloft.net>

Here's a proper patchset based on net-next.

v1 -> v2:
  - Rebased on net-next
  - Add Nicolas's Acks
  - Reorder commits, putting the list_empty() cleanups prior to the
    others.
  - Added commit reverting the GFP_ATOMIC change.

Julia Cartwright (3):
  net: macb: kill useless use of list_empty()
  net: macb: reduce scope of rx_fs_lock-protected regions
  net: macb: change GFP_ATOMIC to GFP_KERNEL

 drivers/net/ethernet/cadence/macb_main.c | 47 ++++++++++++++++----------------
 1 file changed, 23 insertions(+), 24 deletions(-)

-- 
2.14.2

^ permalink raw reply

* Re: [PATCH v2 0/6] bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type
From: Daniel Borkmann @ 2017-12-05 23:51 UTC (permalink / raw)
  To: Hendrik Brueckner, Arnaldo Carvalho de Melo, Alexei Starovoitov,
	Martin Schwidefsky, Will Deacon, Ingo Molnar
  Cc: Arnd Bergmann, Peter Zijlstra, David S. Miller, linux-kernel,
	linux-s390, netdev
In-Reply-To: <e879ca95-25d7-d7b6-e811-81a5017ea5cf@iogearbox.net>

On 12/05/2017 04:53 PM, Daniel Borkmann wrote:
> On 12/04/2017 10:56 AM, Hendrik Brueckner wrote:
>> Perf tool bpf selftests revealed a broken uapi for s390 and arm64.
>> With the BPF_PROG_TYPE_PERF_EVENT program type the bpf_perf_event
>> structure exports the pt_regs structure for all architectures.
>>
>> This fails for s390 and arm64 because pt_regs are not part of the
>> user api and kept in-kernel only.  To mitigate the broken uapi,
>> introduce a wrapper that exports pt_regs in an asm-generic way.
>> For arm64, export the exising user_pt_regs structure.  For s390,
>> introduce a user_pt_regs structure that exports the beginning of
>> pt_regs.
>>
>> Note that user_pt_regs must export from the beginning of pt_regs
>> as BPF_PROG_TYPE_PERF_EVENT program type is not the only type for
>> running BPF programs.
>>
>> Some more background:
>> 	For the bpf_perf_event, there is a uapi definition that is
>> 	passed to the BPF program.  For other "probe" points like
>> 	trace points, kprobes, and uprobes, there is no uapi and the
>> 	BPF program is always passed pt_regs (which is OK as the BPF
>> 	program runs in the kernel context).  The perf tool can attach
>> 	BPF programs to all of these "probe" points and, optionally,
>> 	can create a BPF prologue to access particular arguments
>> 	(passed as registers).  For this, it uses DWARF/CFI
>> 	information to obtain the register and calls a perf-arch
>> 	backend function, regs_query_register_offset().  This function
>> 	returns the index into (user_)pt_regs for a particular
>> 	register.  Then, perf creates a BPF prologue that accesses
>> 	this register based on the passed stucture from the "probe"
>> 	point.
>>
>> Part of this series, are also updates to the testing and bpf selftest
>> to deal with asm-specifics.  To complete the bpf support in perf, the
>> the regs_query_register_offset function is added for s390 to support
>> BPF prologue creation.
>>
>> Changelog v1 -> v2:
>> - Correct kbuild test bot issues by including
>>   asm-generic/bpf_perf_event.h for archictectures that do not have
>>   their own asm version.
>> - Added patch to clean-up whitespace and coding style issues in s390
>>   asm/ptrace.h (#4/6) as suggested by Alexei.
>>
>>
>> Hendrik Brueckner (6):
>>   bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type
>>   s390/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program
>>     type
>>   arm64/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program
>>     type
>>   s390/uapi: correct whitespace & coding style in asm/ptrace.h
>>   selftests/bpf: sync kernel headers and introduce arch support in
>>     Makefile
>>   perf s390: add regs_query_register_offset()
> 
> Series looks good to me, thanks for working on this Hendrik! If nobody
> hollers, I would take the fixes via bpf tree later tonight.

Done, applied to bpf, thanks Hendrik!

^ permalink raw reply

* Re: [PATCH 3/3] make sock_alloc_file() do sock_release() on failures
From: David Miller @ 2017-12-05 23:41 UTC (permalink / raw)
  To: viro; +Cc: netdev
In-Reply-To: <20171205232909.GY21978@ZenIV.linux.org.uk>

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Tue, 5 Dec 2017 23:29:09 +0000

> This changes calling conventions (and simplifies the hell out
> the callers).  New rules: once struct socket had been passed
> to sock_alloc_file(), it's been consumed either by struct file
> or by sock_release() done by sock_alloc_file().  Either way
> the caller should not do sock_release() after that point.
> 
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Applied.

^ permalink raw reply

* Re: [PATCH 2/3] socketpair(): allocate descriptors first
From: David Miller @ 2017-12-05 23:40 UTC (permalink / raw)
  To: viro; +Cc: netdev
In-Reply-To: <20171205232838.GX21978@ZenIV.linux.org.uk>

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Tue, 5 Dec 2017 23:28:38 +0000

> simplifies failure exits considerably...
> 
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Applied.

^ permalink raw reply

* Re: [PATCH 1/3] fix kcm_clone()
From: David Miller @ 2017-12-05 23:40 UTC (permalink / raw)
  To: viro; +Cc: netdev
In-Reply-To: <20171205232757.GW21978@ZenIV.linux.org.uk>

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Tue, 5 Dec 2017 23:27:57 +0000

> 1) it's fput() or sock_release(), not both
> 2) don't do fd_install() until the last failure exit.
> 3) not a bug per se, but... don't attach socket to struct file
>    until it's set up.
> 
> Take reserving descriptor into the caller, move fd_install() to the
> caller, sanitize failure exits and calling conventions.
> 
> Cc: stable@vger.kernel.org # v4.6+
> Acked-by: Tom Herbert <tom@herbertland.com>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Applied.

^ permalink raw reply

* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
From: Paul E. McKenney @ 2017-12-05 23:39 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
	fweisbec, oleg, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20171206000541-mutt-send-email-mst@kernel.org>

On Wed, Dec 06, 2017 at 12:09:36AM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 10:57:00PM +0100, Peter Zijlstra wrote:
> > On Tue, Dec 05, 2017 at 11:24:49PM +0200, Michael S. Tsirkin wrote:
> > > READ_ONCE is really all over the place (some code literally replaced all
> > > memory accesses with READ/WRITE ONCE).
> > 
> > Yeah, so?
> 
> Oh my point was I can't just look for READ_ONCE and go
> *that's the pair*. there are too many of these.
> At Paul's suggestion I will document the pairing *this read once has a
> barrier that is paired with that barrier*.
> 
> > Complain to the compiler people for forcing us into that.
> 
> In some cases when you end up with all accesses
> going through read/write once volatile just might better.

That is in fact what the jiffies counter does.  But you lose READ_ONCE()'s
automatic handling of DEC Alpha when you take that approach.

> > > Would an API like WRITE_POINTER()/smp_store_pointer make sense,
> > > and READ_POINTER for symmetry?
> > 
> > No, the whole point of the exercise was to get away from the fact that
> > dependent loads are special.
> 
> It's a pity that dependent stores are still special.

We can make READ_ONCE() not be special at zero cost on non-Alpha
systems, but both smp_wmb() and smp_store_release() are decidedly
not free of added overhead.

							Thanx, Paul

^ permalink raw reply


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