Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH 4.10 3/6] bpf: Use SHA256 instead of SHA1 for bpf digests
From: Andy Lutomirski @ 2016-12-27  2:08 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, Andy Lutomirski, Netdev, LKML,
	Linux Crypto Mailing List, Jason A. Donenfeld,
	Hannes Frederic Sowa, Eric Dumazet, Eric Biggers, Tom Herbert,
	David S. Miller, Alexei Starovoitov
In-Reply-To: <20161227013644.GA96815@ast-mbp.thefacebook.com>

On Mon, Dec 26, 2016 at 5:36 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Sat, Dec 24, 2016 at 08:59:53PM +0100, Daniel Borkmann wrote:
>> On 12/24/2016 03:22 AM, Andy Lutomirski wrote:
>> >BPF digests are intended to be used to avoid reloading programs that
>> >are already loaded.  For use cases (CRIU?) where untrusted programs
>> >are involved, intentional hash collisions could cause the wrong BPF
>> >program to execute.  Additionally, if BPF digests are ever used
>> >in-kernel to skip verification, a hash collision could give privilege
>> >escalation directly.
>>
>> Just for the record, digests will never ever be used to skip the
>> verification step, so I don't know why this idea even comes up
>> here (?) or is part of the changelog? As this will never be done
>> anyway, rather drop that part so we can avoid confusion on this?
>
> +1 to what Daniel said above.
>
> For the others let me explain what this patch set is actually
> trying to accomplish.

The patch:

a) cleans up the code and

b) uses a cryptographic hash that is actually believed to satisfy the
definition of a cryptographic hash.

There's no excuse for not doing b.

> and I have an obvious NACK for bpf related patches 3,4,5,6.

Did you *read* the ones that were pure cleanups?

>
> sha1 is 20 bytes which is already a bit long to print and copy paste by humans.
> whereas 4 byte jhash is a bit too short, since collisions are not that rare
> and may lead to incorrect assumptions from the users that develop the programs.
> I would prefer something in 6-10 byte range that prevents collisions most of
> the time and short to print as hex, but I don't know of anything like this
> in the existing kernel and inventing bpf specific hash is not great.
> Another requirement for debugging (and prog_digest) that user space
> should be able to produce the same hash without asking kernel, so
> sha1 fits that as well, since it's well known and easy to put into library.

Then truncate them in user space.

^ permalink raw reply

* [PATCH net] net: xdp: remove unused bfp_warn_invalid_xdp_buffer()
From: Jason Wang @ 2016-12-27  2:49 UTC (permalink / raw)
  To: davem, linux-kernel, netdev; +Cc: Jason Wang, Daniel Borkmann, John Fastabend

After commit 73b62bd085f4737679ea9afc7867fa5f99ba7d1b ("virtio-net:
remove the warning before XDP linearizing"), there's no users for
bpf_warn_invalid_xdp_buffer(), so remove it. This is a revert for
commit f23bc46c30ca5ef58b8549434899fcbac41b2cfc.

Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/linux/filter.h | 1 -
 net/core/filter.c      | 6 ------
 2 files changed, 7 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 7023142..a0934e6 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -610,7 +610,6 @@ bool bpf_helper_changes_pkt_data(void *func);
 struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,
 				       const struct bpf_insn *patch, u32 len);
 void bpf_warn_invalid_xdp_action(u32 act);
-void bpf_warn_invalid_xdp_buffer(void);
 
 #ifdef CONFIG_BPF_JIT
 extern int bpf_jit_enable;
diff --git a/net/core/filter.c b/net/core/filter.c
index 7190bd6..b146170 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2972,12 +2972,6 @@ void bpf_warn_invalid_xdp_action(u32 act)
 }
 EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_action);
 
-void bpf_warn_invalid_xdp_buffer(void)
-{
-	WARN_ONCE(1, "Illegal XDP buffer encountered, expect throughput degradation\n");
-}
-EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_buffer);
-
 static u32 sk_filter_convert_ctx_access(enum bpf_access_type type, int dst_reg,
 					int src_reg, int ctx_off,
 					struct bpf_insn *insn_buf,
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net 1/9] virtio-net: remove the warning before XDP linearizing
From: Jason Wang @ 2016-12-27  3:08 UTC (permalink / raw)
  To: Daniel Borkmann, mst, virtualization, netdev, linux-kernel
  Cc: john.r.fastabend
In-Reply-To: <585D7BA9.50509@iogearbox.net>



