Netdev List
 help / color / mirror / Atom feed
* iproute2 won't compile without AF_VSOCK
From: Steve Wise @ 2018-06-19 15:17 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev

Hey David,

I'm trying to compile the latest iproute2 on an RHEL-7.3 distro, and it
fails to compile because AF_VSOCK is not defined.  Should this
functionality be a configure option to disable it on older distros?


Thanks,

Steve.

----

misc
    CC       ss.o
ss.c:301:27: error: ‘AF_VSOCK’ undeclared here (not in a function)
   .families = FAMILY_MASK(AF_VSOCK),
                           ^
ss.c:252:46: note: in definition of macro ‘FAMILY_MASK’
 #define FAMILY_MASK(family) ((uint64_t)1 << (family))
                                              ^
ss.c:334:2: error: array index in initializer not of integer type
  [AF_VSOCK] = {
  ^
ss.c:334:2: error: (near initialization for ‘default_afs’)
make[1]: *** [ss.o] Error 1
make: *** [all] Error 2


^ permalink raw reply

* [PATCH] bpfilter: ignore binary files
From: Matteo Croce @ 2018-06-19 15:21 UTC (permalink / raw)
  To: netdev; +Cc: Alexei Starovoitov

net/bpfilter/bpfilter_umh is a binary file generated when bpfilter is
enabled, add it to .gitignore to avoid committing it.

Fixes: d2ba09c17a064 ("net: add skeleton of bpfilter kernel module")
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 net/bpfilter/.gitignore | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 net/bpfilter/.gitignore

diff --git a/net/bpfilter/.gitignore b/net/bpfilter/.gitignore
new file mode 100644
index 000000000000..e97084e3eea2
--- /dev/null
+++ b/net/bpfilter/.gitignore
@@ -0,0 +1 @@
+bpfilter_umh
-- 
2.17.1

^ permalink raw reply related

* Re: Link modes representation in phylib
From: Andrew Lunn @ 2018-06-19 15:21 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: davem, Russell King - ARM Linux, Florian Fainelli, netdev,
	Antoine Tenart, thomas.petazzoni@bootlin.com, Gregory CLEMENT,
	Miquel Raynal
In-Reply-To: <20180619113053.11df78a2@bootlin.com>

> What I propose is that we add 3 link_mode fields in phy_device, and keep
> the legacy fields for now. It would be up to the driver to fill the new
> "supported" field in config_init, kind of like what's done in the
> marvell10g driver.

Hi Maxime

You can do this conversion in the core. If features == 0, and some
bits are set in the features link_mode, do the conversion at probe
time. The same can be done for lp_advertising, when the call into the
drivers read_status() has completed.

> Would that be acceptable ?

It sounds reasonable. Lets see what the code looks like.

   Andrew

^ permalink raw reply

* Re: [PATCH bpf-net] bpf: Change bpf_fib_lookup to return lookup status
From: Martin KaFai Lau @ 2018-06-19 15:25 UTC (permalink / raw)
  To: David Ahern; +Cc: dsahern, netdev, borkmann, ast, davem
In-Reply-To: <1339f6f2-9dd3-886c-2178-7088b0ae4746@gmail.com>

On Mon, Jun 18, 2018 at 03:35:25PM -0600, David Ahern wrote:
> On 6/18/18 2:55 PM, Martin KaFai Lau wrote:
> >> 	/* rc > 0 case */
> >> 	switch(rc) {
> >> 	case BPF_FIB_LKUP_RET_BLACKHOLE:
> >> 	case BPF_FIB_LKUP_RET_UNREACHABLE:
> >> 	case BPF_FIB_LKUP_RET_PROHIBIT:
> >> 		return XDP_DROP;
> >> 	}
> >>
> >> For the others it becomes a question of do we share why the stack needs
> >> to be involved? Maybe the program wants to collect stats to show traffic
> >> patterns that can be improved (BPF_FIB_LKUP_RET_FRAG_NEEDED) or support
> >> in the kernel needs to be improved (BPF_FIB_LKUP_RET_UNSUPP_LWT) or an
> >> interface is misconfigured (BPF_FIB_LKUP_RET_FWD_DISABLED).
> > Thanks for the explanation.
> > 
> > Agree on the bpf able to collect stats will be useful.
> > 
> > I am wondering, if a new BPF_FIB_LKUP_RET_XYZ is added later,
> > how may the old xdp_prog work/not-work?  As of now, the return value
> > is straight forward, FWD, PASS (to stack) or DROP (error).
> > With this change, the xdp_prog needs to match/switch() the
> > BPF_FIB_LKUP_RET_* to at least PASS and DROP.
> 
> IMO, programs should only call XDP_DROP for known reasons - like the 3
> above. Anything else punt to the stack.
> 
> If a new RET_XYZ comes along:
> 1. the new XYZ is a new ACL response where the packet is to be dropped.
> If the program does not understand XYZ and punts to the stack
> (recommendation), then a second lookup is done during normal packet
> processing and the stack drops it.
> 
> 2. the new XYZ is a new path in the kernel that is unsupported with
> respect to XDP forwarding, nothing new for the program to do.
> 
> Either way I would expect stats on BPF_FIB_LKUP_RET_* to give a hint to
> the program writer.
> 
> Worst case of punting packets to the stack for any rc != 0 means the
> stack is doing 2 lookups - 1 in XDP based on its lookup parameters and 1
> in normal stack processing - to handle the packet.
Instead of having the xdp_prog to follow the meaning of what RET_SYZ is,
should the bpf_*_fib_lookup() return value be kept as is such that
the xdp_prog is clear what to do.  The reason can be returned in
the 'struct bpf_fib_lookup'.  The number of reasons can be extended.
If the xdp_prog does not understand a reason, it still will not
affect its decision because the return value is clear.
I think the situation here is similar to regular syscall which usually
uses -1 to clearly states error and errno to spells out the reason.

> 
> > 
> >>
> >> Arguably BPF_FIB_LKUP_RET_NO_NHDEV is not needed. See below.
> >>
> >>>> @@ -2612,6 +2613,19 @@ struct bpf_raw_tracepoint_args {
> >>>>  #define BPF_FIB_LOOKUP_DIRECT  BIT(0)
> >>>>  #define BPF_FIB_LOOKUP_OUTPUT  BIT(1)
> >>>>  
> >>>> +enum {
> >>>> +	BPF_FIB_LKUP_RET_SUCCESS,      /* lookup successful */
> >>>> +	BPF_FIB_LKUP_RET_BLACKHOLE,    /* dest is blackholed */
> >>>> +	BPF_FIB_LKUP_RET_UNREACHABLE,  /* dest is unreachable */
> >>>> +	BPF_FIB_LKUP_RET_PROHIBIT,     /* dest not allowed */
> >>>> +	BPF_FIB_LKUP_RET_NOT_FWDED,    /* pkt is not forwardded */
> >>> BPF_FIB_LKUP_RET_NOT_FWDED is a catch all?
> >>>
> >>
> >> Destination is local. More precisely, the FIB lookup is not unicast so
> >> not forwarded. It could be RTN_LOCAL, RTN_BROADCAST, RTN_ANYCAST, or
> >> RTN_MULTICAST. The next ones -- blackhole, reachable, prohibit -- are
> >> called out.
> > I think it also includes the tbid not found case.
> 
> Another one of those "should never happen scenarios". The user does not
> specify the table; it is retrieved based on device association. Table
> defaults to the main table - which always exists - and any VRF
> enslavement of a device happens after the VRF device creates the table.
> 
> > 
> >>
> >>>> @@ -4252,16 +4277,19 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
> >>>>  	if (check_mtu) {
> >>>>  		mtu = ipv6_stub->ip6_mtu_from_fib6(f6i, dst, src);
> >>>>  		if (params->tot_len > mtu)
> >>>> -			return 0;
> >>>> +			return BPF_FIB_LKUP_RET_FRAG_NEEDED;
> >>>>  	}
> >>>>  
> >>>>  	if (f6i->fib6_nh.nh_lwtstate)
> >>>> -		return 0;
> >>>> +		return BPF_FIB_LKUP_RET_UNSUPP_LWT;
> >>>>  
> >>>>  	if (f6i->fib6_flags & RTF_GATEWAY)
> >>>>  		*dst = f6i->fib6_nh.nh_gw;
> >>>>  
> >>>>  	dev = f6i->fib6_nh.nh_dev;
> >>>> +	if (unlikely(!dev))
> >>>> +		return BPF_FIB_LKUP_RET_NO_NHDEV;
> >>> Is this a bug fix?
> >>>
> >>
> >> Difference between IPv4 and IPv6. Making them consistent.
> >>
> >> It is a major BUG in the kernel to reach this point in either protocol
> >> to have a unicast route not tied to a device. IPv4 has checks; v6 does
> >> not. I figured this being new code, why not make bpf_ipv{4,6}_fib_lookup
> >> as close to the same as possible.
> > Make sense.  A comment in the commit log will be useful if there is a
> > re-spin.
> > 
> 
> ok.

^ permalink raw reply

* Re: [PATCH] net/phy: Micrel KSZ8061 PHY link failure after cable connect
From: Andrew Lunn @ 2018-06-19 15:28 UTC (permalink / raw)
  To: Onnasch, Alexander (EXT)
  Cc: Florian Fainelli, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <AM6PR01MB4262F8BB6004D246DFF5C3FEF0700@AM6PR01MB4262.eurprd01.prod.exchangelabs.com>

On Tue, Jun 19, 2018 at 02:23:41PM +0000, Onnasch, Alexander (EXT) wrote:
> Hi Andrew
> thanks for the hint. But actually I cannot confirm - or I don't see it yet. 
> 
> Without having tested, just from the code, the struct phy_driver instance for PHY_ID_KSZ8061 in micrel.c does not have a .write_mmd function assigned, thus phy_write_mmd should evaluate to its else-clause (see below) and not to mdiobus_write (as in phy_write).
> 
> Also the ksz8061_extended_write() function which I have added uses the same principle as already existing HW-specific functions in micrel.c for simular reasons (kszphy_extended_write and ksz9031_extended_write).
> They use phy_write all over the place in that file and never phy_write_mmd - for whatever reason they had.
> Thus I thought it would be a good idea ...

Hi Alexander

Please don't top post. And wrap your lines at around 75 characters 

> 		struct mii_bus *bus = phydev->mdio.bus;
> 		int phy_addr = phydev->mdio.addr;
> 
> 		mutex_lock(&bus->mdio_lock);
> 		mmd_phy_indirect(bus, phy_addr, devad, regnum);
> 
> 		/* Write the data into MMD's selected register */
> 		bus->write(bus, phy_addr, MII_MMD_DATA, val);
> 		mutex_unlock(&bus->mdio_lock);


> > +static int ksz8061_extended_write(struct phy_device *phydev,
> > +				  u8 mode, u32 dev_addr, u32 regnum, u16 val) {
> > +	phy_write(phydev, MII_KSZ8061RN_MMD_CTRL_REG, dev_addr);
> > +	phy_write(phydev, MII_KSZ8061RN_MMD_REGDATA_REG, regnum);
> > +	phy_write(phydev, MII_KSZ8061RN_MMD_CTRL_REG, (mode << 14) | dev_addr);
> > +	return phy_write(phydev, MII_KSZ8061RN_MMD_REGDATA_REG, val); }
> 
> Hi Alexander
> 
> This looks a lot like phy_write_mmd().

Look closely at the two implementations. Look at what
mmd_phy_indirect() does. I _think_ these are identical. So don't add
your own helper, please use the core code.

     Andrew

^ permalink raw reply

* Re: [PATCH bpf-net] bpf: Change bpf_fib_lookup to return lookup status
From: David Ahern @ 2018-06-19 15:34 UTC (permalink / raw)
  To: Martin KaFai Lau; +Cc: dsahern, netdev, borkmann, ast, davem
In-Reply-To: <20180619152529.rkzeyyqgmiwsvjp6@kafai-mbp.dhcp.thefacebook.com>

On 6/19/18 9:25 AM, Martin KaFai Lau wrote:
> On Mon, Jun 18, 2018 at 03:35:25PM -0600, David Ahern wrote:
>> On 6/18/18 2:55 PM, Martin KaFai Lau wrote:
>>>> 	/* rc > 0 case */
>>>> 	switch(rc) {
>>>> 	case BPF_FIB_LKUP_RET_BLACKHOLE:
>>>> 	case BPF_FIB_LKUP_RET_UNREACHABLE:
>>>> 	case BPF_FIB_LKUP_RET_PROHIBIT:
>>>> 		return XDP_DROP;
>>>> 	}
>>>>
>>>> For the others it becomes a question of do we share why the stack needs
>>>> to be involved? Maybe the program wants to collect stats to show traffic
>>>> patterns that can be improved (BPF_FIB_LKUP_RET_FRAG_NEEDED) or support
>>>> in the kernel needs to be improved (BPF_FIB_LKUP_RET_UNSUPP_LWT) or an
>>>> interface is misconfigured (BPF_FIB_LKUP_RET_FWD_DISABLED).
>>> Thanks for the explanation.
>>>
>>> Agree on the bpf able to collect stats will be useful.
>>>
>>> I am wondering, if a new BPF_FIB_LKUP_RET_XYZ is added later,
>>> how may the old xdp_prog work/not-work?  As of now, the return value
>>> is straight forward, FWD, PASS (to stack) or DROP (error).
>>> With this change, the xdp_prog needs to match/switch() the
>>> BPF_FIB_LKUP_RET_* to at least PASS and DROP.
>>
>> IMO, programs should only call XDP_DROP for known reasons - like the 3
>> above. Anything else punt to the stack.
>>
>> If a new RET_XYZ comes along:
>> 1. the new XYZ is a new ACL response where the packet is to be dropped.
>> If the program does not understand XYZ and punts to the stack
>> (recommendation), then a second lookup is done during normal packet
>> processing and the stack drops it.
>>
>> 2. the new XYZ is a new path in the kernel that is unsupported with
>> respect to XDP forwarding, nothing new for the program to do.
>>
>> Either way I would expect stats on BPF_FIB_LKUP_RET_* to give a hint to
>> the program writer.
>>
>> Worst case of punting packets to the stack for any rc != 0 means the
>> stack is doing 2 lookups - 1 in XDP based on its lookup parameters and 1
>> in normal stack processing - to handle the packet.
> Instead of having the xdp_prog to follow the meaning of what RET_SYZ is,
> should the bpf_*_fib_lookup() return value be kept as is such that
> the xdp_prog is clear what to do.  The reason can be returned in
> the 'struct bpf_fib_lookup'.  The number of reasons can be extended.
> If the xdp_prog does not understand a reason, it still will not
> affect its decision because the return value is clear.
> I think the situation here is similar to regular syscall which usually
> uses -1 to clearly states error and errno to spells out the reason.
> 

I did consider returning the status in struct bpf_fib_lookup. However,
it is 64 bytes and can not be extended without a big performance
penalty, so the only option there is to make an existing entry a union
the most logical of which is the ifindex. It seemed odd to me to have
the result by hidden in the struct as a union on ifindex and returning
the egress index from the function:

@@ -2625,7 +2636,11 @@ struct bpf_fib_lookup {

        /* total length of packet from network header - used for MTU
check */
        __u16   tot_len;
-       __u32   ifindex;  /* L3 device index for lookup */
+
+       union {
+               __u32   ifindex;  /* input: L3 device index for lookup */
+               __u32   result;   /* output: one of BPF_FIB_LKUP_RET_* */
+       };


It seemed more natural to have ifindex stay ifindex and only change
value on return:

@@ -2625,7 +2639,11 @@ struct bpf_fib_lookup {

 	/* total length of packet from network header - used for MTU check */
 	__u16	tot_len;
-	__u32	ifindex;  /* L3 device index for lookup */
+
+	/* input: L3 device index for lookup
+	 * output: nexthop device index from FIB lookup
+	 */
+	__u32	ifindex;

 	union {
 		/* inputs to lookup */


>From a program's perspective:

rc < 0  -- program is passing incorrect data
rc == 0 -- packet can be forwarded
rc > 0  -- packet can not be forwarded.

BPF programs are not required to track the LKUP_RET values any more than
a function returning multiple negative values - the caller just checks
rc < 0 means failure. If the program cares it can look at specific
values of rc to see the specific value.

The same applies with the LKUP_RET values - they are there to provide
insight into why the packet is not forwarded directly if the program
cares to know why.

^ permalink raw reply

* [PATCH] net: stmmac: socfpga: add additional ocp reset line for Stratix10
From: Dinh Nguyen @ 2018-06-19 15:35 UTC (permalink / raw)
  To: netdev
  Cc: dinguyen, davem, joabreu, alexandre.torgue, peppe.cavallaro,
	linux-kernel

The Stratix10 platform has an additional reset line, OCP(Open Core Protocol),
that also needs to get deasserted for the stmmac ethernet controller to work.
Thus we need to update the Kconfig to include ARCH_STRATIX10 in order to build
dwmac-socfpga.

Also, remove the redundant check for the reset controller pointer. The
reset driver already checks for the pointer and returns 0 if the pointer
is NULL.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig         |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 18 ++++++++++++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index cb5b0f5..edf2036 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -111,7 +111,7 @@ config DWMAC_ROCKCHIP
 config DWMAC_SOCFPGA
 	tristate "SOCFPGA dwmac support"
 	default ARCH_SOCFPGA
-	depends on OF && (ARCH_SOCFPGA || COMPILE_TEST)
+	depends on OF && (ARCH_SOCFPGA || ARCH_STRATIX10 || COMPILE_TEST)
 	select MFD_SYSCON
 	help
 	  Support for ethernet controller on Altera SOCFPGA
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 6e35957..5b3b06a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -55,6 +55,7 @@ struct socfpga_dwmac {
 	struct	device *dev;
 	struct regmap *sys_mgr_base_addr;
 	struct reset_control *stmmac_rst;
+	struct reset_control *stmmac_ocp_rst;
 	void __iomem *splitter_base;
 	bool f2h_ptp_ref_clk;
 	struct tse_pcs pcs;
@@ -262,8 +263,8 @@ static int socfpga_dwmac_set_phy_mode(struct socfpga_dwmac *dwmac)
 		val = SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII;
 
 	/* Assert reset to the enet controller before changing the phy mode */
-	if (dwmac->stmmac_rst)
-		reset_control_assert(dwmac->stmmac_rst);
+	reset_control_assert(dwmac->stmmac_ocp_rst);
+	reset_control_assert(dwmac->stmmac_rst);
 
 	regmap_read(sys_mgr_base_addr, reg_offset, &ctrl);
 	ctrl &= ~(SYSMGR_EMACGRP_CTRL_PHYSEL_MASK << reg_shift);
@@ -288,8 +289,8 @@ static int socfpga_dwmac_set_phy_mode(struct socfpga_dwmac *dwmac)
 	/* Deassert reset for the phy configuration to be sampled by
 	 * the enet controller, and operation to start in requested mode
 	 */
-	if (dwmac->stmmac_rst)
-		reset_control_deassert(dwmac->stmmac_rst);
+	reset_control_deassert(dwmac->stmmac_ocp_rst);
+	reset_control_deassert(dwmac->stmmac_rst);
 	if (phymode == PHY_INTERFACE_MODE_SGMII) {
 		if (tse_pcs_init(dwmac->pcs.tse_pcs_base, &dwmac->pcs) != 0) {
 			dev_err(dwmac->dev, "Unable to initialize TSE PCS");
@@ -324,6 +325,15 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 		goto err_remove_config_dt;
 	}
 
+	dwmac->stmmac_ocp_rst = devm_reset_control_get_optional(dev, "stmmaceth-ocp");
+	if (IS_ERR(dwmac->stmmac_ocp_rst)) {
+		ret = PTR_ERR(dwmac->stmmac_ocp_rst);
+		dev_err(dev, "error getting reset control of ocp %d\n", ret);
+		goto err_remove_config_dt;
+	}
+
+	reset_control_deassert(dwmac->stmmac_ocp_rst);
+
 	ret = socfpga_dwmac_parse_data(dwmac, dev);
 	if (ret) {
 		dev_err(dev, "Unable to parse OF data\n");
-- 
2.7.4

^ permalink raw reply related

* Re: iproute2 won't compile without AF_VSOCK
From: Stephen Hemminger @ 2018-06-19 15:47 UTC (permalink / raw)
  To: Steve Wise; +Cc: David Ahern, netdev
In-Reply-To: <c06b38c7-1680-806d-5b93-a7e04313183d@opengridcomputing.com>

On Tue, 19 Jun 2018 10:17:45 -0500
Steve Wise <swise@opengridcomputing.com> wrote:

> Hey David,
> 
> I'm trying to compile the latest iproute2 on an RHEL-7.3 distro, and it
> fails to compile because AF_VSOCK is not defined.  Should this
> functionality be a configure option to disable it on older distros?
> 
> 
> Thanks,
> 
> Steve.
> 
> ----
> 
> misc
>     CC       ss.o
> ss.c:301:27: error: ‘AF_VSOCK’ undeclared here (not in a function)
>    .families = FAMILY_MASK(AF_VSOCK),
>                            ^
> ss.c:252:46: note: in definition of macro ‘FAMILY_MASK’
>  #define FAMILY_MASK(family) ((uint64_t)1 << (family))
>                                               ^
> ss.c:334:2: error: array index in initializer not of integer type
>   [AF_VSOCK] = {
>   ^
> ss.c:334:2: error: (near initialization for ‘default_afs’)
> make[1]: *** [ss.o] Error 1
> make: *** [all] Error 2
> 

Probably should just add an #ifdef to takeout that if not present

^ permalink raw reply

* [PATCH 0/3] net: davinci_emac: fix suspend/resume (both a regression and a common clk problem)
From: Bartosz Golaszewski @ 2018-06-19 16:09 UTC (permalink / raw)
  To: Grygorii Strashko, David S . Miller, Florian Fainelli,
	Dan Carpenter, Ivan Khoronzhuk, Rob Herring, Lukas Wunner,
	Kevin Hilman, David Lechner, Sekhar Nori, Andrew Lunn
  Cc: linux-omap, netdev, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Earlier today I sent the first patch as a solution to a regression
introduced during the v4.16 merge window, but after testing David's
common clock series on top of 4.18-rc1 + this patch it turned out that
the problem persisted.

This is a follow-up containing the regression fix and two additional
patches that make suspend/resume work with David's changes.

Bartosz Golaszewski (3):
  net: ethernet: fix suspend/resume in davinci_emac
  net: phy: set the of_node in the mdiodev's struct device
  net: davinci_emac: match the mdio device against its compatible if
    possible

 drivers/net/ethernet/ti/davinci_emac.c | 19 +++++++++++++++++--
 drivers/net/phy/phy_device.c           |  1 +
 2 files changed, 18 insertions(+), 2 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH 1/3] net: ethernet: fix suspend/resume in davinci_emac
From: Bartosz Golaszewski @ 2018-06-19 16:09 UTC (permalink / raw)
  To: Grygorii Strashko, David S . Miller, Florian Fainelli,
	Dan Carpenter, Ivan Khoronzhuk, Rob Herring, Lukas Wunner,
	Kevin Hilman, David Lechner, Sekhar Nori, Andrew Lunn
  Cc: linux-omap, netdev, linux-kernel, Bartosz Golaszewski, stable
In-Reply-To: <20180619160950.6283-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This patch reverts commit 3243ff2a05ec ("net: ethernet: davinci_emac:
Deduplicate bus_find_device() by name matching") and adds a comment
which should stop anyone from reintroducing the same "fix" in the future.

We can't use bus_find_device_by_name() here because the device name is
not guaranteed to be 'davinci_mdio'. On some systems it can be
'davinci_mdio.0' so we need to use strncmp() against the first part of
the string to correctly match it.

Fixes: 3243ff2a05ec ("net: ethernet: davinci_emac: Deduplicate bus_find_device() by name matching")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/ethernet/ti/davinci_emac.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 06d7c9e4dcda..a1a6445b5a7e 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1385,6 +1385,11 @@ static int emac_devioctl(struct net_device *ndev, struct ifreq *ifrq, int cmd)
 		return -EOPNOTSUPP;
 }
 
+static int match_first_device(struct device *dev, void *data)
+{
+	return !strncmp(dev_name(dev), "davinci_mdio", 12);
+}
+
 /**
  * emac_dev_open - EMAC device open
  * @ndev: The DaVinci EMAC network adapter
@@ -1484,8 +1489,14 @@ static int emac_dev_open(struct net_device *ndev)
 
 	/* use the first phy on the bus if pdata did not give us a phy id */
 	if (!phydev && !priv->phy_id) {
-		phy = bus_find_device_by_name(&mdio_bus_type, NULL,
-					      "davinci_mdio");
+		/* NOTE: we can't use bus_find_device_by_name() here because
+		 * the device name is not guaranteed to be 'davinci_mdio'. On
+		 * some systems it can be 'davinci_mdio.0' so we need to use
+		 * strncmp() against the first part of the string to correctly
+		 * match it.
+		 */
+		phy = bus_find_device(&mdio_bus_type, NULL, NULL,
+				      match_first_device);
 		if (phy) {
 			priv->phy_id = dev_name(phy);
 			if (!priv->phy_id || !*priv->phy_id)
-- 
2.17.1

^ permalink raw reply related

* [PATCH 2/3] net: phy: set the of_node in the mdiodev's struct device
From: Bartosz Golaszewski @ 2018-06-19 16:09 UTC (permalink / raw)
  To: Grygorii Strashko, David S . Miller, Florian Fainelli,
	Dan Carpenter, Ivan Khoronzhuk, Rob Herring, Lukas Wunner,
	Kevin Hilman, David Lechner, Sekhar Nori, Andrew Lunn
  Cc: linux-omap, netdev, linux-kernel, Bartosz Golaszewski
In-Reply-To: <20180619160950.6283-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Copy the of_node over from mii_bus's struct device. This is needed
for device-tree systems to be able to check the mdio device's
compatible string.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/phy/phy_device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index bd0f339f69fd..a92d5ee61813 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -411,6 +411,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
 	mdiodev->dev.parent = &bus->dev;
 	mdiodev->dev.bus = &mdio_bus_type;
 	mdiodev->dev.type = &mdio_bus_phy_type;
+	mdiodev->dev.of_node = bus->dev.of_node;
 	mdiodev->bus = bus;
 	mdiodev->bus_match = phy_bus_match;
 	mdiodev->addr = addr;
-- 
2.17.1

^ permalink raw reply related

* [PATCH 3/3] net: davinci_emac: match the mdio device against its compatible if possible
From: Bartosz Golaszewski @ 2018-06-19 16:09 UTC (permalink / raw)
  To: Grygorii Strashko, David S . Miller, Florian Fainelli,
	Dan Carpenter, Ivan Khoronzhuk, Rob Herring, Lukas Wunner,
	Kevin Hilman, David Lechner, Sekhar Nori, Andrew Lunn
  Cc: linux-omap, netdev, linux-kernel, Bartosz Golaszewski
In-Reply-To: <20180619160950.6283-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Device tree based systems without of_dev_auxdata will have the mdio
device named differently than "davinci_mdio(.0)". In this case use the
device's compatible string for matching.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/ethernet/ti/davinci_emac.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index a1a6445b5a7e..c28a35bb852f 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1387,6 +1387,10 @@ static int emac_devioctl(struct net_device *ndev, struct ifreq *ifrq, int cmd)
 
 static int match_first_device(struct device *dev, void *data)
 {
+	if (dev->of_node)
+		return of_device_is_compatible(dev->of_node,
+					       "ti,davinci_mdio");
+
 	return !strncmp(dev_name(dev), "davinci_mdio", 12);
 }
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] net: ethernet: fix suspend/resume in davinci_emac
From: Bartosz Golaszewski @ 2018-06-19 16:11 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Grygorii Strashko, David S . Miller, Florian Fainelli,
	Dan Carpenter, Ivan Khoronzhuk, Rob Herring, Kevin Hilman,
	David Lechner, Sekhar Nori, linux-omap, netdev,
	Linux Kernel Mailing List, Bartosz Golaszewski, stable
In-Reply-To: <20180619135219.GA7312@wunner.de>

2018-06-19 15:52 GMT+02:00 Lukas Wunner <lukas@wunner.de>:
> On Tue, Jun 19, 2018 at 02:44:00PM +0200, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> This patch reverts commit 3243ff2a05ec ("net: ethernet: davinci_emac:
>> Deduplicate bus_find_device() by name matching") and adds a comment
>> which should stop anyone from reintroducing the same "fix" in the future.
>>
>> We can't use bus_find_device_by_name() here because the device name is
>> not guaranteed to be 'davinci_mdio'. On some systems it can be
>> 'davinci_mdio.0' so we need to use strncmp() against the first part of
>> the string to correctly match it.
>>
>> Fixes: 3243ff2a05ec ("net: ethernet: davinci_emac: Deduplicate bus_find_device() by name matching")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Acked-by: Lukas Wunner <lukas@wunner.de>
>
> My apologies Bartosz, it wasn't clear to me that the driver deliberately
> only matched against the prefix of the name.  Sorry for the breakage.
>

No issue. I submitted a follow-up series with additional changes
required to make suspend/resume work. Please leave your Acked-by there
as well as I forgot to pick it up.

Thanks in advance,
Bartosz Golaszewski

^ permalink raw reply

* [PATCH] selftests/net: Fix permissions for fib_tests.sh
From: Daniel Díaz @ 2018-06-19 16:20 UTC (permalink / raw)
  To: shuahkh, linux-kselftest
  Cc: Daniel Díaz, David S. Miller, Shuah Khan,
	open list:NETWORKING [GENERAL], open list

fib_tests.sh became non-executable at some point. This is
what happens:
  selftests: net: fib_tests.sh: Warning: file fib_tests.sh is
  not executable, correct this.
  not ok 1..11 selftests: net: fib_tests.sh [FAIL]

Fixes: d69faad76584 ("selftests: fib_tests: Add prefix route tests with metric")

Signed-off-by: Daniel Díaz <daniel.diaz@linaro.org>
---
 tools/testing/selftests/net/fib_tests.sh | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 tools/testing/selftests/net/fib_tests.sh

diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
old mode 100644
new mode 100755
-- 
2.7.4

^ permalink raw reply

* Re: [PATCH bpf-net] bpf: Change bpf_fib_lookup to return lookup status
From: Martin KaFai Lau @ 2018-06-19 16:36 UTC (permalink / raw)
  To: David Ahern; +Cc: dsahern, netdev, borkmann, ast, davem
In-Reply-To: <2d6278d1-45ab-ff53-7b97-d9593203ff3e@gmail.com>

On Tue, Jun 19, 2018 at 09:34:28AM -0600, David Ahern wrote:
> On 6/19/18 9:25 AM, Martin KaFai Lau wrote:
> > On Mon, Jun 18, 2018 at 03:35:25PM -0600, David Ahern wrote:
> >> On 6/18/18 2:55 PM, Martin KaFai Lau wrote:
> >>>> 	/* rc > 0 case */
> >>>> 	switch(rc) {
> >>>> 	case BPF_FIB_LKUP_RET_BLACKHOLE:
> >>>> 	case BPF_FIB_LKUP_RET_UNREACHABLE:
> >>>> 	case BPF_FIB_LKUP_RET_PROHIBIT:
> >>>> 		return XDP_DROP;
> >>>> 	}
> >>>>
> >>>> For the others it becomes a question of do we share why the stack needs
> >>>> to be involved? Maybe the program wants to collect stats to show traffic
> >>>> patterns that can be improved (BPF_FIB_LKUP_RET_FRAG_NEEDED) or support
> >>>> in the kernel needs to be improved (BPF_FIB_LKUP_RET_UNSUPP_LWT) or an
> >>>> interface is misconfigured (BPF_FIB_LKUP_RET_FWD_DISABLED).
> >>> Thanks for the explanation.
> >>>
> >>> Agree on the bpf able to collect stats will be useful.
> >>>
> >>> I am wondering, if a new BPF_FIB_LKUP_RET_XYZ is added later,
> >>> how may the old xdp_prog work/not-work?  As of now, the return value
> >>> is straight forward, FWD, PASS (to stack) or DROP (error).
> >>> With this change, the xdp_prog needs to match/switch() the
> >>> BPF_FIB_LKUP_RET_* to at least PASS and DROP.
> >>
> >> IMO, programs should only call XDP_DROP for known reasons - like the 3
> >> above. Anything else punt to the stack.
> >>
> >> If a new RET_XYZ comes along:
> >> 1. the new XYZ is a new ACL response where the packet is to be dropped.
> >> If the program does not understand XYZ and punts to the stack
> >> (recommendation), then a second lookup is done during normal packet
> >> processing and the stack drops it.
> >>
> >> 2. the new XYZ is a new path in the kernel that is unsupported with
> >> respect to XDP forwarding, nothing new for the program to do.
> >>
> >> Either way I would expect stats on BPF_FIB_LKUP_RET_* to give a hint to
> >> the program writer.
> >>
> >> Worst case of punting packets to the stack for any rc != 0 means the
> >> stack is doing 2 lookups - 1 in XDP based on its lookup parameters and 1
> >> in normal stack processing - to handle the packet.
> > Instead of having the xdp_prog to follow the meaning of what RET_SYZ is,
> > should the bpf_*_fib_lookup() return value be kept as is such that
> > the xdp_prog is clear what to do.  The reason can be returned in
> > the 'struct bpf_fib_lookup'.  The number of reasons can be extended.
> > If the xdp_prog does not understand a reason, it still will not
> > affect its decision because the return value is clear.
> > I think the situation here is similar to regular syscall which usually
> > uses -1 to clearly states error and errno to spells out the reason.
> > 
> 
> I did consider returning the status in struct bpf_fib_lookup. However,
> it is 64 bytes and can not be extended without a big performance
> penalty, so the only option there is to make an existing entry a union
> the most logical of which is the ifindex. It seemed odd to me to have
> the result by hidden in the struct as a union on ifindex and returning
> the egress index from the function:
> 
> @@ -2625,7 +2636,11 @@ struct bpf_fib_lookup {
> 
>         /* total length of packet from network header - used for MTU
> check */
>         __u16   tot_len;
> -       __u32   ifindex;  /* L3 device index for lookup */
> +
> +       union {
> +               __u32   ifindex;  /* input: L3 device index for lookup */
> +               __u32   result;   /* output: one of BPF_FIB_LKUP_RET_* */
> +       };
> 
> 
> It seemed more natural to have ifindex stay ifindex and only change
> value on return:
> 
> @@ -2625,7 +2639,11 @@ struct bpf_fib_lookup {
> 
>  	/* total length of packet from network header - used for MTU check */
>  	__u16	tot_len;
> -	__u32	ifindex;  /* L3 device index for lookup */
> +
> +	/* input: L3 device index for lookup
> +	 * output: nexthop device index from FIB lookup
> +	 */
> +	__u32	ifindex;
> 
>  	union {
>  		/* inputs to lookup */
> 
> 
> From a program's perspective:
> 
> rc < 0  -- program is passing incorrect data
> rc == 0 -- packet can be forwarded
> rc > 0  -- packet can not be forwarded.
> 
> BPF programs are not required to track the LKUP_RET values any more than
> a function returning multiple negative values - the caller just checks
> rc < 0 means failure. If the program cares it can look at specific
> values of rc to see the specific value.
> 
> The same applies with the LKUP_RET values - they are there to provide
> insight into why the packet is not forwarded directly if the program
> cares to know why.
hmm...ic. My concern is, the prog can interpret rc > 0 (in this patch) to be
drop vs pass (although we can advise them in bpf.h to always pass if it does
not understand a rc but it is not a strong contract),  it may catch people
a surprise if a xdp_prog suddenly drops everything when running in a
newer kernel where the upper stack can actually handle it.

while the current behavior (i.e. before this patch, rc == 0) is always pass
to the stack.

I think at least comments should be put in the enum such that
the xdp/tc_prog should expect the enum could be extended later, so
the suggested behavior should be a pass for unknown LKUP_RET and let
the stack to decide.

^ permalink raw reply

* [PATCH] ucc_geth: Add BQL support
From: Joakim Tjernlund @ 2018-06-19 16:30 UTC (permalink / raw)
  To: Li Yang, netdev; +Cc: Joakim Tjernlund

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
 drivers/net/ethernet/freescale/ucc_geth.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index f77ba9fa257b..6c99a9af6647 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3096,6 +3096,7 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	ugeth_vdbg("%s: IN", __func__);
 
+	netdev_sent_queue(dev, skb->len);
 	spin_lock_irqsave(&ugeth->lock, flags);
 
 	dev->stats.tx_bytes += skb->len;
@@ -3242,6 +3243,8 @@ static int ucc_geth_tx(struct net_device *dev, u8 txQ)
 	struct ucc_geth_private *ugeth = netdev_priv(dev);
 	u8 __iomem *bd;		/* BD pointer */
 	u32 bd_status;
+	int howmany = 0;
+	unsigned int bytes_sent = 0;
 
 	bd = ugeth->confBd[txQ];
 	bd_status = in_be32((u32 __iomem *)bd);
@@ -3257,7 +3260,8 @@ static int ucc_geth_tx(struct net_device *dev, u8 txQ)
 		skb = ugeth->tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]];
 		if (!skb)
 			break;
-
+		howmany++;
+		bytes_sent += skb->len;
 		dev->stats.tx_packets++;
 
 		dev_consume_skb_any(skb);
@@ -3279,6 +3283,7 @@ static int ucc_geth_tx(struct net_device *dev, u8 txQ)
 		bd_status = in_be32((u32 __iomem *)bd);
 	}
 	ugeth->confBd[txQ] = bd;
+	netdev_completed_queue(dev, howmany, bytes_sent);
 	return 0;
 }
 
-- 
2.13.6

^ permalink raw reply related

* [PATCH] selftests: net: add config fragments
From: Anders Roxell @ 2018-06-19 16:41 UTC (permalink / raw)
  To: davem, shuah, fw, shannon.nelson
  Cc: netdev, linux-kselftest, linux-kernel, Anders Roxell

Add fragments to pass bridge and vlan tests.

Fixes: 33b01b7b4f19 ("selftests: add rtnetlink test script")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---

Hi,

net/rtnetlink.sh still fails on tc hbt hierarchy, addrlabel and ipsec:
Error: Specified qdisc not found.
RTNETLINK answers: No such file or directory
Error: Parent Qdisc doesn't exists.
We have an error talking to the kernel, -1
Error: Parent Qdisc doesn't exists.
We have an error talking to the kernel, -1
Error: Parent Qdisc doesn't exists.
We have an error talking to the kernel, -1
Error: Parent Qdisc doesn't exists.
We have an error talking to the kernel, -1
Error: Parent Qdisc doesn't exists.
We have an error talking to the kernel, -1
Error: Parent Qdisc doesn't exists.
We have an error talking to the kernel, -1
Error: Invalid handle.
FAIL: tc htb hierarchy

FAIL: ipv6 addrlabel

FAIL: can't add fou port 7777, skipping test
RTNETLINK answers: Operation not supported
FAIL: can't add macsec interface, skipping test
RTNETLINK answers: Protocol not supported
RTNETLINK answers: No such process
RTNETLINK answers: No such process
./rtnetlink.sh: line 527:  5356 Terminated              ip x m >
$tmpfile
FAIL: ipsec


I'm using iproute2 tag: 4.17 and tried the qdisc command from the
function kci_test_tc in net/rtnetlink.sh:
$ tc qdisc add dev lo root handle 1: htb
Error: Specified qdisc not found.

For kci_test_addrlabel it fails on this row:
ip addrlabel list |grep -q "prefix dead::/64 dev lo label 1"

Any idea why these three fails?

Cheers,
Anders

 tools/testing/selftests/net/config | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 7ba089b33e8b..cd3a2f1545b5 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -12,3 +12,5 @@ CONFIG_NET_IPVTI=y
 CONFIG_INET6_XFRM_MODE_TUNNEL=y
 CONFIG_IPV6_VTI=y
 CONFIG_DUMMY=y
+CONFIG_BRIDGE=y
+CONFIG_VLAN_8021Q=y
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH rdma-next v2 00/20] Introduce mlx5 DEVX interface
From: Leon Romanovsky @ 2018-06-19 16:46 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, RDMA mailing list, Joonas Lahtinen, Matan Barak,
	Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <20180619045930.GA7557@mtr-leonro.mtl.com>

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

On Tue, Jun 19, 2018 at 07:59:30AM +0300, Leon Romanovsky wrote:
> On Mon, Jun 18, 2018 at 04:05:04PM -0600, Jason Gunthorpe wrote:
> > On Sun, Jun 17, 2018 at 12:59:46PM +0300, Leon Romanovsky wrote:
> >
> > > Leon Romanovsky (2):
> > >   drm/i915: Move u64-to-ptr helpers to general header
> > >   kernel.h: Reuse u64_to_ptr macro to cast __user pointers
> >
> > I dropped these since they are not needed by this series when using a
> > union.
>
> No problem, it was my idea to reuse existing macro, before it was
> hard-coded implementation, but union makes it cleaner.
>
> >
> > > Matan Barak (5):
> > >   IB/uverbs: Export uverbs idr and fd types
> > >   IB/uverbs: Add PTR_IN attributes that are allocated/copied
> > >     automatically
> >
> > Revised this one, as noted
>
> Thanks
>
> >
> > >   IB/uverbs: Add a macro to define a type with no kernel known size
> > >   IB/uverbs: Allow an empty namespace in ioctl() framework
> > >   IB/uverbs: Refactor uverbs_finalize_objects
> >
> > I put the above in a branch and can apply them if you ack my revisions..
> >
>
> Except the line "return (void *)attr;", which should be "return ERR_CAST(attr);"
> everything looks reasonable. I didn't test it, but I'm not worried, we will have
> enough time to fix if needed.
>
> > >   net/mlx5_core: Prevent warns in dmesg upon firmware commands
> > >   IB/core: Improve uverbs_cleanup_ucontext algorithm
> >
> > I dropped these two (they are linked), need comments addressed and
> > resent.
>
> They are linked only logically, the second patch will trigger warning
> which is suppressed by first patch. So actually mlx5-net branch will have
> only first patch "net/mlx5_core: Prevent warns in dmesg upon firmware commands"
> and you will apply "IB/core: Improve uverbs_cleanup_ucontext algorithm" in
> your rdma-next.
>
> >
> > > Yishai Hadas (13):
> > >   net/mlx5: Expose DEVX ifc structures
> > >   IB/mlx5: Introduce DEVX
> > >   IB/core: Introduce DECLARE_UVERBS_GLOBAL_METHODS
> > >   IB: Expose ib_ucontext from a given ib_uverbs_file
> > >   IB/mlx5: Add support for DEVX general command
> > >   IB/mlx5: Add obj create and destroy functionality
> > >   IB/mlx5: Add DEVX support for modify and query commands
> > >   IB/mlx5: Add support for DEVX query UAR
> > >   IB/mlx5: Add DEVX support for memory registration
> > >   IB/mlx5: Add DEVX query EQN support
> > >   IB/mlx5: Expose DEVX tree
> >
> > I put these in a branch also and can apply them, but I need the first
> > two patches in the mlx5 core branch first please, thanks.
> >
> > Since this requires so many core patches I think I prefer to merge the
> > mlx core branch then apply rather merge a branch.
>
> So to summarize, I'm applying those three patches to mlx5-next:
>  * net/mlx5_core: Prevent warns in dmesg upon firmware commands
>  * net/mlx5: Expose DEVX ifc structures
>  * IB/mlx5: Introduce DEVX

Updated mlx5-next with two patches and squashed ifc and commands bits
from third commit into second one.

>
> And resend:
>  * IB/core: Improve uverbs_cleanup_ucontext algorithm
>

Resent.

> Thanks
>
> >
> > Jason



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* [PATCH net v2] ip: limit use of gso_size to udp
From: Willem de Bruijn @ 2018-06-19 16:47 UTC (permalink / raw)
  To: netdev; +Cc: davem, Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

The ipcm(6)_cookie field gso_size is set only in the udp path. The ip
layer copies this to cork only if sk_type is SOCK_DGRAM. This check
proved too permissive. Ping and l2tp sockets have the same type.

Limit to sockets of type SOCK_DGRAM and protocol IPPROTO_UDP to
exclude ping sockets.

v1 -> v2
- remove irrelevant whitespace changes

Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT")
Reported-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>

---

For net-next, I'll take a look whether ipcm(6)_cookie fields like
these can be initialized uniformly, and then this branch removed
completely.
---
 net/ipv4/ip_output.c  | 3 ++-
 net/ipv6/ip6_output.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index af5a830ff6ad..b3308e9d9762 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1145,7 +1145,8 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
 	cork->fragsize = ip_sk_use_pmtu(sk) ?
 			 dst_mtu(&rt->dst) : rt->dst.dev->mtu;
 
-	cork->gso_size = sk->sk_type == SOCK_DGRAM ? ipc->gso_size : 0;
+	cork->gso_size = sk->sk_type == SOCK_DGRAM &&
+			 sk->sk_protocol == IPPROTO_UDP ? ipc->gso_size : 0;
 	cork->dst = &rt->dst;
 	cork->length = 0;
 	cork->ttl = ipc->ttl;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 021e5aef6ba3..a14fb4fcdf18 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1219,7 +1219,8 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
 	if (mtu < IPV6_MIN_MTU)
 		return -EINVAL;
 	cork->base.fragsize = mtu;
-	cork->base.gso_size = sk->sk_type == SOCK_DGRAM ? ipc6->gso_size : 0;
+	cork->base.gso_size = sk->sk_type == SOCK_DGRAM &&
+			      sk->sk_protocol == IPPROTO_UDP ? ipc6->gso_size : 0;
 
 	if (dst_allfrag(xfrm_dst_path(&rt->dst)))
 		cork->base.flags |= IPCORK_ALLFRAG;
-- 
2.18.0.rc1.244.gcf134e6275-goog

^ permalink raw reply related

* [PATCH] net: nixge: Add __packed attribute to DMA descriptor struct
From: Moritz Fischer @ 2018-06-19 16:54 UTC (permalink / raw)
  To: davem; +Cc: keescook, netdev, linux-kernel, Moritz Fischer

Add __packed attribute to DMA descriptor structure  in order to
make sure that the DMA engine's alignemnt requirements are met.

Fixes commit 492caffa8a1a ("net: ethernet: nixge: Add support for
National Instruments XGE netdev")
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---

Hi David,

this addresses an issue where padding occured breaking the alignment
in the array the descriptors are allocated in coherent memory.
This was discovered when we tried to bring up the driver via a PCIe
bridge on x86.

Thanks,

Moritz

---
 drivers/net/ethernet/ni/nixge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 09f674ec0f9e..fea0e994324b 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -122,7 +122,7 @@ struct nixge_hw_dma_bd {
 	u32 sw_id_offset;
 	u32 reserved5;
 	u32 reserved6;
-};
+} __packed;
 
 struct nixge_tx_skb {
 	struct sk_buff *skb;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 1/3] net: ethernet: fix suspend/resume in davinci_emac
From: Florian Fainelli @ 2018-06-19 16:55 UTC (permalink / raw)
  To: Bartosz Golaszewski, Grygorii Strashko, David S . Miller,
	Dan Carpenter, Ivan Khoronzhuk, Rob Herring, Lukas Wunner,
	Kevin Hilman, David Lechner, Sekhar Nori, Andrew Lunn
  Cc: linux-omap, netdev, linux-kernel, Bartosz Golaszewski, stable
In-Reply-To: <20180619160950.6283-2-brgl@bgdev.pl>

On 06/19/2018 09:09 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> This patch reverts commit 3243ff2a05ec ("net: ethernet: davinci_emac:
> Deduplicate bus_find_device() by name matching") and adds a comment
> which should stop anyone from reintroducing the same "fix" in the future.
> 
> We can't use bus_find_device_by_name() here because the device name is
> not guaranteed to be 'davinci_mdio'. On some systems it can be
> 'davinci_mdio.0' so we need to use strncmp() against the first part of
> the string to correctly match it.
> 
> Fixes: 3243ff2a05ec ("net: ethernet: davinci_emac: Deduplicate bus_find_device() by name matching")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/net/ethernet/ti/davinci_emac.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index 06d7c9e4dcda..a1a6445b5a7e 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -1385,6 +1385,11 @@ static int emac_devioctl(struct net_device *ndev, struct ifreq *ifrq, int cmd)
>  		return -EOPNOTSUPP;
>  }
>  
> +static int match_first_device(struct device *dev, void *data)
> +{
> +	return !strncmp(dev_name(dev), "davinci_mdio", 12);

	const char *bus_name = "davinci_mdio";

	return !strncmp(dev_name(dev), bus_name, strlen(bus_name));

Or even better yet, if you want to make sure this really is a PHY device
that you are trying to match, you could try to use sscanf() with PHY_ID_FMT.
-- 
Florian

^ permalink raw reply

* Re: [PATCH 2/3] net: phy: set the of_node in the mdiodev's struct device
From: Florian Fainelli @ 2018-06-19 16:55 UTC (permalink / raw)
  To: Bartosz Golaszewski, Grygorii Strashko, David S . Miller,
	Dan Carpenter, Ivan Khoronzhuk, Rob Herring, Lukas Wunner,
	Kevin Hilman, David Lechner, Sekhar Nori, Andrew Lunn
  Cc: linux-omap, netdev, linux-kernel, Bartosz Golaszewski
In-Reply-To: <20180619160950.6283-3-brgl@bgdev.pl>

On 06/19/2018 09:09 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Copy the of_node over from mii_bus's struct device. This is needed
> for device-tree systems to be able to check the mdio device's
> compatible string.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/net/phy/phy_device.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index bd0f339f69fd..a92d5ee61813 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -411,6 +411,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
>  	mdiodev->dev.parent = &bus->dev;
>  	mdiodev->dev.bus = &mdio_bus_type;
>  	mdiodev->dev.type = &mdio_bus_phy_type;
> +	mdiodev->dev.of_node = bus->dev.of_node;

That does not quite make sense to me, the mdio device's parent already
points to &bus->dev, which would get you the correct of_node. You are
breaking the parent/child relationship here. From patch 3, see my
comments there, it does not look like you are matching on the right
device level.

>  	mdiodev->bus = bus;
>  	mdiodev->bus_match = phy_bus_match;
>  	mdiodev->addr = addr;
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH 3/3] net: davinci_emac: match the mdio device against its compatible if possible
From: Florian Fainelli @ 2018-06-19 16:56 UTC (permalink / raw)
  To: Bartosz Golaszewski, Grygorii Strashko, David S . Miller,
	Dan Carpenter, Ivan Khoronzhuk, Rob Herring, Lukas Wunner,
	Kevin Hilman, David Lechner, Sekhar Nori, Andrew Lunn
  Cc: linux-omap, netdev, linux-kernel, Bartosz Golaszewski
In-Reply-To: <20180619160950.6283-4-brgl@bgdev.pl>

On 06/19/2018 09:09 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Device tree based systems without of_dev_auxdata will have the mdio
> device named differently than "davinci_mdio(.0)". In this case use the
> device's compatible string for matching.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/net/ethernet/ti/davinci_emac.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index a1a6445b5a7e..c28a35bb852f 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -1387,6 +1387,10 @@ static int emac_devioctl(struct net_device *ndev, struct ifreq *ifrq, int cmd)
>  
>  static int match_first_device(struct device *dev, void *data)
>  {
> +	if (dev->of_node)
> +		return of_device_is_compatible(dev->of_node,
> +					       "ti,davinci_mdio");

Why would we be matching the PHY device with the MDIO controller
compatibe string? Why not check dev->parent.of_node instead which would
make more sense?
-- 
Florian

^ permalink raw reply

* [PATCH net 0/5] net sched actions: code style cleanup and fixes
From: Roman Mashak @ 2018-06-19 16:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak

The patchset fixes a few code stylistic issues and typos, as well as one
detected by sparse semantic checker tool.

No functional changes introduced.

Patch 1 & 2 fix coding style bits caught by the checkpatch.pl script
Patch 3 fixes an issue with a shadowed variable
Patch 4 adds sizeof() operator instead of magic number for buffer length
Patch 5 fixes typos in diagnostics messages

Roman Mashak (5):
  net sched actions: fix coding style in pedit action
  net sched actions: fix coding style in pedit headers
  net sched actions: fix sparse warning
  net sched actions: use sizeof operator for buffer length
  net sched actions: fix misleading text strings in pedit action

 include/net/tc_act/tc_pedit.h        |  1 +
 include/uapi/linux/tc_act/tc_pedit.h |  9 ++++++--
 net/sched/act_pedit.c                | 41 +++++++++++++++++++-----------------
 3 files changed, 30 insertions(+), 21 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH net 1/5] net sched actions: fix coding style in pedit action
From: Roman Mashak @ 2018-06-19 16:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1529427368-17129-1-git-send-email-mrv@mojatatu.com>

Fix coding style issues in tc pedit action detected by the
checkpatch script.

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 net/sched/act_pedit.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 8a925c72db5f..e4b29ee79ba8 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -136,15 +136,15 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 {
 	struct tc_action_net *tn = net_generic(net, pedit_net_id);
 	struct nlattr *tb[TCA_PEDIT_MAX + 1];
-	struct nlattr *pattr;
-	struct tc_pedit *parm;
-	int ret = 0, err;
-	struct tcf_pedit *p;
 	struct tc_pedit_key *keys = NULL;
 	struct tcf_pedit_key_ex *keys_ex;
+	struct tc_pedit *parm;
+	struct nlattr *pattr;
+	struct tcf_pedit *p;
+	int ret = 0, err;
 	int ksize;
 
-	if (nla == NULL)
+	if (!nla)
 		return -EINVAL;
 
 	err = nla_parse_nested(tb, TCA_PEDIT_MAX, nla, pedit_policy, NULL);
@@ -175,7 +175,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 			return ret;
 		p = to_pedit(*a);
 		keys = kmalloc(ksize, GFP_KERNEL);
-		if (keys == NULL) {
+		if (!keys) {
 			tcf_idr_release(*a, bind);
 			kfree(keys_ex);
 			return -ENOMEM;
@@ -220,6 +220,7 @@ static void tcf_pedit_cleanup(struct tc_action *a)
 {
 	struct tcf_pedit *p = to_pedit(a);
 	struct tc_pedit_key *keys = p->tcfp_keys;
+
 	kfree(keys);
 	kfree(p->tcfp_keys_ex);
 }
@@ -284,7 +285,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 	if (p->tcfp_nkeys > 0) {
 		struct tc_pedit_key *tkey = p->tcfp_keys;
 		struct tcf_pedit_key_ex *tkey_ex = p->tcfp_keys_ex;
-		enum pedit_header_type htype = TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
+		enum pedit_header_type htype =
+			TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
 		enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
 
 		for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
@@ -316,16 +318,15 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 						hoffset + tkey->at);
 					goto bad;
 				}
-				d = skb_header_pointer(skb, hoffset + tkey->at, 1,
-						       &_d);
+				d = skb_header_pointer(skb, hoffset + tkey->at,
+						       1, &_d);
 				if (!d)
 					goto bad;
 				offset += (*d & tkey->offmask) >> tkey->shift;
 			}
 
 			if (offset % 4) {
-				pr_info("tc filter pedit"
-					" offset must be on 32 bit boundaries\n");
+				pr_info("tc filter pedit offset must be on 32 bit boundaries\n");
 				goto bad;
 			}
 
@@ -335,7 +336,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 				goto bad;
 			}
 
-			ptr = skb_header_pointer(skb, hoffset + offset, 4, &_data);
+			ptr = skb_header_pointer(skb, hoffset + offset,
+						 4, &_data);
 			if (!ptr)
 				goto bad;
 			/* just do it, baby */
@@ -358,8 +360,9 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 		}
 
 		goto done;
-	} else
+	} else {
 		WARN(1, "pedit BUG: index %d\n", p->tcf_index);
+	}
 
 bad:
 	p->tcf_qstats.overlimits++;
-- 
2.7.4

^ permalink raw reply related


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