Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] Revert "net: Add driver helper functions to determine checksum offloadability"
From: David Miller @ 2016-10-14 14:03 UTC (permalink / raw)
  To: stephen; +Cc: tom, netdev
In-Reply-To: <20161011130409.7e6b54ff@xeon-e3>

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 11 Oct 2016 13:04:09 -0700

> 
> This reverts commit 6ae23ad36253a8033c5714c52b691b84456487c5.
> 
> The code has been in kernel since 4.4 but there are no in tree
> code that uses. Unused code is broken code, remove it.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied to net-next, thanks Stephen.

Please put proper "[PATCH net-next]" tags in your Subject lines when
patches are targetting net-next.

Thanks.

^ permalink raw reply

* Re: [PATCH net] ip6_tunnel: fix ip6_tnl_lookup
From: David Miller @ 2016-10-14 14:02 UTC (permalink / raw)
  To: junk; +Cc: netdev
In-Reply-To: <20161011203454.F2B87341C3E@junk-ub14>

From: Vadim Fedorenko <junk@yandex-team.ru>
Date: Tue, 11 Oct 2016 22:47:20 +0300

> The commit ea3dc9601bda ("ip6_tunnel: Add support for wildcard tunnel
> endpoints.") introduces support for wildcards in tunnels endpoints,
> but in some rare circumstances ip6_tnl_lookup selects wrong tunnel
> interface relying only on source or destination address of the packet
> and not checking presence of wildcard in tunnels endpoints. Later in
> ip6_tnl_rcv this packets can be dicarded because of difference in
> ipproto even if fallback device have proper ipproto configuration.
> 
> This patch adds checks of wildcard endpoint in tunnel avoiding such
> behavior
> 
> Fixes: ea3dc9601bda ("ip6_tunnel: Add support for wildcard tunnel
> endpoints.")
> 
> Signed-off-by: Vadim Fedorenko <junk@yandex-team.ru>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH v2] r8169: set coherent DMA mask as well as streaming DMA mask
From: Ard Biesheuvel @ 2016-10-14 13:48 UTC (permalink / raw)
  To: David Laight
  Cc: romieu@fr.zoreil.com, nic_swsd@realtek.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	davem@davemloft.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB01FF652@AcuExch.aculab.com>


> On 14 Oct 2016, at 14:42, David Laight <David.Laight@ACULAB.COM> wrote:
> 
> From: Of Ard Biesheuvel
>> Sent: 14 October 2016 14:41
>> PCI devices that are 64-bit DMA capable should set the coherent
>> DMA mask as well as the streaming DMA mask. On some architectures,
>> these are managed separately, and so the coherent DMA mask will be
>> left at its default value of 32 if it is not set explicitly. This
>> results in errors such as
>> 
>>     r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>     hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>>     swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>>     CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>>     Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
>> 
>> on systems without memory that is 32-bit addressable by PCI devices.
>> 
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> v2: dropped the hunk that sets the coherent DMA mask to DMA_BIT_MASK(32),
>>    which is unnecessary given that it is the default
>> 
>> drivers/net/ethernet/realtek/r8169.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
>> index e55638c7505a..bf000d819a21 100644
>> --- a/drivers/net/ethernet/realtek/r8169.c
>> +++ b/drivers/net/ethernet/realtek/r8169.c
>> @@ -8273,7 +8273,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>    if ((sizeof(dma_addr_t) > 4) &&
>>        (use_dac == 1 || (use_dac == -1 && pci_is_pcie(pdev) &&
>>                  tp->mac_version >= RTL_GIGA_MAC_VER_18)) &&
>> -        !pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) {
>> +        !pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) &&
>> +        !pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64))) {
> 
> Isn't there a dma_set_mask_and_coherent() function ?
> 

Not of the pci_xxx variety afaik

^ permalink raw reply

* Re: [PATCH v2] r8169: set coherent DMA mask as well as streaming DMA mask
From: Ard Biesheuvel @ 2016-10-14 13:46 UTC (permalink / raw)
  To: David Laight
  Cc: romieu@fr.zoreil.com, nic_swsd@realtek.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	davem@davemloft.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB01FF652@AcuExch.aculab.com>



> On 14 Oct 2016, at 14:42, David Laight<David.Laight@ACULAB.COM> wrote:
> 
> From: Of Ard Biesheuvel
>> Sent: 14 October 2016 14:41
>> PCI devices that are 64-bit DMA capable should set the coherent
>> DMA mask as well as the streaming DMA mask. On some architectures,
>> these are managed separately, and so the coherent DMA mask will be
>> left at its default value of 32 if it is not set explicitly. This
>> results in errors such as
>> 
>>     r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>     hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>>     swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>>     CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>>     Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
>> 
>> on systems without memory that is 32-bit addressable by PCI devices.
>> 
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> v2: dropped the hunk that sets the coherent DMA mask to DMA_BIT_MASK(32),
>>    which is unnecessary given that it is the default
>> 
>> drivers/net/ethernet/realtek/r8169.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
>> index e55638c7505a..bf000d819a21 100644
>> --- a/drivers/net/ethernet/realtek/r8169.c
>> +++ b/drivers/net/ethernet/realtek/r8169.c
>> @@ -8273,7 +8273,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>    if ((sizeof(dma_addr_t) > 4) &&
>>        (use_dac == 1 || (use_dac == -1 && pci_is_pcie(pdev) &&
>>                  tp->mac_version >= RTL_GIGA_MAC_VER_18)) &&
>> -        !pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) {
>> +        !pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) &&
>> +        !pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64))) {
> 
> Isn't there a dma_set_mask_and_coherent() function ?
> 
>    David
> 

^ permalink raw reply

* Re: [PATCH] mac80211: aes_ccm: move struct aead_req off the stack
From: Johannes Berg @ 2016-10-14 13:46 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Andy Lutomirski, Sergey Senozhatsky,
	<netdev@vger.kernel.org>, Herbert Xu, David S. Miller,
	<linux-wireless@vger.kernel.org>,
	linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu81RYq1aZUQ2Aru3Vev4LGgb1zFwWz8gepE0tQEhQi9Uw@mail.gmail.com>


> 
> Is the aad[] actually reused? I would assume it only affects the mac
> on encryption, and the verification on decryption but I don't think
> we actually need it back from the crypto routines.

I don't think it's reused.

> Exactly what you said above :-) My patch only touches CCM but as you
> said,
> 
> """
> 'Also there's B_0/J_0 for CCM/GCM, and the 'zero' thing that GMAC
> has.
> """