On 2016年12月24日 03:31, Daniel Borkmann wrote:
> Hi Jason,
>
> On 12/23/2016 03:37 PM, Jason Wang wrote:
>> Since we use EWMA to estimate the size of rx buffer. When rx buffer
>> size is underestimated, it's usual to have a packet with more than one
>> buffers. Consider this is not a bug, remove the warning and correct
>> the comment before XDP linearizing.
>>
>> Cc: John Fastabend <john.r.fastabend@intel.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>   drivers/net/virtio_net.c | 8 +-------
>>   1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 08327e0..1067253 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -552,14 +552,8 @@ static struct sk_buff *receive_mergeable(struct 
>> net_device *dev,
>>           struct page *xdp_page;
>>           u32 act;
>>
>> -        /* No known backend devices should send packets with
>> -         * more than a single buffer when XDP conditions are
>> -         * met. However it is not strictly illegal so the case
>> -         * is handled as an exception and a warning is thrown.
>> -         */
>> +        /* This happens when rx buffer size is underestimated */
>>           if (unlikely(num_buf > 1)) {
>> -            bpf_warn_invalid_xdp_buffer();
>
> Could you also remove the bpf_warn_invalid_xdp_buffer(), which got added
> just for this?
>
> Thanks. 

Posted.

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

^ permalink raw reply

* Re: [RFC PATCH 1/7] PF driver modified to enable HW filter support, changes works in backward compatibility mode Enable required things in Makefile Enable LZ4 dependecy inside config file
From: Sunil Kovvuri @ 2016-12-27  4:19 UTC (permalink / raw)
  To: Koteshwar Rao, Satha
  Cc: LKML, Goutham, Sunil, Robert Richter, David S. Miller,
	Daney, David, Vatsavayi, Raghu, Chickles, Derek, Romanov, Philip,
	Linux Netdev List, LAKML
In-Reply-To: <DM5PR07MB28428EE7B1679FAEC78103AD8D960@DM5PR07MB2842.namprd07.prod.outlook.com>

>>  #define NIC_MAX_RSS_HASH_BITS          8
>>  #define NIC_MAX_RSS_IDR_TBL_SIZE       (1 << NIC_MAX_RSS_HASH_BITS)
>> +#define NIC_TNS_RSS_IDR_TBL_SIZE       5
>
> So you want to use only 5 queues per VF when TNS is enabled, is it ??
> There are 4096 RSS indices in total, for each VF you can use max 32.
> I guess you wanted to set no of hash bits to 5 instead of table size.
>
> SATHA>>> We enabled 8 queues for VF. Yes Macro name misleads it has to be hash bits, will change this in next version

No, I am not referring to any discrepancy in naming the macro.
If you check your code

- hw->rss_ind_tbl_size = NIC_MAX_RSS_IDR_TBL_SIZE;
+ hw->rss_ind_tbl_size = veb_enabled ? NIC_TNS_RSS_IDR_TBL_SIZE :
+     NIC_MAX_RSS_IDR_TBL_SIZE;

You are setting RSS table size to 5, i.e RSS hash bits will be set to 2.
Hence only 4 queues (not even 5 as i mentioned earlier). Please check
'nicvf_rss_init' you will understand what i am saying.
Have you tested and observed pkts in all 8 queues ?

^ permalink raw reply

* Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: David Miller @ 2016-12-27  4:54 UTC (permalink / raw)
  To: hock.leong.kweh
  Cc: Joao.Pinto, peppe.cavallaro, seraphin.bonnaffe, alexandre.torgue,
	manabian, niklas.cassel, johan, pavel, boon.leong.ong, netdev,
	linux-kernel, weifeng.voon, lars.persson
In-Reply-To: <1482839100-20612-1-git-send-email-hock.leong.kweh@intel.com>

From: "Kweh, Hock Leong"	<hock.leong.kweh@intel.com>
Date: Tue, 27 Dec 2016 19:44:59 +0800

> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> 
> If kernel module stmmac driver being loaded after OS booted, there is a
> race condition between stmmac_open() and stmmac_mdio_register(), which is
> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as
> PHY not found and stmmac_open() failed:
 ...
> The resolution used wait_for_completion_interruptible() to synchronize
> stmmac_open() and stmmac_dvr_probe() to prevent the race condition
> happening.
> 
> Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>

The proper thing to do is to make sure register_netdevice() is not
invoked until it is %100 safe to call stmmac_open().

^ permalink raw reply

* Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: Florian Fainelli @ 2016-12-27  5:10 UTC (permalink / raw)
  To: Kweh, Hock Leong, David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
	seraphin.bonnaffe
  Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
	pavel, Ong Boon Leong, netdev, LKML, weifeng.voon, Lars Persson
In-Reply-To: <1482839100-20612-1-git-send-email-hock.leong.kweh@intel.com>



On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote:
> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> 
> If kernel module stmmac driver being loaded after OS booted, there is a
> race condition between stmmac_open() and stmmac_mdio_register(), which is
> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as
> PHY not found and stmmac_open() failed:
> [  473.919358] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
> 		stmmac_dvr_probe: warning: cannot get CSR clock
> [  473.919382] stmmaceth 0000:01:00.0: no reset control found
> [  473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42
> [  473.919429] stmmaceth 0000:01:00.0: DMA HW capability register supported
> [  473.919436] stmmaceth 0000:01:00.0: RX Checksum Offload Engine supported
> [  473.919443] stmmaceth 0000:01:00.0: TX Checksum insertion supported
> [  473.919451] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
> 		Enable RX Mitigation via HW Watchdog Timer
> [  473.921395] libphy: PHY stmmac-1:00 not found
> [  473.921417] stmmaceth 0000:01:00.0 eth0: Could not attach to PHY
> [  473.921427] stmmaceth 0000:01:00.0 eth0: stmmac_open: Cannot attach to
> 		PHY (error: -19)
> [  473.959710] libphy: stmmac: probed
> [  473.959724] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL
> 		(stmmac-1:00) active
> [  473.959728] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL
> 		(stmmac-1:01)
> [  473.959731] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL
> 		(stmmac-1:02)
> [  473.959734] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL
> 		(stmmac-1:03)
> 
> The resolution used wait_for_completion_interruptible() to synchronize
> stmmac_open() and stmmac_dvr_probe() to prevent the race condition
> happening.

The proper fix for this would be to have register_netdev() be the last
thing done in stmmac_drv_probe(), whereas right now, the last thing done
is stmmac_mdio_register(), leading the window you are seeing here, where
the network interface can be open prior to all resources being set up,
including, but not limited to MDIO devices.
-- 
Florian

^ permalink raw reply

* Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: Florian Fainelli @ 2016-12-27  5:13 UTC (permalink / raw)
  To: Kweh, Hock Leong, David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
	seraphin.bonnaffe
  Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
	pavel, Ong Boon Leong, netdev, LKML, weifeng.voon, Lars Persson
In-Reply-To: <461cd45a-70a5-1f08-816d-3c210d694083@gmail.com>



On 12/26/2016 09:10 PM, Florian Fainelli wrote:
> 
> 
> On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote:
>> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
>>
>> If kernel module stmmac driver being loaded after OS booted, there is a
>> race condition between stmmac_open() and stmmac_mdio_register(), which is
>> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as
>> PHY not found and stmmac_open() failed:
>> [  473.919358] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
>> 		stmmac_dvr_probe: warning: cannot get CSR clock
>> [  473.919382] stmmaceth 0000:01:00.0: no reset control found
>> [  473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42
>> [  473.919429] stmmaceth 0000:01:00.0: DMA HW capability register supported
>> [  473.919436] stmmaceth 0000:01:00.0: RX Checksum Offload Engine supported
>> [  473.919443] stmmaceth 0000:01:00.0: TX Checksum insertion supported
>> [  473.919451] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
>> 		Enable RX Mitigation via HW Watchdog Timer
>> [  473.921395] libphy: PHY stmmac-1:00 not found
>> [  473.921417] stmmaceth 0000:01:00.0 eth0: Could not attach to PHY
>> [  473.921427] stmmaceth 0000:01:00.0 eth0: stmmac_open: Cannot attach to
>> 		PHY (error: -19)
>> [  473.959710] libphy: stmmac: probed
>> [  473.959724] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL
>> 		(stmmac-1:00) active
>> [  473.959728] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL
>> 		(stmmac-1:01)
>> [  473.959731] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL
>> 		(stmmac-1:02)
>> [  473.959734] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL
>> 		(stmmac-1:03)
>>
>> The resolution used wait_for_completion_interruptible() to synchronize
>> stmmac_open() and stmmac_dvr_probe() to prevent the race condition
>> happening.
> 
> The proper fix for this would be to have register_netdev() be the last
> thing done in stmmac_drv_probe(), whereas right now, the last thing done
> is stmmac_mdio_register(), leading the window you are seeing here, where
> the network interface can be open prior to all resources being set up,
> including, but not limited to MDIO devices.

Something like the following untested patch should plug this race:

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bb40382e205d..5910ea51f8f6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3339,13 +3339,6 @@ int stmmac_dvr_probe(struct device *device,

        spin_lock_init(&priv->lock);

-       ret = register_netdev(ndev);
-       if (ret) {
-               netdev_err(priv->dev, "%s: ERROR %i registering the
device\n",
-                          __func__, ret);
-               goto error_netdev_register;
-       }
-
        /* If a specific clk_csr value is passed from the platform
         * this means that the CSR Clock Range selection cannot be
         * changed at run-time and it is fixed. Viceversa the driver'll
try to
@@ -3372,11 +3365,14 @@ int stmmac_dvr_probe(struct device *device,
                }
        }

-       return 0;
+       ret = register_netdev(ndev);
+       if (ret)
+               netdev_err(priv->dev, "%s: ERROR %i registering the
device\n",
+                          __func__, ret);
+
+       return ret;

 error_mdio_register:
-       unregister_netdev(ndev);
-error_netdev_register:
        netif_napi_del(&priv->napi);
 error_hw_init:
        clk_disable_unprepare(priv->pclk);

-- 
Florian

^ permalink raw reply related

* RE: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: Kweh, Hock Leong @ 2016-12-27  5:25 UTC (permalink / raw)
  To: Florian Fainelli, David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
	seraphin.bonnaffe@st.com
  Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
	pavel@ucw.cz, Ong, Boon Leong, netdev, LKML, Voon, Weifeng,
	Lars Persson
In-Reply-To: <6df6425b-bb0c-1e74-b5e1-a221447b761f@gmail.com>

> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Tuesday, December 27, 2016 1:14 PM
> To: Kweh, Hock Leong <hock.leong.kweh@intel.com>; David S. Miller
> <davem@davemloft.net>; Joao Pinto <Joao.Pinto@synopsys.com>; Giuseppe
> CAVALLARO <peppe.cavallaro@st.com>; seraphin.bonnaffe@st.com
> Cc: Alexandre TORGUE <alexandre.torgue@gmail.com>; Joachim Eastwood
> <manabian@gmail.com>; Niklas Cassel <niklas.cassel@axis.com>; Johan Hovold
> <johan@kernel.org>; pavel@ucw.cz; Ong, Boon Leong
> <boon.leong.ong@intel.com>; netdev <netdev@vger.kernel.org>; LKML <linux-
> kernel@vger.kernel.org>; Voon, Weifeng <weifeng.voon@intel.com>; Lars
> Persson <lars.persson@axis.com>
> Subject: Re: [PATCH] net: stmmac: synchronize stmmac_open and
> stmmac_dvr_probe
> 
> 
> 
> On 12/26/2016 09:10 PM, Florian Fainelli wrote:
> >
> >
> > On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote:
> >> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >>
> >> If kernel module stmmac driver being loaded after OS booted, there is a
> >> race condition between stmmac_open() and stmmac_mdio_register(), which
> is
> >> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as
> >> PHY not found and stmmac_open() failed:
> >> [  473.919358] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
> >> 		stmmac_dvr_probe: warning: cannot get CSR clock
> >> [  473.919382] stmmaceth 0000:01:00.0: no reset control found
> >> [  473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42
> >> [  473.919429] stmmaceth 0000:01:00.0: DMA HW capability register
> supported
> >> [  473.919436] stmmaceth 0000:01:00.0: RX Checksum Offload Engine
> supported
> >> [  473.919443] stmmaceth 0000:01:00.0: TX Checksum insertion supported
> >> [  473.919451] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
> >> 		Enable RX Mitigation via HW Watchdog Timer
> >> [  473.921395] libphy: PHY stmmac-1:00 not found
> >> [  473.921417] stmmaceth 0000:01:00.0 eth0: Could not attach to PHY
> >> [  473.921427] stmmaceth 0000:01:00.0 eth0: stmmac_open: Cannot attach
> to
> >> 		PHY (error: -19)
> >> [  473.959710] libphy: stmmac: probed
> >> [  473.959724] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL
> >> 		(stmmac-1:00) active
> >> [  473.959728] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL
> >> 		(stmmac-1:01)
> >> [  473.959731] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL
> >> 		(stmmac-1:02)
> >> [  473.959734] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL
> >> 		(stmmac-1:03)
> >>
> >> The resolution used wait_for_completion_interruptible() to synchronize
> >> stmmac_open() and stmmac_dvr_probe() to prevent the race condition
> >> happening.
> >
> > The proper fix for this would be to have register_netdev() be the last
> > thing done in stmmac_drv_probe(), whereas right now, the last thing done
> > is stmmac_mdio_register(), leading the window you are seeing here, where
> > the network interface can be open prior to all resources being set up,
> > including, but not limited to MDIO devices.
> 
> Something like the following untested patch should plug this race:
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index bb40382e205d..5910ea51f8f6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3339,13 +3339,6 @@ int stmmac_dvr_probe(struct device *device,
> 
>         spin_lock_init(&priv->lock);
> 
> -       ret = register_netdev(ndev);
> -       if (ret) {
> -               netdev_err(priv->dev, "%s: ERROR %i registering the
> device\n",
> -                          __func__, ret);
> -               goto error_netdev_register;
> -       }
> -
>         /* If a specific clk_csr value is passed from the platform
>          * this means that the CSR Clock Range selection cannot be
>          * changed at run-time and it is fixed. Viceversa the driver'll
> try to
> @@ -3372,11 +3365,14 @@ int stmmac_dvr_probe(struct device *device,
>                 }
>         }
> 
> -       return 0;
> +       ret = register_netdev(ndev);
> +       if (ret)
> +               netdev_err(priv->dev, "%s: ERROR %i registering the
> device\n",
> +                          __func__, ret);
> +
> +       return ret;
> 
>  error_mdio_register:
> -       unregister_netdev(ndev);
> -error_netdev_register:
>         netif_napi_del(&priv->napi);
>  error_hw_init:
>         clk_disable_unprepare(priv->pclk);
> 
> --
> Florian

Thanks. Will try out to confirm.

Regards,
Wilson

^ permalink raw reply

* RE: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: Kweh, Hock Leong @ 2016-12-27  5:26 UTC (permalink / raw)
  To: David Miller
  Cc: Joao.Pinto@synopsys.com, peppe.cavallaro@st.com,
	seraphin.bonnaffe@st.com, alexandre.torgue@gmail.com,
	manabian@gmail.com, niklas.cassel@axis.com, johan@kernel.org,
	pavel@ucw.cz, Ong, Boon Leong, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Voon, Weifeng,
	lars.persson@axis.com
In-Reply-To: <20161226.235436.131087168899382517.davem@davemloft.net>

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, December 27, 2016 12:55 PM
> To: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> Cc: Joao.Pinto@synopsys.com; peppe.cavallaro@st.com;
> seraphin.bonnaffe@st.com; alexandre.torgue@gmail.com;
> manabian@gmail.com; niklas.cassel@axis.com; johan@kernel.org;
> pavel@ucw.cz; Ong, Boon Leong <boon.leong.ong@intel.com>;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Voon, Weifeng
> <weifeng.voon@intel.com>; lars.persson@axis.com
> Subject: Re: [PATCH] net: stmmac: synchronize stmmac_open and
> stmmac_dvr_probe
> 
> From: "Kweh, Hock Leong"	<hock.leong.kweh@intel.com>
> Date: Tue, 27 Dec 2016 19:44:59 +0800
> 
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >
> > If kernel module stmmac driver being loaded after OS booted, there is a
> > race condition between stmmac_open() and stmmac_mdio_register(), which is
> > invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as
> > PHY not found and stmmac_open() failed:
>  ...
> > The resolution used wait_for_completion_interruptible() to synchronize
> > stmmac_open() and stmmac_dvr_probe() to prevent the race condition
> > happening.
> >
> > Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> 
> The proper thing to do is to make sure register_netdevice() is not
> invoked until it is %100 safe to call stmmac_open().

Noted & thanks. Will look into it.

Regards,
Wilson

^ permalink raw reply

* Re: driver r8169 suddenly failed
From: Robert Grasso @ 2016-12-27  7:33 UTC (permalink / raw)
  To: Francois Romieu; +Cc: Realtek linux nic maintainers, netdev
In-Reply-To: <20161227001248.GA20172@electric-eye.fr.zoreil.com>

Thank you for your quick answer ! Allow me some days for this test :

1 - I am at work these days
2 - it has been a while since I have dealt with custom kernels, I am a 
bit rusty, it will be good to review this topic

-- 
Robert Grasso
@home
---
UNIX was not designed to stop you from doing stupid things, because
   that would also stop you from doing clever things. -- Doug Gwyn

On 27/12/2016 01:12, Francois Romieu wrote:
> Robert Grasso <robert.grasso@modulonet.fr> :
> [dhcp snafu]
>> First of all, can you confirm that I am doing right in posting to you
>> (addresses found in README.Debian) ?
> It isn't completely wrong.
>
>> If I do, can you help ? I am not very proficient with Ethernet, and I am not
>> able to figure out what my provider changed : their hotline is
>> underqualified, they are just able to tell that "the signal on the line is
>> ok".
> You're spoiled. It is more than decent for a company whose core business
> used to sell cable TV.
>
>> But if you want me to run various tests, try new versions, I would be
>> glad to do so : I would appreciate if I could salvage this Shuttle.
> Please try a recent stable vanilla kernel and send a complete dmesg
> from boot. I need to identify the specific 816x chipset.
>
> Then record some traffic:
>
> # touch gonzo.pcap && tshark -w gonzo.pcap -i eth1
>
> It should only exhibit small outgoing packets but, well, one never knows.
>
> Check the leds activity to be sure that the network interfaces have not
> been renumbered.
>
> Use ethtool to check that tso is disabled. If it isn't, disable it.
>

^ permalink raw reply

* [PATCH] net: fix incorrect original ingress device index in PKTINFO
From: Wei Zhang @ 2016-12-27  7:52 UTC (permalink / raw)
  To: davem, kuznet, jmorris, yoshfuji, kaber; +Cc: netdev, linux-kernel

When we send a packet for our own local address on a non-loopback interface
(e.g. eth0), due to the change had been introduced from commit 0b922b7a829c
("net: original ingress device index in PKTINFO"), the original ingress
device index would be set as the loopback interface. However, the packet
should be considered as if it is being arrived via the sending interface
(eth0), otherwise it would break the expectation of the userspace
application (e.g. the DHCPRELEASE message from dhcp_release binary would
be ignored by the dnsmasq daemon)

Signed-off-by: Wei Zhang <asuka.com@163.com>
---
 net/ipv4/ip_sockglue.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index b8a2d63..b6a6d35 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1202,8 +1202,13 @@ void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb)
 		 * which has interface index (iif) as the first member of the
 		 * underlying inet{6}_skb_parm struct. This code then overlays
 		 * PKTINFO_SKB_CB and in_pktinfo also has iif as the first
-		 * element so the iif is picked up from the prior IPCB
+		 * element so the iif is picked up from the prior IPCB except
+		 * iif is loopback interface which the packet should be 
+		 * considered as if it is being arrived via the sending interface
 		 */
+		if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX) {
+			pktinfo->ipi_ifindex = inet_iif(skb);
+		}
 		pktinfo->ipi_spec_dst.s_addr = fib_compute_spec_dst(skb);
 	} else {
 		pktinfo->ipi_ifindex = 0;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH] net: dev_weight: TX/RX orthogonality
From: Matthias Tafelmeier @ 2016-12-27  8:25 UTC (permalink / raw)
  To: netdev; +Cc: hagen, fw, edumazet, daniel
In-Reply-To: <20161226.115818.1691666598003246082.davem@davemloft.net>

Oftenly, introducing side effects on packet processing on the other half
of the stack by adjusting one of TX/RX via sysctl is not desirable.
There are cases of demand for asymmetric, orthogonal configurability.

This holds true especially for nodes where RPS for RFS usage on top is
configured and therefore use the 'old dev_weight'. This is quite a
common base configuration setup nowadays, even with NICs of superior processing
support (e.g. aRFS).

A good example use case are nodes acting as noSQL data bases with a
large number of tiny requests and rather fewer but large packets as responses.
It's affordable to have large budget and rx dev_weights for the
requests. But as a side effect having this large a number on TX
processed in one run can overwhelm drivers.

This patch therefore introduces an independent configurability via sysctl to
userland.
---
 include/linux/netdevice.h  |  2 ++
 net/core/dev.c             |  4 +++-
 net/core/sysctl_net_core.c | 14 ++++++++++++++
 net/sched/sch_generic.c    |  2 +-
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 994f742..bb331e0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3795,6 +3795,8 @@ void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
 extern int		netdev_max_backlog;
 extern int		netdev_tstamp_prequeue;
 extern int		weight_p;
+extern int		dev_w_rx_bias;
+extern int		dev_w_tx_bias;
 
 bool netdev_has_upper_dev(struct net_device *dev, struct net_device *upper_dev);
 struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
diff --git a/net/core/dev.c b/net/core/dev.c
index 8db5a0b..0dcbd28 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3428,6 +3428,8 @@ EXPORT_SYMBOL(netdev_max_backlog);
 int netdev_tstamp_prequeue __read_mostly = 1;
 int netdev_budget __read_mostly = 300;
 int weight_p __read_mostly = 64;            /* old backlog weight */
+int dev_w_rx_bias __read_mostly = 1;            /* bias for backlog weight */
+int dev_w_tx_bias __read_mostly = 1;            /* bias for output_queue quota */
 
 /* Called with irq disabled */
 static inline void ____napi_schedule(struct softnet_data *sd,
@@ -4833,7 +4835,7 @@ static int process_backlog(struct napi_struct *napi, int quota)
 		net_rps_action_and_irq_enable(sd);
 	}
 
-	napi->weight = weight_p;
+	napi->weight = weight_p * dev_w_rx_bias;
 	while (again) {
 		struct sk_buff *skb;
 
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 2a46e40..a2ab149 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -276,6 +276,20 @@ static struct ctl_table net_core_table[] = {
 		.proc_handler	= proc_dointvec
 	},
 	{
+		.procname	= "dev_w_rx_bias",
+		.data		= &dev_w_rx_bias,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
+	{
+		.procname	= "dev_w_tx_bias",
+		.data		= &dev_w_tx_bias,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
+	{
 		.procname	= "netdev_max_backlog",
 		.data		= &netdev_max_backlog,
 		.maxlen		= sizeof(int),
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 6eb9c8e..4c07780 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -247,7 +247,7 @@ static inline int qdisc_restart(struct Qdisc *q, int *packets)
 
 void __qdisc_run(struct Qdisc *q)
 {
-	int quota = weight_p;
+	int quota = weight_p * dev_w_tx_bias;
 	int packets;
 
 	while (qdisc_restart(q, &packets)) {
-- 
2.7.4

^ permalink raw reply related

* Re:[PATCH] net: fix incorrect original ingress device index in PKTINFO
From: wei zhang @ 2016-12-27  8:29 UTC (permalink / raw)
  To: kuznet, davem, jmorris, yoshfuji, kaber; +Cc: netdev, linux-kernel
In-Reply-To: <1482825138-2125-1-git-send-email-asuka.com@163.com>

At 2016-12-27 15:52:18, "Wei Zhang" <asuka.com@163.com> wrote:
>When we send a packet for our own local address on a non-loopback interface
>(e.g. eth0), due to the change had been introduced from commit 0b922b7a829c
>("net: original ingress device index in PKTINFO"), the original ingress
>device index would be set as the loopback interface. However, the packet
>should be considered as if it is being arrived via the sending interface
>(eth0), otherwise it would break the expectation of the userspace
>application (e.g. the DHCPRELEASE message from dhcp_release binary would
>be ignored by the dnsmasq daemon)
>
>Signed-off-by: Wei Zhang <asuka.com@163.com>
>---
> net/ipv4/ip_sockglue.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
>index b8a2d63..b6a6d35 100644
>--- a/net/ipv4/ip_sockglue.c
>+++ b/net/ipv4/ip_sockglue.c
>@@ -1202,8 +1202,13 @@ void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb)
> 		 * which has interface index (iif) as the first member of the
> 		 * underlying inet{6}_skb_parm struct. This code then overlays
> 		 * PKTINFO_SKB_CB and in_pktinfo also has iif as the first
>-		 * element so the iif is picked up from the prior IPCB
>+		 * element so the iif is picked up from the prior IPCB except
>+		 * iif is loopback interface which the packet should be 
>+		 * considered as if it is being arrived via the sending interface
> 		 */
>+		if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX) {
>+			pktinfo->ipi_ifindex = inet_iif(skb);
>+		}
> 		pktinfo->ipi_spec_dst.s_addr = fib_compute_spec_dst(skb);
> 	} else {
> 		pktinfo->ipi_ifindex = 0;
>-- 
>1.8.3.1
>

  When I upgrade to the 4.9, the dhcp_release could not release the dhcp
lease, the dnsmasq ignored all the DHCPRELEASE message which it think come
from lo. I think this is due to the commit 0b922b7a829c, which set the
IPCB(skb)->iif = skb->skb_iif in the ip_rcv()!

  And I'm very sorry about forgetting checkpatch, I will resend the patch,
hope I'm not bothering you!

Thanks,
Wei Zhang

^ permalink raw reply

* [PATCH net-next] r8169: add support for RTL8168 series add-on card.
From: Chun-Hao Lin @ 2016-12-27  8:29 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, Chun-Hao Lin

This chip is the same as RTL8168, but its device id is 0x8161.

Signed-off-by: Chun-Hao Lin <hau@realtek.com>
---
 drivers/net/ethernet/realtek/r8169.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index f9b97f5..44389c9 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -326,6 +326,7 @@ enum cfg_version {
 static const struct pci_device_id rtl8169_pci_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK,	0x8129), 0, 0, RTL_CFG_0 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK,	0x8136), 0, 0, RTL_CFG_2 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK,	0x8161), 0, 0, RTL_CFG_1 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK,	0x8167), 0, 0, RTL_CFG_0 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK,	0x8168), 0, 0, RTL_CFG_1 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK,	0x8169), 0, 0, RTL_CFG_0 },
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net] net: xdp: remove unused bfp_warn_invalid_xdp_buffer()
From: Daniel Borkmann @ 2016-12-27  9:29 UTC (permalink / raw)
  To: Jason Wang, davem, linux-kernel, netdev; +Cc: John Fastabend
