Netdev List
 help / color / mirror / Atom feed
* Bug in cadence macb driver
From: Steve Bennett @ 2016-10-27  7:22 UTC (permalink / raw)
  To: Nicolas Ferre, netdev, Cyrille Pitchen

Hi,

We are seeing the following problem in the cadence macb driver.

BUG: scheduling while atomic: kworker/0:1/44/0x00000002
Modules linked in: 8021q of_serial beacon_sigproc io_processor emsattr ems_dma_fifo xaxi_dma libgpioreg leds_gpio
CPU: 0 PID: 44 Comm: kworker/0:1 Not tainted 4.4.0 #1
Hardware name: Xilinx Zynq Platform
Workqueue: events macb_tx_error_task
[<c0016a18>] (unwind_backtrace) from [<c0012ad4>] (show_stack+0x10/0x14)
[<c0012ad4>] (show_stack) from [<c01b089c>] (dump_stack+0x88/0xd8)
[<c01b089c>] (dump_stack) from [<c008e94c>] (__schedule_bug+0x48/0x60)
[<c008e94c>] (__schedule_bug) from [<c03592d4>] (__schedule+0x80/0x4c4)
[<c03592d4>] (__schedule) from [<c0359968>] (schedule+0xac/0xd4)
[<c0359968>] (schedule) from [<c035c284>] (schedule_hrtimeout_range_clock+0xc0/0xf4)
[<c035c284>] (schedule_hrtimeout_range_clock) from [<c035bebc>] (usleep_range+0x44/0x48)
[<c035bebc>] (usleep_range) from [<c025ec48>] (macb_tx_error_task+0xa0/0x270)
[<c025ec48>] (macb_tx_error_task) from [<c0035b6c>] (process_one_work+0x1b8/0x2f0)
[<c0035b6c>] (process_one_work) from [<c0035fb4>] (worker_thread+0x2e4/0x3f0)
[<c0035fb4>] (worker_thread) from [<c003ac94>] (kthread+0xdc/0xf0)
[<c003ac94>] (kthread) from [<c000f698>] (ret_from_fork+0x14/0x3c)

Here we are running at 10M half duplex, so tx is slow.
The problem was introduced in e4bfd971b where a spinlock is taken before
calling macb_halt_tx() which may then sleep waiting for the current packet to finish
transmitting.

We don't need the spinlock on our platform since we don't have multiple queues, but
this should be fixed properly in the general case.

Cheers,
Steve

^ permalink raw reply

* Re: [RFC net-next iproute2 1/2] libnetlink: Add test for error code returned from netlink reply
From: Cyrill Gorcunov @ 2016-10-27  6:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Eric Dumazet, David Ahern, Andrey Vagin
In-Reply-To: <20161026205256.162e1d25@xeon-e3>

On Wed, Oct 26, 2016 at 08:52:56PM -0700, Stephen Hemminger wrote:
> On Wed, 26 Oct 2016 22:30:07 +0300
> Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> 
> >  
> >  				if (h->nlmsg_type == NLMSG_DONE) {
> > +					int len;
> > +
> > +					/*
> > +					 * The kernel reports if there is
> > +					 * no inet-diag module present in
> > +					 * the system via negative length
> > +					 * as error code.
> > +					 */
> > +					if (h->nlmsg_len < NLMSG_LENGTH(sizeof(int))) {
> > +						fprintf(stderr, "Truncated length reply\n");
> > +						return -1;
> > +					}
> > +					len = *(int *)NLMSG_DATA(h);
> > +					if (len < 0) {
> > +						errno = -len;
> > +						if (errno == ENOENT ||
> > +						    errno == EOPNOTSUPP)
> > +							return -1;
> > +						perror("RTNETLINK answers");
> > +						return len;
> > +					}
> >  					found_done = 1;
> >  					break; /* process next filter */
> >  				
> 
> This looks like a mistake in how you implemented the functionality in the kernel.
> Despite what it looks like, all netlink request/reply functionality reports
> errors in current implementation by returning error to the sendmsg request.
> 
> What you added implies that the new kernel api is wrong, or many other usages
> are wrong.  Please fix the kernel.

No. This is not my code. This code has been in kernel for the really long time.
I don't know why you've not been doing such test in libnetlink before.

Actually I've hit this problem accidentaly -- I made a patch 2 from this
set and run it on the machine where kernel was unpatched, ie without
raw-diag module, and I found that we can't figure out if kernel notified
us that some diag module simply not present in the system. And here is
the only way to find it out.

^ permalink raw reply

* RE: nfs NULL-dereferencing in net-next
From: Yotam Gigi @ 2016-10-27  6:50 UTC (permalink / raw)
  To: Anna Schumaker, Jakub Kicinski
  Cc: Andy Adamson, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Trond Myklebust,
	Yotam Gigi, mlxsw
In-Reply-To: <0ab5021c-84eb-cb9f-c7f5-4d856fdb0f4b-ZwjVKphTwtPQT0dZR+AlfA@public.gmane.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3636 bytes --]



>-----Original Message-----
>From: Anna Schumaker [mailto:Anna.Schumaker@Netapp.com]
>Sent: Wednesday, October 26, 2016 9:17 PM
>To: Jakub Kicinski <kubakici@wp.pl>
>Cc: Yotam Gigi <yotamg@mellanox.com>; Andy Adamson <andros@netapp.com>;
>linux-nfs@vger.kernel.org; netdev@vger.kernel.org; Trond Myklebust
><Trond.Myklebust@netapp.com>; Yotam Gigi <yotam.gi@gmail.com>; mlxsw
><mlxsw@mellanox.com>
>Subject: Re: nfs NULL-dereferencing in net-next
>
>On 10/26/2016 02:08 PM, Jakub Kicinski wrote:
>> On Wed, 26 Oct 2016 16:15:24 +0000, Yotam Gigi wrote:
>>>> -----Original Message-----
>>>> From: Anna Schumaker [mailto:Anna.Schumaker@Netapp.com]
>>>> Sent: Wednesday, October 26, 2016 5:40 PM
>>>> To: Yotam Gigi <yotamg@mellanox.com>; Jakub Kicinski <kubakici@wp.pl>;
>Andy
>>>> Adamson <andros@netapp.com>; Anna Schumaker
>>>> <Anna.Schumaker@Netapp.com>; linux-nfs@vger.kernel.org
>>>> Cc: netdev@vger.kernel.org; Trond Myklebust
><Trond.Myklebust@netapp.com>;
>>>> Yotam Gigi <yotam.gi@gmail.com>; mlxsw <mlxsw@mellanox.com>
>>>> Subject: Re: nfs NULL-dereferencing in net-next
>>>>
>>>> On 10/25/2016 01:19 PM, Yotam Gigi wrote:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: netdev-owner@vger.kernel.org [mailto:netdev-
>owner@vger.kernel.org]
>>>> On
>>>>>> Behalf Of Jakub Kicinski
>>>>>> Sent: Monday, October 17, 2016 10:20 PM
>>>>>> To: Andy Adamson <andros@netapp.com>; Anna Schumaker
>>>>>> <Anna.Schumaker@Netapp.com>; linux-nfs@vger.kernel.org
>>>>>> Cc: netdev@vger.kernel.org; Trond Myklebust
>>>> <Trond.Myklebust@netapp.com>
>>>>>> Subject: nfs NULL-dereferencing in net-next
>>>>>>
>>>>>> Hi!
>>>>>>
>>>>>> I'm hitting this reliably on net-next, HEAD at 3f3177bb680f
>>>>>> ("fsl/fman: fix error return code in mac_probe()").
>>>>>
>>>>>
>>>>> I see the same thing. It happens constantly on some of my machines, making
>>>> them
>>>>> completely unusable.
>>>>>
>>>>> I bisected it and got to the commit:
>>>>>
>>>>> commit 04ea1b3e6d8ed4978bb608c1748530af3de8c274
>>>>> Author: Andy Adamson <andros@netapp.com>
>>>>> Date:   Fri Sep 9 09:22:27 2016 -0400
>>>>>
>>>>>     NFS add xprt switch addrs test to match client
>>>>>
>>>>>     Signed-off-by: Andy Adamson <andros@netapp.com>
>>>>>     Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>>
>>>> Thanks for reporting on this everyone!  Does this patch help?
>>>
>>> Actually, I still see the same bug with the same trace.
>
>Well, it was worth a shot.  I'll keep poking at it.
>
>>
>> I rebuild the latest net-next and I'm not seeing the trace any more...
>> I'm only seeing this (with or without your patch):
>>
>> [   23.465877] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>> [   23.473784] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>> [   23.588890] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>> [   23.596746] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>> [   23.781574] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>> [   23.789599] NFS: set_pnfs_layoutdriver: cl_exchange_flags 0x0
>
>Interesting, I get that too when I try to use NFS v4.1.  It's weird that the crash would
>stop happening like that, so maybe something is racy in this area.
>
>Thanks for testing, Yotam and Jakub!
>Anna

I just found out that it happens on any of my machines, once I put two nfs entries in
my fstab. If I put only one, I don't see the problem. 

I hope it might be helpful :)

>
>>
>> HTH
>>

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±û"žØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹\x1e®w¥¢¸?™¨è­Ú&¢)ߢ^[f

^ permalink raw reply

* Re: [PATCH] net: avoid uninitialized variable
From: zhong jiang @ 2016-10-27  5:53 UTC (permalink / raw)
  To: Gao Feng
  Cc: David S. Miller, Alex Duyck, Tom Herbert, jiri, hadarh, amir,
	Linux Kernel Network Developers, linux-kernel
In-Reply-To: <CA+6hz4rniDvOBL65k_c2SqN+nTCrdUBLkHoo6XdtqfZUPgRVeQ@mail.gmail.com>

