Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3 0/4] fsl/fman: fixes for ARM
From: Madalin Bucur @ 2016-12-19 16:13 UTC (permalink / raw)
  To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe

The patch set fixes advertised speeds for QSGMII interfaces, disables
A007273 erratum workaround on non-PowerPC platforms where it does not
apply, enables compilation on ARM64 and addresses a probing issue on
non PPC platforms.

Changes from v2: merged fsl/fman changes to avoid a point of failure
Changes from v1: unifying probing on all supported platforms

Madalin Bucur (4):
  fsl/fman: fix 1G support for QSGMII interfaces
  powerpc: fsl/fman: remove fsl,fman from of_device_ids[]
  fsl/fman: A007273 only applies to PPC SoCs
  fsl/fman: enable compilation on ARM64

 arch/powerpc/platforms/85xx/corenet_generic.c |  3 ---
 drivers/net/ethernet/freescale/fman/Kconfig   |  2 +-
 drivers/net/ethernet/freescale/fman/fman.c    | 16 ++++++++++++++++
 drivers/net/ethernet/freescale/fman/mac.c     |  1 +
 4 files changed, 18 insertions(+), 4 deletions(-)

-- 
2.1.0

^ permalink raw reply

* [PATCH net v3 1/4] fsl/fman: fix 1G support for QSGMII interfaces
From: Madalin Bucur @ 2016-12-19 16:13 UTC (permalink / raw)
  To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe
In-Reply-To: <1482164013-6111-1-git-send-email-madalin.bucur@nxp.com>

QSGMII ports were not advertising 1G speed.

Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
Reviewed-by: Camelia Groza <camelia.groza@nxp.com>
---
 drivers/net/ethernet/freescale/fman/mac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 69ca42c..0b31f85 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -594,6 +594,7 @@ static const u16 phy2speed[] = {
 	[PHY_INTERFACE_MODE_RGMII_RXID]	= SPEED_1000,
 	[PHY_INTERFACE_MODE_RGMII_TXID]	= SPEED_1000,
 	[PHY_INTERFACE_MODE_RTBI]		= SPEED_1000,
+	[PHY_INTERFACE_MODE_QSGMII]		= SPEED_1000,
 	[PHY_INTERFACE_MODE_XGMII]		= SPEED_10000
 };
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH net v3 2/4] powerpc: fsl/fman: remove fsl,fman from of_device_ids[]
From: Madalin Bucur @ 2016-12-19 16:13 UTC (permalink / raw)
  To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe
In-Reply-To: <1482164013-6111-1-git-send-email-madalin.bucur@nxp.com>

The fsl/fman drivers will use of_platform_populate() on all
supported platforms. Call of_platform_populate() to probe the
FMan sub-nodes.

Signed-off-by: Igal Liberman <igal.liberman@freescale.com>
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
 arch/powerpc/platforms/85xx/corenet_generic.c | 3 ---
 drivers/net/ethernet/freescale/fman/fman.c    | 8 ++++++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c
index 1179115..824b7f1 100644
--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c
@@ -117,9 +117,6 @@ static const struct of_device_id of_device_ids[] = {
 	{
 		.compatible	= "fsl,qe",
 	},
-	{
-		.compatible    = "fsl,fman",
-	},
 	/* The following two are for the Freescale hypervisor */
 	{
 		.name		= "hypervisor",
diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index dafd9e1..0b7f711 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2868,6 +2868,14 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 
 	fman->dev = &of_dev->dev;
 
+	/* call of_platform_populate in order to probe sub-nodes on arm64 */
+	err = of_platform_populate(fm_node, NULL, NULL, &of_dev->dev);
+	if (err) {
+		dev_err(&of_dev->dev, "%s: of_platform_populate() failed\n",
+			__func__);
+		goto fman_free;
+	}
+
 	return fman;
 
 fman_node_put:
-- 
2.1.0

^ permalink raw reply related

* [PATCH net v3 4/4] fsl/fman: enable compilation on ARM64
From: Madalin Bucur @ 2016-12-19 16:13 UTC (permalink / raw)
  To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe
In-Reply-To: <1482164013-6111-1-git-send-email-madalin.bucur@nxp.com>

Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
 drivers/net/ethernet/freescale/fman/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fman/Kconfig b/drivers/net/ethernet/freescale/fman/Kconfig
index 79b7c84..dc0850b 100644
--- a/drivers/net/ethernet/freescale/fman/Kconfig
+++ b/drivers/net/ethernet/freescale/fman/Kconfig
@@ -1,6 +1,6 @@
 config FSL_FMAN
 	tristate "FMan support"
-	depends on FSL_SOC || COMPILE_TEST
+	depends on FSL_SOC || ARCH_LAYERSCAPE || COMPILE_TEST
 	select GENERIC_ALLOCATOR
 	select PHYLIB
 	default n
-- 
2.1.0

^ permalink raw reply related

* [PATCH net v3 3/4] fsl/fman: A007273 only applies to PPC SoCs
From: Madalin Bucur @ 2016-12-19 16:13 UTC (permalink / raw)
  To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe
In-Reply-To: <1482164013-6111-1-git-send-email-madalin.bucur@nxp.com>

Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
Reviewed-by: Camelia Groza <camelia.groza@nxp.com>
---
 drivers/net/ethernet/freescale/fman/fman.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 0b7f711..003b86d 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -1890,6 +1890,7 @@ static int fman_reset(struct fman *fman)
 
 		goto _return;
 	} else {
+#ifdef CONFIG_PPC
 		struct device_node *guts_node;
 		struct ccsr_guts __iomem *guts_regs;
 		u32 devdisr2, reg;
@@ -1921,6 +1922,7 @@ static int fman_reset(struct fman *fman)
 
 		/* Enable all MACs */
 		iowrite32be(reg, &guts_regs->devdisr2);
+#endif
 
 		/* Perform FMan reset */
 		iowrite32be(FPM_RSTC_FM_RESET, &fman->fpm_regs->fm_rstc);
@@ -1932,25 +1934,31 @@ static int fman_reset(struct fman *fman)
 		} while (((ioread32be(&fman->fpm_regs->fm_rstc)) &
 			 FPM_RSTC_FM_RESET) && --count);
 		if (count == 0) {
+#ifdef CONFIG_PPC
 			iounmap(guts_regs);
 			of_node_put(guts_node);
+#endif
 			err = -EBUSY;
 			goto _return;
 		}
+#ifdef CONFIG_PPC
 
 		/* Restore devdisr2 value */
 		iowrite32be(devdisr2, &guts_regs->devdisr2);
 
 		iounmap(guts_regs);
 		of_node_put(guts_node);
+#endif
 
 		goto _return;
 
+#ifdef CONFIG_PPC
 guts_regs:
 		of_node_put(guts_node);
 guts_node:
 		dev_dbg(fman->dev, "%s: Didn't perform FManV3 reset due to Errata A007273!\n",
 			__func__);
+#endif
 	}
 _return:
 	return err;
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH RFC net-next 2/7] net: add dst_pending_confirm flag to skbuff
From: Eric Dumazet @ 2016-12-19 16:17 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: netdev, linux-sctp, YueHaibing
In-Reply-To: <1482094596-26046-3-git-send-email-ja@ssi.bg>

On Sun, 2016-12-18 at 22:56 +0200, Julian Anastasov wrote:

>  
> +static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour *n)
> +{
> +	if (unlikely(skb->dst_pending_confirm)) {
> +		struct sock *sk = skb->sk;
> +		unsigned long now = jiffies;
> +
> +		/* avoid dirtying neighbour */
> +		if (n->confirmed != now)
> +			n->confirmed = now;
> +		if (sk && sk->sk_dst_pending_confirm)
> +			sk->sk_dst_pending_confirm = 0;
> +	}
> +}
> +

I am still digesting this awesome patch series ;)

Not sure why you used an unlikely() here. TCP for example would hit this
path quite often.

So considering sk_dst_pending_confirm might be dirtied quite often,

I am not sure why you placed it in the cache line that contains
sk_rx_dst (in 1st patch)

Thanks.

^ permalink raw reply