In-Reply-To: <1482806994-14649-1-git-send-email-jasowang@redhat.com>

On 12/27/2016 03:49 AM, Jason Wang wrote:
> After commit 73b62bd085f4737679ea9afc7867fa5f99ba7d1b ("virtio-net:
> remove the warning before XDP linearizing"), there's no users for
> bpf_warn_invalid_xdp_buffer(), so remove it. This is a revert for
> commit f23bc46c30ca5ef58b8549434899fcbac41b2cfc.
>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Thanks for following up.

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

^ permalink raw reply

* [PATCH] net: fix incorrect original ingress device index in PKTINFO
From: Wei Zhang @ 2016-12-27  9:52 UTC (permalink / raw)
  To: davem, kuznet, jmorris, yoshfuji, kaber; +Cc: netdev, linux-kernel

When we send a packet for our own local address on a non-loopback
interface (e.g. eth0), due to the change had been introduced from
commit 0b922b7a829c ("net: original ingress device index in PKTINFO"), the
original ingress device index would be set as the loopback interface.
However, the packet should be considered as if it is being arrived via the
sending interface (eth0), otherwise it would break the expectation of the
userspace application (e.g. the DHCPRELEASE message from dhcp_release
binary would be ignored by the dnsmasq daemon, since it come from lo which
is not the interface dnsmasq bind to)

Signed-off-by: Wei Zhang <asuka.com@163.com>
---
 net/ipv4/ip_sockglue.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index b8a2d63..76d78a7 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1202,8 +1202,14 @@ void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb)
 		 * which has interface index (iif) as the first member of the
 		 * underlying inet{6}_skb_parm struct. This code then overlays
 		 * PKTINFO_SKB_CB and in_pktinfo also has iif as the first