Ah, but we can/should do the same for the others, no?

johannes

^ permalink raw reply

* RE: [PATCH v2] r8169: set coherent DMA mask as well as streaming DMA mask
From: David Laight @ 2016-10-14 13:42 UTC (permalink / raw)
  To: 'Ard Biesheuvel', romieu@fr.zoreil.com,
	nic_swsd@realtek.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, davem@davemloft.net
In-Reply-To: <1476452433-20518-1-git-send-email-ard.biesheuvel@linaro.org>

From: Of Ard Biesheuvel
> Sent: 14 October 2016 14:41
> PCI devices that are 64-bit DMA capable should set the coherent
> DMA mask as well as the streaming DMA mask. On some architectures,
> these are managed separately, and so the coherent DMA mask will be
> left at its default value of 32 if it is not set explicitly. This
> results in errors such as
> 
>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
> 
> on systems without memory that is 32-bit addressable by PCI devices.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v2: dropped the hunk that sets the coherent DMA mask to DMA_BIT_MASK(32),
>     which is unnecessary given that it is the default
> 
>  drivers/net/ethernet/realtek/r8169.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index e55638c7505a..bf000d819a21 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -8273,7 +8273,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if ((sizeof(dma_addr_t) > 4) &&
>  	    (use_dac == 1 || (use_dac == -1 && pci_is_pcie(pdev) &&
>  			      tp->mac_version >= RTL_GIGA_MAC_VER_18)) &&
> -	    !pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) {
> +	    !pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) &&
> +	    !pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64))) {

Isn't there a dma_set_mask_and_coherent() function ?

	David

^ permalink raw reply

* [PATCH v2] r8169: set coherent DMA mask as well as streaming DMA mask
From: Ard Biesheuvel @ 2016-10-14 13:40 UTC (permalink / raw)
  To: romieu, nic_swsd, netdev, linux-kernel, davem; +Cc: Ard Biesheuvel

PCI devices that are 64-bit DMA capable should set the coherent
DMA mask as well as the streaming DMA mask. On some architectures,
these are managed separately, and so the coherent DMA mask will be
left at its default value of 32 if it is not set explicitly. This
results in errors such as

     r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
     hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
     swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
     CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
     Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016

on systems without memory that is 32-bit addressable by PCI devices.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: dropped the hunk that sets the coherent DMA mask to DMA_BIT_MASK(32),
    which is unnecessary given that it is the default

 drivers/net/ethernet/realtek/r8169.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index e55638c7505a..bf000d819a21 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -8273,7 +8273,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if ((sizeof(dma_addr_t) > 4) &&
 	    (use_dac == 1 || (use_dac == -1 && pci_is_pcie(pdev) &&
 			      tp->mac_version >= RTL_GIGA_MAC_VER_18)) &&
-	    !pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) {
+	    !pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) &&
+	    !pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64))) {
 
 		/* CPlusCmd Dual Access Cycle is only needed for non-PCIe */
 		if (!pci_is_pcie(pdev))
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
From: Ard Biesheuvel @ 2016-10-14 13:36 UTC (permalink / raw)
  To: David Miller
  Cc: Francois Romieu, Realtek linux nic maintainers,
	<netdev@vger.kernel.org>, linux-kernel@vger.kernel.org
In-Reply-To: <20161014.093403.1591217631213879011.davem@davemloft.net>

On 14 October 2016 at 14:34, David Miller <davem@davemloft.net> wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date: Fri, 14 Oct 2016 14:32:24 +0100
>
>> On 14 October 2016 at 14:31, David Miller <davem@davemloft.net> wrote:
>>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Date: Fri, 14 Oct 2016 12:39:30 +0100
>>>
>>>> PCI devices that are 64-bit DMA capable should set the coherent
>>>> DMA mask as well as the streaming DMA mask. On some architectures,
>>>> these are managed separately, and so the coherent DMA mask will be
>>>> left at its default value of 32 if it is not set explicitly. This
>>>> results in errors such as
>>>>
>>>>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>>>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>>>>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>>>>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>>>>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
>>>>
>>>> on systems without memory that is 32-bit addressable by PCI devices.
>>>>
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>  ...
>>>> @@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>>               dev->features |= NETIF_F_HIGHDMA;
>>>>       } else {
>>>>               rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>>>> +             if (!rc)
>>>> +                     rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
>>>
>>> As you state 32-bit is the default, therefore this part of your patch is unnecessary.
>>
>> Perhaps, but the original code did not assume that either. Should I
>> remove the other call in a subsequent patch as well?
>
> I simply want you to respin this with the above hunk removed.
>
> Your code changes and your commit message must be consistent.

OK, fair enough

^ permalink raw reply

* Re: [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
From: David Miller @ 2016-10-14 13:34 UTC (permalink / raw)
  To: ard.biesheuvel; +Cc: romieu, nic_swsd, netdev, linux-kernel
In-Reply-To: <CAKv+Gu_HEANqHNh5d_b=-3djA__8MkE7UE5ps8rTFuCuJURcVQ@mail.gmail.com>

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 14 Oct 2016 14:32:24 +0100

> On 14 October 2016 at 14:31, David Miller <davem@davemloft.net> wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Date: Fri, 14 Oct 2016 12:39:30 +0100
>>
>>> PCI devices that are 64-bit DMA capable should set the coherent
>>> DMA mask as well as the streaming DMA mask. On some architectures,
>>> these are managed separately, and so the coherent DMA mask will be
>>> left at its default value of 32 if it is not set explicitly. This
>>> results in errors such as
>>>
>>>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>>>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>>>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>>>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
>>>
>>> on systems without memory that is 32-bit addressable by PCI devices.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>  ...
>>> @@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>               dev->features |= NETIF_F_HIGHDMA;
>>>       } else {
>>>               rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>>> +             if (!rc)
>>> +                     rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
>>
>> As you state 32-bit is the default, therefore this part of your patch is unnecessary.
> 
> Perhaps, but the original code did not assume that either. Should I
> remove the other call in a subsequent patch as well?

I simply want you to respin this with the above hunk removed.

Your code changes and your commit message must be consistent.

^ permalink raw reply

* Re: [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
From: Ard Biesheuvel @ 2016-10-14 13:32 UTC (permalink / raw)
  To: David Miller
  Cc: Francois Romieu, Realtek linux nic maintainers,
	<netdev@vger.kernel.org>, linux-kernel@vger.kernel.org
In-Reply-To: <20161014.093101.1348173909013700977.davem@davemloft.net>

On 14 October 2016 at 14:31, David Miller <davem@davemloft.net> wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date: Fri, 14 Oct 2016 12:39:30 +0100
>
>> PCI devices that are 64-bit DMA capable should set the coherent
>> DMA mask as well as the streaming DMA mask. On some architectures,
>> these are managed separately, and so the coherent DMA mask will be
>> left at its default value of 32 if it is not set explicitly. This
>> results in errors such as
>>
>>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
>>
>> on systems without memory that is 32-bit addressable by PCI devices.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>  ...
>> @@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>               dev->features |= NETIF_F_HIGHDMA;
>>       } else {
>>               rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>> +             if (!rc)
>> +                     rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
>
> As you state 32-bit is the default, therefore this part of your patch is unnecessary.

Perhaps, but the original code did not assume that either. Should I
remove the other call in a subsequent patch as well?

^ permalink raw reply

* Re: [PATCH] r8169: set coherent DMA mask as well as streaming DMA mask
From: David Miller @ 2016-10-14 13:31 UTC (permalink / raw)
  To: ard.biesheuvel; +Cc: romieu, nic_swsd, netdev, linux-kernel
In-Reply-To: <1476445170-4496-1-git-send-email-ard.biesheuvel@linaro.org>

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 14 Oct 2016 12:39:30 +0100

> PCI devices that are 64-bit DMA capable should set the coherent
> DMA mask as well as the streaming DMA mask. On some architectures,
> these are managed separately, and so the coherent DMA mask will be
> left at its default value of 32 if it is not set explicitly. This
> results in errors such as
> 
>      r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>      hwdev DMA mask = 0x00000000ffffffff, dev_addr = 0x00000080fbfff000
>      swiotlb: coherent allocation failed for device 0000:02:00.0 size=4096
>      CPU: 0 PID: 1062 Comm: systemd-udevd Not tainted 4.8.0+ #35
>      Hardware name: AMD Seattle/Seattle, BIOS 10:53:24 Oct 13 2016
> 
> on systems without memory that is 32-bit addressable by PCI devices.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
 ...
> @@ -8281,6 +8282,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		dev->features |= NETIF_F_HIGHDMA;
>  	} else {
>  		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> +		if (!rc)
> +			rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));

As you state 32-bit is the default, therefore this part of your patch is unnecessary.

^ permalink raw reply

* Re: Need help with mdiobus_register and phy
From: Timur Tabi @ 2016-10-14 13:28 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20161014131852.GM5822@lunn.ch>

Andrew Lunn wrote:
> Does the datasheet say anything about this?
>
> I would say for this device, suspend() is too aggressive.

I'll have to find the datasheet.  Let me do some research and get back 
to you.  Thanks for your help so far.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply

* Re: [PATCH] mac80211: aes_ccm: move struct aead_req off the stack
From: Ard Biesheuvel @ 2016-10-14 13:19 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Andy Lutomirski, Sergey Senozhatsky,
	<netdev@vger.kernel.org>, Herbert Xu, David S. Miller,
	<linux-wireless@vger.kernel.org>,
	linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476450941.31114.45.camel@sipsolutions.net>

On 14 October 2016 at 14:15, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 14:13 +0100, Ard Biesheuvel wrote:
>>
>> > But if we allocate things anyway, is it worth expending per-CPU
>> > buffers on these?
>>
>> Ehmm, maybe not. I could spin a v2 that allocates a bigger buffer,
>> and copies aad[] into it as well
>
> Copies in/out, I guess. Also there's B_0/J_0 for CCM/GCM, and the
> 'zero' thing that GMAC has.
>

Is the aad[] actually reused? I would assume it only affects the mac
on encryption, and the verification on decryption but I don't think we
actually need it back from the crypto routines.

>> That does not help the other algos though
>
> What do you mean?
>

Exactly what you said above :-) My patch only touches CCM but as you said,

"""
'Also there's B_0/J_0 for CCM/GCM, and the 'zero' thing that GMAC has.
"""

^ permalink raw reply

* Re: Need help with mdiobus_register and phy
From: Andrew Lunn @ 2016-10-14 13:18 UTC (permalink / raw)
  To: Timur Tabi; +Cc: netdev
In-Reply-To: <5800D796.1030602@codeaurora.org>

On Fri, Oct 14, 2016 at 08:03:18AM -0500, Timur Tabi wrote:
> Andrew Lunn wrote:
> >Have you tried using the ethernet-phy-id device tree property? It
> >looks like that will allow you to skip get_phy_device and just create
> >the phy device. You can then bring the phy out of sleep in the probe
> >function?
> 
> The problem I'm experiencing is with ACPI, so I can't use any of the
> fancy of_ apis like of_get_phy_id().  But I'll look into it.
> 
> Is it possible that at803x_suspend() is too aggressive?  That's it's
> effectively disabling the phy?  While the phy is suspended, should
> it still respond to MII_PHYSID1 and MII_PHYSID2 requests?

That is a basic assumption of the code. If you cannot read the IDs how
are you supposed to know what device it is, and what quirks you need
to work around its broken features...

Does the datasheet say anything about this?

I would say for this device, suspend() is too aggressive.

  Andrew

^ permalink raw reply

* Re: [PATCH] mac80211: aes_ccm: move struct aead_req off the stack
From: Johannes Berg @ 2016-10-14 13:15 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Andy Lutomirski, Sergey Senozhatsky,
	<netdev@vger.kernel.org>, Herbert Xu, David S. Miller,
	<linux-wireless@vger.kernel.org>,
	linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu_eS-C84rH_MLYy--ivC0a99wwJvNDLhETfrBkYXW-XpA@mail.gmail.com>

On Fri, 2016-10-14 at 14:13 +0100, Ard Biesheuvel wrote:
> 
> > But if we allocate things anyway, is it worth expending per-CPU
> > buffers on these?
> 
> Ehmm, maybe not. I could spin a v2 that allocates a bigger buffer,
> and copies aad[] into it as well

Copies in/out, I guess. Also there's B_0/J_0 for CCM/GCM, and the
'zero' thing that GMAC has.

> That does not help the other algos though

What do you mean?

johannes

^ permalink raw reply

* Re: [PATCH] mac80211: aes_ccm: move struct aead_req off the stack
From: Ard Biesheuvel @ 2016-10-14 13:13 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Andy Lutomirski, Sergey Senozhatsky,
	<netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Herbert Xu, David S. Miller,
	<linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jouni Malinen