* Re: [PATCH RFC net-next 2/7] net: add dst_pending_confirm flag to skbuff
From: Hannes Frederic Sowa @ 2016-12-19 16:36 UTC (permalink / raw)
  To: Eric Dumazet, Julian Anastasov; +Cc: netdev, linux-sctp, YueHaibing
In-Reply-To: <1482164252.1521.7.camel@edumazet-glaptop3.roam.corp.google.com>

On 19.12.2016 17:17, Eric Dumazet wrote:
> On Sun, 2016-12-18 at 22:56 +0200, Julian Anastasov wrote:
> 
>>  
>> +static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour *n)
>> +{
>> +	if (unlikely(skb->dst_pending_confirm)) {
>> +		struct sock *sk = skb->sk;
>> +		unsigned long now = jiffies;
>> +
>> +		/* avoid dirtying neighbour */
>> +		if (n->confirmed != now)
>> +			n->confirmed = now;
>> +		if (sk && sk->sk_dst_pending_confirm)
>> +			sk->sk_dst_pending_confirm = 0;
>> +	}
>> +}
>> +
> 
> I am still digesting this awesome patch series ;)
> 
> Not sure why you used an unlikely() here. TCP for example would hit this
> path quite often.
> 
> So considering sk_dst_pending_confirm might be dirtied quite often,
> 
> I am not sure why you placed it in the cache line that contains
> sk_rx_dst (in 1st patch)

Because they have to stay synchronized?

If we modify sk_rx_dst, we automatically also must clear
pending_confirm, otherwise we might end up confirming a wrong neighbor.

Bye,
Hannes

^ permalink raw reply

* Re: [PATCHv2 1/5] sh_eth: add generic wake-on-lan support via magic packet
From: Niklas Söderlund @ 2016-12-19 16:39 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Simon Horman, netdev, linux-renesas-soc, Geert Uytterhoeven
In-Reply-To: <87e3f9e6-0f5a-986c-772d-006cb25b9fd9@cogentembedded.com>

Hi Sergei,

Thanks for the spelling feedback, will include your suggestions in v3.
Which I hope to post once rc1 is released and netdev opens, as you 
suggested to me previously.

On 2016-12-18 23:26:11 +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/12/2016 07:09 PM, Niklas Söderlund wrote:
> 
> > Add generic functionality to support Wake-on-Lan using MagicPacket which
> > are supported by at least a few versions of sh_eth. Only add
> > functionality for WoL, no specific sh_eth version are marked to support
> 
>    Versions.
> 
> > WoL yet.
> > 
> > WoL is enabled in the suspend callback by setting MagicPacket detection
> > and disabling all interrupts expect MagicPacket. In the resume path the
> > driver needs to reset the hardware to rearm the WoL logic, this prevents
> > the driver from simply restoring the registers and to take advantage of
> > that sh_eth was not suspended to reduce resume time. To reset the
> > hardware the driver close and reopens the device just like it would do
> 
>    Closes.
> 
> > in a normal suspend/resume scenario without WoL enabled, but it both
> > close and open the device in the resume callback since the device needs
> 
>    Closes and opens.
> 
> > to be open for WoL to work.
> 
> > One quirk needed for WoL is that the module clock needs to be prevented
> > from being switched off by Runtime PM. To keep the clock alive the
> 
>    I tried to find the code in question and failed, getting muddled in the
> RPM maze. Could you point at this code for my education? :-)

In my investigation I observed this (simplified) call graph with regards
to clocks for suspend:

pm_suspend
  pm_clk_suspend
    clk_disable
      clk_core_disable
        cpg_mstp_clock_disable

The interesting function here are clk_core_disable(). In that function a
'enable_count' for each clock is decremented and the clock is only
turned of if the count reaches zero, hence cpg_mstp_clock_disable() are
only called if the counter reaches 0. At runtime the enable_count can be
displayed by examining /sys/kernel/debug/clk/clk_summary.

However the value for enable_count show at runtime are not the values
which are used when the pm_clk_suspend() are called. For a clock to not
be switched off when suspending the enable_count needs to incremented,
which a few drivers do if they can be used as a wake-up source.

> 
> > suspend callback need to call clk_enable() directly to increase the
> 
>    My main concern is why we need to manipulate the clock directly --
> can't you call RPM to achieve the same effect?

In my early attempts to keep the clock alive when suspending I used 
pm_clk_resume()/pm_clk_suspend() to increment/decrement the clock usage 
count. pm_clk_resume()/pm_clk_suspend() calls clk_enable()/clk_disable() 
for all clocks in the device's PM clock list, among other things.

Geert pointed out to me that this might have side effects and to manages 
its clock directly is preferred. Looking how these functions are used at 
other places I can only agree with Geert that this feels like the wrong 
solution and a layer violation.

> 
> > usage count of the clock. Then when Runtime PM decreases the clock usage
> > count it won't reach 0 and be switched off.
> 
>    You mean it does this even though we don't call pr_runtime_put_sync()
> as done in sh_eth_close()?

Yes.

I had a look at the pm_runtime_* functions in include/linux/pm_runtime.h 
and drivers/base/power/runtime.c and could not find any clock handling.  
Maybe they only deal with power domains?

I might have missed something when looking in to this. But if I do not 
call clk_enable()/clk_disable() (or something equivalent) in the sh_eth 
suspend/resume callbacks WoL do not work. That is it suspends fine but 
sending a MagicPacket do not wake the system :-)

> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> [...]
> 
> MBR, Sergei
> 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply

* Re: [PATCH RFC net-next 2/7] net: add dst_pending_confirm flag to skbuff
From: Eric Dumazet @ 2016-12-19 16:40 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: Julian Anastasov, netdev, linux-sctp, YueHaibing
In-Reply-To: <f9dadb5e-172a-4c39-c564-38be672ef0eb@stressinduktion.org>

On Mon, 2016-12-19 at 17:36 +0100, Hannes Frederic Sowa wrote:
> On 19.12.2016 17:17, Eric Dumazet wrote:
> > On Sun, 2016-12-18 at 22:56 +0200, Julian Anastasov wrote:
> > 
> >>  
> >> +static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour *n)
> >> +{
> >> +	if (unlikely(skb->dst_pending_confirm)) {
> >> +		struct sock *sk = skb->sk;
> >> +		unsigned long now = jiffies;
> >> +
> >> +		/* avoid dirtying neighbour */
> >> +		if (n->confirmed != now)
> >> +			n->confirmed = now;
> >> +		if (sk && sk->sk_dst_pending_confirm)
> >> +			sk->sk_dst_pending_confirm = 0;
> >> +	}
> >> +}
> >> +
> > 
> > I am still digesting this awesome patch series ;)
> > 
> > Not sure why you used an unlikely() here. TCP for example would hit this
> > path quite often.
> > 
> > So considering sk_dst_pending_confirm might be dirtied quite often,
> > 
> > I am not sure why you placed it in the cache line that contains
> > sk_rx_dst (in 1st patch)
> 
> Because they have to stay synchronized?
> 
> If we modify sk_rx_dst, we automatically also must clear
> pending_confirm, otherwise we might end up confirming a wrong neighbor.

Your answer makes little sense really...

For most TCP flows, we set sk_rx_dst exactly once.

Hardly a good reason to have these in the same cache line.

^ permalink raw reply

* Re: [PATCHv2 1/5] sh_eth: add generic wake-on-lan support via magic packet
From: Niklas Söderlund @ 2016-12-19 16:41 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Simon Horman, netdev, linux-renesas-soc, Geert Uytterhoeven
In-Reply-To: <079800f0-2e80-7a0b-01e2-51e09f05345c@cogentembedded.com>

Hi Sergei,