-		 * element so the iif is picked up from the prior IPCB
+		 * element so the iif is picked up from the prior IPCB except
+		 * iif is loopback interface which the packet should be
+		 * considered as if it is being arrived via the sending
+		 * interface
 		 */
+		if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX)
+			pktinfo->ipi_ifindex = inet_iif(skb);
+
 		pktinfo->ipi_spec_dst.s_addr = fib_compute_spec_dst(skb);
 	} else {
 		pktinfo->ipi_ifindex = 0;
-- 
1.8.3.1

^ permalink raw reply related

* Greetings From Mr. Hamidou Kader.
From: Mr. Hamidou Kader @ 2016-12-27  9:50 UTC (permalink / raw)

In-Reply-To: <1424159083.2117965.1482832245161.ref@mail.yahoo.com>



Greetings From Mr. Hamidou Kader.

I have a Mutual/Beneficial Business Project that would be beneficial to you. I only have two questions to ask of you,if you are interested.

1. Can you handle this project?
2. Can I give you this trust ?

Please note that the deal requires high level of maturity, honesty and secrecy. This will involve moving some money from my office, on trust to your hands or bank account. Also note that i will do everything to make sure that the money is moved as a purely legitimate fund, so you will not be exposed to any risk.

I request for your full co-operation. I will give you details and procedure when I receive your reply, To commence this transaction, I require you to immediately indicate your interest by a return reply.

I will be waiting for your response in a timely manner.
please contact me through my private email address: hamidoukader@barid.com
Best Regard,
Mr. Hamidou Kader

^ permalink raw reply

* Re: [RFC PATCH 4.10 1/6] crypto/sha256: Refactor the API so it can be used without shash
From: Herbert Xu @ 2016-12-27  9:58 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Ard Biesheuvel, Andy Lutomirski, Daniel Borkmann, Netdev, LKML,
	Linux Crypto Mailing List, Jason A. Donenfeld,
	Hannes Frederic Sowa, Alexei Starovoitov, Eric Dumazet,
	Eric Biggers, Tom Herbert, David S. Miller
In-Reply-To: <CALCETrXFTQ2AXgbQzPnRcDRHK81=FS1R0zqfoTqGjUsqZy2PvQ@mail.gmail.com>

On Mon, Dec 26, 2016 at 10:08:48AM -0800, Andy Lutomirski wrote:
>
> According to Daniel, the networking folks want to let embedded systems
> include BPF without requiring the crypto core.

Last I checked the IPv4 stack depended on the crypto API so this
sounds bogus.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] net: fix incorrect original ingress device index in PKTINFO
From: Sergei Shtylyov @ 2016-12-27 10:01 UTC (permalink / raw)
  To: Wei Zhang, davem, kuznet, jmorris, yoshfuji, kaber; +Cc: netdev, linux-kernel