In-Reply-To: <1476450635.31114.42.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

On 14 October 2016 at 14:10, Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
>
>> So use kzalloc
>
> Do we really need kzalloc()? We have things on the stack right now, and
> don't initialize, so surely we don't really need to zero things?
>
>> This only addresses one half of the problem. The other problem, i.e.,
>> the fact that the aad[] array lives on the stack of the caller, is
>> handled adequately imo by the change proposed by Johannes.
>
> But if we allocate things anyway, is it worth expending per-CPU buffers
> on these?
>

Ehmm, maybe not. I could spin a v2 that allocates a bigger buffer, and
copies aad[] into it as well
That does not help the other algos though

^ permalink raw reply

* Re: [PATCH] mac80211: aes_ccm: move struct aead_req off the stack
From: Johannes Berg @ 2016-10-14 13:11 UTC (permalink / raw)
  To: Ard Biesheuvel, luto, sergey.senozhatsky.work, netdev, herbert,
	davem, linux-wireless, linux-kernel, j
In-Reply-To: <1476450635.31114.42.camel@sipsolutions.net>

On Fri, 2016-10-14 at 15:10 +0200, Johannes Berg wrote:
> > 
> > So use kzalloc
> 
> Do we really need kzalloc()? We have things on the stack right now,
> and don't initialize, so surely we don't really need to zero things? 

Err, never mind, I'm an idiot - we *do* initialize to 0, of course.

johannes

^ permalink raw reply

* Re: [PATCH] mac80211: aes_ccm: move struct aead_req off the stack
From: Johannes Berg @ 2016-10-14 13:10 UTC (permalink / raw)
  To: Ard Biesheuvel, luto-kltTT9wpgjJwATOyAt5JVQ,
	sergey.senozhatsky.work-Re5JQEeQqe8AvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, j
