* Re: [PATCH 1/2] ethernet: stmmac: make DWMAC_STM32 depend on it's associated SoC
From: Peter Robinson @ 2016-11-16 14:01 UTC (permalink / raw)
To: David Miller; +Cc: mcoquelin.stm32, alexandre.torgue, peppe.cavallaro, netdev
In-Reply-To: <20161106.214128.668519501329683090.davem@davemloft.net>
On Mon, Nov 7, 2016 at 2:41 AM, David Miller <davem@davemloft.net> wrote:
> From: Peter Robinson <pbrobinson@gmail.com>
> Date: Sun, 6 Nov 2016 20:04:37 +0000
>
>> There's not much point, except compile test, enabling the stmmac
>> platform drivers unless the STM32 SoC is enabled. It's not
>> useful without it.
>>
>> Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
>
> Please don't post just some of the patches in a patch series.
>
> Always post the complete series, with a proper "[PATCH 0/2] ..."
> posting at the beginning.
Sorry, it was a stm branch, I should have broke it our differently. I
can recompose and send this one again on it's own.
Peter
^ permalink raw reply
* [PATCH net-next] sfc: remove napi_hash_del() call
From: Eric Dumazet @ 2016-11-16 14:01 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Edward Cree, Bert Kenward
From: Eric Dumazet <edumazet@google.com>
Calling napi_hash_del() after netif_napi_del() is pointless.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Edward Cree <ecree@solarflare.com>
Cc: Bert Kenward <bkenward@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 649bc508fa4b9ea42aa08331c50335e1a51f9259..52ca0404c49121f1c86126ab7d443b1144f20720 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2113,10 +2113,9 @@ static void efx_init_napi(struct efx_nic *efx)
static void efx_fini_napi_channel(struct efx_channel *channel)
{
- if (channel->napi_dev) {
+ if (channel->napi_dev)
netif_napi_del(&channel->napi_str);
- napi_hash_del(&channel->napi_str);
- }
+
channel->napi_dev = NULL;
}
^ permalink raw reply related
* Re: [PATCH net] virtio-net: add a missing synchronize_net()
From: Michael S. Tsirkin @ 2016-11-16 13:55 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Jason Wang
In-Reply-To: <1479277452.8455.156.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, Nov 15, 2016 at 10:24:12PM -0800, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> It seems many drivers do not respect napi_hash_del() contract.
>
> When napi_hash_del() is used before netif_napi_del(), an RCU grace
> period is needed before freeing NAPI object.
>
> Fixes: 91815639d880 ("virtio-net: rx busy polling support")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.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 fd8b1e62301f..7276d5a95bd0 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1497,6 +1497,11 @@ static void virtnet_free_queues(struct virtnet_info *vi)
> netif_napi_del(&vi->rq[i].napi);
> }
>
> + /* We called napi_hash_del() before netif_napi_del(),
> + * we need to respect an RCU grace period before freeing vi->rq
> + */
> + synchronize_net();
> +
> kfree(vi->rq);
> kfree(vi->sq);
> }
>
^ permalink raw reply
* [PATCH net-next] net/mlx5e: remove napi_hash_del() calls
From: Eric Dumazet @ 2016-11-16 13:55 UTC (permalink / raw)
To: David Miller; +Cc: Saeed Mahameed, netdev
From: Eric Dumazet <edumazet@google.com>
Calling napi_hash_del() after netif_napi_del() is pointless.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 ----
1 file changed, 4 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] bpf, mlx5: fix various refcount issues in mlx5e_xdp_set
From: Daniel Borkmann @ 2016-11-16 13:54 UTC (permalink / raw)
To: Saeed Mahameed
Cc: David S. Miller, Alexei Starovoitov, Brenden Blanco, zhiyisun,
Rana Shahout, Saeed Mahameed, Linux Netdev List
In-Reply-To: <CALzJLG_r9tujvSVysMXLSO8cRR6zHaHfmacmGq-hCqg5Ou7w_w@mail.gmail.com>
On 11/16/2016 01:25 PM, Saeed Mahameed wrote:
> On Wed, Nov 16, 2016 at 2:04 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> There are multiple issues in mlx5e_xdp_set():
>>
>> 1) The batched bpf_prog_add() is currently not checked for errors! When
>> doing so, it should be done at an earlier point in time to makes sure
>> that we cannot fail anymore at the time we want to set the program for
>> each channel. This only means that we have to undo the bpf_prog_add()
>> in case we return due to required reset.
>>
>> 2) When swapping the priv->xdp_prog, then no extra reference count must be
>> taken since we got that from call path via dev_change_xdp_fd() already.
>> Otherwise, we'd never be able to free the program. Also, bpf_prog_add()
>> without checking the return code could fail.
>>
>> Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 25 ++++++++++++++++++-----
>> 1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> index ab0c336..cf26672 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> @@ -3142,6 +3142,17 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>> goto unlock;
>> }
>>
>> + if (prog) {
>> + /* num_channels is invariant here, so we can take the
>> + * batched reference right upfront.
>> + */
>> + prog = bpf_prog_add(prog, priv->params.num_channels);
>> + if (IS_ERR(prog)) {
>> + err = PTR_ERR(prog);
>> + goto unlock;
>> + }
>> + }
>> +
>
> With this approach you will end up taking a ref count twice per RQ! on
> the first time xdp_set is called i.e (old_prog = NULL, prog != NULL).
> One ref will be taken per RQ/Channel from the above code, and since
> reset will be TRUE mlx5e_open_locked will be called and another ref
> count will be taken on mlx5e_create_rq.
>
> The issue here is that we have two places for ref count accounting,
> xdp_set and mlx5e_create_rq. Having ref-count updates in
> mlx5e_create_rq is essential for num_channels configuration changes
> (mlx5e_set_ringparam).
>
> Our previous approach made sure that only one path will do the ref
> counting (mlx5e_open_locked vs. mlx5e_xdp_set batch ref update when
> reset == false).
That is correct, for a short time bpf_prog_add() was charged also when
we reset. When we reset, we will then jump to unlock_put and safely undo
this since we took ref from mlx5e_create_rq() already in that case, and
return successfully. That was intentional, so that eventually we end up
just taking a /single/ ref per RQ when we exit mlx5e_xdp_set(), but more
below ...
[...]
> 2. Keep the current approach and make sure to not update the ref count
> twice, you can batch update only if (!reset && was_open) otherwise you
> can rely on mlx5e_open_locked to take the num_channels ref count for
> you.
>
> Personally I prefer option 2, since we will keep the current logic
> which will allow configuration updates such as (mlx5e_set_ringparam)
> to not worry about ref counting since it will be done in the reset
> flow.
... agree on keeping current approach. I actually like the idea, so we'd
end up with this simpler version for the batched ref then.
Note, your "bpf_prog_add(prog, 1) // one ref for the device." is not needed
since this we already got that one through dev_change_xdp_fd() as mentioned.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index ab0c336..09ac27c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3148,11 +3148,21 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
if (was_opened && reset)
mlx5e_close_locked(netdev);
+ if (was_opened && !reset) {
+ /* num_channels is invariant here, so we can take the
+ * batched reference right upfront.
+ */
+ prog = bpf_prog_add(prog, priv->params.num_channels);
+ if (IS_ERR(prog)) {
+ err = PTR_ERR(prog);
+ goto unlock;
+ }
+ }
- /* exchange programs */
+ /* exchange programs, extra prog reference we got from caller
+ * as long as we don't fail from this point onwards.
+ */
old_prog = xchg(&priv->xdp_prog, prog);
- if (prog)
- bpf_prog_add(prog, 1);
if (old_prog)
bpf_prog_put(old_prog);
@@ -3168,7 +3178,6 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
/* exchanging programs w/o reset, we update ref counts on behalf
* of the channels RQs here.
*/
- bpf_prog_add(prog, priv->params.num_channels);
for (i = 0; i < priv->params.num_channels; i++) {
struct mlx5e_channel *c = priv->channel[i];
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v2] net: ethernet: faraday: To support device tree usage.
From: Andrew Lunn @ 2016-11-16 13:53 UTC (permalink / raw)
To: Greentime Hu; +Cc: netdev, linux-kernel, jiri
In-Reply-To: <1479297826-4335-1-git-send-email-green.hu@gmail.com>
On Wed, Nov 16, 2016 at 08:03:46PM +0800, Greentime Hu wrote:
> Signed-off-by: Greentime Hu <green.hu@gmail.com>
> ---
> drivers/net/ethernet/faraday/ftmac100.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
> index dce5f7b..5d70ee9 100644
> --- a/drivers/net/ethernet/faraday/ftmac100.c
> +++ b/drivers/net/ethernet/faraday/ftmac100.c
> @@ -1172,11 +1172,17 @@ static int __exit ftmac100_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct of_device_id ftmac100_of_ids[] = {
> + { .compatible = "andestech,atmac100" },
Please see my comment to v1.
You are also supposed to add a binding document.
Andrew
^ permalink raw reply
* Re: [PATCH v2] net/phy/vitesse: Configure RGMII skew on VSC8601, if needed
From: Andrew Lunn @ 2016-11-16 13:50 UTC (permalink / raw)
To: Alexandru Gagniuc; +Cc: f.fainelli, netdev, linux-kernel, davem
In-Reply-To: <1479286953-11481-1-git-send-email-alex.g@adaptrum.com>
On Wed, Nov 16, 2016 at 01:02:33AM -0800, Alexandru Gagniuc wrote:
> With RGMII, we need a 1.5 to 2ns skew between clock and data lines. The
> VSC8601 can handle this internally. While the VSC8601 can set more
> fine-grained delays, the standard skew settings work out of the box.
> The same heuristic is used to determine when this skew should be enabled
> as in vsc824x_config_init().
>
> Tested on custom board with AM3352 SOC and VSC801 PHY.
>
> Signed-off-by: Alexandru Gagniuc <alex.g@adaptrum.com>
> ---
> Changes since v1:
> * Added comment detailing applicability to different RGMII interfaces.
>
> drivers/net/phy/vitesse.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
> index 2e37eb3..24b4a09 100644
> --- a/drivers/net/phy/vitesse.c
> +++ b/drivers/net/phy/vitesse.c
> @@ -62,6 +62,10 @@
> /* Vitesse Extended Page Access Register */
> #define MII_VSC82X4_EXT_PAGE_ACCESS 0x1f
>
> +/* Vitesse VSC8601 Extended PHY Control Register 1 */
> +#define MII_VSC8601_EPHY_CTL 0x17
> +#define MII_VSC8601_EPHY_CTL_RGMII_SKEW (1 << 8)
> +
> #define PHY_ID_VSC8234 0x000fc620
> #define PHY_ID_VSC8244 0x000fc6c0
> #define PHY_ID_VSC8514 0x00070670
> @@ -111,6 +115,34 @@ static int vsc824x_config_init(struct phy_device *phydev)
> return err;
> }
>
> +/* This adds a skew for both TX and RX clocks, so the skew should only be
> + * applied to "rgmii-id" interfaces. It may not work as expected
> + * on "rgmii-txid", "rgmii-rxid" or "rgmii" interfaces. */
Hi Alexandru
You should be able to make "rgmii" work as expected. If that is the
phy mode, disable the skew.
Andrew
^ permalink raw reply
* [PATCH net-next] net/mlx4_en: remove napi_hash_del() call
From: Eric Dumazet @ 2016-11-16 13:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Tariq Toukan
From: Eric Dumazet <edumazet@google.com>
There is no need calling napi_hash_del()+synchronize_rcu() before
calling netif_napi_del()
netif_napi_del() does this already.
Using napi_hash_del() in a driver is useful only when dealing with
a batch of NAPI structures, so that a single synchronize_rcu() can
be used. mlx4_en_deactivate_cq() is deactivating a single NAPI.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index 03f05c4d1f9897d851c4f36d1f0e37ad3398e76c..09dd3776db7632ae8818c9638507705176233ea4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -185,10 +185,6 @@ void mlx4_en_destroy_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq **pcq)
void mlx4_en_deactivate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq)
{
napi_disable(&cq->napi);
- if (cq->type == RX) {
- napi_hash_del(&cq->napi);
- synchronize_rcu();
- }
netif_napi_del(&cq->napi);
mlx4_cq_free(priv->mdev->dev, &cq->mcq);
^ permalink raw reply related
* Re: [PATCH net] ipv6 addrconf: Implemented enhanced DAD (RFC7527)
From: Hannes Frederic Sowa @ 2016-11-16 13:49 UTC (permalink / raw)
To: Erik Nordmark, netdev
In-Reply-To: <99c143c3-9ec5-0f5f-44ed-6a4c60e723d8@sonic.net>
Hello,
On 16.11.2016 01:44, Erik Nordmark wrote:
> On 11/16/16 1:00 AM, Hannes Frederic Sowa wrote:
>> On 15.11.2016 08:57, Erik Nordmark wrote:
>>> Implemented RFC7527 Enhanced DAD.
>>> IPv6 duplicate address detection can fail if there is some temporary
>>> loopback of Ethernet frames. RFC7527 solves this by including a random
>>> nonce in the NS messages used for DAD, and if an NS is received with the
>>> same nonce it is assumed to be a looped back DAD probe and is ignored.
>>> RFC7527 is disabled by default. Can be enabled by setting either one of
>>> conf/{all,interface}/ipv6_rfc7527 to non-zero.
>>>
>>> Signed-off-by: Erik Nordmark <nordmark@arista.com>
>>>
>>> Index: linux-stable/Documentation/networking/ip-sysctl.txt
>>> ===================================================================
>>> --- linux-stable.orig/Documentation/networking/ip-sysctl.txt
>>> +++ linux-stable/Documentation/networking/ip-sysctl.txt
>>> @@ -1713,6 +1713,15 @@ drop_unsolicited_na - BOOLEAN
>>>
>>> By default this is turned off.
>>>
>>> +ipv6_rfc7527 - BOOLEAN
>> Could you rename the sysctl to enhanced_dad. As it will anyway show up
>> in the ipv6/ directory hierarchy you don't need to prefix it with 'ipv6_'
>
> Thanks for your careful review and in particular finding that
> uninitialized variable.
>
> Yes, I'll rename the sysctl and all the other things to enhanced_dad.
Thanks!
>>> DEVCONF_MAX
>>> };
>>>
>>> Index: linux-stable/net/ipv6/addrconf.c
>>> ===================================================================
>>> --- linux-stable.orig/net/ipv6/addrconf.c
>>> +++ linux-stable/net/ipv6/addrconf.c
>>> @@ -217,6 +217,7 @@ static struct ipv6_devconf ipv6_devconf
>>> .use_oif_addrs_only = 0,
>>> .ignore_routes_with_linkdown = 0,
>>> .keep_addr_on_down = 0,
>>> + .ipv6_rfc7527 = 0,
>> What is your reason to not enable this by default? I haven't fully
>> studied the RFC, but it seems to be backwards compatible.
>
> The concern is that while RFC4861 says that all unknown options must be
> silently ignored, RFC7527 isn't widely implemented yet. Hence if there
> is some broken implementation of RFC4861 it would fail to interoperate
> with Linux if we set this by default.
>
> Perhaps I'm being too conservative?
> In any case such broken implementations would need to be fixed to ignore
> unknown options.
I thought about even removing the sysctl altogether and enable enhanced
DAD by default. ;)
I am in favor of enabling it by default.
But given that there could be broken implementations out there, we
should give users a choice and provide.
Could you always generate a nonce in the interface structure? You could
check the sysctl in the send and receive path to attach and check the
nonce. This has the advantage that you don't need to delete the
interface and recreate it to enable/disable enhanced dad on an interface
(also you can get away with the loop around get_random_bytes to make
sure its value is not zero as we don't depend on a non-zero nonce
variable to signal enaling of the feature, see below).
>
>>
>>> };
>>>
>>> static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
>>> @@ -262,6 +263,7 @@ static struct ipv6_devconf ipv6_devconf_
>>> .use_oif_addrs_only = 0,
>>> .ignore_routes_with_linkdown = 0,
>>> .keep_addr_on_down = 0,
>>> + .ipv6_rfc7527 = 0,
>>> };
>>>
>>> /* Check if a valid qdisc is available */
>>> @@ -3722,12 +3724,18 @@ static void addrconf_dad_kick(struct ine
>>> {
>>> unsigned long rand_num;
>>> struct inet6_dev *idev = ifp->idev;
>>> + u64 nonce;
>>>
>>> if (ifp->flags & IFA_F_OPTIMISTIC)
>>> rand_num = 0;
>>> else
>>> rand_num = prandom_u32() % (idev->cnf.rtr_solicit_delay ? :
>>> 1);
>>>
>>> + nonce = 0;
>>> + if (ifp->idev->cnf.ipv6_rfc7527 ||
>>> + dev_net((ifp->idev)->dev)->ipv6.devconf_all->ipv6_rfc7527)
>> idev should already be in scope, so you can simplify this conditional.
>
> Yes; I'll fix.
>
>>
>>
>>> + get_random_bytes(&nonce, 6);
>> Maybe:
>>
>> do
>> get_random_bytes(&nonce, 6);
>> while (!nonce);
>
> Is that because get_random_bytes() will not fill in anything if there is
> insufficient entropy available?
No, just because 0 is a possible return value from the random number
generator. ;)
>>> @@ -745,6 +756,7 @@ static void ndisc_recv_ns(struct sk_buff
>>> int dad = ipv6_addr_any(saddr);
>>> bool inc;
>>> int is_router = -1;
>>> + u64 nonce;
>>>
>>> if (skb->len < sizeof(struct nd_msg)) {
>>> ND_PRINTK(2, warn, "NS: packet too short\n");
>>> @@ -789,6 +801,8 @@ static void ndisc_recv_ns(struct sk_buff
>>> return;
>>> }
>>> }
>>> + if (ndopts.nd_opts_nonce)
>>> + memcpy(&nonce, (u8 *)(ndopts.nd_opts_nonce + 1), 6);
>> You only initialize 6 bytes of the nonce, with the other 2 being not
>> initialized.
> Mea culpa. Will fix.
>
>>
>>> inc = ipv6_addr_is_multicast(daddr);
>>>
>>> @@ -797,6 +811,16 @@ static void ndisc_recv_ns(struct sk_buff
>>> have_ifp:
>>> if (ifp->flags & (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)) {
>>> if (dad) {
>>> + if (nonce != 0 && ifp->dad_nonce == nonce) {
>>> + /* Matching nonce if looped back */
>>> + if (net_ratelimit())
>>> + ND_PRINTK(2, notice,
>>> + "%s: IPv6 DAD loopback for address %pI6c
>>> nonce %llu ignored\n",
>>> + ifp->idev->dev->name,
>>> + &ifp->addr,
>>> + nonce);
>> If we print the nonce for debugging reasons, we should keep it in
>> correct endianess on the wire vs. in the debug output.
> How about printing it as colon-separated hex bytes since that is more
> clear than decimal?
> Would follow the network byte order in the packet.
I would be totally fine with it. It will be probably easier to switch to
a char[6] array for the nonce then.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH] net: ethernet: faraday: To support device tree usage.
From: Andrew Lunn @ 2016-11-16 13:47 UTC (permalink / raw)
To: Greentime Hu; +Cc: netdev, linux-kernel
In-Reply-To: <1479285795-3105-1-git-send-email-green.hu@gmail.com>
On Wed, Nov 16, 2016 at 04:43:15PM +0800, Greentime Hu wrote:
> To support device tree usage for ftmac100.
>
> Signed-off-by: Greentime Hu <green.hu@gmail.com>
> ---
> drivers/net/ethernet/faraday/ftmac100.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
> index dce5f7b..81dd9e1 100644
> --- a/drivers/net/ethernet/faraday/ftmac100.c
> +++ b/drivers/net/ethernet/faraday/ftmac100.c
> @@ -1172,11 +1172,17 @@ static int __exit ftmac100_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct of_device_id mac_of_ids[] = {
> + { .compatible = "andestech,atmac100" },
> + { }
andestech is not in
Documentation/devicetree/bindings/vendor-prefixes.txt Please provide a
separate patch adding it.
Humm, why andestech? Why not something based around faraday
technology?
Andrew
^ permalink raw reply
* Re: [PATCH net] sctp: use new rhlist interface on sctp transport rhashtable
From: Xin Long @ 2016-11-16 13:34 UTC (permalink / raw)
To: Neil Horman
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Vlad Yasevich, Herbert Xu, Phil Sutter
In-Reply-To: <20161115180427.GC11283@hmsreliant.think-freely.org>
On Wed, Nov 16, 2016 at 2:04 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Tue, Nov 15, 2016 at 11:23:11PM +0800, Xin Long wrote:
>> Now sctp transport rhashtable uses hash(lport, dport, daddr) as the key
>> to hash a node to one chain. If in one host thousands of assocs connect
>> to one server with the same lport and different laddrs (although it's
>> not a normal case), all the transports would be hashed into the same
>> chain.
>>
>> It may cause to keep returning -EBUSY when inserting a new node, as the
>> chain is too long and sctp inserts a transport node in a loop, which
>> could even lead to system hangs there.
>>
>> The new rhlist interface works for this case that there are many nodes
>> with the same key in one chain. It puts them into a list then makes this
>> list be as a node of the chain.
>>
>> This patch is to replace rhashtable_ interface with rhltable_ interface.
>> Since a chain would not be too long and it would not return -EBUSY with
>> this fix when inserting a node, the reinsert loop is also removed here.
>>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> Does this really buy us anything in this case though? If the use case is that a
> majority of the associations map to the same key, then you might avoid EBUSY for
> the individual associaion that doesn't map there, but you still have to cope
> with a huge linear search for the majority of the keys.
>
This patch is NOT for improving performance, it is to reorganize
transports in rhashtable in another way to avoid EBUSY, rhlist is born
for this.
Before this patch, the transport insert codes are pretty bad, if it returns
EBUSY, it would retry in a loop. now this patch avoid this and even
removed that loop, it's a fix for this issue.
> Might be more reasonable to mix saddr into the hash function so that your use
> case gets spread through the hash table more evenly.
we can not do this:
1. it will increase rhashtable's size when using multihome, if a host has
N addrs, the size for one assoc will be N times bigger than now.
2. the hash node is inside transport, if we mix saddrs, when using multihome
one transport would be hashed many times with different saddrs, we
would have to define new structure to link transport.
we do not need to do this:
1. as the changelog said, "it's not a normal case", in one host (client), it
shouldn't connect to the same server with different saddrs.
2. now as long as paddr+dport+lport are different, rhashtable can hash
it evenly.
^ permalink raw reply
* Re: [PATCH 1/2] can: holt_hi311x: document device tree bindings
From: Rob Herring @ 2016-11-16 13:34 UTC (permalink / raw)
To: Akshay Bhat
Cc: wg, mkl, mark.rutland, linux-can, netdev, devicetree,
linux-kernel, Akshay Bhat
In-Reply-To: <1479146144-29143-1-git-send-email-akshay.bhat@timesys.com>
On Mon, Nov 14, 2016 at 12:55:43PM -0500, Akshay Bhat wrote:
> Document the HOLT HI-311x CAN device tree bindings.
>
> Signed-off-by: Akshay Bhat <nodeax@gmail.com>
> ---
> .../devicetree/bindings/net/can/holt_hi311x.txt | 24 ++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/can/holt_hi311x.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH net 1/3] net: phy: realtek: add eee advertisement disable options
From: Andrew Lunn @ 2016-11-16 13:23 UTC (permalink / raw)
To: Jerome Brunet
Cc: Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Carlo Caione, Kevin Hilman,
Giuseppe Cavallaro, Alexandre TORGUE, Martin Blumenstingl,
Andre Roth, Neil Armstrong,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479290189.17538.25.camel-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> There two kind of PHYs supporting eee, the one advertising eee by
> default (like realtek) and the one not advertising it (like micrel).
I don't know too much about EEE. So maybe a dumb question. Does the
MAC need to be involved? Or is it just the PHY?
If the MAC needs to be involved, the PHY should not be advertising EEE
unless the MAC asks for it by calling phy_init_eee(). If this is true,
maybe we need to change the realtek driver, and others in that class.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/1] ixgbe: write flush vfta registers
From: zyjzyj2000 @ 2016-11-16 12:40 UTC (permalink / raw)
To: e1000-devel, netdev, jeffrey.t.kirsher, intel-wired-lan; +Cc: Zhu Yanjun
From: Zhu Yanjun <zyjzyj2000@gmail.com>
Sometimes vfta registers can not be written successfully in dcb mode.
This is very occassional. When the ixgbe nic runs for a very long time,
sometimes this bug occurs. But after IXGBE_WRITE_FLUSH is executed,
this bug never occurs.
Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bd93d82..1221cfb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4138,8 +4138,10 @@ static void ixgbe_vlan_promisc_enable(struct ixgbe_adapter *adapter)
}
/* Set all bits in the VLAN filter table array */
- for (i = hw->mac.vft_size; i--;)
+ for (i = hw->mac.vft_size; i--;) {
IXGBE_WRITE_REG(hw, IXGBE_VFTA(i), ~0U);
+ IXGBE_WRITE_FLUSH(hw);
+ }
}
#define VFTA_BLOCK_SIZE 8
@@ -4186,6 +4188,7 @@ static void ixgbe_scrub_vfta(struct ixgbe_adapter *adapter, u32 vfta_offset)
vfta[i] |= adapter->active_vlans[word] >> bits;
IXGBE_WRITE_REG(hw, IXGBE_VFTA(vfta_offset + i), vfta[i]);
+ IXGBE_WRITE_FLUSH(hw);
}
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v2 3/4] bpf, mlx5: drop priv->xdp_prog reference on netdev cleanup
From: Saeed Mahameed @ 2016-11-16 12:51 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Alexei Starovoitov, Brenden Blanco, zhiyisun,
Rana Shahout, Saeed Mahameed, Linux Netdev List
In-Reply-To: <4617cac95e3ca1b608f5d0a55709b0534c168943.1479253024.git.daniel@iogearbox.net>
On Wed, Nov 16, 2016 at 2:04 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> mlx5e_xdp_set() is currently the only place where we drop reference on the
> prog sitting in priv->xdp_prog when it's exchanged by a new one. We also
> need to make sure that we eventually release that reference, for example,
> in case the netdev is dismantled.
>
> Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index cf26672..60fe54c 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -3715,6 +3715,9 @@ static void mlx5e_nic_cleanup(struct mlx5e_priv *priv)
>
> if (MLX5_CAP_GEN(mdev, vport_group_manager))
> mlx5_eswitch_unregister_vport_rep(esw, 0);
> +
> + if (priv->xdp_prog)
> + bpf_prog_put(priv->xdp_prog);
> }
>
I thought that on unregister_netdev ndo_xdp_set will be called with
NULL prog to cleanup. like any other resources (Vlans/mac_lists/
etc..), why xdp should be different ?
Anyway if this is the case, I am ok with this fix, you can even send
it to net (it looks like a serious leak).
Saeed.
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] bpf, mlx5: fix various refcount issues in mlx5e_xdp_set
From: Saeed Mahameed @ 2016-11-16 12:25 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Alexei Starovoitov, Brenden Blanco, zhiyisun,
Rana Shahout, Saeed Mahameed, Linux Netdev List
In-Reply-To: <0899adb6ace97b7e7f94b04dd11a0e55270d2b54.1479253024.git.daniel@iogearbox.net>
On Wed, Nov 16, 2016 at 2:04 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> There are multiple issues in mlx5e_xdp_set():
>
> 1) The batched bpf_prog_add() is currently not checked for errors! When
> doing so, it should be done at an earlier point in time to makes sure
> that we cannot fail anymore at the time we want to set the program for
> each channel. This only means that we have to undo the bpf_prog_add()
> in case we return due to required reset.
>
> 2) When swapping the priv->xdp_prog, then no extra reference count must be
> taken since we got that from call path via dev_change_xdp_fd() already.
> Otherwise, we'd never be able to free the program. Also, bpf_prog_add()
> without checking the return code could fail.
>
> Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 25 ++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index ab0c336..cf26672 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -3142,6 +3142,17 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
> goto unlock;
> }
>
> + if (prog) {
> + /* num_channels is invariant here, so we can take the
> + * batched reference right upfront.
> + */
> + prog = bpf_prog_add(prog, priv->params.num_channels);
> + if (IS_ERR(prog)) {
> + err = PTR_ERR(prog);
> + goto unlock;
> + }
> + }
> +
With this approach you will end up taking a ref count twice per RQ! on
the first time xdp_set is called i.e (old_prog = NULL, prog != NULL).
One ref will be taken per RQ/Channel from the above code, and since
reset will be TRUE mlx5e_open_locked will be called and another ref
count will be taken on mlx5e_create_rq.
The issue here is that we have two places for ref count accounting,
xdp_set and mlx5e_create_rq. Having ref-count updates in
mlx5e_create_rq is essential for num_channels configuration changes
(mlx5e_set_ringparam).
Our previous approach made sure that only one path will do the ref
counting (mlx5e_open_locked vs. mlx5e_xdp_set batch ref update when
reset == false).
We have two options:
1. remove ref count updates from mlx5e_create_rq and only do batch
updates in mlx5e_set_ringparam and mlx5e_xdp_set, the only two places
num_channels/priv->xdp_prog could change.
2. Keep the current approach and make sure to not update the ref count
twice, you can batch update only if (!reset && was_open) otherwise you
can rely on mlx5e_open_locked to take the num_channels ref count for
you.
Personally I prefer option 2, since we will keep the current logic
which will allow configuration updates such as (mlx5e_set_ringparam)
to not worry about ref counting since it will be done in the reset
flow.
Basically we want on xdp_set
-bpf_prog_add(prog, 1) // one ref for the device.
- if not going to reset bpf_prog_add(prog, num_channels) //batch update
- old_prog = xchg(&priv->xdp_prog, prog);
- if going to reset then reset . // reset will handle ref-counting
per channel.
- bpf_prog_put(old_prog) // put one ref for the device.
- if (!reset) bpf_prog_put(old_prog, num_channels) // if we didn't
reset, we need to release ref count on old_prog ourselves
Saeed.
^ permalink raw reply
* Re: [PATCH] net: ethernet: faraday: To support device tree usage.
From: Jiri Pirko @ 2016-11-16 12:16 UTC (permalink / raw)
To: Greentime Hu; +Cc: netdev, linux-kernel
In-Reply-To: <CAEbi=3fFbCyCeh=349bJoRDNF0wabF=zJ07u0AYx395Sieo94A@mail.gmail.com>
Wed, Nov 16, 2016 at 01:08:57PM CET, green.hu@gmail.com wrote:
>You are right. I didn't notice that. I should use ftmac100.
Don't top-post please.
>
>On Wed, Nov 16, 2016 at 7:53 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Wed, Nov 16, 2016 at 09:43:15AM CET, green.hu@gmail.com wrote:
>>>To support device tree usage for ftmac100.
>>>
>>>Signed-off-by: Greentime Hu <green.hu@gmail.com>
>>>---
>>> drivers/net/ethernet/faraday/ftmac100.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>>diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
>>>index dce5f7b..81dd9e1 100644
>>>--- a/drivers/net/ethernet/faraday/ftmac100.c
>>>+++ b/drivers/net/ethernet/faraday/ftmac100.c
>>>@@ -1172,11 +1172,17 @@ static int __exit ftmac100_remove(struct platform_device *pdev)
>>> return 0;
>>> }
>>>
>>>+static const struct of_device_id mac_of_ids[] = {
>>
>> Prefix of everything in this file is "ftmac100", yet here, you use
>> "mac". I wonder why?!?
>>
>>
>>
>>>+ { .compatible = "andestech,atmac100" },
>>>+ { }
>>>+};
>>>+
>>> static struct platform_driver ftmac100_driver = {
>>> .probe = ftmac100_probe,
>>> .remove = __exit_p(ftmac100_remove),
>>> .driver = {
>>> .name = DRV_NAME,
>>>+ .of_match_table = mac_of_ids
>>> },
>>> };
>>>
>>>@@ -1200,3 +1206,4 @@ static void __exit ftmac100_exit(void)
>>> MODULE_AUTHOR("Po-Yu Chuang <ratbert@faraday-tech.com>");
>>> MODULE_DESCRIPTION("FTMAC100 driver");
>>> MODULE_LICENSE("GPL");
>>>+MODULE_DEVICE_TABLE(of, mac_of_ids);
>>>--
>>>1.7.9.5
>>>
^ permalink raw reply
* Netperf UDP issue with connected sockets
From: Jesper Dangaard Brouer @ 2016-11-16 12:16 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev@vger.kernel.org, Rick Jones, brouer, Eric Dumazet
In-Reply-To: <1409757426.26422.41.camel@edumazet-glaptop2.roam.corp.google.com>
While optimizing the kernel RX path, I've run into an issue where I
cannot use netperf UDP_STREAM for testing, because the sender is
slower than receiver. Thus, it cannot show my receiver improvements
(as receiver have idle cycles).
Eric Dumazet previously told me[1] this was related to netperf need to
use connected socket for UDP. Netperf options "-- -n -N" should
enable connected UDP sockets, but it never worked! Options are
documented, but netperf seems to have a bug.
Called like:
netperf -H 198.18.50.1 -t UDP_STREAM -l 120 -- -m 1472 -n -N
Problem on sender-side is "__ip_select_ident".
Samples: 15K of event 'cycles', Event count (approx.): 16409681913
Overhead Command Shared Object Symbol
+ 11.18% netperf [kernel.vmlinux] [k] __ip_select_ident
+ 6.93% netperf [kernel.vmlinux] [k] _raw_spin_lock
+ 6.12% netperf [kernel.vmlinux] [k] copy_user_enhanced_fast_string
+ 4.31% netperf [kernel.vmlinux] [k] __ip_make_skb
+ 3.97% netperf [kernel.vmlinux] [k] fib_table_lookup
+ 3.51% netperf [mlx5_core] [k] mlx5e_sq_xmit
+ 2.43% netperf [kernel.vmlinux] [k] __ip_route_output_key_hash
+ 2.24% netperf netperf [.] send_omni_inner
+ 2.17% netperf netperf [.] send_data
[1] Subj: High perf top ip_idents_reserve doing netperf UDP_STREAM
- https://www.spinics.net/lists/netdev/msg294752.html
Not fixed in version 2.7.0.
- ftp://ftp.netperf.org/netperf/netperf-2.7.0.tar.gz
Used extra netperf configure compile options:
./configure --enable-histogram --enable-demo
It seems like some fix attempts exists in the SVN repository::
svn checkout http://www.netperf.org/svn/netperf2/trunk/ netperf2-svn
svn log -r709
# A quick stab at getting remote connect going for UDP_STREAM
svn diff -r708:709
Testing with SVN version, still show __ip_select_ident() in top#1.
(p.s. is netperf ever going to be converted from SVN to git?)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
(old email below)
On Wed, 03 Sep 2014 08:17:06 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2014-09-03 at 16:59 +0200, Jesper Dangaard Brouer wrote:
> > Hi Eric,
> >
> > When doing:
> > super_netperf 120 -H 192.168.8.2 -t UDP_STREAM -l 100 -- -m 256
> >
> > I'm seeing function ip_idents_reserve() consuming most CPU. Could you
> > help, explain what is going on, and how I can avoid this?
> >
> > Perf top:
> > 11.67% [kernel] [k] ip_idents_reserve
> > 8.37% [kernel] [k] fib_table_lookup
> > 4.46% [kernel] [k] _raw_spin_lock
> > 3.21% [kernel] [k] copy_user_enhanced_fast_string
> > 2.92% [kernel] [k] sock_alloc_send_pskb
> > 2.88% [kernel] [k] udp_sendmsg
> >
>
> Because you use a single destination, all flows compete on a single
> atomic to get their next IP identifier.
>
> You can try to use netperf options (-- -N -n) so that netperf uses
> connected UDP sockets.
>
> In this case, the IP identifier generator is held in each socket.
^ permalink raw reply
* Re: [PATCH] net: ethernet: faraday: To support device tree usage.
From: Greentime Hu @ 2016-11-16 12:08 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, linux-kernel
In-Reply-To: <20161116115343.GB1791@nanopsycho.orion>
You are right. I didn't notice that. I should use ftmac100.
On Wed, Nov 16, 2016 at 7:53 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Wed, Nov 16, 2016 at 09:43:15AM CET, green.hu@gmail.com wrote:
>>To support device tree usage for ftmac100.
>>
>>Signed-off-by: Greentime Hu <green.hu@gmail.com>
>>---
>> drivers/net/ethernet/faraday/ftmac100.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>>diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
>>index dce5f7b..81dd9e1 100644
>>--- a/drivers/net/ethernet/faraday/ftmac100.c
>>+++ b/drivers/net/ethernet/faraday/ftmac100.c
>>@@ -1172,11 +1172,17 @@ static int __exit ftmac100_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>>+static const struct of_device_id mac_of_ids[] = {
>
> Prefix of everything in this file is "ftmac100", yet here, you use
> "mac". I wonder why?!?
>
>
>
>>+ { .compatible = "andestech,atmac100" },
>>+ { }
>>+};
>>+
>> static struct platform_driver ftmac100_driver = {
>> .probe = ftmac100_probe,
>> .remove = __exit_p(ftmac100_remove),
>> .driver = {
>> .name = DRV_NAME,
>>+ .of_match_table = mac_of_ids
>> },
>> };
>>
>>@@ -1200,3 +1206,4 @@ static void __exit ftmac100_exit(void)
>> MODULE_AUTHOR("Po-Yu Chuang <ratbert@faraday-tech.com>");
>> MODULE_DESCRIPTION("FTMAC100 driver");
>> MODULE_LICENSE("GPL");
>>+MODULE_DEVICE_TABLE(of, mac_of_ids);
>>--
>>1.7.9.5
>>
^ permalink raw reply
* [PATCH v2] net: ethernet: faraday: To support device tree usage.
From: Greentime Hu @ 2016-11-16 12:03 UTC (permalink / raw)
To: netdev, linux-kernel, jiri; +Cc: Greentime Hu
Signed-off-by: Greentime Hu <green.hu@gmail.com>
---
drivers/net/ethernet/faraday/ftmac100.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
index dce5f7b..5d70ee9 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -1172,11 +1172,17 @@ static int __exit ftmac100_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id ftmac100_of_ids[] = {
+ { .compatible = "andestech,atmac100" },
+ { }
+};
+
static struct platform_driver ftmac100_driver = {
.probe = ftmac100_probe,
.remove = __exit_p(ftmac100_remove),
.driver = {
.name = DRV_NAME,
+ .of_match_table = ftmac100_of_ids
},
};
@@ -1200,3 +1206,4 @@ static void __exit ftmac100_exit(void)
MODULE_AUTHOR("Po-Yu Chuang <ratbert@faraday-tech.com>");
MODULE_DESCRIPTION("FTMAC100 driver");
MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(of, ftmac100_of_ids);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net] net: nsid cannot be allocated for a dead netns
From: Sergei Shtylyov @ 2016-11-16 12:00 UTC (permalink / raw)
To: Nicolas Dichtel, avagin; +Cc: davem, netdev, xiyou.wangcong
In-Reply-To: <1479285671-1298-1-git-send-email-nicolas.dichtel@6wind.com>
Hello.
On 11/16/2016 11:41 AM, Nicolas Dichtel wrote:
> Andrei reports the following kmemleak error:
> unreferenced object 0xffff91badb543950 (size 2096):
> comm "kworker/u4:0", pid 6, jiffies 4295152553 (age 28.418s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 cb 5f df ba 91 ff ff .........._.....
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffffb1865bea>] kmemleak_alloc+0x4a/0xa0
> [<ffffffffb1243b38>] kmem_cache_alloc+0x128/0x280
> [<ffffffffb142f5ab>] idr_layer_alloc+0x2b/0x90
> [<ffffffffb142f9cd>] idr_get_empty_slot+0x34d/0x370
> [<ffffffffb142fa4e>] idr_alloc+0x5e/0x110
> [<ffffffffb170ac3d>] __peernet2id_alloc+0x6d/0x90
> [<ffffffffb170bda5>] peernet2id_alloc+0x55/0xb0
> [<ffffffffb1731216>] rtnl_fill_ifinfo+0xaa6/0x10a0
> [<ffffffffb1733073>] rtmsg_ifinfo_build_skb+0x73/0xd0
> [<ffffffffb17125d5>] rollback_registered_many+0x295/0x390
> [<ffffffffb1712765>] unregister_netdevice_many+0x25/0x80
> [<ffffffffb17138a5>] default_device_exit_batch+0x145/0x170
> [<ffffffffb170ae52>] ops_exit_list.isra.4+0x52/0x60
> [<ffffffffb170c17f>] cleanup_net+0x1bf/0x2a0
> [<ffffffffb10b616f>] process_one_work+0x1ff/0x660
> [<ffffffffb10b661e>] worker_thread+0x4e/0x480
>
> There is no reason to try to allocate an nsid for a netns which is dying.
>
> Reported-by: Andrei Vagin <avagin@gmail.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>
> Thank you for the report. Can you test this patch?
>
> Regards,
> Nicolas
>
> net/core/net_namespace.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index f61c0e02a413..f1340ed0d8df 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -159,6 +159,9 @@ static int alloc_netid(struct net *net, struct net *peer, int reqid)
> max = reqid + 1;
> }
>
> + if (!atomic_read(&net->count) || !&atomic_read(peer->count))
Looks like & is misplaced in the second case?
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] net: ethernet: faraday: To support device tree usage.
From: Jiri Pirko @ 2016-11-16 11:53 UTC (permalink / raw)
To: Greentime Hu; +Cc: netdev, linux-kernel
In-Reply-To: <1479285795-3105-1-git-send-email-green.hu@gmail.com>
Wed, Nov 16, 2016 at 09:43:15AM CET, green.hu@gmail.com wrote:
>To support device tree usage for ftmac100.
>
>Signed-off-by: Greentime Hu <green.hu@gmail.com>
>---
> drivers/net/ethernet/faraday/ftmac100.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
>index dce5f7b..81dd9e1 100644
>--- a/drivers/net/ethernet/faraday/ftmac100.c
>+++ b/drivers/net/ethernet/faraday/ftmac100.c
>@@ -1172,11 +1172,17 @@ static int __exit ftmac100_remove(struct platform_device *pdev)
> return 0;
> }
>
>+static const struct of_device_id mac_of_ids[] = {
Prefix of everything in this file is "ftmac100", yet here, you use
"mac". I wonder why?!?
>+ { .compatible = "andestech,atmac100" },
>+ { }
>+};
>+
> static struct platform_driver ftmac100_driver = {
> .probe = ftmac100_probe,
> .remove = __exit_p(ftmac100_remove),
> .driver = {
> .name = DRV_NAME,
>+ .of_match_table = mac_of_ids
> },
> };
>
>@@ -1200,3 +1206,4 @@ static void __exit ftmac100_exit(void)
> MODULE_AUTHOR("Po-Yu Chuang <ratbert@faraday-tech.com>");
> MODULE_DESCRIPTION("FTMAC100 driver");
> MODULE_LICENSE("GPL");
>+MODULE_DEVICE_TABLE(of, mac_of_ids);
>--
>1.7.9.5
>
^ permalink raw reply
* Re: Debugging Ethernet issues
From: Sebastian Frias @ 2016-11-16 11:10 UTC (permalink / raw)
To: Florian Fainelli, Mason
Cc: Andrew Lunn, netdev, Mans Rullgard, Sergei Shtylyov, Tom Lendacky,
Zach Brown, Shaohui Xie, Tim Beale, Brian Hill, Vince Bridgers,
Balakumaran Kannan, David S. Miller, Kirill Kapranov
In-Reply-To: <1fd492b4-3c32-7939-791c-56cf6d8c0078@gmail.com>
Hi Florian,
On 11/14/2016 10:00 PM, Florian Fainelli wrote:
> On 11/14/2016 12:27 PM, Mason wrote:
>> On 14/11/2016 19:20, Florian Fainelli wrote:
>>
>>> On 11/14/2016 09:59 AM, Sebastian Frias wrote:
>>>
>>>> Could you confirm that Mason's patch is correct and/or that it does not
>>>> has negative side-effects?
>>>
>>> The patch is not correct nor incorrect per-se, it changes the default
>>> policy of having pause frames advertised by default to not having them
>>> advertised by default. This influences both your Ethernet MAC and the
>>> link partner in that the result is either flow control is enabled
>>> (before) or it is not (with the patch). There must be something amiss if
>>> you see packet loss or some kind of problem like that with an early
>>> exchange such as DHCP. Flow control tend to kick in under higher packet
>>> rates (at least, that's what you expect).
>>
>> Did you note that, without the change under discussion (i.e. with
>> the eth driver as it is upstream), when the board is connected to
>> a 100 Mbps switch, then *nothing* works *systematically (no ping,
>> no DHCP; are there other relevant low-level network tools?).
>
> No I missed that, way too many emails, really. So how about you compare
> the register settings that could be (that is, all that could be modified
> by the PHYLIB adjust_link function) and try to spot where things could
> go wrong?
The registers that make this work or fail are modified in
'nb8800_pause_config()' which is called from 'adjust_link', see below.
>Any other register that can be influenced by the link speed?
It may not be the link-speed per se, but the way these "pause" parameters
are interacting.
Indeed, on the 1000 Mbps switch we get (1):
"Link partner advertised pause frame use: Symmetric Receive-only"
And on the 100 Mbps switch we get (2):
"Link partner advertised pause frame use: Symmetric"
Since the 100 Mbps is advertising "symmetric", the code in
'drivers/net/ethernet/aurora/nb8800.c:nb8800_pause_config()' enables the
flow control bit:
nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
If we force this to be disabled, the network works on 100 Mbps.
Keep in mind that the same driver (net and phy) work on a different board,
could it be just by chance?
>
> It seems like a possible (yet after re-reading, very unlikely) scenario,
> considering that priv->speed, priv->duplex and priv->link are initially
> zero-initialized (because nb8800_priv is zero initialized) may not force
> a correct link transition and a full MAC reconfiguration in
> nb8800_link_reconfigure() where some of the cached values are used.
>
> NB: you will see most drivers initialize the previous link,
Could you suggest another driver to compare this with?
It is not easy to know which drivers follow the conventions so that
they can be used as examples.
>speed,
> duplex values to -1, because those are outside of the range of values
> that PHYLIB would assign to phydev->{link,duplex,speed}, and therefore,
> this is guaranteed to make the adjust_link callback that tries to
> minimize these settings to force a transition.
>
>>
>> Also, maybe this comment was lost in my own noise:
>>
>> If I manually set the link up, then down, then run udhcpc
>> => then nothing works, as if something is wedged somewhere
>> (a kernel thread gets borked by a race condition?)
>
> Well then start seriously debugging the problem: firs thing you need to
> check is is the RUNNING flag set on the interface (which indicates a
> carrier on?) without that, the networking stack won't even send packets.
'ifconfig' reports "eth0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>"
Packets get sent, as the statistics show for example the following values
increasing:
tx_bytes: 756
tx_frames: 3
However, the statistics show that RX values increase and then stop.
Basically, if we do:
# ethtool -S eth0 > /tmp/toto
# udhcpc (ctrl-C because it won't return because it keeps trying)
# ethtool -S eth0 > /tmp/titi
# diff /tmp/toto /tmp/titi
we can see that the RX values go from 0 to X, but get stuck at X, while
the TX values keep increasing:
rx_bytes_ok: 887
rx_frames_ok: 8
...
tx_bytes: 1576
tx_frames: 7
Like if the Flow Control is not working properly. However we don't know
how to double check it, since the older driver (for kernel 3.4) does not
seem to use the feature.
> If it is not set, why is not it set? Did nb8800_mac_config() get called
> in the first place to configure the MAC wrt. the link settings?
>
> When you transmit, do transmit counters increase? That would indicate
> the TX DMA does its job. When transmission occurs, it is successful or
> is it reporting errors? If the PHY supports it, can you access PHY
> counters and look for success/error counters changing? Finally, try to
> put another golden (working) host and if your switch supports it,
> configure port mirroring to look at packets. If the switch does not
> support it, then try different link partners.
>
So far, our limited testing indicates that only switches that advertise:
"Link partner advertised pause frame use: Symmetric"
are problematic.
Would the information presented here give you guys other ideas of what
to look for? Are those switches known to give issues?
Could the "pause" feature be timing dependent? (since it works on other
boards)
Best regards,
Sebastian
----
(1): ethtool eth0 when connected to 1000 Mbps switch
Supported ports: [ TP MII ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Supported pause frame use: Symmetric
Supports auto-negotiation: Yes
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Advertised pause frame use: Symmetric
Advertised auto-negotiation: Yes
Link partner advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Half 1000baseT/Full
Link partner advertised pause frame use: Symmetric Receive-only
Link partner advertised auto-negotiation: Yes
Speed: 1000Mb/s
Duplex: Full
Port: MII
PHYAD: 4
Transceiver: external
Auto-negotiation: on
Link detected: yes
(2): ethtool eth0 when connected to 100 Mbps switch
Supported ports: [ TP MII ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Supported pause frame use: Symmetric
Supports auto-negotiation: Yes
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Advertised pause frame use: Symmetric
Advertised auto-negotiation: Yes
Link partner advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
Link partner advertised pause frame use: Symmetric
Link partner advertised auto-negotiation: Yes
Speed: 100Mb/s
Duplex: Full
Port: MII
PHYAD: 4
Transceiver: external
Auto-negotiation: on
Link detected: yes
>>
>> Could not advertising pause frames result in making such a
>> race condition impossible? (I don't really believe in a race,
>> due to the 100% nature of the problem.)
>>
>>>> Right now we know that Mason's patch makes this work, but we do not understand
>>>> why nor its implications.
>>>
>>> You need to understand why, right now, the way this problem is
>>> presented, you came up with a workaround, not with the root cause or the
>>> solution. What does your link partner (switch?) reports, that is, what
>>> is the ethtool output when you have a link up from your nb8800 adapter?
>>
>> Isn't that what ethtool -a eth0 prints?
>
> No, ethtool -a prints the local pause settings.
>
>> How do I get the link partner information?
>
> ethtool eth0:
>
> # ethtool eth0
> Settings for eth0:
> Supported ports: [ TP MII ]
> Supported link modes: 10baseT/Half 10baseT/Full
> 100baseT/Half 100baseT/Full
> Supported pause frame use: Symmetric Receive-only
> Supports auto-negotiation: Yes
> Advertised link modes: 10baseT/Half 10baseT/Full
> 100baseT/Half 100baseT/Full
> Advertised pause frame use: No
> Advertised auto-negotiation: Yes
> Link partner advertised link modes: 10baseT/Half 10baseT/Full
> 100baseT/Half 100baseT/Full
>
> ^======================
>
> Link partner advertised pause frame use: Symmetric
> Link partner advertised auto-negotiation: Yes
>
> ^========================
>
> Speed: 100Mb/s
> Duplex: Full
> Port: MII
> PHYAD: 1
> Transceiver: internal
> Auto-negotiation: on
> Supports Wake-on: gs
> Wake-on: d
> SecureOn password: 00:00:00:00:00:00
> Current message level: 0x00000007 (7)
> drv probe link
> Link detected: yes
> #
>
>
>> Just ethtool eth0?
>
> Yes, just that.
>
^ permalink raw reply
* [PATCH 1/2] net: netcp: replace IS_ERR_OR_NULL by IS_ERR
From: Julia Lawall @ 2016-11-16 10:43 UTC (permalink / raw)
To: Wingman Kwok
Cc: kernel-janitors, Murali Karicheri, netdev, linux-kernel,
Christophe JAILLET
In-Reply-To: <1479293014-18811-1-git-send-email-Julia.Lawall@lip6.fr>
knav_queue_open always returns an ERR_PTR value, never NULL. This can be
confirmed by unfolding the function calls and conforms to the function's
documentation. Thus, replace IS_ERR_OR_NULL by IS_ERR in error checks.
The change is made using the following semantic patch:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression x;
statement S;
@@
x = knav_queue_open(...);
if (
- IS_ERR_OR_NULL
+ IS_ERR
(x)) S
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The patches in this series are completely independent of each other, except
for having been made from the same specification.
drivers/net/ethernet/ti/netcp_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 78b4c83..7981b99 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -1568,7 +1568,7 @@ static int netcp_setup_navigator_resources(struct net_device *ndev)
/* open Tx completion queue */
snprintf(name, sizeof(name), "tx-compl-%s", ndev->name);
netcp->tx_compl_q = knav_queue_open(name, netcp->tx_compl_qid, 0);
- if (IS_ERR_OR_NULL(netcp->tx_compl_q)) {
+ if (IS_ERR(netcp->tx_compl_q)) {
ret = PTR_ERR(netcp->tx_compl_q);
goto fail;
}
@@ -1588,7 +1588,7 @@ static int netcp_setup_navigator_resources(struct net_device *ndev)
/* open Rx completion queue */
snprintf(name, sizeof(name), "rx-compl-%s", ndev->name);
netcp->rx_queue = knav_queue_open(name, netcp->rx_queue_id, 0);
- if (IS_ERR_OR_NULL(netcp->rx_queue)) {
+ if (IS_ERR(netcp->rx_queue)) {
ret = PTR_ERR(netcp->rx_queue);
goto fail;
}
@@ -1610,7 +1610,7 @@ static int netcp_setup_navigator_resources(struct net_device *ndev)
++i) {
snprintf(name, sizeof(name), "rx-fdq-%s-%d", ndev->name, i);
netcp->rx_fdq[i] = knav_queue_open(name, KNAV_QUEUE_GP, 0);
- if (IS_ERR_OR_NULL(netcp->rx_fdq[i])) {
+ if (IS_ERR(netcp->rx_fdq[i])) {
ret = PTR_ERR(netcp->rx_fdq[i]);
goto fail;
}
^ permalink raw reply related
* [PATCH 0/2] replace IS_ERR_OR_NULL by IS_ERR
From: Julia Lawall @ 2016-11-16 10:43 UTC (permalink / raw)
To: netdev; +Cc: kernel-janitors, linux-kernel, linux-arm-kernel,
Christophe JAILLET
Replace IS_ERR_OR_NULL by IS_ERR on knav_queue_open calls.
---
drivers/net/ethernet/ti/netcp_core.c | 6 +++---
drivers/soc/ti/knav_qmss_queue.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox