Netdev List
 help / color / mirror / Atom feed
* RE: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
From: Ran Rozenstein @ 2019-06-25  9:04 UTC (permalink / raw)
  To: Tariq Toukan, Florian Westphal, netdev@vger.kernel.org
  Cc: Maor Gottlieb, edumazet@google.com
In-Reply-To: <08e102a0-8051-e582-56c8-d721bfc9e8b9@mellanox.com>



> -----Original Message-----
> From: Tariq Toukan
> Sent: Monday, June 17, 2019 19:16
> To: Florian Westphal <fw@strlen.de>; netdev@vger.kernel.org
> Cc: Ran Rozenstein <ranro@mellanox.com>; Maor Gottlieb
> <maorg@mellanox.com>; edumazet@google.com
> Subject: Re: [PATCH net-next 0/2] net: ipv4: remove erroneous
> advancement of list pointer
> 
> 
> 
> On 6/17/2019 5:02 PM, Florian Westphal wrote:
> > Tariq reported a soft lockup on net-next that Mellanox was able to
> > bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
> >
> > While reviewing above patch I found a regression when addresses have a
> > lifetime specified.
> >
> > Second patch extends rtnetlink.sh to trigger crash (without first
> > patch applied).
> >
> 
> Thanks Florian.
> 
> Ran, can you please test?

Tested, still reproduce.

^ permalink raw reply

* Re: [PATCH v5 4/5] net: macb: add support for high speed interface
From: Russell King - ARM Linux admin @ 2019-06-25  9:03 UTC (permalink / raw)
  To: Parshuram Raju Thombare
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <SN2PR07MB2480CF15E11D54DA8C3B7319C1E30@SN2PR07MB2480.namprd07.prod.outlook.com>

On Tue, Jun 25, 2019 at 08:49:33AM +0000, Parshuram Raju Thombare wrote:
> >>  	switch (state->interface) {
> >>  	case PHY_INTERFACE_MODE_NA:
> >> +	case PHY_INTERFACE_MODE_USXGMII:
> >> +	case PHY_INTERFACE_MODE_10GKR:
> >> +		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
> >> +			phylink_set(mask, 10000baseCR_Full);
> >> +			phylink_set(mask, 10000baseER_Full);
> >> +			phylink_set(mask, 10000baseKR_Full);
> >> +			phylink_set(mask, 10000baseLR_Full);
> >> +			phylink_set(mask, 10000baseLRM_Full);
> >> +			phylink_set(mask, 10000baseSR_Full);
> >> +			phylink_set(mask, 10000baseT_Full);
> >> +			phylink_set(mask, 5000baseT_Full);
> >> +			phylink_set(mask, 2500baseX_Full);
> >> +			phylink_set(mask, 1000baseX_Full);
> >> +		}
> >If MACB_CAPS_GIGABIT_MODE_AVAILABLE is not set, are these interface
> >modes supported by the hardware?  If the PHY interface mode is not
> >supported, then the returned support mask must be cleared.[] 
> There are some configs which uses this macro to limit data rate to 100M 
> even if hardware support higher rates.

I'm sorry, this response does not address my statement, maybe I wasn't
clear enough.  I am asking about the *PHY* interface modes, in
other words (e.g.) PHY_INTERFACE_MODE_USXGMII.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* RE: [PATCH v5 4/5] net: macb: add support for high speed interface
From: Parshuram Raju Thombare @ 2019-06-25  8:55 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190624134755.u3oq3xr6uergnfs5@shell.armlinux.org.uk>

>> +static inline void gem_mac_configure(struct macb *bp, int speed)
>> +	switch (speed) {
>> +	case SPEED_1000:
>> +		gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> +			   gem_readl(bp, NCFGR));
>> +		break;
>> +	case SPEED_100:
>> +		macb_writel(bp, NCFGR, MACB_BIT(SPD) |
>> +			    macb_readl(bp, NCFGR));
>What happens to the NCFGR register if we call mac_config() first for
>a 1G speed, then 100M and finally 10M - what value does the NCFGR
>register end up with?
>
>I suspect it ends up with both the GBE and SPD bits set, and that is
>probably not what you want.

No, In gem_mac_config GBE and SPD bits are always cleared
before setting appropriate bits as per requested speed, duplex mode.


Regards,
Parshuram Thombare

^ permalink raw reply

* Re: [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence
From: Nicolas.Ferre @ 2019-06-25  8:54 UTC (permalink / raw)
  To: palmer, harinik; +Cc: davem, netdev, linux-kernel
In-Reply-To: <20190625084828.540-3-palmer@sifive.com>

On 25/06/2019 at 10:48, Palmer Dabbelt wrote:
> The help text makes it look like NET_VENDOR_CADENCE enables support for
> Atmel devices, when in reality it's a driver written by Atmel that
> supports Cadence devices.  This may confuse users that have this device
> on a non-Atmel SoC.
> 
> The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig
> help text as that change caused it to go over 80 characters.
> 
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   drivers/net/ethernet/cadence/Kconfig | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index 64d8d6ee7739..f4b3bd85dfe3 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -1,6 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   #
> -# Atmel device configuration
> +# Cadence device configuration
>   #
>   
>   config NET_VENDOR_CADENCE
> @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE
>   	  If unsure, say Y.
>   
>   	  Note that the answer to this question doesn't directly affect the
> -	  kernel: saying N will just cause the configurator to skip all
> -	  the remaining Atmel network card questions. If you say Y, you will be
> +	  kernel: saying N will just cause the configurator to skip all the
> +	  remaining Cadence network card questions. If you say Y, you will be
>   	  asked for your specific card in the following questions.
>   
>   if NET_VENDOR_CADENCE
> 


-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK
From: Nicolas.Ferre @ 2019-06-25  8:54 UTC (permalink / raw)
  To: palmer, harinik; +Cc: davem, netdev, linux-kernel
In-Reply-To: <20190625084828.540-2-palmer@sifive.com>

On 25/06/2019 at 10:48, Palmer Dabbelt wrote:
> commit c218ad559020 ("macb: Add support for SiFive FU540-C000") added a
> dependency on the common clock framework to the macb driver, but didn't
> express that dependency in Kconfig.  As a result macb now fails to
> compile on systems without COMMON_CLK, which specifically causes a build
> failure on powerpc allyesconfig.
> 
> This patch adds the dependency, which results in the macb driver no
> longer being selectable on systems without the common clock framework.
> All known systems that have this device already support the common clock
> framework, so this should not cause trouble for any uses.  Supporting
> both the FU540-C000 and systems without COMMON_CLK is quite ugly.
> 
> I've build tested this on powerpc allyesconfig and RISC-V defconfig
> (which selects MACB), but I have not even booted the resulting kernels.
> 
> Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000")
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks!

> ---
>   drivers/net/ethernet/cadence/Kconfig | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index 1766697c9c5a..64d8d6ee7739 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -21,7 +21,7 @@ if NET_VENDOR_CADENCE
>   
>   config MACB
>   	tristate "Cadence MACB/GEM support"
> -	depends on HAS_DMA
> +	depends on HAS_DMA && COMMON_CLK
>   	select PHYLIB
>   	---help---
>   	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
> @@ -42,7 +42,7 @@ config MACB_USE_HWSTAMP
>   
>   config MACB_PCI
>   	tristate "Cadence PCI MACB/GEM support"
> -	depends on MACB && PCI && COMMON_CLK
> +	depends on MACB && PCI
>   	---help---
>   	  This is PCI wrapper for MACB driver.
>   
> 


-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH v3] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: linmiaohe @ 2019-06-25  8:51 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: kadlec@blackhole.kfki.hu, fw@strlen.de, davem@davemloft.net,
	kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	dsahern@gmail.com, Mingfangsen


On Wed, Jun 19, 2019 at 09:49:04AM +0000, linmiaohe wrote:
> 
> On 2019/6/18 23:58, Pablo Neira Ayuso wrote:
> > On Thu, Apr 25, 2019 at 09:43:53PM +0800, linmiaohe wrote:
> >> From: Miaohe Lin <linmiaohe@huawei.com>
> >>
> >> When firewalld is enabled with ipv4/ipv6 rpfilter, vrf
> >> ipv4/ipv6 packets will be dropped because in device is vrf but out 
> >> device is an enslaved device. So failed with the check of the 
> >> rpfilter.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> >> ---
> >> --- a/net/ipv4/netfilter/ipt_rpfilter.c
> >> +++ b/net/ipv4/netfilter/ipt_rpfilter.c
> >> @@ -81,6 +81,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
> >>  	flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
> >>  	flow.flowi4_tos = RT_TOS(iph->tos);
> >>  	flow.flowi4_scope = RT_SCOPE_UNIVERSE;
> >> +	flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
> >>
> >>  	return rpfilter_lookup_reverse(xt_net(par), &flow, xt_in(par),
> >> --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> >> +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> >> @@ -58,7 +58,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> >>  	if (rpfilter_addr_linklocal(&iph->saddr)) {
> >>  		lookup_flags |= RT6_LOOKUP_F_IFACE;
> >>  		fl6.flowi6_oif = dev->ifindex;
> >> -	} else if ((flags & XT_RPFILTER_LOOSE) == 0)
> >> +	} else if (((flags & XT_RPFILTER_LOOSE) == 0) ||
> >> +		   (netif_is_l3_master(dev)) ||
> >> +		   (netif_is_l3_slave(dev)))
> >>  		fl6.flowi6_oif = dev->ifindex;
> >>
> >>  	rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags); @@
> >> -73,6 +75,12 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> >>  		goto out;
> >>  	}
> >>
> >> +	if (netif_is_l3_master(dev)) {
> >> +		dev = dev_get_by_index_rcu(dev_net(dev), IP6CB(skb)->iif);
> >> +		if (!dev)
> >> +			goto out;
> >> +	}
> > 
> > So, for the l3 device cases this makes:
> > 
> > #1 ip6_route_lookup() to fetch the route, using the device in xt_in()
> >    (the _LOOSE flag is ignored for the l3 device case).
> > 
> > #2 If this is a l3dev master, then you make a global lookup for the
> >    device using IP6CB(skb)->iif.
> > 
> > #3 You check if route matches with the device, using the new device
> >    from the lookup:
> > 
> >    if (rt->rt6i_idev->dev == dev ...
> > 
> > If there is no other way to fix this, OK, that's fair enough.
> > 
> > Still this fix looks a bit tricky to me.
> > 
> > And this assymmetric between the IPv4 and IPv6 codebase looks rare.
> > 
> > Probably someone can explain me this in more detail? I'd appreciate.
> > 
> > Thanks!
> > 
> Thanks for explaining.
>
> Something must be wrong in all these helper function logic because this new code logic is hard to follow for the IPv6 chunk...
>
> Can you explore a more readable fix?
>
> So I'm not inclined to quickly take this patch.
>
> Thanks.

Thanks, I will try a more readable fix.

^ permalink raw reply

* RE: [PATCH v5 4/5] net: macb: add support for high speed interface
From: Parshuram Raju Thombare @ 2019-06-25  8:49 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190624134755.u3oq3xr6uergnfs5@shell.armlinux.org.uk>

>>  	switch (state->interface) {
>>  	case PHY_INTERFACE_MODE_NA:
>> +	case PHY_INTERFACE_MODE_USXGMII:
>> +	case PHY_INTERFACE_MODE_10GKR:
>> +		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
>> +			phylink_set(mask, 10000baseCR_Full);
>> +			phylink_set(mask, 10000baseER_Full);
>> +			phylink_set(mask, 10000baseKR_Full);
>> +			phylink_set(mask, 10000baseLR_Full);
>> +			phylink_set(mask, 10000baseLRM_Full);
>> +			phylink_set(mask, 10000baseSR_Full);
>> +			phylink_set(mask, 10000baseT_Full);
>> +			phylink_set(mask, 5000baseT_Full);
>> +			phylink_set(mask, 2500baseX_Full);
>> +			phylink_set(mask, 1000baseX_Full);
>> +		}
>If MACB_CAPS_GIGABIT_MODE_AVAILABLE is not set, are these interface
>modes supported by the hardware?  If the PHY interface mode is not
>supported, then the returned support mask must be cleared.[] 
There are some configs which uses this macro to limit data rate to 100M 
even if hardware support higher rates.
Empty link mode mask is initialized at the beginning and supported link 
modes are added to it and at the end of this function this mask is AND'ed 
with supported mask.

>> +static inline void gem_mac_configure(struct macb *bp, int speed)
>> +{
>> +	if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
>> +		gem_writel(bp, NCFGR, GEM_BIT(SGMIIEN) |
>> +			   GEM_BIT(PCSSEL) |
>> +			   gem_readl(bp, NCFGR));
>Is this still necessary?
Sorry, missed this. I will remove in next patch set.

Regards,
Parshuram Thombare

^ permalink raw reply

* Re: [PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
From: Nicolas Dichtel @ 2019-06-25  8:49 UTC (permalink / raw)
  To: Lucas Bates, davem
  Cc: netdev, jhs, xiyou.wangcong, jiri, mleitner, vladbu, dcaratti,
	kernel
In-Reply-To: <1561424427-9949-1-git-send-email-lucasb@mojatatu.com>

Le 25/06/2019 à 03:00, Lucas Bates a écrit :
> This patch restores the original behaviour for tdc prior to the
> introduction of the plugin system, where the network namespace
> functionality was split from the main script.
> 
> It introduces the concept of required plugins for testcases,
> and will automatically load any plugin that isn't already
> enabled when said plugin is required by even one testcase.
> 
> Additionally, the -n option for the nsPlugin is deprecated
> so the default action is to make use of the namespaces.
> Instead, we introduce -N to not use them, but still create
> the veth pair.
> 
> buildebpfPlugin's -B option is also deprecated.
> 
> If a test cases requires the features of a specific plugin
> in order to pass, it should instead include a new key/value
> pair describing plugin interactions:
> 
>         "plugins": {
>                 "requires": "buildebpfPlugin"
>         },
> 
> A test case can have more than one required plugin: a list
> can be inserted as the value for 'requires'.
> 
> Signed-off-by: Lucas Bates <lucasb@mojatatu.com>

Thank you for the follow up!

Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

[snip]

> @@ -550,6 +614,7 @@ def filter_tests_by_category(args, testlist):
>  
>      return answer
>  
> +
>  def get_test_cases(args):
>      """
>      If a test case file is specified, retrieve tests from that file.
nit: this new line is probably a leftover of a previous version ;-)

^ permalink raw reply

* net: macb: Fix compilation on systems without COMMON_CLK, v2
From: Palmer Dabbelt @ 2019-06-25  8:48 UTC (permalink / raw)
  To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel

Our patch to add support for the FU540-C000 broke compilation on at
least powerpc allyesconfig, which was found as part of the linux-next
build regression tests.  This must have somehow slipped through the
cracks, as the patch has been reverted in linux-next for a while now.
This patch applies on top of the offending commit, which is the only one
I've even tried it on as I'm not sure how this subsystem makes it to
Linus.

This patch set fixes the issue by adding a dependency of COMMON_CLK to
the MACB Kconfig entry, which avoids the build failure by disabling MACB
on systems where it wouldn't compile.  All known users of MACB have
COMMON_CLK, so this shouldn't cause any issues.  This is a significantly
simpler approach than disabling just the FU540-C000 support.

I've also included a second patch to indicate this is a driver for a
Cadence device that was originally written by an engineer at Atmel.  The
only relation is that I stumbled across it when writing the first patch.

Changes since v1 <20190624061603.1704-1-palmer@sifive.com>:

* Disable MACB on systems without COMMON_CLK, instead of just disabling
  the FU540-C000 support on these systems.
* Update the commit message to reflect the driver was written by Atmel.



^ permalink raw reply

* [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK
From: Palmer Dabbelt @ 2019-06-25  8:48 UTC (permalink / raw)
  To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel, Palmer Dabbelt
In-Reply-To: <20190625084828.540-1-palmer@sifive.com>

commit c218ad559020 ("macb: Add support for SiFive FU540-C000") added a
dependency on the common clock framework to the macb driver, but didn't
express that dependency in Kconfig.  As a result macb now fails to
compile on systems without COMMON_CLK, which specifically causes a build
failure on powerpc allyesconfig.

This patch adds the dependency, which results in the macb driver no
longer being selectable on systems without the common clock framework.
All known systems that have this device already support the common clock
framework, so this should not cause trouble for any uses.  Supporting
both the FU540-C000 and systems without COMMON_CLK is quite ugly.

I've build tested this on powerpc allyesconfig and RISC-V defconfig
(which selects MACB), but I have not even booted the resulting kernels.

Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000")
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 drivers/net/ethernet/cadence/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 1766697c9c5a..64d8d6ee7739 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -21,7 +21,7 @@ if NET_VENDOR_CADENCE
 
 config MACB
 	tristate "Cadence MACB/GEM support"
-	depends on HAS_DMA
+	depends on HAS_DMA && COMMON_CLK
 	select PHYLIB
 	---help---
 	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
@@ -42,7 +42,7 @@ config MACB_USE_HWSTAMP
 
 config MACB_PCI
 	tristate "Cadence PCI MACB/GEM support"
-	depends on MACB && PCI && COMMON_CLK
+	depends on MACB && PCI
 	---help---
 	  This is PCI wrapper for MACB driver.
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence
From: Palmer Dabbelt @ 2019-06-25  8:48 UTC (permalink / raw)
  To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel, Palmer Dabbelt
In-Reply-To: <20190625084828.540-1-palmer@sifive.com>

The help text makes it look like NET_VENDOR_CADENCE enables support for
Atmel devices, when in reality it's a driver written by Atmel that
supports Cadence devices.  This may confuse users that have this device
on a non-Atmel SoC.

The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig
help text as that change caused it to go over 80 characters.

Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 drivers/net/ethernet/cadence/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 64d8d6ee7739..f4b3bd85dfe3 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 #
-# Atmel device configuration
+# Cadence device configuration
 #
 
 config NET_VENDOR_CADENCE
@@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE
 	  If unsure, say Y.
 
 	  Note that the answer to this question doesn't directly affect the
-	  kernel: saying N will just cause the configurator to skip all
-	  the remaining Atmel network card questions. If you say Y, you will be
+	  kernel: saying N will just cause the configurator to skip all the
+	  remaining Cadence network card questions. If you say Y, you will be
 	  asked for your specific card in the following questions.
 
 if NET_VENDOR_CADENCE
-- 
2.21.0


^ permalink raw reply related

* [PATCH v1 2/2] net: dsa: qca8k: introduce reset via gpio feature
From: Christian Lamparter @ 2019-06-25  8:41 UTC (permalink / raw)
  To: netdev
  Cc: Mark Rutland, Rob Herring, David S . Miller, Florian Fainelli,
	Vivien Didelot, Andrew Lunn
In-Reply-To: <08e0fd513620f03a2207b9f32637cdb434ed8def.1561452044.git.chunkeey@gmail.com>

The QCA8337(N) has a RESETn signal on Pin B42 that
triggers a chip reset if the line is pulled low.
The datasheet says that: "The active low duration
must be greater than 10 ms".

This can hopefully fix some of the issues related
to pin strapping in OpenWrt for the EA8500 which
suffers from detection issues after a SoC reset.

Please note that the qca8k_probe() function does
currently require to read the chip's revision
register for identification purposes.

Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 drivers/net/dsa/qca8k.c | 15 +++++++++++++++
 drivers/net/dsa/qca8k.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index c4fa400efdcc..27709f866c23 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -14,6 +14,7 @@
 #include <linux/of_platform.h>
 #include <linux/if_bridge.h>
 #include <linux/mdio.h>
+#include <linux/gpio.h>
 #include <linux/etherdevice.h>
 
 #include "qca8k.h"
@@ -1046,6 +1047,20 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
 	priv->bus = mdiodev->bus;
 	priv->dev = &mdiodev->dev;
 
+	priv->reset_gpio = devm_gpiod_get_optional(priv->dev, "reset",
+						   GPIOD_ASIS);
+	if (IS_ERR(priv->reset_gpio))
+		return PTR_ERR(priv->reset_gpio);
+
+	if (priv->reset_gpio) {
+		gpiod_set_value_cansleep(priv->reset_gpio, 1);
+		/* The active low duration must be greater than 10 ms
+		 * and checkpatch.pl wants 20 ms.
+		 */
+		msleep(20);
+		gpiod_set_value_cansleep(priv->reset_gpio, 0);
+	}
+
 	/* read the switches ID register */
 	id = qca8k_read(priv, QCA8K_REG_MASK_CTRL);
 	id >>= QCA8K_MASK_CTRL_ID_S;
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 91557433ce2f..42d6ea24eb14 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -10,6 +10,7 @@
 
 #include <linux/delay.h>
 #include <linux/regmap.h>
+#include <linux/gpio.h>
 
 #define QCA8K_NUM_PORTS					7
 
@@ -174,6 +175,7 @@ struct qca8k_priv {
 	struct mutex reg_mutex;
 	struct device *dev;
 	struct dsa_switch_ops ops;
+	struct gpio_desc *reset_gpio;
 };
 
 struct qca8k_mib_desc {
-- 
2.20.1


^ permalink raw reply related

* [PATCH v1 1/2] dt-bindings: net: dsa: qca8k: document reset-gpios property
From: Christian Lamparter @ 2019-06-25  8:41 UTC (permalink / raw)
  To: netdev
  Cc: Mark Rutland, Rob Herring, David S . Miller, Florian Fainelli,
	Vivien Didelot, Andrew Lunn

This patch documents the qca8k's reset-gpios property that
can be used if the QCA8337N ends up in a bad state during
reset.

Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/qca8k.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
index 93a7469e70d4..ccbc6d89325d 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
@@ -9,6 +9,10 @@ Required properties:
 - #size-cells: must be 0
 - #address-cells: must be 1
 
+Optional properties:
+
+- reset-gpios: GPIO to be used to reset the whole device
+
 Subnodes:
 
 The integrated switch subnode should be specified according to the binding
@@ -66,6 +70,7 @@ for the external mdio-bus configuration:
 			#address-cells = <1>;
 			#size-cells = <0>;
 
+			reset-gpios = <&gpio 42 GPIO_ACTIVE_LOW>;
 			reg = <0x10>;
 
 			ports {
@@ -123,6 +128,7 @@ for the internal master mdio-bus configuration:
 			#address-cells = <1>;
 			#size-cells = <0>;
 
+			reset-gpios = <&gpio 42 GPIO_ACTIVE_LOW>;
 			reg = <0x10>;
 
 			ports {
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next 04/12] net: sched: add tcf_block_setup()
From: Pablo Neira Ayuso @ 2019-06-25  8:31 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
	ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
	salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
	jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
	vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
	ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
	venkatkumar.duvvuru, cphealy
In-Reply-To: <20190621171603.GF2414@nanopsycho.orion>

On Fri, Jun 21, 2019 at 07:16:03PM +0200, Jiri Pirko wrote:
> Thu, Jun 20, 2019 at 09:49:09PM CEST, pablo@netfilter.org wrote:
> 
> [...]
> 
> > 
> >+static LIST_HEAD(tcf_block_cb_list);
> 
> I still don't like the global list. Have to go throught the code more
> carefully, but why you can't pass the priv/ctx from tc/netfilter. From
> tc it would be tcf_block as it is now, from netfilter something else.

This tcf_block_cb_list should go away at some point, once drivers know
how to deal with multiple subsystems using the setup block
infrastructure. As I said in my previous email, only one can set up
the block at this stage, the ones coming later will hit busy.

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: sched: protect against stack overflow in TC act_mirred
From: Eyal Birger @ 2019-06-25  8:30 UTC (permalink / raw)
  To: John Hurley
  Cc: netdev, davem, fw, jhs, simon.horman, jakub.kicinski, oss-drivers,
	shmulik
In-Reply-To: <1561414416-29732-3-git-send-email-john.hurley@netronome.com>

Hi John,

On Mon, 24 Jun 2019 23:13:36 +0100
John Hurley <john.hurley@netronome.com> wrote:

> TC hooks allow the application of filters and actions to packets at
> both ingress and egress of the network stack. It is possible, with
> poor configuration, that this can produce loops whereby an ingress
> hook calls a mirred egress action that has an egress hook that
> redirects back to the first ingress etc. The TC core classifier
> protects against loops when doing reclassifies but there is no
> protection against a packet looping between multiple hooks and
> recursively calling act_mirred. This can lead to stack overflow
> panics.
> 
> Add a per CPU counter to act_mirred that is incremented for each
> recursive call of the action function when processing a packet. If a
> limit is passed then the packet is dropped and CPU counter reset.
> 
> Note that this patch does not protect against loops in TC datapaths.
> Its aim is to prevent stack overflow kernel panics that can be a
> consequence of such loops.
> 
> Signed-off-by: John Hurley <john.hurley@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> ---
>  net/sched/act_mirred.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 8c1d736..c3fce36 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -27,6 +27,9 @@
>  static LIST_HEAD(mirred_list);
>  static DEFINE_SPINLOCK(mirred_list_lock);
>  
> +#define MIRRED_RECURSION_LIMIT    4

Could you increase the limit to maybe 6 or 8? I am aware of cases where
mirred ingress is used for cascading several layers of logical network
interfaces and 4 seems a little limiting.

Thanks,
Eyal.

^ permalink raw reply

* RE: [PATCH v4 4/5] net: macb: add support for high speed interface
From: Parshuram Raju Thombare @ 2019-06-25  8:26 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, linux@armlinux.org.uk,
	netdev@vger.kernel.org, hkallweit1@gmail.com,
	linux-kernel@vger.kernel.org, Rafal Ciepiela, Anil Joy Varughese,
	Piotr Sroka
In-Reply-To: <20190624131307.GA17872@lunn.ch>

Hi Andrew,

>What i'm saying is that the USXGMII rate is fixed. So why do you need a device
>tree property for the SERDES rate?
This is based on Cisco USXGMII specification, it specify USXGMII 5G and USXGMII 10G.
Sorry I can't share that document here.

Regards,
Parshuram Thombare

^ permalink raw reply

* Re: [PATCH RFC net-next 5/5] net: dsa: mt7530: Add mediatek,ephy-handle to isolate external phy
From: René van Dorst @ 2019-06-25  8:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: sean.wang, f.fainelli, linux, davem, matthias.bgg, vivien.didelot,
	frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190624215248.GC31306@lunn.ch>

Quoting Andrew Lunn <andrew@lunn.ch>:

Hi Andrew,

>> +static int mt7530_isolate_ephy(struct dsa_switch *ds,
>> +			       struct device_node *ephy_node)
>> +{
>> +	struct phy_device *phydev = of_phy_find_device(ephy_node);
>> +	int ret;
>> +
>> +	if (!phydev)
>> +		return 0;
>> +
>> +	ret = phy_modify(phydev, MII_BMCR, 0, (BMCR_ISOLATE | BMCR_PDOWN));
>
> genphy_suspend() does what you want.

In case my device has AT8033 PHY which act as a RGMII-to-SGMII  
converter for the
SFP cage.

Qoute of the AR8031/33 datasheet:

The AR8033 device supports the low power mode with software power-down.
To enter the standard IEEE power-down mode, set the bit[11] POWER_DOWN of
Control register - copper page or Control register — fiber page to 1.
In this mode, AR8033 ignores all MAC interface signals except the MDC/MDIO and
does not respond to any activity on the media side. AR8033 cannot wake  
up on its
own and is only waken up by setting the POWER_DOWN bit to 0.


Does "standard IEEE power-down mode" describ this behavior that in power-down
mode the RGMII are also put in tri-state?

Reading the datasheet does not give me any clues.
Putting RGMII signals in tri-state is important in this case.

>
>> +	if (ret)
>> +		dev_err(ds->dev, "Failed to put phy %s in isolation mode!\n",
>> +			ephy_node->full_name);
>> +	else
>> +		dev_info(ds->dev, "Phy %s in isolation mode!\n",
>> +			 ephy_node->full_name);
>
> No need to clog up the system with yet more kernel messages.

OK, I remove it.

>
>    Andrew

Greats,

René




^ permalink raw reply

* Re: [PATCH net-next 11/12] net: flow_offload: don't allow block sharing until drivers support this
From: Pablo Neira Ayuso @ 2019-06-25  8:22 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
	ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
	salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
	jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
	vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
	ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
	venkatkumar.duvvuru, cphealy
In-Reply-To: <20190625081627.GA2630@nanopsycho>

On Tue, Jun 25, 2019 at 10:16:27AM +0200, Jiri Pirko wrote:
> I don't understand the purpose of this patch. Could you please provide
> some description about what this is about. mlxsw supports block sharing
> between ports. Or what kind of "sharing" do you have in mind?

I'm refering to ethtool, tc and netfilter potentially using the same
tc setup infrastructure infrastructure.

At this stage, if one of them sets up the flow block, then, other
subsystems will hit busy.

^ permalink raw reply

* [PATCH net-next] net: stmmac: Fix the case when PHY handle is not present
From: Jose Abreu @ 2019-06-25  8:19 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue

Some DT bindings do not have the PHY handle. Let's fallback to manually
discovery in case phylink_of_phy_connect() fails.

Reported-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
Hello Katsuhiro,

Can you please test this patch ?
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a48751989fa6..f4593d2d9d20 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -950,9 +950,12 @@ static int stmmac_init_phy(struct net_device *dev)
 
 	node = priv->plat->phylink_node;
 
-	if (node) {
+	if (node)
 		ret = phylink_of_phy_connect(priv->phylink, node, 0);
-	} else {
+
+	/* Some DT bindings do not set-up the PHY handle. Let's try to
+	 * manually parse it */
+	if (!node || ret) {
 		int addr = priv->plat->phy_addr;
 		struct phy_device *phydev;
 
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v3] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: Pablo Neira Ayuso @ 2019-06-25  8:17 UTC (permalink / raw)
  To: linmiaohe
  Cc: kadlec@blackhole.kfki.hu, fw@strlen.de, davem@davemloft.net,
	kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	dsahern@gmail.com, Mingfangsen
In-Reply-To: <30442ee669c44d9db01fb374b73fd2dd@huawei.com>

On Wed, Jun 19, 2019 at 09:49:04AM +0000, linmiaohe wrote:
> 
> On 2019/6/18 23:58, Pablo Neira Ayuso wrote:
> > On Thu, Apr 25, 2019 at 09:43:53PM +0800, linmiaohe wrote:
> >> From: Miaohe Lin <linmiaohe@huawei.com>
> >>
> >> When firewalld is enabled with ipv4/ipv6 rpfilter, vrf
> >> ipv4/ipv6 packets will be dropped because in device is vrf but out 
> >> device is an enslaved device. So failed with the check of the 
> >> rpfilter.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> >> ---
> >> --- a/net/ipv4/netfilter/ipt_rpfilter.c
> >> +++ b/net/ipv4/netfilter/ipt_rpfilter.c
> >> @@ -81,6 +81,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
> >>  	flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
> >>  	flow.flowi4_tos = RT_TOS(iph->tos);
> >>  	flow.flowi4_scope = RT_SCOPE_UNIVERSE;
> >> +	flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
> >>
> >>  	return rpfilter_lookup_reverse(xt_net(par), &flow, xt_in(par),
> >> --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> >> +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> >> @@ -58,7 +58,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> >>  	if (rpfilter_addr_linklocal(&iph->saddr)) {
> >>  		lookup_flags |= RT6_LOOKUP_F_IFACE;
> >>  		fl6.flowi6_oif = dev->ifindex;
> >> -	} else if ((flags & XT_RPFILTER_LOOSE) == 0)
> >> +	} else if (((flags & XT_RPFILTER_LOOSE) == 0) ||
> >> +		   (netif_is_l3_master(dev)) ||
> >> +		   (netif_is_l3_slave(dev)))
> >>  		fl6.flowi6_oif = dev->ifindex;
> >>
> >>  	rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags); @@
> >> -73,6 +75,12 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> >>  		goto out;
> >>  	}
> >>
> >> +	if (netif_is_l3_master(dev)) {
> >> +		dev = dev_get_by_index_rcu(dev_net(dev), IP6CB(skb)->iif);
> >> +		if (!dev)
> >> +			goto out;
> >> +	}
> > 
> > So, for the l3 device cases this makes:
> > 
> > #1 ip6_route_lookup() to fetch the route, using the device in xt_in()
> >    (the _LOOSE flag is ignored for the l3 device case).
> > 
> > #2 If this is a l3dev master, then you make a global lookup for the
> >    device using IP6CB(skb)->iif.
> > 
> > #3 You check if route matches with the device, using the new device
> >    from the lookup:
> > 
> >    if (rt->rt6i_idev->dev == dev ...
> > 
> > If there is no other way to fix this, OK, that's fair enough.
> > 
> > Still this fix looks a bit tricky to me.
> > 
> > And this assymmetric between the IPv4 and IPv6 codebase looks rare.
> > 
> > Probably someone can explain me this in more detail? I'd appreciate.
> > 
> > Thanks!
> > 
>     Thanks for your reply. I will try to explain this in more detail.
>     Vrf device will pass through netfilter hook twice. One with skb->dev=l3mdev
> Slave device and another one with skb->dev=l3mdev master deivce.
>     If a device is an l3mdev,  l3mdev_master_ifindex_rcu will return l3mdev
> master device ifindex otherwise 0 . So for non l3mdev cases,  v4 version is
> as same as the previous one. And for l3mdev cases,  flow.flowi4_oif
> will be l3mdev master device ifindex, so we can do a fib lookup in l3mdev
> domain as expected. Since fib_info_nh_uses_dev help us handle the case with
> dev=l3mdev slave or master and  XT_RPFILTER_LOOSE do not lookup route
> table, we finish v4.
>     For v6 version we need to set fl6.flowi6_oif as we are supposed to lookup 
> fib in l3mdev domain even in XT_RPFILTER_LOOSE mode.
>     And fib result rt->rt6i_idev->dev is l3mdev slave device, we need change
> dev to enslaved l3mdev device when dev passed in is l3mdev master device.
>     The key is l3mdev will pass through netfilter hook twice with skb dev is l3mdev slave
> and master . And we need to set flowi6_oif as fib lookup should in the l3mdev
> domain.

Thanks for explaining.

Something must be wrong in all these helper function logic because
this new code logic is hard to follow for the IPv6 chunk...

Can you explore a more readable fix?

So I'm not inclined to quickly take this patch.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 11/12] net: flow_offload: don't allow block sharing until drivers support this
From: Jiri Pirko @ 2019-06-25  8:16 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
	ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
	salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
	jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
	vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
	ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
	venkatkumar.duvvuru, cphealy
In-Reply-To: <20190620194917.2298-12-pablo@netfilter.org>

I don't understand the purpose of this patch. Could you please provide
some description about what this is about. mlxsw supports block sharing
between ports. Or what kind of "sharing" do you have in mind?

^ permalink raw reply

* Re: [RFC bpf-next 0/7] Programming socket lookup with BPF
From: Jakub Sitnicki @ 2019-06-25  8:11 UTC (permalink / raw)
  To: Joe Stringer; +Cc: Florian Westphal, netdev, bpf, kernel-team
In-Reply-To: <CAOftzPhGVeLpqbffLwBP8JCvY1t65-uXztEsZV0qJEQapywRgg@mail.gmail.com>

On Fri, Jun 21, 2019 at 06:50 PM CEST, Joe Stringer wrote:
> On Fri, Jun 21, 2019 at 1:44 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>>
>> On Fri, Jun 21, 2019, 00:20 Joe Stringer <joe@wand.net.nz> wrote:
>>>
>>> On Wed, Jun 19, 2019 at 2:14 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>>> >
>>> > Hey Florian,
>>> >
>>> > Thanks for taking a look at it.
>>> >
>>> > On Tue, Jun 18, 2019 at 03:52 PM CEST, Florian Westphal wrote:
>>> > > Jakub Sitnicki <jakub@cloudflare.com> wrote:
>>> > >>  - XDP programs using bpf_sk_lookup helpers, like load balancers, can't
>>> > >>    find the listening socket to check for SYN cookies with TPROXY redirect.
>>> > >
>>> > > Sorry for the question, but where is the problem?
>>> > > (i.e., is it with TPROXY or bpf side)?
>>> >
>>> > The way I see it is that the problem is that we have mappings for
>>> > steering traffic into sockets split between two places: (1) the socket
>>> > lookup tables, and (2) the TPROXY rules.
>>> >
>>> > BPF programs that need to check if there is a socket the packet is
>>> > destined for have access to the socket lookup tables, via the mentioned
>>> > bpf_sk_lookup helper, but are unaware of TPROXY redirects.
>>> >
>>> > For TCP we're able to look up from BPF if there are any established,
>>> > request, and "normal" listening sockets. The listening sockets that
>>> > receive connections via TPROXY are invisible to BPF progs.
>>> >
>>> > Why are we interested in finding all listening sockets? To check if any
>>> > of them had SYN queue overflow recently and if we should honor SYN
>>> > cookies.
>>>
>>> Why are they invisible? Can't you look them up with bpf_skc_lookup_tcp()?
>>
>>
>> They are invisible in that sense that you can't look them up using the packet 4-tuple. You have to somehow make the XDP/TC progs aware of the TPROXY redirects to find the target sockets.
>
> Isn't that what you're doing in the example from the cover letter
> (reincluded below for reference), except with the new program type
> rather than XDP/TC progs?
>
>        switch (bpf_ntohl(ctx->local_ip4) >> 8) {
>         case NET1:
>                 ctx->local_ip4 = bpf_htonl(IP4(127, 0, 0, 1));
>                 ctx->local_port = 81;
>                 return BPF_REDIRECT;
>         case NET2:
>                 ctx->local_ip4 = bpf_htonl(IP4(127, 0, 0, 1));
>                 ctx->local_port = 82;
>                 return BPF_REDIRECT;
>         }
>
> That said, I appreciate that even if you find the sockets from XDP,
> you'd presumably need some way to retain the socket reference beyond
> XDP execution to convince the stack to guide the traffic into that
> socket, which would be a whole other effort. For your use case it may
> or may not make the most sense.

Granted we're just moving steering logic from one place to another, that
is from TPROXY rules to a BPF program.

The key here is that the BPF prog runs during inet_lookup.  This let's
"lower level" BPF progs like XDP or TC check if there is a destination
socket, without having to know about steering rules.

If there is a local socket, we don't need to do socket dispatch from
BPF. Just pass the packet up the stack.

-Jakub

^ permalink raw reply

* Re: [PATCH bpf-next] libbpf: add xsk_ring_prod__free() function
From: Eelco Chaudron @ 2019-06-25  7:58 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Karlsson, Magnus, Networking, Alexei Starovoitov, Daniel Borkmann,
	Martin Lau, Song Liu, Yonghong Song
In-Reply-To: <CAEf4Bzb7A5abJaxxrS5sudCE=Ca0C9rY0B23OjK8c2RCCx=Y6g@mail.gmail.com>



On 24 Jun 2019, at 18:42, Andrii Nakryiko wrote:

> On Mon, Jun 24, 2019 at 2:37 AM Eelco Chaudron <echaudro@redhat.com> 
> wrote:
>>
>>
>>
>> On 21 Jun 2019, at 21:13, Andrii Nakryiko wrote:
>>
>>> On Fri, Jun 21, 2019 at 8:26 AM Eelco Chaudron <echaudro@redhat.com>
>>> wrote:
>>>>
>>>> When an AF_XDP application received X packets, it does not mean X
>>>> frames can be stuffed into the producer ring. To make it easier for
>>>> AF_XDP applications this API allows them to check how many frames 
>>>> can
>>>> be added into the ring.
>>>>
>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>>> ---
>>>>  tools/lib/bpf/xsk.h | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>>>> index 82ea71a0f3ec..86f3d485e957 100644
>>>> --- a/tools/lib/bpf/xsk.h
>>>> +++ b/tools/lib/bpf/xsk.h
>>>> @@ -95,6 +95,12 @@ static inline __u32 xsk_prod_nb_free(struct
>>>> xsk_ring_prod *r, __u32 nb)
>>>>         return r->cached_cons - r->cached_prod;
>>>>  }
>>>>
>>>> +static inline __u32 xsk_ring_prod__free(struct xsk_ring_prod *r)
>>>
>>> This is a very bad name choice. __free is used for functions that 
>>> free
>>> memory and resources. One function below I see avail is used in the
>>> name, why not xsk_ring_prog__avail?
>>
>> Must agree that free sound like you are freeing entries… However, I
>> just kept the naming already in the API/file (see above,
>> xsk_prod_nb_free()).
>> Reading the code there is a difference as xx_avail() means available
>> filled entries, where xx_free() means available free entries.
>>
>> So I could rename it to xsk_ring_prod__nb_free() maybe?
>
> I'm fine with __nb_free, yes. Thanks!

Ok, will rework the patch and use xsk_ring_prod__nb_free(). Will also 
take Magnus suggestion into account, and create a cached version (and 
use it internally).

>>
>> Forgot to include Magnus in the email, so copied him in, for some
>> comments.
>>
>>>> +{
>>>> +       r->cached_cons = *r->consumer + r->size;
>>>> +       return r->cached_cons - r->cached_prod;
>>>> +}
>>>> +
>>>>  static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, 
>>>> __u32
>>>> nb)
>>>>  {
>>>>         __u32 entries = r->cached_prod - r->cached_cons;
>>>> --
>>>> 2.20.1
>>>>

^ permalink raw reply

* Re: [PATCH] net: stmmac: add sanity check to device_property_read_u32_array call
From: Colin Ian King @ 2019-06-25  7:58 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: alexandre.torgue, davem, joabreu, kernel-janitors,
	linux-arm-kernel, linux-kernel, linux-stm32, mcoquelin.stm32,
	netdev, peppe.cavallaro
In-Reply-To: <CAFBinCC-LLpfXQRFcKBbUpCfKc0S9Xtt60QrhEThsOFV-T7vFw@mail.gmail.com>

On 25/06/2019 05:44, Martin Blumenstingl wrote:
> Hi Colin,
> 
> On Thu, Jun 20, 2019 at 3:34 AM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
>>
>> Hi Colin,
>>
>> On Wed, Jun 19, 2019 at 8:55 AM Colin Ian King <colin.king@canonical.com> wrote:
>>>
>>> On 19/06/2019 06:13, Martin Blumenstingl wrote:
>>>> Hi Colin,
>>>>
>>>>> Currently the call to device_property_read_u32_array is not error checked
>>>>> leading to potential garbage values in the delays array that are then used
>>>>> in msleep delays.  Add a sanity check to the property fetching.
>>>>>
>>>>> Addresses-Coverity: ("Uninitialized scalar variable")
>>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>>> I have also sent a patch [0] to fix initialize the array.
>>>> can you please look at my patch so we can work out which one to use?
>>>>
>>>> my concern is that the "snps,reset-delays-us" property is optional,
>>>> the current dt-bindings documentation states that it's a required
>>>> property. in reality it isn't, there are boards (two examples are
>>>> mentioned in my patch: [0]) without it.
>>>>
>>>> so I believe that the resulting behavior has to be:
>>>> 1. don't delay if this property is missing (instead of delaying for
>>>>    <garbage value> ms)
>>>> 2. don't error out if this property is missing
>>>>
>>>> your patch covers #1, can you please check whether #2 is also covered?
>>>> I tested case #2 when submitting my patch and it worked fine (even
>>>> though I could not reproduce the garbage values which are being read
>>>> on some boards)
> in the meantime I have tested your patch.
> when I don't set the "snps,reset-delays-us" property then I get the
> following error:
>   invalid property snps,reset-delays-us
> 
> my patch has landed in the meantime: [0]
> how should we proceed with your patch?

I'm out of the office today. I'll get back to you on this tomorrow.

Colin
> 
> 
> Martin
> 
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c?id=84ce4d0f9f55b4f4ca4d4edcbb54a23d9dad1aae
> 


^ permalink raw reply

* Re: stmmac regression on ASUS TinkerBoard
From: Heiko Stübner @ 2019-06-25  7:50 UTC (permalink / raw)
  To: Jose Abreu
  Cc: Katsuhiro Suzuki, Giuseppe Cavallaro, Alexandre Torgue,
	Maxime Coquelin, netdev@vger.kernel.org, Andrew Lunn,
	linux-arm-kernel, Linux Kernel Mailing List
In-Reply-To: <78EB27739596EE489E55E81C33FEC33A0B9D7065@DE02WEMBXB.internal.synopsys.com>

Hi,

Am Dienstag, 25. Juni 2019, 09:46:12 CEST schrieb Jose Abreu:
> From: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> 
> > I checked drivers/net/ethernet/stmicro/stmmac/stmmac_main.c and found
> > stmmac_init_phy() is going to fail if ethernet device node does not
> > have following property:
> >    - phy-handle
> >    - phy
> >    - phy-device
> > 
> > This commit broke the device-trees such as TinkerBoard. The mdio
> > subnode creating a mdio bus is changed to required or still optional?
> 
> Yeah, with PHYLINK the PHY binding is always required ...
> 
> How do you want to proceed ? I think DT bindings can never break between 
> releases so I will probably need to cook a patch for stmmac.

Correct ... old devicetrees on new kernels should not break.
Especially as this affects a big number of boards potentially loosing
network support and in the devicetree binding the phy property is also
marked as optional.

Heiko



^ 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