In-Reply-To: <1482825138-2125-1-git-send-email-asuka.com@163.com>

Hello!

On 12/27/2016 10:52 AM, Wei Zhang wrote:

> When we send a packet for our own local address on a non-loopback interface
> (e.g. eth0), due to the change had been introduced from commit 0b922b7a829c
> ("net: original ingress device index in PKTINFO"), the original ingress
> device index would be set as the loopback interface. However, the packet
> should be considered as if it is being arrived via the sending interface
> (eth0), otherwise it would break the expectation of the userspace
> application (e.g. the DHCPRELEASE message from dhcp_release binary would
> be ignored by the dnsmasq daemon)
>
> Signed-off-by: Wei Zhang <asuka.com@163.com>
> ---
>  net/ipv4/ip_sockglue.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index b8a2d63..b6a6d35 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -1202,8 +1202,13 @@ void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb)
>  		 * which has interface index (iif) as the first member of the
>  		 * underlying inet{6}_skb_parm struct. This code then overlays
>  		 * PKTINFO_SKB_CB and in_pktinfo also has iif as the first
> -		 * element so the iif is picked up from the prior IPCB
> +		 * element so the iif is picked up from the prior IPCB except
> +		 * iif is loopback interface which the packet should be

    Tail space.

> +		 * considered as if it is being arrived via the sending interface
>  		 */
> +		if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX) {
> +			pktinfo->ipi_ifindex = inet_iif(skb);
> +		}

    {} not needed here.