On 2016-12-18 00:50:59 +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 12/12/2016 07:09 PM, Niklas Söderlund wrote:
> 
>    Not the complete review yet, just some superficial comments.
> 
> > Add generic functionality to support Wake-on-Lan using MagicPacket which
> 
>    LAN.
> 
> > are supported by at least a few versions of sh_eth. Only add
> > functionality for WoL, no specific sh_eth version are marked to support
> > WoL yet.
> 
> [...]
> 
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> > index 05b0dc5..87640b9 100644
> > --- a/drivers/net/ethernet/renesas/sh_eth.c
> > +++ b/drivers/net/ethernet/renesas/sh_eth.c
> [...]
> > @@ -3150,15 +3187,67 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
> [...]
> > +static int sh_eth_wol_restore(struct net_device *ndev)
> > +{
> > +	struct sh_eth_private *mdp = netdev_priv(ndev);
> > +	int ret;
> > +
> > +	napi_enable(&mdp->napi);
> > +
> > +	/* Disable MagicPacket */
> > +	sh_eth_modify(ndev, ECMR, ECMR_PMDE, 0);
> > +
> > +	/* The device need to be reset to restore MagicPacket logic
> 
>    Needs.
> 
> [...]
> > index d050f37..4ceed00 100644
> > --- a/drivers/net/ethernet/renesas/sh_eth.h
> > +++ b/drivers/net/ethernet/renesas/sh_eth.h
> > @@ -493,6 +493,7 @@ struct sh_eth_cpu_data {
> >  	unsigned shift_rd0:1;	/* shift Rx descriptor word 0 right by 16 */
> >  	unsigned rmiimode:1;	/* EtherC has RMIIMODE register */
> >  	unsigned rtrate:1;	/* EtherC has RTRATE register */
> > +	unsigned magic:1;	/* EtherC has ECMR.PMDE and ECSR.MPD */
> 
>    After looking at the SH7734/63 manuals it became obvious that PMDE was a
> result of typo, the bit is called MPDE actually, the current name doesn't
> make sense anyway. Care to fix?

Sure, I add fixup patch to the front of this series which cleans this up 
before adding WoL support. Or would you prefer I sent a separate patch 
and have the WoL series depend on it?

> 
> MBR, Sergei
> 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply

* Re: [PATCH RFC net-next 2/7] net: add dst_pending_confirm flag to skbuff
From: Hannes Frederic Sowa @ 2016-12-19 16:53 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Julian Anastasov, netdev, linux-sctp, YueHaibing
In-Reply-To: <1482165628.1521.9.camel@edumazet-glaptop3.roam.corp.google.com>

On 19.12.2016 17:40, Eric Dumazet wrote:
> On Mon, 2016-12-19 at 17:36 +0100, Hannes Frederic Sowa wrote:
>> On 19.12.2016 17:17, Eric Dumazet wrote:
>>> On Sun, 2016-12-18 at 22:56 +0200, Julian Anastasov wrote:
>>>
>>>>  
>>>> +static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour *n)
>>>> +{
>>>> +	if (unlikely(skb->dst_pending_confirm)) {
>>>> +		struct sock *sk = skb->sk;
>>>> +		unsigned long now = jiffies;
>>>> +
>>>> +		/* avoid dirtying neighbour */
>>>> +		if (n->confirmed != now)
>>>> +			n->confirmed = now;
>>>> +		if (sk && sk->sk_dst_pending_confirm)
>>>> +			sk->sk_dst_pending_confirm = 0;
>>>> +	}
>>>> +}
>>>> +
>>>
>>> I am still digesting this awesome patch series ;)
>>>
>>> Not sure why you used an unlikely() here. TCP for example would hit this
>>> path quite often.
>>>
>>> So considering sk_dst_pending_confirm might be dirtied quite often,
>>>
>>> I am not sure why you placed it in the cache line that contains
>>> sk_rx_dst (in 1st patch)
>>
>> Because they have to stay synchronized?
>>
>> If we modify sk_rx_dst, we automatically also must clear
>> pending_confirm, otherwise we might end up confirming a wrong neighbor.
> 
> Your answer makes little sense really...
> 
> For most TCP flows, we set sk_rx_dst exactly once.
> 
> Hardly a good reason to have these in the same cache line.

Right :) , and I didn't actually wanted to make an argument in favor of
that. Just noted they are probably semantically grouped together as an
explanation.

Sorry,
Hannes

^ permalink raw reply

* Re: [PATCHv2 1/5] sh_eth: add generic wake-on-lan support via magic packet
From: Sergei Shtylyov @ 2016-12-19 16:56 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Simon Horman, netdev, linux-renesas-soc, Geert Uytterhoeven
In-Reply-To: <20161219164159.GF21006@bigcity.dyn.berto.se>

Hello!

On 12/19/2016 07:41 PM, Niklas Söderlund wrote:

[...]