In-Reply-To: <1476450540-1760-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>


> So use kzalloc

Do we really need kzalloc()? We have things on the stack right now, and
don't initialize, so surely we don't really need to zero things?

> This only addresses one half of the problem. The other problem, i.e.,
> the fact that the aad[] array lives on the stack of the caller, is
> handled adequately imo by the change proposed by Johannes.

But if we allocate things anyway, is it worth expending per-CPU buffers
on these?

johannes

^ permalink raw reply

* [PATCH] mac80211: aes_ccm: move struct aead_req off the stack
From: Ard Biesheuvel @ 2016-10-14 13:09 UTC (permalink / raw)
  To: johannes, luto, sergey.senozhatsky.work, netdev, herbert, davem,
	linux-wireless, linux-kernel, j
  Cc: Ard Biesheuvel

Some CCM implementations (such as the generic CCM wrapper in crypto/)
use scatterlists to map fields of struct aead_req. This means these
data structures cannot live in the vmalloc area, which means that in
the near future, they can no longer live on the stack either.

Given that these data structures have implementation specific context fields,
it really depends on the particular driver whether this issue is likely to
occur or not, and so it seems best to simply move the entire data structure
into the direct mapped kernel heap.

So use kzalloc/kfree to allocate and free the data structures. This pattern
already exists in the IPsec ESP driver, but in the future, we may need to
improve upon this by either moving the request into the SKB, or using a
slab cache to allocate/free the data structures.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

This only addresses one half of the problem. The other problem, i.e., the
fact that the aad[] array lives on the stack of the caller, is handled
adequately imo by the change proposed by Johannes.

 net/mac80211/aes_ccm.c | 24 ++++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
index 7663c28ba353..a0ae8cebbe4e 100644
--- a/net/mac80211/aes_ccm.c
+++ b/net/mac80211/aes_ccm.c
@@ -23,13 +23,10 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
 			       size_t mic_len)
 {
 	struct scatterlist sg[3];
+	struct aead_request *aead_req;
 
-	char aead_req_data[sizeof(struct aead_request) +
-			   crypto_aead_reqsize(tfm)]
-		__aligned(__alignof__(struct aead_request));
-	struct aead_request *aead_req = (void *) aead_req_data;
-
-	memset(aead_req, 0, sizeof(aead_req_data));
+	aead_req = kzalloc(sizeof(struct aead_request) +
+			   crypto_aead_reqsize(tfm), GFP_ATOMIC);
 
 	sg_init_table(sg, 3);
 	sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
@@ -41,6 +38,7 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
 	aead_request_set_ad(aead_req, sg[0].length);
 
 	crypto_aead_encrypt(aead_req);
+	kfree(aead_req);
 }
 
 int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