>  		pktinfo->ipi_spec_dst.s_addr = fib_compute_spec_dst(skb);
>  	} else {
>  		pktinfo->ipi_ifindex = 0;

MBR, Sergei

^ permalink raw reply

* Re: [PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe
From: Pavel Machek @ 2016-12-27 10:48 UTC (permalink / raw)
  To: Kweh, Hock Leong
  Cc: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
	seraphin.bonnaffe, f.fainelli, Alexandre TORGUE, Joachim Eastwood,
	Niklas Cassel, Johan Hovold, Ong Boon Leong, weifeng.voon,
	lars.persson, Netdev list, LKML
In-Reply-To: <1482849756-21817-1-git-send-email-hock.leong.kweh@intel.com>

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

On Tue 2016-12-27 22:42:36, Kweh, Hock Leong wrote:
> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> 
> If kernel module stmmac driver being loaded after OS booted, there is a
> race condition between stmmac_open() and stmmac_mdio_register(), which is
> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as
> PHY not found and stmmac_open() failed:
> [  473.919358] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
>                 stmmac_dvr_probe: warning: cannot get CSR clock
> [  473.919382] stmmaceth 0000:01:00.0: no reset control found
> [  473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42
> [  473.919429] stmmaceth 0000:01:00.0: DMA HW capability register supported
> [  473.919436] stmmaceth 0000:01:00.0: RX Checksum Offload Engine supported
> [  473.919443] stmmaceth 0000:01:00.0: TX Checksum insertion supported
> [  473.919451] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
>                 Enable RX Mitigation via HW Watchdog Timer
> [  473.921395] libphy: PHY stmmac-1:00 not found
> [  473.921417] stmmaceth 0000:01:00.0 eth0: Could not attach to PHY
> [  473.921427] stmmaceth 0000:01:00.0 eth0: stmmac_open: Cannot attach to
>                 PHY (error: -19)
> [  473.959710] libphy: stmmac: probed
> [  473.959724] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL
>                 (stmmac-1:00) active
> [  473.959728] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL
>                 (stmmac-1:01)
> [  473.959731] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL
>                 (stmmac-1:02)
> [  473.959734] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL
>                 (stmmac-1:03)
> 
> The resolution moved the register_netdev() function call to the end of
> stmmac_dvr_probe() after stmmac_mdio_register().
> 
> Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: Kweh, Hock Leong @ 2016-12-27 11:44 UTC (permalink / raw)
  To: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
	seraphin.bonnaffe
  Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
	pavel, Kweh, Hock Leong, Ong Boon Leong, netdev, LKML,
	weifeng.voon, Lars Persson

From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

If kernel module stmmac driver being loaded after OS booted, there is a
race condition between stmmac_open() and stmmac_mdio_register(), which is
invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as
PHY not found and stmmac_open() failed:
[  473.919358] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
		stmmac_dvr_probe: warning: cannot get CSR clock
[  473.919382] stmmaceth 0000:01:00.0: no reset control found
[  473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42
[  473.919429] stmmaceth 0000:01:00.0: DMA HW capability register supported
[  473.919436] stmmaceth 0000:01:00.0: RX Checksum Offload Engine supported
[  473.919443] stmmaceth 0000:01:00.0: TX Checksum insertion supported
[  473.919451] stmmaceth 0000:01:00.0 (unnamed net_device) (uninitialized):
		Enable RX Mitigation via HW Watchdog Timer
[  473.921395] libphy: PHY stmmac-1:00 not found
[  473.921417] stmmaceth 0000:01:00.0 eth0: Could not attach to PHY
[  473.921427] stmmaceth 0000:01:00.0 eth0: stmmac_open: Cannot attach to
		PHY (error: -19)
[  473.959710] libphy: stmmac: probed
[  473.959724] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL
		(stmmac-1:00) active
[  473.959728] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL
		(stmmac-1:01)
[  473.959731] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL
		(stmmac-1:02)
[  473.959734] stmmaceth 0000:01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL
		(stmmac-1:03)

The resolution used wait_for_completion_interruptible() to synchronize
stmmac_open() and stmmac_dvr_probe() to prevent the race condition
happening.

Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |    1 +
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index eab04ae..5daf8a5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -131,6 +131,7 @@ struct stmmac_priv {
 	u32 rx_tail_addr;
 	u32 tx_tail_addr;
 	u32 mss;
+	struct completion probe_done;
 
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dbgfs_dir;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bb40382..28e85f6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1770,6 +1770,14 @@ static int stmmac_open(struct net_device *dev)
 	struct stmmac_priv *priv = netdev_priv(dev);
 	int ret;
 
+	ret = wait_for_completion_interruptible(&priv->probe_done);
+	if (ret) {
+		netdev_err(priv->dev,
+			   "%s: Interrupted while waiting probe completion\n",
+			   __func__);
+		return ret;
+	}
+
 	stmmac_check_ether_addr(priv);
 
 	if (priv->hw->pcs != STMMAC_PCS_RGMII &&
@@ -3226,6 +3234,7 @@ int stmmac_dvr_probe(struct device *device,
 	priv = netdev_priv(ndev);
 	priv->device = device;
 	priv->dev = ndev;
+	init_completion(&priv->probe_done);
 
 	stmmac_set_ethtool_ops(ndev);
 	priv->pause = pause;
@@ -3372,6 +3381,7 @@ int stmmac_dvr_probe(struct device *device,
 		}
 	}
 
+	complete_all(&priv->probe_done);
 	return 0;
 
 error_mdio_register:
-- 
1.7.9.5

^ permalink raw reply related

* Platform init code in stmmac_main
From: Joao Pinto @ 2016-12-27 11:47 UTC (permalink / raw)
  To: pavel@ucw.cz, Niklas Cassel, Giuseppe CAVALLARO, Florian Fainelli,
	netdev

Hello,

I spotted in stmmac_main.c, *_dvr_probe() function, some clocks initializations
that are related with platform based setups:


	priv->pclk = devm_clk_get(priv->device, "pclk");
	if (IS_ERR(priv->pclk)) {
		if (PTR_ERR(priv->pclk) == -EPROBE_DEFER) {
			ret = -EPROBE_DEFER;
			goto error_pclk_get;
		}
		priv->pclk = NULL;
	}
	clk_prepare_enable(priv->pclk);

	priv->stmmac_rst = devm_reset_control_get(priv->device,
						  STMMAC_RESOURCE_NAME);
	if (IS_ERR(priv->stmmac_rst)) {
		if (PTR_ERR(priv->stmmac_rst) == -EPROBE_DEFER) {
			ret = -EPROBE_DEFER;
			goto error_hw_init;
		}
		dev_info(priv->device, "no reset control found\n");
		priv->stmmac_rst = NULL;
	}
	if (priv->stmmac_rst)
		reset_control_deassert(priv->stmmac_rst);

I am using PCI based setup, so these initializations are executed since they are
in common space in stmmac_main.c. Wouldn't be better to map these clocks in
stmmac_platform? This hasn't any problem for me, but I think it would improve
code readbility.

I would like to have your opinion about the subject!

Thanks,
Joao

^ permalink raw reply

* [PATCH] net: stmmac: fix incorrect bit set in gmac4 mdio addr register
From: Kweh, Hock Leong @ 2016-12-27 20:07 UTC (permalink / raw)
  To: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
	seraphin.bonnaffe, f.fainelli
  Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
	pavel, Ong Boon Leong, Kweh, Hock Leong, weifeng.voon,
	lars.persson, netdev, LKML

From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

Fixing the gmac4 mdio write access to use MII_GMAC4_WRITE only instead of
OR together with MII_WRITE.

Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index fda01f7..b0344c2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -116,7 +116,7 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 	unsigned int mii_address = priv->hw->mii.addr;
 	unsigned int mii_data = priv->hw->mii.data;
 
-	u32 value = MII_WRITE | MII_BUSY;
+	u32 value = MII_BUSY;
 
 	value |= (phyaddr << priv->hw->mii.addr_shift)
 		& priv->hw->mii.addr_mask;
@@ -126,6 +126,8 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 		& priv->hw->mii.clk_csr_mask;
 	if (priv->plat->has_gmac4)
 		value |= MII_GMAC4_WRITE;
+	else
+		value |= MII_WRITE;
 
 	/* Wait until any existing MII operation is complete */
 	if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] net: stmmac: fix incorrect bit set in gmac4 mdio addr register
From: Joao Pinto @ 2016-12-27 12:11 UTC (permalink / raw)
  To: Kweh, Hock Leong, David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
	seraphin.bonnaffe, f.fainelli
  Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
	pavel, Ong Boon Leong, weifeng.voon, lars.persson, netdev, LKML
In-Reply-To: <1482869261-23803-1-git-send-email-hock.leong.kweh@intel.com>

Às 8:07 PM de 12/27/2016, Kweh, Hock Leong escreveu:
> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> 
> Fixing the gmac4 mdio write access to use MII_GMAC4_WRITE only instead of
> OR together with MII_WRITE.
> 
> Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> index fda01f7..b0344c2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> @@ -116,7 +116,7 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
>  	unsigned int mii_address = priv->hw->mii.addr;
>  	unsigned int mii_data = priv->hw->mii.data;
>  
> -	u32 value = MII_WRITE | MII_BUSY;
> +	u32 value = MII_BUSY;
>  
>  	value |= (phyaddr << priv->hw->mii.addr_shift)
>  		& priv->hw->mii.addr_mask;
> @@ -126,6 +126,8 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
>  		& priv->hw->mii.clk_csr_mask;
>  	if (priv->plat->has_gmac4)
>  		value |= MII_GMAC4_WRITE;
> +	else
> +		value |= MII_WRITE;
>  
>  	/* Wait until any existing MII operation is complete */
>  	if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
> 

Acked-By: Joao Pinto <jpinto@synopsys.com>

^ permalink raw reply

* [PATCH 01/12] Make and configuration files.
From: David VomLehn @ 2016-12-27 13:17 UTC (permalink / raw)
  To: netdev
  Cc: Simon Edelhaus, David VomLehn, Dmitrii Tarakanov,
	Alexander Loktionov

Patches to create the make and configuration files.

Signed-off-by: Dmitrii Tarakanov <Dmitrii.Tarakanov@aquantia.com>
Signed-off-by: Alexander Loktionov <Alexander.Loktionov@aquantia.com>
Signed-off-by: David M. VomLehn <vomlehn@texas.net>
---
 drivers/net/ethernet/aquantia/atlantic/Kconfig  |  9 ++++++
 drivers/net/ethernet/aquantia/atlantic/Makefile | 40 +++++++++++++++++++++++++
 drivers/net/ethernet/aquantia/atlantic/ver.h    | 18 +++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 drivers/net/ethernet/aquantia/atlantic/Kconfig
 create mode 100644 drivers/net/ethernet/aquantia/atlantic/Makefile
 create mode 100644 drivers/net/ethernet/aquantia/atlantic/ver.h

diff --git a/drivers/net/ethernet/aquantia/atlantic/Kconfig b/drivers/net/ethernet/aquantia/atlantic/Kconfig
new file mode 100644
index 0000000..33f1eb6
--- /dev/null
+++ b/drivers/net/ethernet/aquantia/atlantic/Kconfig
@@ -0,0 +1,9 @@
+#
+# Aquantia device configuration
+#
+
+config AQTION
+	tristate "Aquantia AQtion Support"
+	depends on PCI
+	---help---
+	  This enables the support for the Aquantia AQtion Ethernet card.
\ No newline at end of file
diff --git a/drivers/net/ethernet/aquantia/atlantic/Makefile b/drivers/net/ethernet/aquantia/atlantic/Makefile
new file mode 100644
index 0000000..f0d961f
--- /dev/null
+++ b/drivers/net/ethernet/aquantia/atlantic/Makefile
@@ -0,0 +1,40 @@
+TARGET:=atlantic
+
+CC = gcc
+
+ifeq "$(CC)" "gcc"
+	ccflags-y := -Wall
+endif
+
+ifneq ($(KERNELRELEASE),)
+	$(TARGET)-objs:=aq_main.o aq_nic.o aq_pci_func.o aq_nic.o aq_vec.o \
+	aq_ring.o aq_hw_utils.o aq_ethtool.o hw_atl/hw_atl_a0.o \
+	hw_atl/hw_atl_utils.o hw_atl/hw_atl_llh.o
+
+	obj-m:=$(TARGET).o
+else
+	ifndef KDIR
+		BUILD_DIR:=/lib/modules/$(shell uname -r)/build
+	else
+		BUILD_DIR:=$(KDIR)
+	endif
+
+	PWD:=$(shell pwd)
+
+all:
+	$(MAKE) -j4 CC=$(CC) -C $(BUILD_DIR) M=$(PWD) modules
+
+dox:	.doxygen
+	@doxygen $<
+
+clean:
+	$(MAKE) -j4 -C $(BUILD_DIR) M=$(PWD) clean
+	@-rm -rdf doc/html 2 > /dev/null
+
+load:
+	insmod ./$(TARGET).ko
+
+unload:
+	rmmod ./$(TARGET).ko
+
+endif
diff --git a/drivers/net/ethernet/aquantia/atlantic/ver.h b/drivers/net/ethernet/aquantia/atlantic/ver.h
new file mode 100644
index 0000000..225f561
--- /dev/null
+++ b/drivers/net/ethernet/aquantia/atlantic/ver.h
@@ -0,0 +1,18 @@
+/*
+ * Aquantia Corporation Network Driver
+ * Copyright (C) 2014-2016 Aquantia Corporation. All rights reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#ifndef VER_H
+#define VER_H
+
+#define NIC_MAJOR_DRIVER_VERSION           1
+#define NIC_MINOR_DRIVER_VERSION           4
+#define NIC_BUILD_DRIVER_VERSION           1671
+#define NIC_REVISION_DRIVER_VERSION        0
+
+#endif				/* VER_H */
-- 
2.7.4

^ permalink raw reply related


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