>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
>>> index 05b0dc5..87640b9 100644
>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
[...]
>>> index d050f37..4ceed00 100644
>>> --- a/drivers/net/ethernet/renesas/sh_eth.h
>>> +++ b/drivers/net/ethernet/renesas/sh_eth.h
>>> @@ -493,6 +493,7 @@ struct sh_eth_cpu_data {
>>>  	unsigned shift_rd0:1;	/* shift Rx descriptor word 0 right by 16 */
>>>  	unsigned rmiimode:1;	/* EtherC has RMIIMODE register */
>>>  	unsigned rtrate:1;	/* EtherC has RTRATE register */
>>> +	unsigned magic:1;	/* EtherC has ECMR.PMDE and ECSR.MPD */
>>
>>    After looking at the SH7734/63 manuals it became obvious that PMDE was a
>> result of typo, the bit is called MPDE actually, the current name doesn't
>> make sense anyway. Care to fix?
>
> Sure, I add fixup patch to the front of this series which cleans this up
> before adding WoL support.

    That would be fine, TIA!

[...]

MBR, Sergei

^ permalink raw reply

* Re: ipv6: handle -EFAULT from skb_copy_bits
From: Dave Jones @ 2016-12-19 17:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20161217.104120.945262627572958024.davem@davemloft.net>

On Sat, Dec 17, 2016 at 10:41:20AM -0500, David Miller wrote:

 > > It seems to be possible to craft a packet for sendmsg that triggers
 > > the -EFAULT path in skb_copy_bits resulting in a BUG_ON that looks like:
 > > 
 > > RIP: 0010:[<ffffffff817c6390>] [<ffffffff817c6390>] rawv6_sendmsg+0xc30/0xc40
 > > RSP: 0018:ffff881f6c4a7c18  EFLAGS: 00010282
 > > RAX: 00000000fffffff2 RBX: ffff881f6c681680 RCX: 0000000000000002
 > > RDX: ffff881f6c4a7cf8 RSI: 0000000000000030 RDI: ffff881fed0f6a00
 > > RBP: ffff881f6c4a7da8 R08: 0000000000000000 R09: 0000000000000009
 > > R10: ffff881fed0f6a00 R11: 0000000000000009 R12: 0000000000000030
 > > R13: ffff881fed0f6a00 R14: ffff881fee39ba00 R15: ffff881fefa93a80
 > > 
 > > Call Trace:
 > >  [<ffffffff8118ba23>] ? unmap_page_range+0x693/0x830
 > >  [<ffffffff81772697>] inet_sendmsg+0x67/0xa0
 > >  [<ffffffff816d93f8>] sock_sendmsg+0x38/0x50
 > >  [<ffffffff816d982f>] SYSC_sendto+0xef/0x170
 > >  [<ffffffff816da27e>] SyS_sendto+0xe/0x10
 > >  [<ffffffff81002910>] do_syscall_64+0x50/0xa0
 > >  [<ffffffff817f7cbc>] entry_SYSCALL64_slow_path+0x25/0x25
 > > 
 > > Handle this in rawv6_push_pending_frames and jump to the failure path.
 > > 
 > > Signed-off-by: Dave Jones <davej@codemonkey.org.uk>
 > 
 > Hmmm, that's interesting.  Becaue the code in __ip6_append_data(), which
 > sets up the ->cork.base.length value, seems to be defensively trying to
 > avoid this possibility.
 > 
 > For example, it checks things like:
 > 
 > 	if (cork->length + length > mtu - headersize && ipc6->dontfrag &&
 > 	    (sk->sk_protocol == IPPROTO_UDP ||
 > 	     sk->sk_protocol == IPPROTO_RAW)) {
 > 
 > This is why the transport offset plus the length should never exceed
 > the total length for that skb_copy_bits() call.
 > 
 > Perhaps this protocol check in the code above is incomplete?  Do you
 > know what the sk->sk_protocol value was when that BUG triggered?  That
 > might shine some light on what is really happening here.

Hm.
  sk_protocol = 7, 






struct sock {
  __sk_common = {
    {
      skc_addrpair = 0, 
      {
        skc_daddr = 0, 
        skc_rcv_saddr = 0
      }
    }, 
    {
      skc_hash = 0, 
      skc_u16hashes = {0, 0}
    }, 
    {
      skc_portpair = 458752, 
      {
        skc_dport = 0, 
        skc_num = 7
      }
    }, 
    skc_family = 10, 
    skc_state = 7 '\a', 
    skc_reuse = 1 '\001', 
    skc_reuseport = 0 '\000', 
    skc_ipv6only = 0 '\000', 
    skc_net_refcnt = 1 '\001', 
    skc_bound_dev_if = 0, 
    {
      skc_bind_node = {
        next = 0x0, 
        pprev = 0x0
      }, 
      skc_portaddr_node = {
        next = 0x0, 
        pprev = 0x0
      }
    }, 
    skc_prot = 0xffffffff81cf3bc0 <rawv6_prot>, 
    skc_net = {
      net = 0xffffffff81ce78c0 <init_net>
    }, 
    skc_v6_daddr = {
      in6_u = {
        u6_addr8 = "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000", 
        u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, 
        u6_addr32 = {0, 0, 0, 0}
      }
    }, 
    }, 
    skc_v6_rcv_saddr = {
      in6_u = {
        u6_addr8 = "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000", 
        u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, 
        u6_addr32 = {0, 0, 0, 0}
      }
    }, 
    skc_cookie = {
      counter = 0
    }, 
    {
      skc_flags = 256, 
      skc_listener = 0x100, 
      skc_tw_dr = 0x100
    }, 
    skc_dontcopy_begin = 0xffff881fd1ce9b68, 
    {
      skc_node = {
        next = 0x0, 
        pprev = 0x0
      }, 
      skc_nulls_node = {
        next = 0x0, 
        pprev = 0x0
      }
    }, 
    skc_tx_queue_mapping = -1, 
    {
      skc_incoming_cpu = -1, 
      skc_rcv_wnd = 4294967295, 
      skc_tw_rcv_nxt = 4294967295
    }, 
    skc_refcnt = {
      counter = 1
    }, 
    skc_dontcopy_end = 0xffff881fd1ce9b84, 
    {
      skc_rxhash = 0, 
      skc_window_clamp = 0, 
      skc_tw_snd_nxt = 0
    }
  }, 
  sk_lock = {
    slock = {
      {
        rlock = {
          raw_lock = {
            val = {
              counter = 0
            }
          }
        }
      }
    }, 
    owned = 1, 
    wq = {
      lock = {
        {
          rlock = {
            raw_lock = {
              val = {
                counter = 0
              }
            }
          }
        }
      }, 
      task_list = {
        next = 0xffff881fd1ce9b98, 
        prev = 0xffff881fd1ce9b98
      }
    }
  }, 
  sk_receive_queue = {
    next = 0xffff881fd1ce9ba8, 
    prev = 0xffff881fd1ce9ba8, 
    qlen = 0, 
    lock = {
      {
        rlock = {
          raw_lock = {
            val = {
              counter = 0
            }
          }
        }
      }
    }
  }, 
  sk_backlog = {
    rmem_alloc = {
      counter = 0
    }, 
    len = 0, 
    head = 0x0, 
    tail = 0x0
  }, 
  sk_forward_alloc = 0, 
  sk_txhash = 0, 
  sk_napi_id = 0, 
  sk_ll_usec = 0, 
  sk_drops = {
    counter = 0
  }, 
  sk_rcvbuf = 1024000, 
  sk_filter = 0x0, 
  {
    sk_wq = 0xffff881fc4b3ab00, 
    sk_wq_raw = 0xffff881fc4b3ab00
  }, 
  sk_policy = {0x0, 0x0}, 
  sk_rx_dst = 0x0, 
  sk_dst_cache = 0x0, 
  sk_wmem_alloc = {
    counter = 769
  }, 
  sk_omem_alloc = {
    counter = 640
  }, 
  sk_sndbuf = 212992, 
  sk_write_queue = {
    next = 0xffff881f6fc60b00, 
    prev = 0xffff881f6fc60b00, 
    qlen = 1, 
    lock = {
      {
        rlock = {
          raw_lock = {
            val = {
              counter = 0
            }
          }
        }
      }
    }
  }, 
  sk_shutdown = 0, 
  sk_no_check_tx = 0, 
  sk_no_check_rx = 0, 
  sk_userlocks = 0, 
  sk_protocol = 7, 
  sk_type = 3, 
  sk_wmem_queued = 0, 
  sk_allocation = 37748928, 
  sk_pacing_rate = 4294967295, 
  sk_max_pacing_rate = 4294967295, 
  sk_route_caps = 0, 
  sk_route_nocaps = 0, 
  sk_gso_type = 0, 
  sk_gso_max_size = 0, 
  sk_gso_max_segs = 0, 
  sk_rcvlowat = 1, 
  sk_lingertime = 0, 
  sk_error_queue = {
    next = 0xffff881fd1ce9c88, 
    prev = 0xffff881fd1ce9c88, 
    qlen = 0, 
    lock = {
      {
        rlock = {
          raw_lock = {
            val = {
              counter = 0
            }
          }
        }
      }
    }
  }, 
  sk_prot_creator = 0xffffffff81cf3bc0 <rawv6_prot>, 
  sk_callback_lock = {
    raw_lock = {
      cnts = {
        counter = 0
      }, 
      wait_lock = {
        val = {
          counter = 0
        }
      }
    }
  }, 
  sk_err = 0, 
  sk_err_soft = 0, 
  sk_ack_backlog = 0, 
  sk_max_ack_backlog = 0, 
  sk_priority = 0, 
  sk_mark = 0, 
  sk_peer_pid = 0x0, 
  sk_peer_cred = 0x0, 
  sk_rcvtimeo = 9223372036854775807, 
  sk_sndtimeo = 9223372036854775807, 
  sk_timer = {
    entry = {
      next = 0x0, 
      pprev = 0x0
    }, 
    expires = 0, 
    function = 0x0, 
    data = 0, 
    flags = 4, 
    slack = -1, 
    start_pid = -1, 
    start_site = 0x0, 
    start_comm = "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"
  }, 
  sk_stamp = {
    tv64 = 1481510503238183349
  }, 
  sk_tsflags = 0, 
  sk_tskey = 0, 
  sk_socket = 0xffff881fd0d24b00, 
  sk_user_data = 0x0, 
  sk_frag = {
    page = 0x0, 
    offset = 0, 
    size = 0
  }, 
  sk_send_head = 0x0, 
  sk_peek_off = -1, 
  sk_write_pending = 0, 
  sk_security = 0x0, 
  sk_cgrp_data = {
    {
      {
        is_data = 48 '0', 
        padding = 109 'm', 
        prioidx = 33292, 
        classid = 4294967295
      }, 
      val = 18446744071596436784
    }
  }, 
  sk_memcg = 0x0, 
  sk_state_change = 0xffffffff816dbed0 <sock_def_wakeup>, 
  sk_data_ready = 0xffffffff816dcd20 <sock_def_readable>, 
  sk_write_space = 0xffffffff816dcc90 <sock_def_write_space>, 
  sk_error_report = 0xffffffff816dcc30 <sock_def_error_report>, 
  sk_backlog_rcv = 0xffffffff817c5690 <rawv6_rcv_skb>, 
  sk_destruct = 0xffffffff81772460 <inet_sock_destruct>, 
  sk_reuseport_cb = 0x0
}

^ permalink raw reply

* Re: [PATCH v5 1/4] siphash: add cryptographically secure PRF
From: Jason A. Donenfeld @ 2016-12-19 17:08 UTC (permalink / raw)
  To: noloader
  Cc: Netdev, kernel-hardening, LKML, Linux Crypto Mailing List,
	David Laight, Ted Tso, Hannes Frederic Sowa, Linus Torvalds,
	Eric Biggers, Tom Herbert, George Spelvin, Vegard Nossum,
	Andi Kleen, David Miller, Andy Lutomirski, Jean-Philippe Aumasson,
	Daniel J . Bernstein
In-Reply-To: <CAH8yC8nE5CLfbv8gCRCeiNW3JLCAB3062-6pNO8mEXJYcsFUsw@mail.gmail.com>

On Sat, Dec 17, 2016 at 3:55 PM, Jeffrey Walton <noloader@gmail.com> wrote:
> It may be prudent to include the endian reversal in the test to ensure
> big endian machines produce expected results. Some closely related
> testing on an old Apple PowerMac G5 revealed that result needed to be
> reversed before returning it to a caller.

The function [1] returns a u64. Originally I had it returning a
__le64, but that was considered unnecessary by many prior reviewers on
the list. It returns an integer. If you want uniform bytes out of it,
then use the endian conversion function, the same as you would do with
any other type of integer.

Additionally, this function is *not* meant for af_alg or any of the
crypto/* code. It's very unlikely to find a use there.


> Forgive my ignorance... I did not find reading on using the primitive
> in a PRNG. Does anyone know what Aumasson or Bernstein have to say?
> Aumasson's site does not seem to discuss the use case:

He's on this thread so I suppose he can speak up for himself. But in
my conversations with him, the primary take-away was, "seems okay to
me!". But please -- JP - correct me if I've misinterpreted.

^ permalink raw reply

* Re: [PATCH] stmmac: fix memory barriers
From: Joao Pinto @ 2016-12-19 17:10 UTC (permalink / raw)
  To: David Miller, pavel
  Cc: LinoSanfilippo, peppe.cavallaro, alexandre.torgue, linux-kernel,
	netdev, niklas.cassel, Joao.Pinto
In-Reply-To: <20161219.110655.448567935176535028.davem@davemloft.net>

Hi,

I am trying to built net-next git tree and it is failing:

  CC      drivers/pnp/card.o
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function
‘stmmac_hw_fix_mac_speed’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:224:34: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
  struct phy_device *phydev = priv->phydev;
                                  ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_eee_init’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:298:24: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   if (phy_init_eee(priv->phydev, 1)) {
                        ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:330:44: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   priv->hw->mac->set_eee_pls(priv->hw, priv->phydev->link);
                                            ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_ptp’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:672:2: error: void value not
ignored as it ought to be
  return stmmac_ptp_register(priv);
  ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_adjust_link’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:694:34: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
  struct phy_device *phydev = priv->phydev;
                                  ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_phy’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:884:6: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
  priv->phydev = phydev;
      ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_open’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1810:10: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
  if (priv->phydev)
          ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1811:17: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   phy_start(priv->phydev);
                 ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1858:10: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
  if (priv->phydev)
          ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1859:22: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   phy_disconnect(priv->phydev);
                      ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_release’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1878:10: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
  if (priv->phydev) {
          ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1879:16: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   phy_stop(priv->phydev);
                ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1880:22: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   phy_disconnect(priv->phydev);
                      ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1881:7: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   priv->phydev = NULL;
       ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_ioctl’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2883:12: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   if (!priv->phydev)
            ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2885:27: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   ret = phy_mii_ioctl(priv->phydev, rq, cmd);
                           ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_suspend’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3438:10: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
  if (priv->phydev)
          ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3439:16: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   phy_stop(priv->phydev);
                ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_resume’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3533:10: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
  if (priv->phydev)
          ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3534:17: error: ‘struct
stmmac_priv’ has no member named ‘phydev’
   phy_start(priv->phydev);
                 ^
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_ptp’:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:673:1: warning: control
reaches end of non-void function [-Wreturn-type]
 }
 ^
  CC      drivers/pnp/driver.o
make[5]: *** [drivers/net/ethernet/stmicro/stmmac/stmmac_main.o] Error 1
make[4]: *** [drivers/net/ethernet/stmicro/stmmac] Error 2
make[3]: *** [drivers/net/ethernet/stmicro] Error 2
make[3]: *** Waiting for unfinished jobs....
  CC      drivers/pnp/resource.o

Thanks,
Joao

Às 4:06 PM de 12/19/2016, David Miller escreveu:
> From: Pavel Machek <pavel@ucw.cz>
> Date: Sun, 18 Dec 2016 21:38:12 +0100
> 
>> Fix up memory barriers in stmmac driver. They are meant to protect
>> against DMA engine, so smp_ variants are certainly wrong, and dma_
>> variants are preferable.
>>     
>> Signed-off-by: Pavel Machek <pavel@denx.de>
> 
> Applied.
> 

^ permalink raw reply

* Re: [PATCHv2 1/5] sh_eth: add generic wake-on-lan support via magic packet
From: Geert Uytterhoeven @ 2016-12-19 17:11 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Sergei Shtylyov, Simon Horman, netdev@vger.kernel.org,
	Linux-Renesas
In-Reply-To: <20161219163904.GE21006@bigcity.dyn.berto.se>

Hi Niklas,

On Mon, Dec 19, 2016 at 5:39 PM, Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
> On 2016-12-18 23:26:11 +0300, Sergei Shtylyov wrote:
>> On 12/12/2016 07:09 PM, Niklas Söderlund wrote:
>> > One quirk needed for WoL is that the module clock needs to be prevented
>> > from being switched off by Runtime PM. To keep the clock alive the
>>
>>    I tried to find the code in question and failed, getting muddled in the
>> RPM maze. Could you point at this code for my education? :-)
>
> In my investigation I observed this (simplified) call graph with regards
> to clocks for suspend:
>
> pm_suspend
>   pm_clk_suspend
>     clk_disable
>       clk_core_disable
>         cpg_mstp_clock_disable
>
> The interesting function here are clk_core_disable(). In that function a
> 'enable_count' for each clock is decremented and the clock is only
> turned of if the count reaches zero, hence cpg_mstp_clock_disable() are
> only called if the counter reaches 0. At runtime the enable_count can be
> displayed by examining /sys/kernel/debug/clk/clk_summary.
>
> However the value for enable_count show at runtime are not the values
> which are used when the pm_clk_suspend() are called. For a clock to not
> be switched off when suspending the enable_count needs to incremented,
> which a few drivers do if they can be used as a wake-up source.
>
>>
>> > suspend callback need to call clk_enable() directly to increase the
>>
>>    My main concern is why we need to manipulate the clock directly --
>> can't you call RPM to achieve the same effect?
>
> In my early attempts to keep the clock alive when suspending I used
> pm_clk_resume()/pm_clk_suspend() to increment/decrement the clock usage
> count. pm_clk_resume()/pm_clk_suspend() calls clk_enable()/clk_disable()
> for all clocks in the device's PM clock list, among other things.
>
> Geert pointed out to me that this might have side effects and to manages
> its clock directly is preferred. Looking how these functions are used at
> other places I can only agree with Geert that this feels like the wrong
> solution and a layer violation.

>> > usage count of the clock. Then when Runtime PM decreases the clock usage
>> > count it won't reach 0 and be switched off.
>>
>>    You mean it does this even though we don't call pr_runtime_put_sync()
>> as done in sh_eth_close()?
>
> Yes.
>
> I had a look at the pm_runtime_* functions in include/linux/pm_runtime.h
> and drivers/base/power/runtime.c and could not find any clock handling.
> Maybe they only deal with power domains?

There should be a generic way to prevent a device from being suspended.
This will make sure the module clock is not disabled, and the power domain
(if applicable) is not powered down.

Note that the clock handling is a special form of power domain handling,
as it uses a clock domain.

Gr{oetje,eeting}s,

                        Geert

^ permalink raw reply

* Re: [PATCH] stmmac: fix memory barriers
From: Niklas Cassel @ 2016-12-19 17:19 UTC (permalink / raw)
  To: Joao Pinto, David Miller, pavel
  Cc: LinoSanfilippo, peppe.cavallaro, alexandre.torgue, linux-kernel,
	netdev
In-Reply-To: <79efbe1a-bdc3-2aca-88a2-42a1b29f292d@synopsys.com>

On 12/19/2016 06:10 PM, Joao Pinto wrote:
> Hi,
>
> I am trying to built net-next git tree and it is failing:
>
>   CC      drivers/pnp/card.o
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function
> ‘stmmac_hw_fix_mac_speed’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:224:34: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>   struct phy_device *phydev = priv->phydev;

Are you really building net-next?
http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#n224

The phy_device initializer does not appear to match your output.

>                                   ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_eee_init’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:298:24: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    if (phy_init_eee(priv->phydev, 1)) {
>                         ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:330:44: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    priv->hw->mac->set_eee_pls(priv->hw, priv->phydev->link);
>                                             ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_ptp’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:672:2: error: void value not
> ignored as it ought to be
>   return stmmac_ptp_register(priv);
>   ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_adjust_link’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:694:34: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>   struct phy_device *phydev = priv->phydev;
>                                   ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_phy’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:884:6: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>   priv->phydev = phydev;
>       ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_open’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1810:10: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>   if (priv->phydev)
>           ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1811:17: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    phy_start(priv->phydev);
>                  ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1858:10: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>   if (priv->phydev)
>           ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1859:22: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    phy_disconnect(priv->phydev);
>                       ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_release’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1878:10: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>   if (priv->phydev) {
>           ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1879:16: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    phy_stop(priv->phydev);
>                 ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1880:22: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    phy_disconnect(priv->phydev);
>                       ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1881:7: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    priv->phydev = NULL;
>        ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_ioctl’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2883:12: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    if (!priv->phydev)
>             ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2885:27: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    ret = phy_mii_ioctl(priv->phydev, rq, cmd);
>                            ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_suspend’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3438:10: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>   if (priv->phydev)
>           ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3439:16: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    phy_stop(priv->phydev);
>                 ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_resume’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3533:10: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>   if (priv->phydev)
>           ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3534:17: error: ‘struct
> stmmac_priv’ has no member named ‘phydev’
>    phy_start(priv->phydev);
>                  ^
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_ptp’:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:673:1: warning: control
> reaches end of non-void function [-Wreturn-type]
>  }
>  ^
>   CC      drivers/pnp/driver.o
> make[5]: *** [drivers/net/ethernet/stmicro/stmmac/stmmac_main.o] Error 1
> make[4]: *** [drivers/net/ethernet/stmicro/stmmac] Error 2
> make[3]: *** [drivers/net/ethernet/stmicro] Error 2
> make[3]: *** Waiting for unfinished jobs....
>   CC      drivers/pnp/resource.o
>
> Thanks,
> Joao
>
> Às 4:06 PM de 12/19/2016, David Miller escreveu:
>> From: Pavel Machek <pavel@ucw.cz>
>> Date: Sun, 18 Dec 2016 21:38:12 +0100
>>
>>> Fix up memory barriers in stmmac driver. They are meant to protect
>>> against DMA engine, so smp_ variants are certainly wrong, and dma_
>>> variants are preferable.
>>>     
>>> Signed-off-by: Pavel Machek <pavel@denx.de>
>> Applied.
>>

^ permalink raw reply

* Re: [PATCH v5 1/4] siphash: add cryptographically secure PRF
From: Jean-Philippe Aumasson @ 2016-12-19 17:19 UTC (permalink / raw)
  To: Jason A. Donenfeld, noloader
  Cc: Netdev, kernel-hardening, LKML, Linux Crypto Mailing List,
	David Laight, Ted Tso, Hannes Frederic Sowa, Linus Torvalds,
	Eric Biggers, Tom Herbert, George Spelvin, Vegard Nossum,
	Andi Kleen, David Miller, Andy Lutomirski, Daniel J . Bernstein
In-Reply-To: <CAHmME9o6CFuQJEp3QJgkh78r5Z2F8NJnpTCCFUiSnAiJxPGCJQ@mail.gmail.com>

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

Yeah you can use the PRF properties to build a DRBG, but that may not be
optimal in terms of performance.
On Mon, 19 Dec 2016 at 18:08, Jason A. Donenfeld <Jason@zx2c4.com> wrote:

> On Sat, Dec 17, 2016 at 3:55 PM, Jeffrey Walton <noloader@gmail.com>
> wrote:
> > It may be prudent to include the endian reversal in the test to ensure
> > big endian machines produce expected results. Some closely related
> > testing on an old Apple PowerMac G5 revealed that result needed to be
> > reversed before returning it to a caller.
>
> The function [1] returns a u64. Originally I had it returning a
> __le64, but that was considered unnecessary by many prior reviewers on
> the list. It returns an integer. If you want uniform bytes out of it,
> then use the endian conversion function, the same as you would do with
> any other type of integer.
>
> Additionally, this function is *not* meant for af_alg or any of the
> crypto/* code. It's very unlikely to find a use there.
>
>
> > Forgive my ignorance... I did not find reading on using the primitive
> > in a PRNG. Does anyone know what Aumasson or Bernstein have to say?
> > Aumasson's site does not seem to discuss the use case:
>
> He's on this thread so I suppose he can speak up for himself. But in
> my conversations with him, the primary take-away was, "seems okay to
> me!". But please -- JP - correct me if I've misinterpreted.
>

[-- Attachment #2: Type: text/html, Size: 2216 bytes --]

^ permalink raw reply

* RE: [PATCH v8 3/8] thunderbolt: Communication with the ICM (firmware)
From: Mario.Limonciello @ 2016-12-19 17:21 UTC (permalink / raw)
  To: mika.westerberg, luto
  Cc: amir.jer.levy, gregkh, andreas.noever, bhelgaas, corbet,
	linux-kernel, linux-pci, netdev, linux-doc, thunderbolt-linux,
	tomas.winkler, xiong.y.zhang
In-Reply-To: <20161219122407.GQ1460@lahna.fi.intel.com>

Dell - Internal Use - Confidential  

> 
> There is small problem, though. On non-Apple systems the host controller only
> appears when something is connected to thunderbolt ports. So the char device
> would not be there all the time. However, I think we can still notify the
> userspace by sending an extra uevent when we detect there is a PCIe device or
> inter-domain connection plugged in.
> 

Why couldn't you just create it the first time a device is plugged into a Thunderbolt
port and leave it until the module is cleaned up?  If the host controller goes to sleep
an event could be sent to the daemon to let it know it disappeared and not to expect
data on the char device for now, but leave the node around. 

^ permalink raw reply

* Re: [kernel-hardening] Re: [PATCH v5 1/4] siphash: add cryptographically secure PRF
From: Jason A. Donenfeld @ 2016-12-19 17:21 UTC (permalink / raw)
  To: Theodore Ts'o, kernel-hardening, Jason, George Spelvin,
	Andi Kleen, David Miller, David Laight, Daniel J . Bernstein,
	Eric Biggers, Hannes Frederic Sowa, Jean-Philippe Aumasson,
	Linux Crypto Mailing List, LKML, Andy Lutomirski, Netdev,
	Tom Herbert, Linus Torvalds, Vegard Nossum
In-Reply-To: <20161217154152.5oug7mzb4tmfknwv@thunk.org>

Hi Ted,

On Sat, Dec 17, 2016 at 4:41 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Fri, Dec 16, 2016 at 09:15:03PM -0500, George Spelvin wrote:
>> >> - Ted, Andy Lutorminski and I will try to figure out a construction of
>> >>   get_random_long() that we all like.
>
> We don't have to find the most optimal solution right away; we can
> approach this incrementally, after all.

Thanks to your call for moderation. This is the impression I have too.
And with all the back and forth of these threads, I fear nothing will
get done. I'm going to collect the best ideas amongst all of these,
and try to get it merged. Then after that we can incrementally improve
on it.

David Miller -- would you merge something into your 4.11 tree? Seems
like you might be the guy for this, since the changes primarily affect
net/*.

Latest patches are here:
https://git.zx2c4.com/linux-dev/log/?h=siphash


> So long as we replace get_random_{long,int}() with something which is
> (a) strictly better in terms of security given today's use of MD5, and
> (b) which is strictly *faster* than the current construction on 32-bit
> and 64-bit systems, we can do that, and can try to make it be faster
> while maintaining some minimum level of security which is sufficient
> for all current users of get_random_{long,int}() and which can be
> clearly artificulated for future users of get_random_{long,int}().
>
> The main worry at this point I have is benchmarking siphash on a
> 32-bit system.  It may be that simply batching the chacha20 output so
> that we're using the urandom construction more efficiently is the
> better way to go, since that *does* meet the criteron of strictly more
> secure and strictly faster than the current MD5 solution.  I'm open to
> using siphash, but I want to see the the 32-bit numbers first.

Sure, I'll run some benchmarks and report back.

> As far as half-siphash is concerned, it occurs to me that the main
> problem will be those users who need to guarantee that output can't be
> guessed over a long period of time.  For example, if you have a
> long-running process, then the output needs to remain unguessable over
> potentially months or years, or else you might be weakening the ASLR
> protections.  If on the other hand, the hash table or the process will
> be going away in a matter of seconds or minutes, the requirements with
> respect to cryptographic strength go down significantly.
>
> Now, maybe this doesn't matter that much if we can guarantee (or make
> assumptions) that the attacker doesn't have unlimited access the
> output stream of get_random_{long,int}(), or if it's being used in an
> anti-DOS use case where it ultimately only needs to be harder than
> alternate ways of attacking the system.

The only acceptable usage of HalfSipHash is for hash table lookups
where top security isn't actually a goal. I'm still a bit queasy about
going with it, but George S has very aggressively been pursuing a
"save every last cycle" agenda, which makes sense given how
performance sensitive certain hash tables are, and so I suspect this
could be an okay compromise between performance and security. But,
only for hash tables. Certainly not for the RNG.

>
> Rekeying every five minutes doesn't necessarily help the with respect
> to ASLR, but it might reduce the amount of the output stream that
> would be available to the attacker in order to be able to attack the
> get_random_{long,int}() generator, and it also reduces the value of
> doing that attack to only compromising the ASLR for those processes
> started within that five minute window.

The current implemention of get_random_int/long in my branch uses
128-bit key siphash, so the chances of compromising the key are pretty
much nil. The periodic rekeying is to protect against direct-ram
attacks or other kernel exploits -- a concern brought up by Andy.

With siphash, which takes a 128-bit key, if you got an RNG output once
every picosecond, I believe it would take approximately 10^19 years...

Jason

^ permalink raw reply

* Re: [PATCH] stmmac: fix memory barriers
From: Joao Pinto @ 2016-12-19 17:25 UTC (permalink / raw)
  To: Niklas Cassel, Joao Pinto, David Miller, pavel
  Cc: LinoSanfilippo, peppe.cavallaro, alexandre.torgue, linux-kernel,
	netdev
In-Reply-To: <886b24a5-6d54-4bd5-2143-d8f2cbe6c746@axis.com>



Às 5:19 PM de 12/19/2016, Niklas Cassel escreveu:
> On 12/19/2016 06:10 PM, Joao Pinto wrote:
>> Hi,
>>
>> I am trying to built net-next git tree and it is failing:
>>
>>   CC      drivers/pnp/card.o
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function
>> ‘stmmac_hw_fix_mac_speed’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:224:34: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>   struct phy_device *phydev = priv->phydev;
> 
> Are you really building net-next?
> https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fmain.c-23n224&d=DgID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=4j4xIgbdV5gZT6wNKZXizwY4xE3ZoFeyGzRKeNvDsN0&s=vY4rQn8skcuhsdzk3vdZEyYF2TL8r5Dxc0gDGRjqLTQ&e= 
> 
> The phy_device initializer does not appear to match your output.

Spookie! The fetch process left half merge! I will try it again! Thanks!

> 
>>                                   ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_eee_init’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:298:24: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    if (phy_init_eee(priv->phydev, 1)) {
>>                         ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:330:44: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    priv->hw->mac->set_eee_pls(priv->hw, priv->phydev->link);
>>                                             ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_ptp’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:672:2: error: void value not
>> ignored as it ought to be
>>   return stmmac_ptp_register(priv);
>>   ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_adjust_link’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:694:34: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>   struct phy_device *phydev = priv->phydev;
>>                                   ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_phy’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:884:6: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>   priv->phydev = phydev;
>>       ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_open’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1810:10: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>   if (priv->phydev)
>>           ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1811:17: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    phy_start(priv->phydev);
>>                  ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1858:10: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>   if (priv->phydev)
>>           ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1859:22: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    phy_disconnect(priv->phydev);
>>                       ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_release’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1878:10: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>   if (priv->phydev) {
>>           ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1879:16: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    phy_stop(priv->phydev);
>>                 ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1880:22: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    phy_disconnect(priv->phydev);
>>                       ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1881:7: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    priv->phydev = NULL;
>>        ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_ioctl’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2883:12: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    if (!priv->phydev)
>>             ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2885:27: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    ret = phy_mii_ioctl(priv->phydev, rq, cmd);
>>                            ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_suspend’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3438:10: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>   if (priv->phydev)
>>           ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3439:16: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    phy_stop(priv->phydev);
>>                 ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_resume’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3533:10: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>   if (priv->phydev)
>>           ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3534:17: error: ‘struct
>> stmmac_priv’ has no member named ‘phydev’
>>    phy_start(priv->phydev);
>>                  ^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function ‘stmmac_init_ptp’:
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:673:1: warning: control
>> reaches end of non-void function [-Wreturn-type]
>>  }
>>  ^
>>   CC      drivers/pnp/driver.o
>> make[5]: *** [drivers/net/ethernet/stmicro/stmmac/stmmac_main.o] Error 1
>> make[4]: *** [drivers/net/ethernet/stmicro/stmmac] Error 2
>> make[3]: *** [drivers/net/ethernet/stmicro] Error 2
>> make[3]: *** Waiting for unfinished jobs....
>>   CC      drivers/pnp/resource.o
>>
>> Thanks,
>> Joao
>>
>> Às 4:06 PM de 12/19/2016, David Miller escreveu:
>>> From: Pavel Machek <pavel@ucw.cz>
>>> Date: Sun, 18 Dec 2016 21:38:12 +0100
>>>
>>>> Fix up memory barriers in stmmac driver. They are meant to protect
>>>> against DMA engine, so smp_ variants are certainly wrong, and dma_
>>>> variants are preferable.
>>>>     
>>>> Signed-off-by: Pavel Machek <pavel@denx.de>
>>> Applied.
>>>
> 

^ permalink raw reply

* HalfSipHash Acceptable Usage
From: Jason A. Donenfeld @ 2016-12-19 17:32 UTC (permalink / raw)
  To: Jean-Philippe Aumasson
  Cc: Theodore Ts'o, Hannes Frederic Sowa, LKML, Eric Biggers,
	Daniel J . Bernstein, David Laight, David Miller, Andi Kleen,
	George Spelvin, kernel-hardening, Andy Lutomirski,
	Linux Crypto Mailing List, Tom Herbert, Vegard Nossum, Netdev,
	Linus Torvalds

Hi JP,

With the threads getting confusing, I've been urged to try and keep
the topics and threads more closely constrained. Here's where we're
at, and here's the current pressing security concern. It'd be helpful
to have a definitive statement on what you think is best, so we can
just build on top of that, instead of getting lost in the chorus of
opinions.

1) Anything that requires actual long-term security will use
SipHash2-4, with the 64-bit output and the 128-bit key. This includes
things like TCP sequence numbers. This seems pretty uncontroversial to
me. Seem okay to you?

2) People seem to want something competitive, performance-wise, with
jhash if it's going to replace jhash. The kernel community
instinctively pushes back on anything that could harm performance,
especially in networking and in critical data structures, so there
have been some calls for something faster than SipHash. So, questions
regarding this:

2a) George thinks that HalfSipHash on 32-bit systems will have roughly
comparable speed as SipHash on 64-bit systems, so the idea would be to
use HalfSipHash on 32-bit systems' hash tables and SipHash on 64-bit
systems' hash tables. The big obvious question is: does HalfSipHash
have a sufficient security margin for hashtable usage and hashtable
attacks? I'm not wondering about the security margin for other usages,
but just of the hashtable usage. In your opinion, does HalfSipHash cut
it?

2b) While I certainly wouldn't consider making the use case in
question (1) employ a weaker function, for this question (2), there
has been some discussion about using HalfSipHash1-3 (or SipHash1-3 on
64-bit) instead of 2-4. So, the same question is therefore posed:
would using HalfSipHash1-3 give a sufficient security margin for
hashtable usage and hashtable attacks?

My plan is essentially to implement things according to your security
recommendation. The thread started with me pushing a heavy duty
security solution -- SipHash2-4 -- for _everything_. I've received
understandable push back on that notion for certain use cases. So now
I'm trying to discover what the most acceptable compromise is. Your
answers on (2a) and (2b) will direct that compromise.

Thanks again,
Jason

^ permalink raw reply

* Re: [PATCH net] virtio_net: reject XDP programs using header adjustment
From: John Fastabend @ 2016-12-19 17:50 UTC (permalink / raw)
  To: Jakub Kicinski, netdev; +Cc: kafai, daniel, alexei.starovoitov, mst
In-Reply-To: <20161219150500.2600-1-jakub.kicinski@netronome.com>

On 16-12-19 07:05 AM, Jakub Kicinski wrote:
> commit 17bedab27231 ("bpf: xdp: Allow head adjustment in XDP prog")
> added a new XDP helper to prepend and remove data from a frame.
> Make virtio_net reject programs making use of this helper until
> proper support is added.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>  drivers/net/virtio_net.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 08327e005ccc..db761f37783e 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1677,6 +1677,11 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
>  	u16 xdp_qp = 0, curr_qp;
>  	int i, err;
>  
> +	if (prog && prog->xdp_adjust_head) {
> +		netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
>  	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6)) {
>  		netdev_warn(dev, "can't set XDP while host is implementing LRO, disable LRO first\n");
> 

Acked-by: John Fastabend <john.r.fastabend@intel.com>

Thanks patch looks good. Alternatively we could push a "bug fix" to
support the adjust header feature depending on how DaveM and MST feel
about that. I don't have a strong opinion but I have the patch on my
queue it just needs some more testing. The adjust header bits were
merged in between some of the later versions of the patch which is how
this check got missed.

Thanks,
John

^ permalink raw reply

* Re: Synopsys Ethernet QoS
From: Niklas Cassel @ 2016-12-19 17:58 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Giuseppe CAVALLARO, Joao Pinto, Florian Fainelli, Andy Shevchenko,
	David Miller, larper, rabinv, netdev, CARLOS.PALMINHA, Jie.Deng1,
	Stephen Warren
In-Reply-To: <20161214125735.GA19542@amd>

On 12/14/2016 01:57 PM, Pavel Machek wrote:
> Hi!
>
>> So if there is a long time before handling interrupts,
>> I guess that it makes sense that one stream could
>> get an advantage in the net scheduler.
>>
>> If I find the time, and if no one beats me to it, I will try to replace
>> the normal timers with HR timers + a smaller default timeout.
>>
> Can you try something like this? Highres timers will be needed, too,
> but this fixes the logic problem.
>
> You'll need to apply it twice as code is copy&pasted.
>
> Best regards,

Hello Pavel

If I first apply your other fix "stmmac: fix memory barriers",
then I can apply this fix without getting a netdev watchdog timeout on tx queue0,
and everything appears to work as it should.

Regarding to fairness, I can't really see a difference, with or without your patch.
I've noticed that for TX, the streams actually stabilize after 5 seconds or so,
but with the default test length of 10 seconds, it's easy to get confused
by the test result summary. So I guess from a fairness point of view,
TX is not really a problem.

For RX fairness, it is very much a real issue. The streams never stabilize.
One key difference is that RX coalescing is implemented by using the
RX watchdog.
Here is an iperf3 run that went for 30 seconds:

[  4]   0.00-30.00  sec  1.54 GBytes   440 Mbits/sec    0             sender
[  4]   0.00-30.00  sec  1.54 GBytes   440 Mbits/sec                  receiver
[  6]   0.00-30.00  sec   600 MBytes   168 Mbits/sec    0             sender
[  6]   0.00-30.00  sec   599 MBytes   168 Mbits/sec                  receiver
[  8]   0.00-30.00  sec  1.17 GBytes   334 Mbits/sec    0             sender
[  8]   0.00-30.00  sec  1.17 GBytes   334 Mbits/sec                  receiver
[SUM]   0.00-30.00  sec  3.29 GBytes   942 Mbits/sec    0             sender
[SUM]   0.00-30.00  sec  3.29 GBytes   942 Mbits/sec                  receiver


> 									Pavel
>
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>
>  	 */
>  	priv->tx_count_frames += nfrags + 1;
>  	if (likely(priv->tx_coal_frames > priv->tx_count_frames)) {
> -		mod_timer(&priv->txtimer,
> -			  STMMAC_COAL_TIMER(priv->tx_coal_timer));
> +		if (priv->tx_count_frames == nfrags + 1)
> +			mod_timer(&priv->txtimer,
> +				  STMMAC_COAL_TIMER(priv->tx_coal_timer));
>  	} else {
>  		priv->tx_count_frames = 0;
>  		priv->hw->desc->set_tx_ic(desc);
>
>

^ permalink raw reply

* Re: Soft lockup in tc_classify
From: Cong Wang @ 2016-12-19 17:58 UTC (permalink / raw)
  To: Shahar Klein
  Cc: Or Gerlitz, Daniel Borkmann, Linux Netdev List, Roi Dayan,
	David Miller, Jiri Pirko, John Fastabend, Hadar Hen Zion
In-Reply-To: <18a64d65-1241-6c72-8333-47b0ae933139@mellanox.com>

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

Hello,

On Mon, Dec 19, 2016 at 8:39 AM, Shahar Klein <shahark@mellanox.com> wrote:
>
>
> On 12/13/2016 12:51 AM, Cong Wang wrote:
>>
>> On Mon, Dec 12, 2016 at 1:18 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>
>>> On Mon, Dec 12, 2016 at 3:28 PM, Daniel Borkmann <daniel@iogearbox.net>
>>> wrote:
>>>
>>>> Note that there's still the RCU fix missing for the deletion race that
>>>> Cong will still send out, but you say that the only thing you do is to
>>>> add a single rule, but no other operation in involved during that test?
>>>
>>>
>>> What's missing to have the deletion race fixed? making a patch or
>>> testing to a patch which was sent?
>>
>>
>> If you think it would help for this problem, here is my patch rebased
>> on the latest net-next.
>>
>> Again, I don't see how it could help this case yet, especially I don't
>> see how we could have a loop in this singly linked list.
>>
>
> I've applied cong's patch and hit a different lockup(full log attached):


Are you sure this is really different? For me, it is still inside the loop
in tc_classify(), with only a slightly different offset.


>
> Daniel suggested I'll add a print:
>                 case RTM_DELTFILTER:
> -                   err = tp->ops->delete(tp, fh);
> +                 printk(KERN_ERR "DEBUGG:SK %s:%d\n", __func__, __LINE__);
> +                 err = tp->ops->delete(tp, fh, &last);
>                         if (err == 0) {
>
> and I couldn't see this print in the output.....

Hmm, that is odd, if this never prints, then my patch should not make any
difference.

There are still two other cases where we could change tp->next, so do you
mind to add two more printk's for debugging?

Attached is the delta patch.

Thanks!

[-- Attachment #2: tc-filter-debug.diff --]
[-- Type: text/plain, Size: 880 bytes --]

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index f9179e0..45bfe9f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -317,6 +317,8 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
 		if (n->nlmsg_type == RTM_DELTFILTER && t->tcm_handle == 0) {
 			struct tcf_proto *next = rtnl_dereference(tp->next);
 
+			printk(KERN_ERR "DEBUGG:SK delete filter by: %pf\n", tp->ops->get);
+
 			RCU_INIT_POINTER(*back, next);
 
 			tfilter_notify(net, skb, n, tp, fh,
@@ -370,6 +372,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
 			      n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE);
 	if (err == 0) {
 		if (tp_created) {
+			printk(KERN_ERR "DEBUGG:SK add/change filter by: %pf\n", tp->ops->change);
 			RCU_INIT_POINTER(tp->next, rtnl_dereference(*back));
 			rcu_assign_pointer(*back, tp);
 		}

^ 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