@@ -48,15 +46,14 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
 			      size_t mic_len)
 {
 	struct scatterlist sg[3];
-	char aead_req_data[sizeof(struct aead_request) +
-			   crypto_aead_reqsize(tfm)]
-		__aligned(__alignof__(struct aead_request));
-	struct aead_request *aead_req = (void *) aead_req_data;
+	struct aead_request *aead_req;
+	int err;
 
 	if (data_len == 0)
 		return -EINVAL;
 
-	memset(aead_req, 0, sizeof(aead_req_data));
+	aead_req = kzalloc(sizeof(struct aead_request) +
+			   crypto_aead_reqsize(tfm), GFP_ATOMIC);
 
 	sg_init_table(sg, 3);
 	sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
@@ -67,7 +64,10 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
 	aead_request_set_crypt(aead_req, sg, sg, data_len + mic_len, b_0);
 	aead_request_set_ad(aead_req, sg[0].length);
 
-	return crypto_aead_decrypt(aead_req);
+	err = crypto_aead_decrypt(aead_req);
+	kfree(aead_req);
+
+	return err;
 }
 
 struct crypto_aead *ieee80211_aes_key_setup_encrypt(const u8 key[],
-- 
2.7.4

^ permalink raw reply related

* Re: Need help with mdiobus_register and phy
From: Timur Tabi @ 2016-10-14 13:03 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20161014125736.GK5822@lunn.ch>

Andrew Lunn wrote:
> Have you tried using the ethernet-phy-id device tree property? It
> looks like that will allow you to skip get_phy_device and just create
> the phy device. You can then bring the phy out of sleep in the probe
> function?

The problem I'm experiencing is with ACPI, so I can't use any of the 
fancy of_ apis like of_get_phy_id().  But I'll look into it.

Is it possible that at803x_suspend() is too aggressive?  That's it's 
effectively disabling the phy?  While the phy is suspended, should it 
still respond to MII_PHYSID1 and MII_PHYSID2 requests?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply

* Re: Need help with mdiobus_register and phy
From: Andrew Lunn @ 2016-10-14 12:57 UTC (permalink / raw)
  To: Timur Tabi; +Cc: netdev
In-Reply-To: <5800D474.1030303@codeaurora.org>

On Fri, Oct 14, 2016 at 07:49:56AM -0500, Timur Tabi wrote:
> Andrew Lunn wrote:
> >So are you seeing that the reads to MII_PHYSID1 and MII_PHYSID2 return
> >0xffff, when called from get_phy_id()?

Have you tried using the ethernet-phy-id device tree property? It
looks like that will allow you to skip get_phy_device and just create
the phy device. You can then bring the phy out of sleep in the probe
function?

	Andrew

^ permalink raw reply

* Re: Need help with mdiobus_register and phy
From: Andrew Lunn @ 2016-10-14 12:49 UTC (permalink / raw)
  To: Timur Tabi; +Cc: netdev
In-Reply-To: <5800D214.70808@codeaurora.org>

> It's the at803x driver.

The at803x_resume() just does normal MDIO transactions. Which suggests
the MDIO bus side of the device is still away. Or at least, the
MII_BMCR register is.

So are you seeing that the reads to MII_PHYSID1 and MII_PHYSID2 return
0xffff, when called from get_phy_id()?

	Andrew

^ permalink raw reply

* Re: Need help with mdiobus_register and phy
From: Timur Tabi @ 2016-10-14 12:49 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20161014124928.GJ5822@lunn.ch>

Andrew Lunn wrote:
> So are you seeing that the reads to MII_PHYSID1 and MII_PHYSID2 return
> 0xffff, when called from get_phy_id()?

Yes.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply

* Re: Need help with mdiobus_register and phy
From: Timur Tabi @ 2016-10-14 12:39 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20161014120624.GG5822@lunn.ch>

Andrew Lunn wrote:
> Please can you tell us what PHY which is, and how it is put to sleep
> and woken up.

It's the at803x driver.

http://lxr.free-electrons.com/source/drivers/net/phy/at803x.c

It goes to sleep in its at803x_suspend() function, which is called by 
phy_suspend().

There is a corresponding at803x_resume().  The problem is that this is 
not called by mdiobus_register().  I'm guessing that mdiobus_register() 
assumes that the phy is awake.

It seems like a catch-22.  mdiobus_register() assumes that the phy is 
awake, but you can't wake up the phy until after you call 
mdiobus_register().

> If the PHY cannot be woken up using MDIO, then maybe you need to look
> at the mdio bus reset call?

I looked at that, but it won't work because there is no phydev when the 
reset function is called:

http://lxr.free-electrons.com/source/drivers/net/phy/mdio_bus.c#L328

It's the same catch-22.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply

* Re: [PATCH v2] net: Require exact match for TCP socket lookups if dif is l3mdev
From: David Ahern @ 2016-10-14 12:21 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1476426789.5650.34.camel@edumazet-glaptop3.roam.corp.google.com>

On 10/14/16 12:33 AM, Eric Dumazet wrote:
> There is a catch here.
> TCP moves IP6CB() in a different location.
> 
> Reference :
> 
> 971f10eca186 ("tcp: better TCP_SKB_CB layout to reduce cache line misses")

thanks for the reference.


> Problem is that the lookup can happen from IP early demux, before TCP
> moved IP{6}CB around.

For TCP we only need the exact_dif match for listen sockets, so early demux does not apply.

> 
> So you might need to let the caller pass IP6CB(skb)->flags (or
> TCP_SKB_CB(skb)->header.h6.flags ) instead of skb since
> inet6_exact_dif_match() does not know where to fetch the flags.
> 
> Same issue for IPv4.

I'll update the match functions to pull from TCP_SKB_CB instead of IP6CB and make a note of the above.

Thanks for the review

^ 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