On 2016/10/27 12:02, Gao Feng wrote:
> On Thu, Oct 27, 2016 at 11:56 AM, zhongjiang <zhongjiang@huawei.com> wrote:
>> From: zhong jiang <zhongjiang@huawei.com>
>>
>> when I compiler the newest kernel, I hit the following error with
>> Werror=may-uninitalized.
>>
>> net/core/flow_dissector.c: In function ?._skb_flow_dissect?
>> include/uapi/linux/swab.h:100:46: error: ?.lan?.may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> net/core/flow_dissector.c:248:26: note: ?.lan?.was declared here
>>
>> This adds an additional check for proto to explicitly tell the compiler
>> that vlan pointer have the correct value before it is used.
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>>  net/core/flow_dissector.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index 1a7b80f..a04d9cf 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> @@ -245,7 +245,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>         }
>>         case htons(ETH_P_8021AD):
>>         case htons(ETH_P_8021Q): {
>> -               const struct vlan_hdr *vlan;
>> +               const struct vlan_hdr *vlan = NULL;
>>
>>                 if (skb_vlan_tag_present(skb))
>>                         proto = skb->protocol;
>> @@ -276,7 +276,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>                                 key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
>>                                 key_vlan->vlan_priority =
>>                                         (skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);
>> -                       } else {
>> +                       } else if (proto == cpu_to_be16(ETH_P_8021Q) ||
>> +                                               proto == cpu_to_be16(ETH_P_8021AD)) {
>>                                 key_vlan->vlan_id = ntohs(vlan->h_vlan_TCI) &
>>                                         VLAN_VID_MASK;
>>                                 key_vlan->vlan_priority =
>> --
>> 1.8.3.1
>>
> It seems there is one similar patch already.
> You could refer to https://patchwork.kernel.org/patch/9389565/
>
> Regards
> Feng
>
> .
>
 sorry, I doesn't notice that patch.

 Thanks
 zhongjiang 

^ permalink raw reply

* RE: [v12, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
From: Y.B. Lu @ 2016-10-27  4:34 UTC (permalink / raw)
  To: Scott Wood, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	Arnd Bergmann
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Russell King, Bhupesh Sharma,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Santosh Shilimkar,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jochen Friedrich, X.B. Xie, M.H. Lian,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Claudiu Manoil, Kumar Gala, Leo Li,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" <l
In-Reply-To: <1477501566.6812.9.camel-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org>

Hi Scott,


> -----Original Message-----
> From: Scott Wood [mailto:oss@buserror.net]
> Sent: Thursday, October 27, 2016 1:06 AM
> To: Y.B. Lu; linux-mmc@vger.kernel.org; ulf.hansson@linaro.org; Arnd
> Bergmann
> Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> foundation.org; netdev@vger.kernel.org; Mark Rutland; Rob Herring;
> Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil; Bhupesh
> Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B. Xie; M.H.
> Lian
> Subject: Re: [v12, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
> 
> On Wed, 2016-09-21 at 14:57 +0800, Yangbo Lu wrote:
> > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new
> > file mode 100644 index 0000000..b99764c
> > --- /dev/null
> > +++ b/drivers/soc/fsl/Kconfig
> > @@ -0,0 +1,19 @@
> > +#
> > +# Freescale SOC drivers
> > +#
> > +
> > +source "drivers/soc/fsl/qe/Kconfig"
> > +
> > +config FSL_GUTS
> > +	bool "Freescale QorIQ GUTS driver"
> > +	select SOC_BUS
> > +	help
> > +	  The global utilities block controls power management, I/O device
> > +	  enabling, power-onreset(POR) configuration monitoring, alternate
> > +	  function selection for multiplexed signals,and clock control.
> > +	  This driver is to manage and access global utilities block.
> > +	  Initially only reading SVR and registering soc device are
> > supported.
> > +	  Other guts accesses, such as reading RCW, should eventually be
> > moved
> > +	  into this driver as well.
> > +
> > +	  If you want GUTS driver support, you should say Y here.
> 
> This is user-enablable without dependencies, which means it will break
> some randconfigs.  If this is to be enabled via select then remove the
> text after "bool".

[Lu Yangbo-B47093] Will enable it via select and remove text after 'bool'.
 
> 
> > +/* SoC die attribute definition for QorIQ platform */ static const
> > +struct fsl_soc_die_attr fsl_soc_die[] = { #ifdef CONFIG_PPC
> > +	/*
> > +	 * Power Architecture-based SoCs T Series
> > +	 */
> > +
> > +	/* Die: T4240, SoC: T4240/T4160/T4080 */
> > +	{ .die		= "T4240",
> > +	  .svr		= 0x82400000,
> > +	  .mask		= 0xfff00000,
> > +	},
> > +	/* Die: T1040, SoC: T1040/T1020/T1042/T1022 */
> > +	{ .die		= "T1040",
> > +	  .svr		= 0x85200000,
> > +	  .mask		= 0xfff00000,
> > +	},
> > +	/* Die: T2080, SoC: T2080/T2081 */
> > +	{ .die		= "T2080",
> > +	  .svr		= 0x85300000,
> > +	  .mask		= 0xfff00000,
> > +	},
> > +	/* Die: T1024, SoC: T1024/T1014/T1023/T1013 */
> > +	{ .die		= "T1024",
> > +	  .svr		= 0x85400000,
> > +	  .mask		= 0xfff00000,
> > +	},
> > +#endif /* CONFIG_PPC */
> > +#if defined(CONFIG_ARCH_MXC) || defined(CONFIG_ARCH_LAYERSCAPE)
> 
> Will this driver ever be probed on MXC?  Why do we need these ifdefs at
> all?

[Lu Yangbo-B47093] Will remove them. In the previous version, we use too many members for soc definition, so I add #ifdef for ARCH. 
CONFIG_ARCH_MXC was for ls1021a.

> 
> 
> > +	/*
> > +	 * ARM-based SoCs LS Series
> > +	 */
> > +
> > +	/* Die: LS1043A, SoC: LS1043A/LS1023A */
> > +	{ .die		= "LS1043A",
> > +	  .svr		= 0x87920000,
> > +	  .mask		= 0xffff0000,
> > +	},
> > +	/* Die: LS2080A, SoC: LS2080A/LS2040A/LS2085A */
> > +	{ .die		= "LS2080A",
> > +	  .svr		= 0x87010000,
> > +	  .mask		= 0xff3f0000,
> > +	},
> > +	/* Die: LS1088A, SoC: LS1088A/LS1048A/LS1084A/LS1044A */
> > +	{ .die		= "LS1088A",
> > +	  .svr		= 0x87030000,
> > +	  .mask		= 0xff3f0000,
> > +	},
> > +	/* Die: LS1012A, SoC: LS1012A */
> > +	{ .die		= "LS1012A",
> > +	  .svr		= 0x87040000,
> > +	  .mask		= 0xffff0000,
> > +	},
> > +	/* Die: LS1046A, SoC: LS1046A/LS1026A */
> > +	{ .die		= "LS1046A",
> > +	  .svr		= 0x87070000,
> > +	  .mask		= 0xffff0000,
> > +	},
> > +	/* Die: LS2088A, SoC: LS2088A/LS2048A/LS2084A/LS2044A */
> > +	{ .die		= "LS2088A",
> > +	  .svr		= 0x87090000,
> > +	  .mask		= 0xff3f0000,
> > +	},
> > +	/* Die: LS1021A, SoC: LS1021A/LS1020A/LS1022A
> > +	 * Note: Put this die at the end in cause of incorrect
> > identification
> > +	 */
> > +	{ .die		= "LS1021A",
> > +	  .svr		= 0x87000000,
> > +	  .mask		= 0xfff00000,
> > +	},
> > +#endif /* CONFIG_ARCH_MXC || CONFIG_ARCH_LAYERSCAPE */
> 
> Instead of relying on ordering, add more bits to the mask so that there's
> no overlap.  I think 0xfff70000 would work.

[Lu Yangbo-B47093] Ok, Will do that. Then we add 3 bits of 'Various Personalities' field for ls1021a die identification.

> 
> > +out:
> > +	kfree(soc_dev_attr.machine);
> > +	kfree(soc_dev_attr.family);
> > +	kfree(soc_dev_attr.soc_id);
> > +	kfree(soc_dev_attr.revision);
> > +	iounmap(guts->regs);
> > +out_free:
> > +	kfree(guts);
> > +	return ret;
> > +}
> 
> Please use devm.

[Lu Yangbo-B47093] Sorry for forgetting this. Will do that and send out the new version soon.
Thanks for your comments.

> 
> -Scott

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply

* Re: [PATCH] net: avoid uninitialized variable
From: Gao Feng @ 2016-10-27  4:02 UTC (permalink / raw)
  To: zhongjiang
  Cc: David S. Miller, Alex Duyck, Tom Herbert, jiri, hadarh, amir,
	Linux Kernel Network Developers, linux-kernel
In-Reply-To: <1477540569-12199-1-git-send-email-zhongjiang@huawei.com>

On Thu, Oct 27, 2016 at 11:56 AM, zhongjiang <zhongjiang@huawei.com> wrote:
> From: zhong jiang <zhongjiang@huawei.com>
>
> when I compiler the newest kernel, I hit the following error with
> Werror=may-uninitalized.
>
> net/core/flow_dissector.c: In function ?._skb_flow_dissect?
> include/uapi/linux/swab.h:100:46: error: ?.lan?.may be used uninitialized in this function [-Werror=maybe-uninitialized]
> net/core/flow_dissector.c:248:26: note: ?.lan?.was declared here
>
> This adds an additional check for proto to explicitly tell the compiler
> that vlan pointer have the correct value before it is used.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
>  net/core/flow_dissector.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 1a7b80f..a04d9cf 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -245,7 +245,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>         }
>         case htons(ETH_P_8021AD):
>         case htons(ETH_P_8021Q): {
> -               const struct vlan_hdr *vlan;
> +               const struct vlan_hdr *vlan = NULL;
>
>                 if (skb_vlan_tag_present(skb))
>                         proto = skb->protocol;
> @@ -276,7 +276,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>                                 key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
>                                 key_vlan->vlan_priority =
>                                         (skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);
> -                       } else {
> +                       } else if (proto == cpu_to_be16(ETH_P_8021Q) ||
> +                                               proto == cpu_to_be16(ETH_P_8021AD)) {
>                                 key_vlan->vlan_id = ntohs(vlan->h_vlan_TCI) &
>                                         VLAN_VID_MASK;
>                                 key_vlan->vlan_priority =
> --
> 1.8.3.1
>

It seems there is one similar patch already.
You could refer to https://patchwork.kernel.org/patch/9389565/

Regards
Feng

^ permalink raw reply

* [PATCH] net: avoid uninitialized variable
From: zhongjiang @ 2016-10-27  3:56 UTC (permalink / raw)
  To: davem, aduyck, tom, jiri, fgao, hadarh, amir; +Cc: netdev, linux-kernel

From: zhong jiang <zhongjiang@huawei.com>

when I compiler the newest kernel, I hit the following error with
Werror=may-uninitalized.

net/core/flow_dissector.c: In function ?._skb_flow_dissect?
include/uapi/linux/swab.h:100:46: error: ?.lan?.may be used uninitialized in this function [-Werror=maybe-uninitialized]
net/core/flow_dissector.c:248:26: note: ?.lan?.was declared here

This adds an additional check for proto to explicitly tell the compiler
that vlan pointer have the correct value before it is used.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 net/core/flow_dissector.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 1a7b80f..a04d9cf 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -245,7 +245,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	}
 	case htons(ETH_P_8021AD):
 	case htons(ETH_P_8021Q): {
-		const struct vlan_hdr *vlan;
+		const struct vlan_hdr *vlan = NULL;
 
 		if (skb_vlan_tag_present(skb))
 			proto = skb->protocol;
@@ -276,7 +276,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 				key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
 				key_vlan->vlan_priority =
 					(skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);
-			} else {
+			} else if (proto == cpu_to_be16(ETH_P_8021Q) ||
+						proto == cpu_to_be16(ETH_P_8021AD)) {
 				key_vlan->vlan_id = ntohs(vlan->h_vlan_TCI) &
 					VLAN_VID_MASK;
 				key_vlan->vlan_priority =
-- 
1.8.3.1

^ permalink raw reply related

* Re: [RFC net-next iproute2 1/2] libnetlink: Add test for error code returned from netlink reply
From: Stephen Hemminger @ 2016-10-27  3:52 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: netdev, Eric Dumazet, David Ahern, Andrey Vagin
In-Reply-To: <1477510208-20292-2-git-send-email-gorcunov@gmail.com>

On Wed, 26 Oct 2016 22:30:07 +0300
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

>  
>  				if (h->nlmsg_type == NLMSG_DONE) {
> +					int len;
> +
> +					/*
> +					 * The kernel reports if there is
> +					 * no inet-diag module present in
> +					 * the system via negative length
> +					 * as error code.
> +					 */
> +					if (h->nlmsg_len < NLMSG_LENGTH(sizeof(int))) {
> +						fprintf(stderr, "Truncated length reply\n");
> +						return -1;
> +					}
> +					len = *(int *)NLMSG_DATA(h);
> +					if (len < 0) {
> +						errno = -len;
> +						if (errno == ENOENT ||
> +						    errno == EOPNOTSUPP)
> +							return -1;
> +						perror("RTNETLINK answers");
> +						return len;
> +					}
>  					found_done = 1;
>  					break; /* process next filter */
>  				

This looks like a mistake in how you implemented the functionality in the kernel.
Despite what it looks like, all netlink request/reply functionality reports
errors in current implementation by returning error to the sendmsg request.

What you added implies that the new kernel api is wrong, or many other usages
are wrong.  Please fix the kernel.

^ permalink raw reply

* Re: [PATCH v7 0/6] Add eBPF hooks for cgroups
From: Alexei Starovoitov @ 2016-10-27  3:35 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Daniel Mack, htejun, daniel, ast, davem, kafai, fw, harald,
	netdev, sargun, cgroups
In-Reply-To: <20161026195933.GA2031@salvia>

On Wed, Oct 26, 2016 at 09:59:33PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Oct 25, 2016 at 12:14:08PM +0200, Daniel Mack wrote:
> [...]
> >   Dumping programs once they are installed is problematic because of
> >   the internal optimizations done to the eBPF program during its
> >   lifetime. Also, the references to maps etc. would need to be
> >   restored during the dump.
> > 
> >   Just exposing whether or not a program is attached would be
> >   trivial to do, however, most easily through another bpf(2)
> >   command. That can be added later on though.
> 
> I don't know if anyone told you, but during last netconf, this topic
> took a bit of time of discussion and it was controversial, I would say
> 1/3 of netdev hackers there showed their concerns, and that's
> something that should not be skipped IMO.

Though I attended netconf over hangouts, I think it was pretty
clear that bpf needs 'introspection' of loaded bpf programs and it
was a universal desire of everyone. Not 1/3 of hackers.
As commit log says it's an orthogonal work and over the last
month we've been discussing pros and cons of different approaches.
The audit infra, tracepoints and other ideas.
We kept the discussion in private because, unfortunately, public
discussions are not fruitful due to threads like this one.

The further points below were disputed many times in the past.
Let's address them one more time:

> path. But this is adding hooks to push bpf programs in the middle of
> our generic stack, this is way different domain.

incorrect. look at socket filters, cls_bpf.
bpf was running in the middle of the stack for years.
Even unix pipe and netfilter can be filtered with bpf.

> around this socket code in functions so we can invoke it. I guess
> filtering of UDP and TCP should be good for you at this stage.

DanielM mentioned few times that it's not only about UDP and TCP.

> This
> would require more work though, but this would come with no hooks in
> the stack and packets will not have to consume *lots of cycles* just
> to be dropped before entering the socket queue.

packets don't consume 'lost of cycles'. This is not a typical
n-tuple firewall framework. Not a DoS mitigation either. Please read
the cover letter and earlier submissions.
It's a framework centered around cgroups.
_Nothing_ in the current stack provides cgroup based monitoring
and application protection. Earlier cgroupv1 controllers don't
scale and we really cannot have more of v1 net controllers.
At the same time we've been brainstorming how this patch set
can work with v1. It's not easy. We're not giving up though.
For now it's v2 only.
Note that another two patchsets depend on this core cgroup+bpf framework.
It's not about hooks (socket/skb inspections points).
Both rx and tx hooks _can_ be moved in the future.
Unlike netfilter hooks, the hooks in patches 4 and 5 can be moved
if we find better place for them. It won't affect userspace.
The only assumption of BPF_CGROUP_INET_[E|IN]GRESS is that
there will be some place in packet delivery from nic to userspace
where bpf program can look at L3+ packet only if application is
under particular cgroup. Patches 4 and 5 are the best places so far.
It took months to converge on these points. Please see earlier
discussions on pro/con of every other place we've tried.

> How useful can be to drop lots of unwanted traffic at such a late
> stage? How would the performance numbers to drop packets would look
> like? Extremely bad, I predict.

facts check please. Daniel provided numbers before that show
excellent performance.

^ permalink raw reply

* Re: [PATCH net] MAINTAINERS: Update qlogic networking drivers
From: David Miller @ 2016-10-27  3:29 UTC (permalink / raw)
  To: Yuval.Mintz
  Cc: netdev, ameen.rahman, Ariel.Elior, harish.patil, rasesh.mody,
	Manish.Chopra, Sudarsana.Kalluru, rahul.verma
In-Reply-To: <1477288089-3894-1-git-send-email-Yuval.Mintz@cavium.com>

From: Yuval Mintz <Yuval.Mintz@cavium.com>
Date: Mon, 24 Oct 2016 08:48:09 +0300

> Following Cavium's acquisition of qlogic we need to update all the qlogic
> drivers maintainer's entries to point to our new e-mail addresses,
> as well as update some of the driver's maintainers as those are no longer
> working for Cavium.
> 
> I would like to thank Sony Chacko and Rajesh Borundia for their support
> and development of our various networking drivers.
> 
> Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] netvsc: fix incorrect receive checksum offloading
From: David Miller @ 2016-10-27  3:19 UTC (permalink / raw)
  To: stephen; +Cc: kys, haiyangz, netdev
In-Reply-To: <20161023213247.75d43c48@xeon-e3>

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 23 Oct 2016 21:32:47 -0700

> From: Stephen Hemminger <sthemmin@microsoft.com>
> 
> The Hyper-V netvsc driver was looking at the incorrect status bits
> in the checksum info. It was setting the receive checksum unnecessary
> flag based on the IP header checksum being correct. The checksum
> flag is skb is about TCP and UDP checksum status. Because of this
> bug, any packet received with bad TCP checksum would be passed
> up the stack and to the application causing data corruption.
> The problem is reproducible via netcat and netem.
> 
> This had a side effect of not doing receive checksum offload
> on IPv6. The driver was also also always doing checksum offload
> independent of the checksum setting done via ethtool.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net-next]ldmvsw: tx queue stuck in stopped state after LDC reset
From: David Miller @ 2016-10-27  3:11 UTC (permalink / raw)
  To: Aaron.Young; +Cc: netdev, sowmini.varadhan
In-Reply-To: <b8d6abc7188e5cac885905854067444cb89a5f3b.1477503707.git.Aaron.Young@oracle.com>

From: Aaron Young <Aaron.Young@oracle.com>
Date: Wed, 26 Oct 2016 20:15:22 -0400

>    From: Aaron Young <aaron.young@oracle.com>
> 
>    The following patch fixes an issue with the ldmvsw driver where
>    the network connection of a guest domain becomes non-functional after
>    a guest domain has panic'd and rebooted (resulting in a LDC reset).
> 

Please fix this indentation, make this text begin on the
first column of every line.

I bet I know how this got this way, you copy and pasted the
commit message from "git show" output or similar and didn't
remove the indentation that adds.

^ permalink raw reply

* Re: [PATCH v2 2/5] kconfig: introduce the "suggest" keyword
From: Nicolas Pitre @ 2016-10-27  2:39 UTC (permalink / raw)
  To: Paul Bolle
  Cc: John Stultz, Richard Cochran, Michal Marek, Thomas Gleixner,
	Josh Triplett, Edward Cree, netdev, linux-kbuild, linux-kernel
In-Reply-To: <1477527045.2121.26.camel@tiscali.nl>

On Thu, 27 Oct 2016, Paul Bolle wrote:

> On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote:
> > Similar to "imply" but with no added restrictions on the target symbol's
> > value. Useful for providing a default value to another symbol.
> > 
> > Suggested by Edward Cree.
> > 
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> 
> As far as I can see this series doesn't add a user of "suggest". So I
> see no reason to add it.
> 
> NAK.

Fine.

Given the discussion from the "imply" patch I thought "suggest" could 
become popular. But if/when someone actually wants it then this patch 
could be picked up later.


Nicolas

^ permalink raw reply

* [PATCH net-next v2 1/1] driver: tun: Use new macro SOCK_IOC_TYPE instead of literal number 0x89
From: fgao @ 2016-10-27  1:05 UTC (permalink / raw)
  To: davem, jasowang, edumazet, pabeni, netdev; +Cc: gfree.wind, Gao Feng

From: Gao Feng <fgao@ikuai8.com>

The current codes use _IOC_TYPE(cmd) == 0x89 to check if the cmd is one
socket ioctl command like SIOCGIFHWADDR. But the literal number 0x89 may
confuse readers. So create one macro SOCK_IOC_TYPE to enhance the readability.

Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
 v2: Rename the macro SOCK_IOC_MAGIC to SOCK_IOC_TYPE
 v1: Initial version

 drivers/net/tun.c            | 2 +-
 include/uapi/linux/sockios.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 9328568..bb49ccb 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1985,7 +1985,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	int le;
 	int ret;
 
-	if (cmd == TUNSETIFF || cmd == TUNSETQUEUE || _IOC_TYPE(cmd) == 0x89) {
+	if (cmd == TUNSETIFF || cmd == TUNSETQUEUE || _IOC_TYPE(cmd) == SOCK_IOC_TYPE) {
 		if (copy_from_user(&ifr, argp, ifreq_len))
 			return -EFAULT;
 	} else {
diff --git a/include/uapi/linux/sockios.h b/include/uapi/linux/sockios.h
index 8e7890b..c977d0e 100644
--- a/include/uapi/linux/sockios.h
+++ b/include/uapi/linux/sockios.h
@@ -24,6 +24,8 @@
 #define SIOCINQ		FIONREAD
 #define SIOCOUTQ	TIOCOUTQ        /* output queue size (not sent + not acked) */
 
+#define SOCK_IOC_TYPE	0x89
+
 /* Routing table calls. */
 #define SIOCADDRT	0x890B		/* add routing table entry	*/
 #define SIOCDELRT	0x890C		/* delete routing table entry	*/
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 net-next 5/5] samples: bpf: add userspace example for modifying sk_bound_dev_if
From: David Ahern @ 2016-10-27  0:58 UTC (permalink / raw)
  To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern
In-Reply-To: <1477529922-4806-1-git-send-email-dsa@cumulusnetworks.com>

Add a simple program to demonstrate the ability to attach a bpf program
to a cgroup that sets sk_bound_dev_if for AF_INET{6} sockets when they
are created.

v2
- removed bpf_sock_store_u32 references
- changed BPF_CGROUP_INET_SOCK_CREATE to BPF_CGROUP_INET_SOCK
- remove BPF_PROG_TYPE_CGROUP_SOCK prog type and add prog_subtype

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 samples/bpf/Makefile          |  2 ++
 samples/bpf/test_cgrp2_sock.c | 84 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 samples/bpf/test_cgrp2_sock.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 2624d5d7ce8b..ec4ef37a2dbc 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -22,6 +22,7 @@ hostprogs-y += map_perf_test
 hostprogs-y += test_overhead
 hostprogs-y += test_cgrp2_array_pin
 hostprogs-y += test_cgrp2_attach
+hostprogs-y += test_cgrp2_sock
 hostprogs-y += xdp1
 hostprogs-y += xdp2
 hostprogs-y += test_current_task_under_cgroup
@@ -48,6 +49,7 @@ map_perf_test-objs := bpf_load.o libbpf.o map_perf_test_user.o
 test_overhead-objs := bpf_load.o libbpf.o test_overhead_user.o
 test_cgrp2_array_pin-objs := libbpf.o test_cgrp2_array_pin.o
 test_cgrp2_attach-objs := libbpf.o test_cgrp2_attach.o
+test_cgrp2_sock-objs := libbpf.o test_cgrp2_sock.o
 xdp1-objs := bpf_load.o libbpf.o xdp1_user.o
 # reuse xdp1 source intentionally
 xdp2-objs := bpf_load.o libbpf.o xdp1_user.o
diff --git a/samples/bpf/test_cgrp2_sock.c b/samples/bpf/test_cgrp2_sock.c
new file mode 100644
index 000000000000..9cc3307052e0
--- /dev/null
+++ b/samples/bpf/test_cgrp2_sock.c
@@ -0,0 +1,84 @@
+/* eBPF example program:
+ *
+ * - Loads eBPF program
+ *
+ *   The eBPF program sets the sk_bound_dev_if index in new AF_INET{6}
+ *   sockets opened by processes in the cgroup.
+ *
+ * - Attaches the new program to a cgroup using BPF_PROG_ATTACH
+ */
+
+#define _GNU_SOURCE
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <string.h>
+#include <unistd.h>
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/bpf.h>
+
+#include "libbpf.h"
+
+static int prog_load(int idx)
+{
+	struct bpf_insn prog[] = {
+		BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+		BPF_MOV64_IMM(BPF_REG_3, idx),
+		BPF_MOV64_IMM(BPF_REG_2, offsetof(struct bpf_sock, bound_dev_if)),
+		BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_3, offsetof(struct bpf_sock, bound_dev_if)),
+		BPF_MOV64_IMM(BPF_REG_0, 1), /* r0 = verdict */
+		BPF_EXIT_INSN(),
+	};
+	union bpf_prog_subtype prog_subtype = { .cgroup.sock = 1 };
+
+	return bpf_prog_load(BPF_PROG_TYPE_CGROUP, prog, sizeof(prog), "GPL",
+			     0, &prog_subtype);
+}
+
+static int usage(const char *argv0)
+{
+	printf("Usage: %s <cg-path> device-index\n", argv0);
+	return EXIT_FAILURE;
+}
+
+int main(int argc, char **argv)
+{
+	int cg_fd, prog_fd, ret;
+	int idx = 0;
+
+	if (argc < 2)
+		return usage(argv[0]);
+
+	idx = atoi(argv[2]);
+	if (!idx) {
+		printf("Invalid device index\n");
+		return EXIT_FAILURE;
+	}
+
+	cg_fd = open(argv[1], O_DIRECTORY | O_RDONLY);
+	if (cg_fd < 0) {
+		printf("Failed to open cgroup path: '%s'\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	prog_fd = prog_load(idx);
+	printf("Output from kernel verifier:\n%s\n-------\n", bpf_log_buf);
+
+	if (prog_fd < 0) {
+		printf("Failed to load prog: '%s'\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = bpf_prog_detach(cg_fd, BPF_CGROUP_INET_SOCK);
+	ret = bpf_prog_attach(prog_fd, cg_fd, BPF_CGROUP_INET_SOCK);
+	if (ret < 0) {
+		printf("Failed to attach prog to cgroup: '%s'\n",
+		       strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	return EXIT_SUCCESS;
+}
-- 
2.1.4

^ permalink raw reply related

* [PATCH net-next 4/5] samples: bpf: Add prog_subtype to bpf_prog_load
From: David Ahern @ 2016-10-27  0:58 UTC (permalink / raw)
  To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern
In-Reply-To: <1477529922-4806-1-git-send-email-dsa@cumulusnetworks.com>

Add bpf_prog_subtype argument to bpf_prog_load. If arg is non-NULL,
it is added to the attr passed to the bpf system call.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 samples/bpf/bpf_load.c     | 2 +-
 samples/bpf/fds_example.c  | 2 +-
 samples/bpf/libbpf.c       | 5 ++++-
 samples/bpf/libbpf.h       | 3 ++-
 samples/bpf/sock_example.c | 2 +-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index 97913e109b14..634fcd7f7498 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -77,7 +77,7 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
 		return -1;
 	}
 
-	fd = bpf_prog_load(prog_type, prog, size, license, kern_version);
+	fd = bpf_prog_load(prog_type, prog, size, license, kern_version, NULL);
 	if (fd < 0) {
 		printf("bpf_prog_load() err=%d\n%s", errno, bpf_log_buf);
 		return -1;
diff --git a/samples/bpf/fds_example.c b/samples/bpf/fds_example.c
index 625e797be6ef..df38b68f3586 100644
--- a/samples/bpf/fds_example.c
+++ b/samples/bpf/fds_example.c
@@ -59,7 +59,7 @@ static int bpf_prog_create(const char *object)
 		return prog_fd[0];
 	} else {
 		return bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER,
-				     insns, sizeof(insns), "GPL", 0);
+				     insns, sizeof(insns), "GPL", 0, NULL);
 	}
 }
 
diff --git a/samples/bpf/libbpf.c b/samples/bpf/libbpf.c
index 9ce707bf02a7..53987b1ab191 100644
--- a/samples/bpf/libbpf.c
+++ b/samples/bpf/libbpf.c
@@ -82,7 +82,8 @@ char bpf_log_buf[LOG_BUF_SIZE];
 
 int bpf_prog_load(enum bpf_prog_type prog_type,
 		  const struct bpf_insn *insns, int prog_len,
-		  const char *license, int kern_version)
+		  const char *license, int kern_version,
+		  union bpf_prog_subtype *prog_subtype)
 {
 	union bpf_attr attr = {
 		.prog_type = prog_type,
@@ -98,6 +99,8 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
 	 * padding is zero initialized
 	 */
 	attr.kern_version = kern_version;
+	if (prog_subtype)
+		attr.prog_subtype = *prog_subtype;
 
 	bpf_log_buf[0] = 0;
 
diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h
index d0a799a52eaf..949c89c85c70 100644
--- a/samples/bpf/libbpf.h
+++ b/samples/bpf/libbpf.h
@@ -13,7 +13,8 @@ int bpf_get_next_key(int fd, void *key, void *next_key);
 
 int bpf_prog_load(enum bpf_prog_type prog_type,
 		  const struct bpf_insn *insns, int insn_len,
-		  const char *license, int kern_version);
+		  const char *license, int kern_version,
+		  union bpf_prog_subtype *prog_subtype);
 
 int bpf_prog_attach(int prog_fd, int attachable_fd, enum bpf_attach_type type);
 int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
diff --git a/samples/bpf/sock_example.c b/samples/bpf/sock_example.c
index 28b60baa9fa8..521f918ab34d 100644
--- a/samples/bpf/sock_example.c
+++ b/samples/bpf/sock_example.c
@@ -56,7 +56,7 @@ static int test_sock(void)
 	};
 
 	prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, prog, sizeof(prog),
-				"GPL", 0);
+				"GPL", 0, NULL);
 	if (prog_fd < 0) {
 		printf("failed to load prog '%s'\n", strerror(errno));
 		goto cleanup;
-- 
2.1.4

^ permalink raw reply related

* [PATCH v2 net-next 3/5] bpf: Add new cgroup attach type to enable sock modifications
From: David Ahern @ 2016-10-27  0:58 UTC (permalink / raw)
  To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern
In-Reply-To: <1477529922-4806-1-git-send-email-dsa@cumulusnetworks.com>

Allow BPF_PROG_TYPE_CGROUP programs with cgroup.sock subtype to modify
sk_bound_dev_if for newly created AF_INET or AF_INET6 sockets. The program
can be attached to a cgroup using attach type BPF_CGROUP_INET_SOCK. The
cgroup verifier ops are updated to handle the sock offsets as well as the
existing skb accesses.

This allows a cgroup to be configured such that AF_INET{6} sockets opened
by processes are automatically bound to a specific device. In turn, this
enables the running of programs that do not support SO_BINDTODEVICE in a
specific VRF context / L3 domain.

v2
- dropped the bpf_sock_store_u32 helper
- dropped the new prog type BPF_PROG_TYPE_CGROUP_SOCK
- moved valid access and context conversion to use subtype
- dropped CREATE from BPF_CGROUP_INET_SOCK and related function names
- moved running of filter from sk_alloc to inet{6}_create

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/linux/filter.h   |  2 +-
 include/uapi/linux/bpf.h |  5 ++++
 kernel/bpf/cgroup.c      |  9 ++++++
 kernel/bpf/syscall.c     |  2 ++
 net/core/filter.c        | 77 ++++++++++++++++++++++++++++++++++++++++++++++--
 net/ipv4/af_inet.c       |  4 +++
 net/ipv6/af_inet6.c      |  3 ++
 7 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 88470cdd3ee1..ffde714f3a98 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -409,7 +409,7 @@ struct bpf_prog {
 	union bpf_prog_subtype	subtype;	/* For fine-grained verifications */
 	struct bpf_prog_aux	*aux;		/* Auxiliary fields */
 	struct sock_fprog_kern	*orig_prog;	/* Original BPF program */
-	unsigned int		(*bpf_func)(const struct sk_buff *skb,
+	unsigned int		(*bpf_func)(const void *ctx,
 					    const struct bpf_insn *filter);
 	/* Instructions for interpreter */
 	union {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 160c24ffdce2..546e84b1792f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -104,6 +104,7 @@ enum bpf_prog_type {
 enum bpf_attach_type {
 	BPF_CGROUP_INET_INGRESS,
 	BPF_CGROUP_INET_EGRESS,
+	BPF_CGROUP_INET_SOCK,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -532,6 +533,10 @@ struct bpf_tunnel_key {
 	__u32 tunnel_label;
 };
 
+struct bpf_sock {
+	__u32 bound_dev_if;
+};
+
 /* User return codes for XDP prog type.
  * A valid XDP program must return one of these defined values. All other
  * return codes are reserved for future use. Unknown return codes will result
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index d5746aec8f34..796e39aa28f5 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -117,6 +117,12 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
 	}
 }
 
+static int __cgroup_bpf_run_filter_sock(struct sock *sk,
+					struct bpf_prog *prog)
+{
+	return prog->bpf_func(sk, prog->insnsi) == 1 ? 0 : -EPERM;
+}
+
 static int __cgroup_bpf_run_filter_skb(struct sk_buff *skb,
 				       struct bpf_prog *prog)
 {
@@ -171,6 +177,9 @@ int __cgroup_bpf_run_filter(struct sock *sk,
 		case BPF_CGROUP_INET_EGRESS:
 			ret = __cgroup_bpf_run_filter_skb(skb, prog);
 			break;
+		case BPF_CGROUP_INET_SOCK:
+			ret = __cgroup_bpf_run_filter_sock(sk, prog);
+			break;
 		/* make gcc happy else complains about missing enum value */
 		default:
 			return 0;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index fbf81156e49d..bc3be0b19b57 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -843,6 +843,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
 	switch (attr->attach_type) {
 	case BPF_CGROUP_INET_INGRESS:
 	case BPF_CGROUP_INET_EGRESS:
+	case BPF_CGROUP_INET_SOCK:
 		prog = bpf_prog_get_type(attr->attach_bpf_fd,
 					 BPF_PROG_TYPE_CGROUP);
 		if (IS_ERR(prog))
@@ -880,6 +881,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
 	switch (attr->attach_type) {
 	case BPF_CGROUP_INET_INGRESS:
 	case BPF_CGROUP_INET_EGRESS:
+	case BPF_CGROUP_INET_SOCK:
 		cgrp = cgroup_get_from_fd(attr->target_fd);
 		if (IS_ERR(cgrp))
 			return PTR_ERR(cgrp);
diff --git a/net/core/filter.c b/net/core/filter.c
index 4207ab2e56ba..7193eb7fe892 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2634,6 +2634,40 @@ static bool sk_filter_is_valid_access(int off, int size,
 	return __is_valid_access(off, size, type);
 }
 
+static bool sock_filter_is_valid_access(int off, int size,
+					enum bpf_access_type type)
+{
+	if (type == BPF_WRITE) {
+		switch (off) {
+		case offsetof(struct bpf_sock, bound_dev_if):
+			break;
+		default:
+			return false;
+		}
+	}
+
+	if (off < 0 || off + size > sizeof(struct bpf_sock))
+		return false;
+
+	/* The verifier guarantees that size > 0. */
+	if (off % size != 0)
+		return false;
+
+	return true;
+}
+
+static bool cgroup_is_valid_access(int off, int size,
+				   enum bpf_access_type type,
+				   enum bpf_reg_type *reg_type,
+				   union bpf_prog_subtype *prog_subtype)
+{
+	if (prog_subtype->cgroup.sock)
+		return sock_filter_is_valid_access(off, size, type);
+
+	return sk_filter_is_valid_access(off, size, type, reg_type,
+					 prog_subtype);
+}
+
 static int tc_cls_act_prologue(struct bpf_insn *insn_buf, bool direct_write,
 			       const struct bpf_prog *prog)
 {
@@ -2894,6 +2928,45 @@ static u32 sk_filter_convert_ctx_access(enum bpf_access_type type, int dst_reg,
 	return insn - insn_buf;
 }
 
+static u32 sock_filter_convert_ctx_access(enum bpf_access_type type,
+					  int dst_reg, int src_reg,
+					  int ctx_off,
+					  struct bpf_insn *insn_buf,
+					  struct bpf_prog *prog)
+{
+	struct bpf_insn *insn = insn_buf;
+
+	switch (ctx_off) {
+	case offsetof(struct bpf_sock, bound_dev_if):
+		BUILD_BUG_ON(FIELD_SIZEOF(struct sock, sk_bound_dev_if) != 4);
+
+		if (type == BPF_WRITE)
+			*insn++ = BPF_STX_MEM(BPF_W, dst_reg, src_reg,
+					offsetof(struct sock, sk_bound_dev_if));
+		else
+			*insn++ = BPF_LDX_MEM(BPF_W, dst_reg, src_reg,
+				      offsetof(struct sock, sk_bound_dev_if));
+		break;
+	}
+
+	return insn - insn_buf;
+}
+
+static u32 cgroup_convert_ctx_access(enum bpf_access_type type, int dst_reg,
+				     int src_reg, int ctx_off,
+				     struct bpf_insn *insn_buf,
+				     struct bpf_prog *prog)
+{
+	union bpf_prog_subtype *prog_subtype = &prog->subtype;
+
+	if (prog_subtype->cgroup.sock)
+		return sock_filter_convert_ctx_access(type, dst_reg, src_reg,
+						      ctx_off, insn_buf, prog);
+
+	return sk_filter_convert_ctx_access(type, dst_reg, src_reg, ctx_off,
+					    insn_buf, prog);
+}
+
 static u32 tc_cls_act_convert_ctx_access(enum bpf_access_type type, int dst_reg,
 					 int src_reg, int ctx_off,
 					 struct bpf_insn *insn_buf,
@@ -2963,8 +3036,8 @@ static const struct bpf_verifier_ops xdp_ops = {
 
 static const struct bpf_verifier_ops cgroup_ops = {
 	.get_func_proto		= cgroup_func_proto,
-	.is_valid_access	= sk_filter_is_valid_access,
-	.convert_ctx_access	= sk_filter_convert_ctx_access,
+	.is_valid_access	= cgroup_is_valid_access,
+	.convert_ctx_access	= cgroup_convert_ctx_access,
 };
 
 static struct bpf_prog_type_list sk_filter_type __read_mostly = {
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1effc986739e..c0934f7483cb 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -377,6 +377,10 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
 		if (err)
 			sk_common_release(sk);
 	}
+
+	if (!kern)
+		cgroup_bpf_run_filter(sk, NULL, BPF_CGROUP_INET_SOCK);
+
 out:
 	return err;
 out_rcu_unlock:
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 46ad699937fd..c499ae3c472e 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -257,6 +257,9 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
 			goto out;
 		}
 	}
+
+	if (!kern)
+		cgroup_bpf_run_filter(sk, NULL, BPF_CGROUP_INET_SOCK);
 out:
 	return err;
 out_rcu_unlock:
-- 
2.1.4

^ permalink raw reply related

* [PATCH net-next 2/5] bpf: Add eBPF program subtype and is_valid_subtype() verifier
From: David Ahern @ 2016-10-27  0:58 UTC (permalink / raw)
  To: netdev
  Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern,
	Mickaël Salaün
In-Reply-To: <1477529922-4806-1-git-send-email-dsa@cumulusnetworks.com>

The program subtype's goal is to be able to have different static
fine-grained verifications for a unique program type.

The struct bpf_verifier_ops gets a new optional function:
is_valid_subtype(). This new verifier is called at the begening of the
eBPF program verification to check if the (optional) program subtype is
valid.

For now, only Landlock eBPF programs are using a program subtype but
this could be used by other program types in the future.

Cf. the next commit to see how the subtype is used by Landlock LSM.

Changes since v3:
* remove the "origin" field
* add an "option" field
* cleanup comments

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
[ Patch from Mickaël modified to add subtype arg to pe_prog_is_valid_access
  which was missing, removed the landlock references since they are not
  relevant for this use case and added cgroup struct to subtype, and
  fixed some checkpatch errors ]

---
 include/linux/bpf.h      |  7 +++++--
 include/linux/filter.h   |  1 +
 include/uapi/linux/bpf.h |  8 ++++++++
 kernel/bpf/syscall.c     |  7 +++++--
 kernel/bpf/verifier.c    | 10 ++++++++--
 kernel/trace/bpf_trace.c | 16 +++++++++++-----
 net/core/filter.c        | 26 ++++++++++++++++----------
 7 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index edcd96ded8aa..1c602a0938e8 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -152,18 +152,21 @@ struct bpf_prog;
 
 struct bpf_verifier_ops {
 	/* return eBPF function prototype for verification */
-	const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id);
+	const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id,
+					union bpf_prog_subtype *prog_subtype);
 
 	/* return true if 'size' wide access at offset 'off' within bpf_context
 	 * with 'type' (read or write) is allowed
 	 */
 	bool (*is_valid_access)(int off, int size, enum bpf_access_type type,
-				enum bpf_reg_type *reg_type);
+				enum bpf_reg_type *reg_type,
+				union bpf_prog_subtype *prog_subtype);
 	int (*gen_prologue)(struct bpf_insn *insn, bool direct_write,
 			    const struct bpf_prog *prog);
 	u32 (*convert_ctx_access)(enum bpf_access_type type, int dst_reg,
 				  int src_reg, int ctx_off,
 				  struct bpf_insn *insn, struct bpf_prog *prog);
+	bool (*is_valid_subtype)(union bpf_prog_subtype *prog_subtype);
 };
 
 struct bpf_prog_type_list {
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 1f09c521adfe..88470cdd3ee1 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -406,6 +406,7 @@ struct bpf_prog {
 	kmemcheck_bitfield_end(meta);
 	u32			len;		/* Number of filter blocks */
 	enum bpf_prog_type	type;		/* Type of BPF program */
+	union bpf_prog_subtype	subtype;	/* For fine-grained verifications */
 	struct bpf_prog_aux	*aux;		/* Auxiliary fields */
 	struct sock_fprog_kern	*orig_prog;	/* Original BPF program */
 	unsigned int		(*bpf_func)(const struct sk_buff *skb,
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 73da296c2125..160c24ffdce2 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -118,6 +118,13 @@ enum bpf_attach_type {
 
 #define BPF_F_NO_PREALLOC	(1U << 0)
 
+union bpf_prog_subtype {
+	struct {
+		__u32 sock:1,    /* ebpf prog applies to sock struct */
+		      reserved:31;
+	} cgroup;
+} __attribute__((aligned(8)));
+
 union bpf_attr {
 	struct { /* anonymous struct used by BPF_MAP_CREATE command */
 		__u32	map_type;	/* one of enum bpf_map_type */
@@ -146,6 +153,7 @@ union bpf_attr {
 		__u32		log_size;	/* size of user buffer */
 		__aligned_u64	log_buf;	/* user supplied buffer */
 		__u32		kern_version;	/* checked when prog_type=kprobe */
+		union bpf_prog_subtype prog_subtype;
 	};
 
 	struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 3c4c9ed2d576..fbf81156e49d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -578,7 +578,9 @@ static void fixup_bpf_calls(struct bpf_prog *prog)
 				continue;
 			}
 
-			fn = prog->aux->ops->get_func_proto(insn->imm);
+			fn = prog->aux->ops->get_func_proto(insn->imm,
+							   &prog->subtype);
+
 			/* all functions that have prototype and verifier allowed
 			 * programs to call them, must be real in-kernel functions
 			 */
@@ -716,7 +718,7 @@ struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type)
 EXPORT_SYMBOL_GPL(bpf_prog_get_type);
 
 /* last field in 'union bpf_attr' used by this command */
-#define	BPF_PROG_LOAD_LAST_FIELD kern_version
+#define	BPF_PROG_LOAD_LAST_FIELD prog_subtype
 
 static int bpf_prog_load(union bpf_attr *attr)
 {
@@ -774,6 +776,7 @@ static int bpf_prog_load(union bpf_attr *attr)
 	err = find_prog_type(type, prog);
 	if (err < 0)
 		goto free_prog;
+	prog->subtype = attr->prog_subtype;
 
 	/* run eBPF verifier */
 	err = bpf_check(&prog, attr);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 846d7ceaf202..e9c660363c41 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -655,7 +655,8 @@ static int check_ctx_access(struct bpf_verifier_env *env, int off, int size,
 		return 0;
 
 	if (env->prog->aux->ops->is_valid_access &&
-	    env->prog->aux->ops->is_valid_access(off, size, t, reg_type)) {
+	    env->prog->aux->ops->is_valid_access(off, size, t, reg_type,
+						&env->prog->subtype)) {
 		/* remember the offset of last byte accessed in ctx */
 		if (env->prog->aux->max_ctx_offset < off + size)
 			env->prog->aux->max_ctx_offset = off + size;
@@ -1177,7 +1178,8 @@ static int check_call(struct bpf_verifier_env *env, int func_id)
 	}
 
 	if (env->prog->aux->ops->get_func_proto)
-		fn = env->prog->aux->ops->get_func_proto(func_id);
+		fn = env->prog->aux->ops->get_func_proto(func_id,
+							&env->prog->subtype);
 
 	if (!fn) {
 		verbose("unknown func %d\n", func_id);
@@ -3076,6 +3078,10 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
 	if ((*prog)->len <= 0 || (*prog)->len > BPF_MAXINSNS)
 		return -E2BIG;
 
+	if ((*prog)->aux->ops->is_valid_subtype &&
+	    !(*prog)->aux->ops->is_valid_subtype(&(*prog)->subtype))
+		return -EINVAL;
+
 	/* 'struct bpf_verifier_env' can be global, but since it's not small,
 	 * allocate/free it every time bpf_check() is called
 	 */
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index fa77311dadb2..922bf7b99811 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -437,7 +437,9 @@ static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
 	}
 }
 
-static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func_id)
+static const
+struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func_id,
+					      union bpf_prog_subtype *prog_subtype)
 {
 	switch (func_id) {
 	case BPF_FUNC_perf_event_output:
@@ -451,7 +453,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
 
 /* bpf+kprobe programs can access fields of 'struct pt_regs' */
 static bool kprobe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
-					enum bpf_reg_type *reg_type)
+					enum bpf_reg_type *reg_type,
+					union bpf_prog_subtype *prog_subtype)
 {
 	if (off < 0 || off >= sizeof(struct pt_regs))
 		return false;
@@ -519,7 +522,8 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
-static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
+static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id,
+		union bpf_prog_subtype *prog_subtype)
 {
 	switch (func_id) {
 	case BPF_FUNC_perf_event_output:
@@ -532,7 +536,8 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
 }
 
 static bool tp_prog_is_valid_access(int off, int size, enum bpf_access_type type,
-				    enum bpf_reg_type *reg_type)
+				    enum bpf_reg_type *reg_type,
+				    union bpf_prog_subtype *prog_subtype)
 {
 	if (off < sizeof(void *) || off >= PERF_MAX_TRACE_SIZE)
 		return false;
@@ -554,7 +559,8 @@ static struct bpf_prog_type_list tracepoint_tl = {
 };
 
 static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
-				    enum bpf_reg_type *reg_type)
+				    enum bpf_reg_type *reg_type,
+				    union bpf_prog_subtype *prog_subtype)
 {
 	if (off < 0 || off >= sizeof(struct bpf_perf_event_data))
 		return false;
diff --git a/net/core/filter.c b/net/core/filter.c
index 61e229c3a862..4207ab2e56ba 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2483,7 +2483,8 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
 };
 
 static const struct bpf_func_proto *
-sk_filter_func_proto(enum bpf_func_id func_id)
+sk_filter_func_proto(enum bpf_func_id func_id,
+		     union bpf_prog_subtype *prog_subtype)
 {
 	switch (func_id) {
 	case BPF_FUNC_map_lookup_elem:
@@ -2511,7 +2512,8 @@ sk_filter_func_proto(enum bpf_func_id func_id)
 }
 
 static const struct bpf_func_proto *
-tc_cls_act_func_proto(enum bpf_func_id func_id)
+tc_cls_act_func_proto(enum bpf_func_id func_id,
+		      union bpf_prog_subtype *prog_subtype)
 {
 	switch (func_id) {
 	case BPF_FUNC_skb_store_bytes:
@@ -2565,12 +2567,12 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
 	case BPF_FUNC_skb_under_cgroup:
 		return &bpf_skb_under_cgroup_proto;
 	default:
-		return sk_filter_func_proto(func_id);
+		return sk_filter_func_proto(func_id, prog_subtype);
 	}
 }
 
 static const struct bpf_func_proto *
-xdp_func_proto(enum bpf_func_id func_id)
+xdp_func_proto(enum bpf_func_id func_id, union bpf_prog_subtype *prog_subtype)
 {
 	switch (func_id) {
 	case BPF_FUNC_perf_event_output:
@@ -2578,18 +2580,19 @@ xdp_func_proto(enum bpf_func_id func_id)
 	case BPF_FUNC_get_smp_processor_id:
 		return &bpf_get_smp_processor_id_proto;
 	default:
-		return sk_filter_func_proto(func_id);
+		return sk_filter_func_proto(func_id, prog_subtype);
 	}
 }
 
 static const struct bpf_func_proto *
-cgroup_func_proto(enum bpf_func_id func_id)
+cgroup_func_proto(enum bpf_func_id func_id,
+		  union bpf_prog_subtype *prog_subtype)
 {
 	switch (func_id) {
 	case BPF_FUNC_skb_load_bytes:
 		return &bpf_skb_load_bytes_proto;
 	default:
-		return sk_filter_func_proto(func_id);
+		return sk_filter_func_proto(func_id, prog_subtype);
 	}
 }
 
@@ -2608,7 +2611,8 @@ static bool __is_valid_access(int off, int size, enum bpf_access_type type)
 
 static bool sk_filter_is_valid_access(int off, int size,
 				      enum bpf_access_type type,
-				      enum bpf_reg_type *reg_type)
+				      enum bpf_reg_type *reg_type,
+				      union bpf_prog_subtype *prog_subtype)
 {
 	switch (off) {
 	case offsetof(struct __sk_buff, tc_classid):
@@ -2671,7 +2675,8 @@ static int tc_cls_act_prologue(struct bpf_insn *insn_buf, bool direct_write,
 
 static bool tc_cls_act_is_valid_access(int off, int size,
 				       enum bpf_access_type type,
-				       enum bpf_reg_type *reg_type)
+				       enum bpf_reg_type *reg_type,
+				       union bpf_prog_subtype *prog_subtype)
 {
 	if (type == BPF_WRITE) {
 		switch (off) {
@@ -2714,7 +2719,8 @@ static bool __is_valid_xdp_access(int off, int size,
 
 static bool xdp_is_valid_access(int off, int size,
 				enum bpf_access_type type,
-				enum bpf_reg_type *reg_type)
+				enum bpf_reg_type *reg_type,
+				union bpf_prog_subtype *prog_subtype)
 {
 	if (type == BPF_WRITE)
 		return false;
-- 
2.1.4

^ permalink raw reply related

* [PATCH v2 net-next 1/5] bpf: Refactor cgroups code in prep for new type
From: David Ahern @ 2016-10-27  0:58 UTC (permalink / raw)
  To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern
In-Reply-To: <1477529922-4806-1-git-send-email-dsa@cumulusnetworks.com>

Code move only and rename only; no functional change intended.

v2
- fix bpf_prog_run_clear_cb to bpf_prog_run_save_cb as caught by Daniel

- rename BPF_PROG_TYPE_CGROUP_SKB and its cg_skb functions to
  BPF_PROG_TYPE_CGROUP and cgroup

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/uapi/linux/bpf.h        |  2 +-
 kernel/bpf/cgroup.c             | 27 ++++++++++++++++++++++-----
 kernel/bpf/syscall.c            |  2 +-
 net/core/filter.c               | 14 +++++++-------
 samples/bpf/test_cgrp2_attach.c |  4 ++--
 5 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 6b62ee9a2f78..73da296c2125 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -98,7 +98,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_TRACEPOINT,
 	BPF_PROG_TYPE_XDP,
 	BPF_PROG_TYPE_PERF_EVENT,
-	BPF_PROG_TYPE_CGROUP_SKB,
+	BPF_PROG_TYPE_CGROUP,
 };
 
 enum bpf_attach_type {
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index a0ab43f264b0..d5746aec8f34 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -117,6 +117,19 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
 	}
 }
 
+static int __cgroup_bpf_run_filter_skb(struct sk_buff *skb,
+				       struct bpf_prog *prog)
+{
+	unsigned int offset = skb->data - skb_network_header(skb);
+	int ret;
+
+	__skb_push(skb, offset);
+	ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM;
+	__skb_pull(skb, offset);
+
+	return ret;
+}
+
 /**
  * __cgroup_bpf_run_filter() - Run a program for packet filtering
  * @sk: The socken sending or receiving traffic
@@ -153,11 +166,15 @@ int __cgroup_bpf_run_filter(struct sock *sk,
 
 	prog = rcu_dereference(cgrp->bpf.effective[type]);
 	if (prog) {
-		unsigned int offset = skb->data - skb_network_header(skb);
-
-		__skb_push(skb, offset);
-		ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM;
-		__skb_pull(skb, offset);
+		switch (type) {
+		case BPF_CGROUP_INET_INGRESS:
+		case BPF_CGROUP_INET_EGRESS:
+			ret = __cgroup_bpf_run_filter_skb(skb, prog);
+			break;
+		/* make gcc happy else complains about missing enum value */
+		default:
+			return 0;
+		}
 	}
 
 	rcu_read_unlock();
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 1814c010ace6..3c4c9ed2d576 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -841,7 +841,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
 	case BPF_CGROUP_INET_INGRESS:
 	case BPF_CGROUP_INET_EGRESS:
 		prog = bpf_prog_get_type(attr->attach_bpf_fd,
-					 BPF_PROG_TYPE_CGROUP_SKB);
+					 BPF_PROG_TYPE_CGROUP);
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
 
diff --git a/net/core/filter.c b/net/core/filter.c
index 4552b8c93b99..61e229c3a862 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2583,7 +2583,7 @@ xdp_func_proto(enum bpf_func_id func_id)
 }
 
 static const struct bpf_func_proto *
-cg_skb_func_proto(enum bpf_func_id func_id)
+cgroup_func_proto(enum bpf_func_id func_id)
 {
 	switch (func_id) {
 	case BPF_FUNC_skb_load_bytes:
@@ -2955,8 +2955,8 @@ static const struct bpf_verifier_ops xdp_ops = {
 	.convert_ctx_access	= xdp_convert_ctx_access,
 };
 
-static const struct bpf_verifier_ops cg_skb_ops = {
-	.get_func_proto		= cg_skb_func_proto,
+static const struct bpf_verifier_ops cgroup_ops = {
+	.get_func_proto		= cgroup_func_proto,
 	.is_valid_access	= sk_filter_is_valid_access,
 	.convert_ctx_access	= sk_filter_convert_ctx_access,
 };
@@ -2981,9 +2981,9 @@ static struct bpf_prog_type_list xdp_type __read_mostly = {
 	.type	= BPF_PROG_TYPE_XDP,
 };
 
-static struct bpf_prog_type_list cg_skb_type __read_mostly = {
-	.ops	= &cg_skb_ops,
-	.type	= BPF_PROG_TYPE_CGROUP_SKB,
+static struct bpf_prog_type_list cgroup_type __read_mostly = {
+	.ops	= &cgroup_ops,
+	.type	= BPF_PROG_TYPE_CGROUP,
 };
 
 static int __init register_sk_filter_ops(void)
@@ -2992,7 +2992,7 @@ static int __init register_sk_filter_ops(void)
 	bpf_register_prog_type(&sched_cls_type);
 	bpf_register_prog_type(&sched_act_type);
 	bpf_register_prog_type(&xdp_type);
-	bpf_register_prog_type(&cg_skb_type);
+	bpf_register_prog_type(&cgroup_type);
 
 	return 0;
 }
diff --git a/samples/bpf/test_cgrp2_attach.c b/samples/bpf/test_cgrp2_attach.c
index 63ef2083f766..468e68e6d511 100644
--- a/samples/bpf/test_cgrp2_attach.c
+++ b/samples/bpf/test_cgrp2_attach.c
@@ -69,8 +69,8 @@ static int prog_load(int map_fd, int verdict)
 		BPF_EXIT_INSN(),
 	};
 
-	return bpf_prog_load(BPF_PROG_TYPE_CGROUP_SKB,
-			     prog, sizeof(prog), "GPL", 0);
+	return bpf_prog_load(BPF_PROG_TYPE_CGROUP,
+			     prog, sizeof(prog), "GPL", 0, NULL);
 }
 
 static int usage(const char *argv0)
-- 
2.1.4

^ permalink raw reply related

* [PATCH v2 net-next 0/5] Add bpf support to set sk_bound_dev_if
From: David Ahern @ 2016-10-27  0:58 UTC (permalink / raw)
  To: netdev; +Cc: daniel, ast, daniel, maheshb, tgraf, David Ahern

The recently added VRF support in Linux leverages the bind-to-device
API for programs to specify an L3 domain for a socket. While
SO_BINDTODEVICE has been around for ages, not every ipv4/ipv6 capable
program has support for it. Even for those programs that do support it,
the API requires processes to be started as root (CAP_NET_RAW) which
is not desirable from a general security perspective.

This patch set leverages Daniel Mack's work to attach bpf programs to
a cgroup:

    https://www.mail-archive.com/netdev@vger.kernel.org/msg134028.html

to provide a capability to set sk_bound_dev_if for all AF_INET{6}
sockets opened by a process in a cgroup when the sockets are allocated.

This capability enables running any program in a VRF context and is key
to deploying Management VRF, a fundamental configuration for networking
gear, with any Linux OS installation.

v2
- addressed Daniel's comments: dropped the bpf_sock_store_u32 helper
  and used bpf_prog_run_save_cb on the code move

- picked up Mickaël Salaün's subtype patch with a few small tweaks

- removed new prog type in favor of a subtype on the BPF_PROG_TYPE_CGROUP
  from Daniel Mack's patch set

- moved the filter hook from sk_alloc to inet{6}_create


David Ahern (5):
  bpf: Refactor cgroups code in prep for new type
  bpf: Add eBPF program subtype and is_valid_subtype() verifier
  bpf: Add new cgroup attach type to enable sock modifications
  samples: bpf: Add prog_subtype to bpf_prog_load
  samples: bpf: add userspace example for modifying sk_bound_dev_if

 include/linux/bpf.h             |   7 ++-
 include/linux/filter.h          |   3 +-
 include/uapi/linux/bpf.h        |  15 +++++-
 kernel/bpf/cgroup.c             |  36 +++++++++++--
 kernel/bpf/syscall.c            |  11 ++--
 kernel/bpf/verifier.c           |  10 +++-
 kernel/trace/bpf_trace.c        |  16 ++++--
 net/core/filter.c               | 115 +++++++++++++++++++++++++++++++++-------
 net/ipv4/af_inet.c              |   4 ++
 net/ipv6/af_inet6.c             |   3 ++
 samples/bpf/Makefile            |   2 +
 samples/bpf/bpf_load.c          |   2 +-
 samples/bpf/fds_example.c       |   2 +-
 samples/bpf/libbpf.c            |   5 +-
 samples/bpf/libbpf.h            |   3 +-
 samples/bpf/sock_example.c      |   2 +-
 samples/bpf/test_cgrp2_attach.c |   4 +-
 samples/bpf/test_cgrp2_sock.c   |  84 +++++++++++++++++++++++++++++
 18 files changed, 280 insertions(+), 44 deletions(-)
 create mode 100644 samples/bpf/test_cgrp2_sock.c

-- 
2.1.4

^ permalink raw reply

* [PATCH net-next]ldmvsw: tx queue stuck in stopped state after LDC reset
From: Aaron Young @ 2016-10-27  0:15 UTC (permalink / raw)
  To: netdev, davem; +Cc: sowmini.varadhan, aaron.young, Aaron Young

   From: Aaron Young <aaron.young@oracle.com>

   The following patch fixes an issue with the ldmvsw driver where
   the network connection of a guest domain becomes non-functional after
   a guest domain has panic'd and rebooted (resulting in a LDC reset).

  The root cause was determined to be from the following series of
  events:

  1. Guest domain panics - resulting in the guest no longer fielding
     network packets from the ldmvsw driver
  2. The ldmvsw driver (in the control domain) eventually exerts flow
     control due to no more available tx drings and stops the tx queue
     for the guest domain
  3. The LDC of the network connection for the guest is reset when
     the guest domain reboots after the panic.
  4. The LDC reset event is received by the ldmvsw driver and the ldmvsw
     responds by clearing the tx queue for the guest.
  5. The guest is eventually rebooted and ldmvsw waits indefinitely for a
     DATA ACK from the guest - which is the normal method to re-enable
     the tx queue. But the ACK never comes because the tx queue was cleared
     due to the LDC reset.

  Fix is, in addition to clearing the tx queue, to re-enable the tx queue
  on a LDC reset. This prevents the ldmvsw from getting caught in this deadlocked
  state of waiting for a DATA ACK which will never come.

Signed-off-by: Aaron Young <Aaron.Young@oracle.com>
Acked-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 drivers/net/ethernet/sun/sunvnet_common.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index 58efe69..8878b75 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -704,9 +704,8 @@ static int handle_mcast(struct vnet_port *port, void *msgbuf)
 	return 0;
 }
 
-/* Got back a STOPPED LDC message on port. If the queue is stopped,
- * wake it up so that we'll send out another START message at the
- * next TX.
+/* If the queue is stopped, wake it up so that we'll
+ * send out another START message at the next TX.
  */
 static void maybe_tx_wakeup(struct vnet_port *port)
 {
@@ -734,6 +733,7 @@ bool sunvnet_port_is_up_common(struct vnet_port *vnet)
 
 static int vnet_event_napi(struct vnet_port *port, int budget)
 {
+	struct net_device *dev = VNET_PORT_TO_NET_DEVICE(port);
 	struct vio_driver_state *vio = &port->vio;
 	int tx_wakeup, err;
 	int npkts = 0;
@@ -747,6 +747,16 @@ static int vnet_event_napi(struct vnet_port *port, int budget)
 		if (event == LDC_EVENT_RESET) {
 			vnet_port_reset(port);
 			vio_port_up(vio);
+
+			/* If the device is running but its tx queue was
+			 * stopped (due to flow control), restart it.
+			 * This is necessary since vnet_port_reset()
+			 * clears the tx drings and thus we may never get
+			 * back a VIO_TYPE_DATA ACK packet - which is
+			 * the normal mechanism to restart the tx queue.
+			 */
+			if (netif_running(dev))
+				maybe_tx_wakeup(port);
 		}
 		port->rx_event = 0;
 		return 0;
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH v2 2/5] kconfig: introduce the "suggest" keyword
From: Paul Bolle @ 2016-10-27  0:10 UTC (permalink / raw)
  To: Nicolas Pitre, John Stultz, Richard Cochran, Michal Marek
  Cc: Thomas Gleixner, Josh Triplett, Edward Cree, netdev, linux-kbuild,
	linux-kernel
In-Reply-To: <1477448931-29051-3-git-send-email-nicolas.pitre@linaro.org>

On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote:
> Similar to "imply" but with no added restrictions on the target symbol's
> value. Useful for providing a default value to another symbol.
> 
> Suggested by Edward Cree.
> 
> Signed-off-by: Nicolas Pitre <nico@linaro.org>

As far as I can see this series doesn't add a user of "suggest". So I
see no reason to add it.

NAK.


Paul Bolle

^ permalink raw reply

* Re: [PATCH v2 0/5] make POSIX timers optional with some Kconfig help
From: Paul Bolle @ 2016-10-26 23:52 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: John Stultz, Richard Cochran, Michal Marek, Thomas Gleixner,
	Josh Triplett, Edward Cree, netdev, linux-kbuild, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1610261938410.14694@knanqh.ubzr>

On Wed, 2016-10-26 at 19:41 -0400, Nicolas Pitre wrote:
> On Thu, 27 Oct 2016, Paul Bolle wrote:
> > If you'll have to send another update don't bother including Yann. Yann
> > hasn't be heard of in years. MAINTAINERS doesn't reflect reality for
> > kconfig.
> 
> What about updating it then?

I don't know why Yann is still mentioned after M: in MAINTAINERS. In
practice Michal has (again) been taking care of kconfig.


Paul Bolle

^ permalink raw reply

* Re: [PATCH v2 1/5] kconfig: introduce the "imply" keyword
From: Nicolas Pitre @ 2016-10-26 23:44 UTC (permalink / raw)
  To: Paul Bolle
  Cc: John Stultz, Richard Cochran, Michal Marek, Thomas Gleixner,
	Josh Triplett, Edward Cree, netdev, linux-kbuild, linux-kernel
In-Reply-To: <1477524527.2121.15.camel@tiscali.nl>

On Thu, 27 Oct 2016, Paul Bolle wrote:

> On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote:
> > SUBSYSTEM_X can still be configured out, and it can be set as a 
> > module when none of the drivers are selected or all of them are also 
> > modular.
> 
> Short note, to highlight a pet peeve: "select" (and therefor
> "selected") has a special meaning in kconfig land. I guess you meant
> something neutral like "set" here. Is that correct?

Right. Will fix.

> By the way: "also" makes this sentence hard to parse.

s/also //


Nicolas

^ permalink raw reply

* Re: [PATCH v2 0/5] make POSIX timers optional with some Kconfig help
From: Nicolas Pitre @ 2016-10-26 23:41 UTC (permalink / raw)
  To: Paul Bolle
  Cc: John Stultz, Richard Cochran, Michal Marek, Thomas Gleixner,
	Josh Triplett, Edward Cree, netdev, linux-kbuild, linux-kernel
In-Reply-To: <1477523669.2121.8.camel@tiscali.nl>

On Thu, 27 Oct 2016, Paul Bolle wrote:

> On Tue, 2016-10-25 at 22:28 -0400, Nicolas Pitre wrote:
> > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > Subject: [PATCH v2 0/5] make POSIX timers optional with some Kconfig help
> 
> This doesn't work. I received this message with an empty subject.

Hmmm... must have dome something stupid with git send-patch.

> If you'll have to send another update don't bother including Yann. Yann
> hasn't be heard of in years. MAINTAINERS doesn't reflect reality for
> kconfig.

What about updating it then?


Nicolas

^ 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