Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 16:31 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
	ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <20180323.122035.1380806748695640531.davem@davemloft.net>

On 3/23/2018 12:20 PM, David Miller wrote:
> From: Sinan Kaya <okaya@codeaurora.org>
> Date: Thu, 22 Mar 2018 13:10:00 -0400
> 
>> Code includes wmb() followed by writel(). writel() already has a
>> barrier on some architectures like arm64.
>  ...
>> @@ -4155,7 +4155,7 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>  	txdata->tx_db.data.prod += nbd;
>>  	barrier();
>>  
>> -	DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
>> +	DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
>>  
>>  	mmiowb();
>  ...
>> @@ -2592,7 +2592,7 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode)
>>  
>>  	txdata->tx_db.data.prod += 2;
>>  	barrier();
>> -	DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
>> +	DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
> 
> These are compiler barriers being used here, not wmb().
> 
> Look, if I can't see a clear:
> 
> 	wmb()
> 	writel()
> 
> sequence in the patch hunks, I am going to keep pushing back on
> these changes.

Sorry, you got me confused now.

If you look at the code closer, you'll see this.

	wmb();

	txdata->tx_db.data.prod += nbd;
	barrier();

	DOORBELL(bp, txdata->cid, txdata->tx_db.raw);

and you also asked me to rename DOORBELL to DOORBELL_RELAXED() to make
it obvious that we have a relaxed operator inside the macro.

Did I miss something?

of course, treating barrier() universally as a write barrier is wrong.

> 
> Thank you.
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* Re: [PATCH v2 net-next 0/6] TLS Rx
From: David Miller @ 2018-03-23 16:27 UTC (permalink / raw)
  To: davejwatson
  Cc: tom, alexei.starovoitov, herbert, linux-crypto, netdev, borisp,
	atul.gupta, vakul.garg, hannes, steffen.klassert, john.fastabend,
	daniel
In-Reply-To: <20180322170944.GA67793@GeorgeHnsiPhone.dhcp.thefacebook.com>

From: Dave Watson <davejwatson@fb.com>
Date: Thu, 22 Mar 2018 10:09:44 -0700

> TLS tcp socket RX implementation, to match existing TX code.

Looks great Dave.  Applied, and once my build tests finish I'll push
this out to net-next.

In some of the new functions, reverse christmas tree ordering of local
variables was not adhered to.

I know it can be difficult in some situations, but please fix this up in
followon patches.  You can move the assignments down into the function
body to accomodate this, if necessary.

Thanks.

^ permalink raw reply

* [PATCH net-next] net: bridge: fix direct access to bridge vlan_enabled and use helper
From: Nikolay Aleksandrov @ 2018-03-23 16:27 UTC (permalink / raw)
  To: netdev; +Cc: Nikolay Aleksandrov, roopa, bridge, 3chas3, davem
In-Reply-To: <20180322153406.17760-1-3chas3@gmail.com>

We need to use br_vlan_enabled() helper otherwise we'll break builds
without bridge vlans:
net/bridge//br_if.c: In function ‘br_mtu’:
net/bridge//br_if.c:458:8: error: ‘const struct net_bridge’ has no
member named ‘vlan_enabled’
  if (br->vlan_enabled)
        ^
net/bridge//br_if.c:462:1: warning: control reaches end of non-void
function [-Wreturn-type]
 }
 ^
scripts/Makefile.build:324: recipe for target 'net/bridge//br_if.o'
failed

Fixes: 419d14af9e07 ("bridge: Allow max MTU when multiple VLANs present")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 net/bridge/br_if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 48dc4d2e2be3..87b2afd455c7 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -455,7 +455,7 @@ static int __br_mtu(const struct net_bridge *br, bool (compare_fn)(int, int))
 
 int br_mtu(const struct net_bridge *br)
 {
-	if (br->vlan_enabled)
+	if (br_vlan_enabled(br->dev))
 		return __br_mtu(br, max_mtu);
 	else
 		return __br_mtu(br, min_mtu);
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: David Miller @ 2018-03-23 16:20 UTC (permalink / raw)
  To: okaya
  Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
	ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <1521738603-23596-4-git-send-email-okaya@codeaurora.org>

From: Sinan Kaya <okaya@codeaurora.org>
Date: Thu, 22 Mar 2018 13:10:00 -0400

> Code includes wmb() followed by writel(). writel() already has a
> barrier on some architectures like arm64.
 ...
> @@ -4155,7 +4155,7 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	txdata->tx_db.data.prod += nbd;
>  	barrier();
>  
> -	DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
> +	DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
>  
>  	mmiowb();
 ...
> @@ -2592,7 +2592,7 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode)
>  
>  	txdata->tx_db.data.prod += 2;
>  	barrier();
> -	DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
> +	DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);

These are compiler barriers being used here, not wmb().

Look, if I can't see a clear:

	wmb()
	writel()

sequence in the patch hunks, I am going to keep pushing back on
these changes.

Thank you.

^ permalink raw reply

* Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present
From: Nikolay Aleksandrov @ 2018-03-23 16:20 UTC (permalink / raw)
  To: David Miller, 3chas3; +Cc: netdev, stephen
In-Reply-To: <20180323.121747.2120157916968005999.davem@davemloft.net>

On 23/03/18 18:17, David Miller wrote:
> From: Chas Williams <3chas3@gmail.com>
> Date: Thu, 22 Mar 2018 11:34:06 -0400
> 
>> If the bridge is allowing multiple VLANs, some VLANs may have
>> different MTUs.  Instead of choosing the minimum MTU for the
>> bridge interface, choose the maximum MTU of the bridge members.
>> With this the user only needs to set a larger MTU on the member
>> ports that are participating in the large MTU VLANS.
>>
>> Signed-off-by: Chas Williams <3chas3@gmail.com>
> 
> Applied, thanks.
> 

Argh, this will break on builds without vlans because br->vlan_enabled shouldn't
be accessed directly. I missed that when reviewing.
I'll send a follow up fix in a second that uses br_vlan_enabled().

^ permalink raw reply

* Re: [net-next v2] intel: add SPDX identifiers to all the Intel drivers
From: David Miller @ 2018-03-23 16:18 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20180322170848.21107-1-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 22 Mar 2018 10:08:48 -0700

> Add the SPDX identifiers to all the Intel wired LAN driver files, as
> outlined in Documentation/process/license-rules.rst.
> 
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> ---
> v2: based on community feedback, went back to the v2.6 version of the
>     SPDX identifier string until the kernel documentation gets updated

Applied, thank you.

^ permalink raw reply

* Re: [bpf-next V5 PATCH 09/15] mlx5: register a memory model when XDP is enabled
From: Sergei Shtylyov @ 2018-03-23 16:18 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, netdev, BjörnTöpel,
	magnus.karlsson
  Cc: eugenia, Jason Wang, John Fastabend, Eran Ben Elisha,
	Saeed Mahameed, galp, Daniel Borkmann, Alexei Starovoitov,
	Tariq Toukan
In-Reply-To: <152180752460.20167.1098864148126196703.stgit@firesoul>

Hello!

On 03/23/2018 03:18 PM, Jesper Dangaard Brouer wrote:

> Now all the users of ndo_xdp_xmit have been converted to use xdp_return_frame.
> This enable a different memory model, thus activating another code path
> in the xdp_return_frame API.
> 
> V2: Fixed issues pointed out by Tariq.
> 
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index da94c8cba5ee..2e4ca0f15b62 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -506,6 +506,14 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
>  		rq->mkey_be = c->mkey_be;
>  	}
>  
> +	/* This must only be activate for order-0 pages */

   Activated?

[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present
From: David Miller @ 2018-03-23 16:17 UTC (permalink / raw)
  To: 3chas3; +Cc: netdev, stephen
In-Reply-To: <20180322153406.17760-1-3chas3@gmail.com>

From: Chas Williams <3chas3@gmail.com>
Date: Thu, 22 Mar 2018 11:34:06 -0400

> If the bridge is allowing multiple VLANs, some VLANs may have
> different MTUs.  Instead of choosing the minimum MTU for the
> bridge interface, choose the maximum MTU of the bridge members.
> With this the user only needs to set a larger MTU on the member
> ports that are participating in the large MTU VLANS.
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH v2 net-next] virtio-net: Fix operstate for virtio when no VIRTIO_NET_F_STATUS
From: David Miller @ 2018-03-23 16:17 UTC (permalink / raw)
  To: jay.vosburgh; +Cc: netdev, mst, jasowang, ben
In-Reply-To: <23981.1521729761@nyx>

From: Jay Vosburgh <jay.vosburgh@canonical.com>
Date: Thu, 22 Mar 2018 14:42:41 +0000

> 	The operstate update logic will leave an interface in the
> default UNKNOWN operstate if the interface carrier state never changes
> from the default carrier up state set at creation.  This includes the
> case of an explicit call to netif_carrier_on, as the carrier on to on
> transition has no effect on operstate.
> 
> 	This affects virtio-net for the case that the virtio peer does
> not support VIRTIO_NET_F_STATUS (the feature that provides carrier state
> updates).  Without this feature, the virtio specification states that
> "the link should be assumed active," so, logically, the operstate should
> be UP instead of UNKNOWN.  This has impact on user space applications
> that use the operstate to make availability decisions for the interface.
> 
> 	Resolve this by changing the virtio probe logic slightly to call
> netif_carrier_off for both the "with" and "without" VIRTIO_NET_F_STATUS
> cases, and then the existing call to netif_carrier_on for the "without"
> case will cause an operstate transition.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: Drop NETDEV_UNREGISTER_FINAL
From: Jason Gunthorpe @ 2018-03-23 16:15 UTC (permalink / raw)
  To: Kirill Tkhai
  Cc: davem, Michal.Kalderon, Ariel.Elior, dledford, benve, 1dgoodell,
	daniel, jakub.kicinski, ast, edumazet, linux, john.fastabend,
	brouer, dsahern, netdev
In-Reply-To: <152179796956.13076.17482079697536507473.stgit@localhost.localdomain>

On Fri, Mar 23, 2018 at 12:39:33PM +0300, Kirill Tkhai wrote:
> Last user is gone after bdf5bd7f2132 "rds: tcp: remove
> register_netdevice_notifier infrastructure.", so we can
> remove this netdevice command. This allows to delete
> rtnl_lock() in netdev_run_todo(), which is hot path for
> net namespace unregistration.
> 
> dev_change_net_namespace() and netdev_wait_allrefs()
> have rcu_barrier() before NETDEV_UNREGISTER_FINAL call,
> and the source commits say they were introduced to
> delemit the call with NETDEV_UNREGISTER, but this patch
> leaves them on the places, since they require additional
> analysis, whether we need in them for something else.
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>  drivers/infiniband/hw/qedr/main.c |    4 ++--
>  include/linux/netdevice.h         |    1 -
>  include/rdma/ib_verbs.h           |    4 ++--
>  net/core/dev.c                    |    6 ------
>  4 files changed, 4 insertions(+), 11 deletions(-)

No problem to take the rdma part of this through netdev

Acked-by: Jason Gunthorpe <jgg@mellanox.com>

Jason

^ permalink raw reply

* Re: [PATCH net-next] devlink: Remove top_hierarchy arg for DEVLINK disabled path
From: David Miller @ 2018-03-23 16:14 UTC (permalink / raw)
  To: dsahern; +Cc: netdev
In-Reply-To: <20180323150948.6087-1-dsahern@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Fri, 23 Mar 2018 08:09:48 -0700

> Earlier change missed the path where CONFIG_NET_DEVLINK is disabled.
> Thanks to Jiri for spotting.
> 
> Fixes: 145307460ba9 ("devlink: Remove top_hierarchy arg to devlink_resource_register")
> Signed-off-by: David Ahern <dsahern@gmail.com>

Applied, thanks David.

^ permalink raw reply

* Re: [patch iproute2] devlink: fix port new monitoring message typo
From: David Ahern @ 2018-03-23 16:11 UTC (permalink / raw)
  To: Jiri Pirko, netdev; +Cc: stephen
In-Reply-To: <20180323121913.12598-1-jiri@resnulli.us>

On 3/23/18 6:19 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> s/net/new/
> 
> Fixes: a3c4b484a1ed ("add devlink tool")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  devlink/devlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [PATCH net] udp6: set dst cache for a connected sk before udp_v6_send_skb
From: Eric Dumazet @ 2018-03-23 15:50 UTC (permalink / raw)
  To: Alexey Kodanev, netdev; +Cc: David Miller
In-Reply-To: <1521815989-26412-1-git-send-email-alexey.kodanev@oracle.com>



On 03/23/2018 07:39 AM, Alexey Kodanev wrote:
> After commit 33c162a980fe ("ipv6: datagram: Update dst cache of a
> connected datagram sk during pmtu update"), when the error occurs on
> sending datagram in udpv6_sendmsg() due to ICMPV6_PKT_TOOBIG type,
> error handler can trigger the following path and call ip6_dst_store():
> 
>     udpv6_err()
>         ip6_sk_update_pmtu()
>             ip6_datagram_dst_update()
>                 ip6_dst_lookup_flow(), can create a RTF_CACHE clone
>                 ...
>                 ip6_dst_store()
> 
> It can happen before a connected UDP socket invokes ip6_dst_store()
> in the end of udpv6_sendmsg(), on destination release, as a result,
> the last one changes dst to the old one, preventing getting updated
> dst cache on the next udpv6_sendmsg() call.
> 
> This patch moves ip6_dst_store() in udpv6_sendmsg(), so that it is
> invoked after ip6_sk_dst_lookup_flow() and before udp_v6_send_skb().
> 


A Fixes: tag would be nice, for automatic tools (and humans as well)

> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
> ---
>  net/ipv6/udp.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 52e3ea0..0d413c6 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1299,6 +1299,16 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>  	if (ipc6.hlimit < 0)
>  		ipc6.hlimit = ip6_sk_dst_hoplimit(np, &fl6, dst);
>  
> +	if (connected)
> +		ip6_dst_store(sk, dst,
> +			      ipv6_addr_equal(&fl6.daddr, &sk->sk_v6_daddr) ?
> +			      &sk->sk_v6_daddr : NULL,
> +#ifdef CONFIG_IPV6_SUBTREES
> +			      ipv6_addr_equal(&fl6.saddr, &np->saddr) ?
> +			      &np->saddr :
> +#endif
> +			      NULL);
> +

What about the MSG_CONFIRM stuff ?

>  	if (msg->msg_flags&MSG_CONFIRM)
>  		goto do_confirm;
>  back_from_confirm:

Should not you move the above code here instead ?

Also ip6_dst_store() does not increment dst refcount.

I fear that as soon as dst is visible to other cpus, it might be stolen.

> @@ -1350,18 +1360,8 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>  
>  release_dst:
>  	if (dst) {
> -		if (connected) {
> -			ip6_dst_store(sk, dst,
> -				      ipv6_addr_equal(&fl6.daddr, &sk->sk_v6_daddr) ?
> -				      &sk->sk_v6_daddr : NULL,
> -#ifdef CONFIG_IPV6_SUBTREES
> -				      ipv6_addr_equal(&fl6.saddr, &np->saddr) ?
> -				      &np->saddr :
> -#endif
> -				      NULL);
> -		} else {
> +		if (!connected)
>  			dst_release(dst);
> -		}
>  		dst = NULL;
>  	}
>  
> 

^ permalink raw reply

* Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Andrew Lunn @ 2018-03-23 15:44 UTC (permalink / raw)
  To: Vicentiu Galanopulo
  Cc: netdev, linux-kernel, robh+dt, mark.rutland, davem, marcel,
	devicetree, madalin.bucur, alexandru.marginean
In-Reply-To: <20180323150522.9603-1-vicentiu.galanopulo@nxp.com>

> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -24,6 +24,8 @@
>  
>  #define DEFAULT_GPIO_RESET_DELAY	10	/* in microseconds */
>  
> +struct phy_c45_device_ids mdio_c45_ids = {0};

You do know that Linux is multi-threaded. It could be probing two MDIO
busses at once.

	Andrew

^ permalink raw reply

* [GIT] 'net' merged into 'net-next'
From: David Miller @ 2018-03-23 15:40 UTC (permalink / raw)
  To: netdev; +Cc: jgg, dledford, idosch, dsahern, sd


This merge was a little bit more hectic than usual.

But thankfully, I had some sample conflict resolutions to work
with, in particular for the mlx5 infiniband changes which were
the most difficult to resolve.

Please double check my work and provide any fixup patches if
necessary.

Thank you.

^ permalink raw reply

* RE: [PATCH net-next,2/2] hv_netvsc: Add range checking for rx packet offset and length
From: Haiyang Zhang @ 2018-03-23 15:25 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Haiyang Zhang
  Cc: davem@davemloft.net, netdev@vger.kernel.org, KY Srinivasan,
	Stephen Hemminger, olaf@aepfle.de, devel@linuxdriverproject.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <87sh8q4y9s.fsf@vitty.brq.redhat.com>



> -----Original Message-----
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> Sent: Friday, March 23, 2018 11:17 AM
> To: Haiyang Zhang <haiyangz@linuxonhyperv.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de;
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net-next,2/2] hv_netvsc: Add range checking for rx packet
> offset and length
> 
> Haiyang Zhang <haiyangz@linuxonhyperv.com> writes:
> 
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > This patch adds range checking for rx packet offset and length.
> > It may only happen if there is a host side bug.
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> >  drivers/net/hyperv/hyperv_net.h |  1 +
> >  drivers/net/hyperv/netvsc.c     | 17 +++++++++++++++--
> >  2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/hyperv_net.h
> > b/drivers/net/hyperv/hyperv_net.h index 0db3bd1ea06f..49c05ac894e5
> > 100644
> > --- a/drivers/net/hyperv/hyperv_net.h
> > +++ b/drivers/net/hyperv/hyperv_net.h
> > @@ -793,6 +793,7 @@ struct netvsc_device {
> >
> >  	/* Receive buffer allocated by us but manages by NetVSP */
> >  	void *recv_buf;
> > +	u32 recv_buf_size; /* allocated bytes */
> >  	u32 recv_buf_gpadl_handle;
> >  	u32 recv_section_cnt;
> >  	u32 recv_section_size;
> > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> > index 1ddb2c39b6e4..a6700d65f206 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -289,6 +289,8 @@ static int netvsc_init_buf(struct hv_device *device,
> >  		goto cleanup;
> >  	}
> >
> > +	net_device->recv_buf_size = buf_size;
> > +
> >  	/*
> >  	 * Establish the gpadl handle for this buffer on this
> >  	 * channel.  Note: This call uses the vmbus connection rather @@
> > -1095,11 +1097,22 @@ static int netvsc_receive(struct net_device
> > *ndev,
> >
> >  	/* Each range represents 1 RNDIS pkt that contains 1 ethernet frame */
> >  	for (i = 0; i < count; i++) {
> > -		void *data = recv_buf
> > -			+ vmxferpage_packet->ranges[i].byte_offset;
> > +		u32 offset = vmxferpage_packet->ranges[i].byte_offset;
> >  		u32 buflen = vmxferpage_packet->ranges[i].byte_count;
> > +		void *data;
> >  		int ret;
> >
> > +		if (unlikely(offset + buflen > net_device->recv_buf_size)) {
> > +			status = NVSP_STAT_FAIL;
> > +			netif_err(net_device_ctx, rx_err, ndev,
> > +				  "Packet offset:%u + len:%u too big\n",
> > +				  offset, buflen);
> 
> This shouldn't happen, of course, but I'd rather ratelimit this error or even used
> something like netdev_WARN_ONCE().

Actually I thought about ratelimit, but this range check is only to catch host side bug. 
It should not happen. 
But if it happens, the VM should not be used anymore. And we need to debug
the host. Similarly, some other this kind of checks in the same function are not using
ratelimit:

        if (unlikely(nvsp->hdr.msg_type != NVSP_MSG1_TYPE_SEND_RNDIS_PKT)) {
                netif_err(net_device_ctx, rx_err, ndev,
                          "Unknown nvsp packet type received %u\n",
                          nvsp->hdr.msg_type);

Thanks,
- Haiyang

^ permalink raw reply

* Re: [patch net-next RFC 00/12] devlink: introduce port flavours and common phys_port_name generation
From: Andrew Lunn @ 2018-03-23 15:24 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, idosch, jakub.kicinski, mlxsw, vivien.didelot,
	f.fainelli, michael.chan, ganeshgr, saeedm, simon.horman,
	pieter.jansenvanvuuren, john.hurley, dirk.vandermerwe,
	alexander.h.duyck, ogerlitz, dsahern, vijaya.guvva,
	satananda.burla, raghu.vatsavayi, felix.manlunas, gospo,
	sathya.perla, vasundhara-v.volam, tariqt, eranbe,
	jeffrey.t.kirsher
In-Reply-To: <20180323145935.GC2125@nanopsycho>

On Fri, Mar 23, 2018 at 03:59:35PM +0100, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 02:43:57PM CET, andrew@lunn.ch wrote:
> >> I tested this for mlxsw and nfp. I have no way to test this on DSA hw,
> >> I would really appretiate DSA guys to test this.
> >
> >Hi Jiri
> >
> >With the missing break added, i get:
> >
> >root@zii-devel-b:~# ./iproute2/devlink/devlink port 
> >mdio_bus/0.1:00/0: type eth netdev lan0 flavour physical number 0
> >mdio_bus/0.1:00/1: type eth netdev lan1 flavour physical number 1
> >mdio_bus/0.1:00/2: type eth netdev lan2 flavour physical number 2
> >mdio_bus/0.1:00/3: type notset
> >mdio_bus/0.1:00/4: type notset
> >mdio_bus/0.1:00/5: type notset flavour dsa number 5
> >mdio_bus/0.1:00/6: type notset flavour cpu number 6
> >mdio_bus/0.2:00/0: type eth netdev lan3 flavour physical number 0
> >mdio_bus/0.2:00/1: type eth netdev lan4 flavour physical number 1
> >mdio_bus/0.2:00/2: type eth netdev lan5 flavour physical number 2
> >mdio_bus/0.2:00/3: type notset
> >mdio_bus/0.2:00/4: type notset
> >mdio_bus/0.2:00/5: type notset flavour dsa number 5
> >mdio_bus/0.2:00/6: type notset flavour dsa number 6
> >mdio_bus/0.4:00/0: type eth netdev lan6 flavour physical number 0
> >mdio_bus/0.4:00/1: type eth netdev lan7 flavour physical number 1
> >mdio_bus/0.4:00/2: type eth netdev lan8 flavour physical number 2
> >mdio_bus/0.4:00/3: type eth netdev optical3 flavour physical number 3
> >mdio_bus/0.4:00/4: type eth netdev optical4 flavour physical number 4
> >mdio_bus/0.4:00/5: type notset
> >mdio_bus/0.4:00/6: type notset
> >mdio_bus/0.4:00/7: type notset
> >mdio_bus/0.4:00/8: type notset
> >mdio_bus/0.4:00/9: type notset flavour dsa number 9

> That is basically front panel number for physical ports.

You cannot make that assumption. As you can see here, we have 3 ports
with the number 0.

Look at clearfog, armada-388-clearfog.dts. port 0=lan5, port 1=lan4
port 2=lan3, port 3=lan2, port 4=lan1, port 5=cpu, port 6=lan6.

The hardware and mechanical engineer is free to wire switch ports to
the front panel however they want. That is why we put the netdev name
in device tree.

    Andrew

^ permalink raw reply

* Re: [PATCH net-next, 2/2] hv_netvsc: Add range checking for rx packet offset and length
From: Vitaly Kuznetsov @ 2018-03-23 15:17 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: olaf, sthemmin, netdev, haiyangz, linux-kernel, devel, davem
In-Reply-To: <20180322190114.25596-3-haiyangz@linuxonhyperv.com>

Haiyang Zhang <haiyangz@linuxonhyperv.com> writes:

> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> This patch adds range checking for rx packet offset and length.
> It may only happen if there is a host side bug.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/net/hyperv/hyperv_net.h |  1 +
>  drivers/net/hyperv/netvsc.c     | 17 +++++++++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index 0db3bd1ea06f..49c05ac894e5 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -793,6 +793,7 @@ struct netvsc_device {
>
>  	/* Receive buffer allocated by us but manages by NetVSP */
>  	void *recv_buf;
> +	u32 recv_buf_size; /* allocated bytes */
>  	u32 recv_buf_gpadl_handle;
>  	u32 recv_section_cnt;
>  	u32 recv_section_size;
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 1ddb2c39b6e4..a6700d65f206 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -289,6 +289,8 @@ static int netvsc_init_buf(struct hv_device *device,
>  		goto cleanup;
>  	}
>
> +	net_device->recv_buf_size = buf_size;
> +
>  	/*
>  	 * Establish the gpadl handle for this buffer on this
>  	 * channel.  Note: This call uses the vmbus connection rather
> @@ -1095,11 +1097,22 @@ static int netvsc_receive(struct net_device *ndev,
>
>  	/* Each range represents 1 RNDIS pkt that contains 1 ethernet frame */
>  	for (i = 0; i < count; i++) {
> -		void *data = recv_buf
> -			+ vmxferpage_packet->ranges[i].byte_offset;
> +		u32 offset = vmxferpage_packet->ranges[i].byte_offset;
>  		u32 buflen = vmxferpage_packet->ranges[i].byte_count;
> +		void *data;
>  		int ret;
>
> +		if (unlikely(offset + buflen > net_device->recv_buf_size)) {
> +			status = NVSP_STAT_FAIL;
> +			netif_err(net_device_ctx, rx_err, ndev,
> +				  "Packet offset:%u + len:%u too big\n",
> +				  offset, buflen);

This shouldn't happen, of course, but I'd rather ratelimit this error or
even used something like netdev_WARN_ONCE().

> +
> +			continue;
> +		}
> +
> +		data = recv_buf + offset;
> +
>  		trace_rndis_recv(ndev, q_idx, data);
>
>  		/* Pass it to the upper layer */

-- 
  Vitaly

^ permalink raw reply

* Re: [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink
From: David Ahern @ 2018-03-23 15:13 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski,
	David Ahern
In-Reply-To: <20180323150516.GE2125@nanopsycho>

On 3/23/18 9:05 AM, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 04:03:40PM CET, dsa@cumulusnetworks.com wrote:
>> On 3/23/18 9:01 AM, Jiri Pirko wrote:
>>> Fri, Mar 23, 2018 at 03:31:02PM CET, dsa@cumulusnetworks.com wrote:
>>>> On 3/23/18 12:50 AM, Jiri Pirko wrote:
>>>>>> +void nsim_devlink_setup(struct netdevsim *ns)
>>>>>> +{
>>>>>> +	struct net *net = dev_net(ns->netdev);
>>>>>> +	bool *reg_devlink = net_generic(net, nsim_devlink_id);
>>>>>> +	struct devlink *devlink;
>>>>>> +	int err = -ENOMEM;
>>>>>> +
>>>>>> +	/* only one device per namespace controls devlink */
>>>>>> +	if (!*reg_devlink) {
>>>>>> +		ns->devlink = NULL;
>>>>>> +		return;
>>>>>> +	}
>>>>>> +
>>>>>> +	devlink = devlink_alloc(&nsim_devlink_ops, 0);
>>>>>> +	if (!devlink)
>>>>>> +		return;
>>>>>> +
>>>>>> +	devlink_net_set(devlink, net);
>>>>>> +	err = devlink_register(devlink, &ns->dev);
>>>>>
>>>>> This reg_devlink construct looks odd. Why don't you leave the devlink
>>>>> instance in init_ns?
>>>>
>>>> It is a per-network namespace resource controller. Since struct devlink
>>>
>>> Wait a second. What do you mean by "per-network namespace"? Devlink
>>> instance is always associated with one physical device. Like an ASIC.
>>>
>>>
>>>> has a net entry, the simplest design is to put it into the namespace of
>>>> the controller. Without it, controlling resource sizes in namespace
>>>> 'foobar' has to be done from init_net, which is just wrong.
>>
>> you need to look at how netdevsim creates a device per netdevice.
> 
> That means one devlink instance for each netdevsim device, doesn't it?
> 

yes.

^ permalink raw reply

* Re: [PATCH] of_net: Implement of_get_nvmem_mac_address helper
From: Andrew Lunn @ 2018-03-23 15:11 UTC (permalink / raw)
  To: Mike Looijmans
  Cc: netdev, linux-kernel, devicetree, f.fainelli, robh+dt,
	frowand.list
In-Reply-To: <1521815074-30424-1-git-send-email-mike.looijmans@topic.nl>

On Fri, Mar 23, 2018 at 03:24:34PM +0100, Mike Looijmans wrote:
> It's common practice to store MAC addresses for network interfaces into
> nvmem devices. However the code to actually do this in the kernel lacks,
> so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
> address from an nvmem cell provider.
> 
> This is particulary useful on devices where the ethernet interface cannot
> be configured by the bootloader, for example because it's in an FPGA.
> 
> Tested by adapting the cadence macb driver to call this instead of
> of_get_mac_address().

Hi Mike

Please can you document the device tree binding. I assume you are
adding a nvmen-cells and nvmem-cell-names to the Ethernet node in
device tree.

> +/**
> + * Search the device tree for a MAC address, by calling of_get_mac_address
> + * and if that doesn't provide an address, fetch it from an nvmem provider
> + * using the name 'mac-address'.
> + * On success, copies the new address is into memory pointed to by addr and
> + * returns 0. Returns a negative error code otherwise.
> + * @dev:	Pointer to the device containing the device_node
> + * @addr:	Pointer to receive the MAC address using ether_addr_copy()
> + */
> +int of_get_nvmem_mac_address(struct device *dev, char *addr)
> +{
> +	const char *mac;
> +	struct nvmem_cell *cell;
> +	size_t len;
> +	int ret;
> +
> +	mac = of_get_mac_address(dev->of_node);
> +	if (mac) {
> +		ether_addr_copy(addr, mac);
> +		return 0;
> +	}

Is there a need to add a new API? Could of_get_mac_address() be
extended to look in NVMEM? The MAC driver does not care. It is saying,
using OF get me a MAC address. One API seems sufficient, and would
mean you don't need to change the MAC drivers.

     Andrew

^ permalink raw reply

* [PATCH net-next] devlink: Remove top_hierarchy arg for DEVLINK disabled path
From: David Ahern @ 2018-03-23 15:09 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern

Earlier change missed the path where CONFIG_NET_DEVLINK is disabled.
Thanks to Jiri for spotting.

Fixes: 145307460ba9 ("devlink: Remove top_hierarchy arg to devlink_resource_register")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/devlink.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index d5b707375e48..e21d8cadd480 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -559,7 +559,6 @@ devlink_dpipe_match_put(struct sk_buff *skb,
 static inline int
 devlink_resource_register(struct devlink *devlink,
 			  const char *resource_name,
-			  bool top_hierarchy,
 			  u64 resource_size,
 			  u64 resource_id,
 			  u64 parent_resource_id,
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH net-next] ibmvnic: Potential NULL dereference in clean_one_tx_pool()
From: Thomas Falcon @ 2018-03-23 15:06 UTC (permalink / raw)
  To: Dan Carpenter, Benjamin Herrenschmidt
  Cc: netdev, kernel-janitors, John Allen, Paul Mackerras, linuxppc-dev
In-Reply-To: <20180323113615.GA28518@mwanda>

On 03/23/2018 06:36 AM, Dan Carpenter wrote:
> There is an && vs || typo here, which potentially leads to a NULL
> dereference.

Thanks for catching that!

>
> Fixes: e9e1e97884b7 ("ibmvnic: Update TX pool cleaning routine")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 5632c030811b..0389a7a52152 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1135,7 +1135,7 @@ static void clean_one_tx_pool(struct ibmvnic_adapter *adapter,
>  	u64 tx_entries;
>  	int i;
>
> -	if (!tx_pool && !tx_pool->tx_buff)
> +	if (!tx_pool || !tx_pool->tx_buff)
>  		return;
>
>  	tx_entries = tx_pool->num_buffers;
>

^ permalink raw reply

* [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Vicentiu Galanopulo @ 2018-03-23 15:05 UTC (permalink / raw)
  To: netdev, linux-kernel, robh+dt, mark.rutland, davem, marcel,
	devicetree
  Cc: madalin.bucur, alexandru.marginean

Reason for this patch is that the Inphi PHY has a
vendor specific address space for accessing the
C45 MDIO registers - starting from 0x1e.

A search of the dev-addr property is done in of_mdiobus_register.
If the property is found in the PHY node,
of_mdiobus_register_static_phy is called. This is a
wrapper function for of_mdiobus_register_phy which finds the
device in package based on dev-addr and fills devices_addrs:
devices_addrs is a new field added to phy_c45_device_ids.
This new field will store the dev-addr property on the same
index where the device in package has been found.
In order to have dev-addr in get_phy_c45_ids(), mdio_c45_ids is
passed from of_mdio.c to phy_device.c as an external variable.
In get_phy_device a copy of the mdio_c45_ids is done over the
local c45_ids (wich are empty). After the copying, the c45_ids
will also contain the static device found from dev-addr.
Having dev-addr stored in devices_addrs, in get_phy_c45_ids(),
when probing the identifiers, dev-addr can be extracted from
devices_addrs and probed if devices_addrs[current_identifier]
is not 0.
This way changing the kernel API is avoided completely.

As a plus to this patch, num_ids in get_phy_c45_ids,
has the value 8 (ARRAY_SIZE(c45_ids->device_ids)),
but the u32 *devs can store 32 devices in the bitfield.
If a device is stored in *devs, in bits 32 to 9, it
will not be found. This is the reason for changing
in phy.h, the size of device_ids array.

Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@nxp.com>
---
 Documentation/devicetree/bindings/net/phy.txt |  6 ++
 drivers/net/phy/phy_device.c                  | 22 +++++-
 drivers/of/of_mdio.c                          | 98 ++++++++++++++++++++++++++-
 include/linux/phy.h                           |  5 +-
 4 files changed, 125 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index d2169a5..82692e2 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -61,6 +61,11 @@ Optional Properties:
 - reset-deassert-us: Delay after the reset was deasserted in microseconds.
   If this property is missing the delay will be skipped.
 
+- dev-addr: If set, it indicates the device address of the PHY to be used
+  when accessing the C45 PHY registers over MDIO. It is used for vendor specific
+  register space addresses that do no conform to standard address for the MDIO
+  registers (e.g. MMD30)
+
 Example:
 
 ethernet-phy@0 {
@@ -72,4 +77,5 @@ ethernet-phy@0 {
 	reset-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
 	reset-assert-us = <1000>;
 	reset-deassert-us = <2000>;
+	dev-addr = <0x1e>;
 };
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b285323..f5051cf6 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -71,6 +71,7 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
 
 static struct phy_driver genphy_driver;
 extern struct phy_driver genphy_10g_driver;
+extern struct phy_c45_device_ids mdio_c45_ids;
 
 static LIST_HEAD(phy_fixup_list);
 static DEFINE_MUTEX(phy_fixup_lock);
@@ -457,7 +458,7 @@ static int get_phy_c45_devs_in_pkg(struct mii_bus *bus, int addr, int dev_addr,
 static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
 			   struct phy_c45_device_ids *c45_ids) {
 	int phy_reg;
-	int i, reg_addr;
+	int i, reg_addr, dev_addr;
 	const int num_ids = ARRAY_SIZE(c45_ids->device_ids);
 	u32 *devs = &c45_ids->devices_in_package;
 
@@ -493,13 +494,23 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
 		if (!(c45_ids->devices_in_package & (1 << i)))
 			continue;
 
-		reg_addr = MII_ADDR_C45 | i << 16 | MII_PHYSID1;
+		/* if c45_ids->devices_addrs for the current id is not 0,
+		 * then dev-addr was defined in the device tree node,
+		 * and the PHY as been seen as a valid device, and added,
+		 * in the package. In this case we can use the
+		 * dev-addr(c45_ids->devices_addrs[i]) to do the MDIO
+		 * reading of the PHY ID.
+		 */
+		dev_addr = !!c45_ids->devices_addrs[i] ?
+					c45_ids->devices_addrs[i] : i;
+
+		reg_addr = MII_ADDR_C45 | dev_addr << 16 | MII_PHYSID1;
 		phy_reg = mdiobus_read(bus, addr, reg_addr);
 		if (phy_reg < 0)
 			return -EIO;
 		c45_ids->device_ids[i] = (phy_reg & 0xffff) << 16;
 
-		reg_addr = MII_ADDR_C45 | i << 16 | MII_PHYSID2;
+		reg_addr = MII_ADDR_C45 | dev_addr << 16 | MII_PHYSID2;
 		phy_reg = mdiobus_read(bus, addr, reg_addr);
 		if (phy_reg < 0)
 			return -EIO;
@@ -566,6 +577,11 @@ struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
 	u32 phy_id = 0;
 	int r;
 
+	/* copy the external mdio_c45_ids (which may contain the id's found
+	 * by serching the device tree dev-addr property) to local c45_ids
+	 */
+	memcpy(&c45_ids, &mdio_c45_ids, sizeof(struct phy_c45_device_ids));
+
 	r = get_phy_id(bus, addr, &phy_id, is_c45, &c45_ids);
 	if (r)
 		return ERR_PTR(r);
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 8c0c927..cbc34f6 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -24,6 +24,8 @@
 
 #define DEFAULT_GPIO_RESET_DELAY	10	/* in microseconds */
 
+struct phy_c45_device_ids mdio_c45_ids = {0};
+
 MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
 MODULE_LICENSE("GPL");
 
@@ -190,6 +192,71 @@ static bool of_mdiobus_child_is_phy(struct device_node *child)
 	return false;
 }
 
+static void of_fill_c45ids_devs_addrs(u32 dev_addr)
+{
+	int i;
+	const int num_ids = ARRAY_SIZE(mdio_c45_ids.device_ids);
+
+	/* Search through all Device Identifiers and set
+	 * dev_addr in mdio_c45_ids.devices_addrs,
+	 * if the device bit is set in
+	 * mdio_c45_ids.devices_in_package
+	 */
+	for (i = 1; i < num_ids; i++) {
+		if (!(mdio_c45_ids.devices_in_package & (1 << i)))
+			continue;
+
+		mdio_c45_ids.devices_addrs[i] = dev_addr;
+		break;
+	}
+}
+
+static int of_find_devaddr_in_pkg(struct mii_bus *bus, u32 addr,
+				  u32 dev_addr)
+{
+	u32 *devs = &mdio_c45_ids.devices_in_package;
+	int phy_reg, reg_addr;
+
+	reg_addr = MII_ADDR_C45 | dev_addr << 16 | MDIO_DEVS2;
+	phy_reg = mdiobus_read(bus, addr, reg_addr);
+	if (phy_reg < 0)
+		return -EIO;
+
+	*devs = (phy_reg & 0xffff) << 16;
+
+	reg_addr = MII_ADDR_C45 | dev_addr << 16 | MDIO_DEVS1;
+	phy_reg = mdiobus_read(bus, addr, reg_addr);
+	if (phy_reg < 0)
+		return -EIO;
+
+	*devs |= (phy_reg & 0xffff);
+
+	return 0;
+}
+
+/*
+ * Finds the device in package and populates the mdio_c45_ids
+ * if any device is found at dev_addr address. After this
+ * the PHY is registered
+ */
+static int of_mdiobus_register_static_phy(struct mii_bus *mdio,
+					  struct device_node *child,
+					  u32 addr, u32 dev_addr)
+{
+	int dev_err = 0;
+
+	if (!dev_addr)
+		goto exit_register_phy;
+
+	dev_err = of_find_devaddr_in_pkg(mdio, addr, dev_addr);
+
+	if (!dev_err)
+		of_fill_c45ids_devs_addrs(dev_addr);
+
+exit_register_phy:
+	return of_mdiobus_register_phy(mdio, child, addr);
+}
+
 /**
  * of_mdiobus_register - Register mii_bus and create PHYs from the device tree
  * @mdio: pointer to mii_bus structure
@@ -202,7 +269,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 {
 	struct device_node *child;
 	bool scanphys = false;
+	bool dev_addr_found = true;
 	int addr, rc;
+	int dev_addr = 0;
+	int ret;
 
 	/* Do not continue if the node is disabled */
 	if (!of_device_is_available(np))
@@ -226,6 +296,14 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 
 	/* Loop over the child nodes and register a phy_device for each phy */
 	for_each_available_child_of_node(np, child) {
+		/* Check if dev-addr is set in the PHY node */
+		ret = of_property_read_u32(child, "dev-addr", &dev_addr);
+
+		if (ret < 0) {
+			/* either not set or invalid */
+			dev_addr_found = false;
+		}
+
 		addr = of_mdio_parse_addr(&mdio->dev, child);
 		if (addr < 0) {
 			scanphys = true;
@@ -233,7 +311,11 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 		}
 
 		if (of_mdiobus_child_is_phy(child))
-			rc = of_mdiobus_register_phy(mdio, child, addr);
+			if (dev_addr_found)
+				rc = of_mdiobus_register_static_phy(mdio, child,
+								    addr, dev_addr);
+			else
+				rc = of_mdiobus_register_phy(mdio, child, addr);
 		else
 			rc = of_mdiobus_register_device(mdio, child, addr);
 
@@ -248,8 +330,16 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 	if (!scanphys)
 		return 0;
 
+	/* reset device found variable */
+	dev_addr_found = true;
+
 	/* auto scan for PHYs with empty reg property */
 	for_each_available_child_of_node(np, child) {
+		/* Check if dev-addr is set in the PHY node,
+		 * for PHYs which don't have reg property set
+		 */
+		ret = of_property_read_u32(child, "dev-addr", &dev_addr);
+
 		/* Skip PHYs with reg property set */
 		if (of_find_property(child, "reg", NULL))
 			continue;
@@ -264,7 +354,11 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 				 child->name, addr);
 
 			if (of_mdiobus_child_is_phy(child)) {
-				rc = of_mdiobus_register_phy(mdio, child, addr);
+				if (dev_addr_found)
+					rc = of_mdiobus_register_static_phy(mdio, child,
+									    addr, dev_addr);
+				else
+					rc = of_mdiobus_register_phy(mdio, child, addr);
 				if (rc && rc != -ENODEV)
 					goto unregister;
 			}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5a9b175..161ad90 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -357,10 +357,13 @@ enum phy_state {
  * struct phy_c45_device_ids - 802.3-c45 Device Identifiers
  * @devices_in_package: Bit vector of devices present.
  * @device_ids: The device identifer for each present device.
+ * @devices_addrs: The devices addresses from the device tree
+ *		   for each present device.
  */
 struct phy_c45_device_ids {
 	u32 devices_in_package;
-	u32 device_ids[8];
+	u32 device_ids[32];
+	u32 devices_addrs[32];
 };
 
 /* phy_device: An instance of a PHY
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink
From: Jiri Pirko @ 2018-03-23 15:05 UTC (permalink / raw)
  To: David Ahern
  Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski,
	David Ahern
In-Reply-To: <e7900c83-3dbd-5e7a-57ec-2647e0371e75@cumulusnetworks.com>

Fri, Mar 23, 2018 at 04:03:40PM CET, dsa@cumulusnetworks.com wrote:
>On 3/23/18 9:01 AM, Jiri Pirko wrote:
>> Fri, Mar 23, 2018 at 03:31:02PM CET, dsa@cumulusnetworks.com wrote:
>>> On 3/23/18 12:50 AM, Jiri Pirko wrote:
>>>>> +void nsim_devlink_setup(struct netdevsim *ns)
>>>>> +{
>>>>> +	struct net *net = dev_net(ns->netdev);
>>>>> +	bool *reg_devlink = net_generic(net, nsim_devlink_id);
>>>>> +	struct devlink *devlink;
>>>>> +	int err = -ENOMEM;
>>>>> +
>>>>> +	/* only one device per namespace controls devlink */
>>>>> +	if (!*reg_devlink) {
>>>>> +		ns->devlink = NULL;
>>>>> +		return;
>>>>> +	}
>>>>> +
>>>>> +	devlink = devlink_alloc(&nsim_devlink_ops, 0);
>>>>> +	if (!devlink)
>>>>> +		return;
>>>>> +
>>>>> +	devlink_net_set(devlink, net);
>>>>> +	err = devlink_register(devlink, &ns->dev);
>>>>
>>>> This reg_devlink construct looks odd. Why don't you leave the devlink
>>>> instance in init_ns?
>>>
>>> It is a per-network namespace resource controller. Since struct devlink
>> 
>> Wait a second. What do you mean by "per-network namespace"? Devlink
>> instance is always associated with one physical device. Like an ASIC.
>> 
>> 
>>> has a net entry, the simplest design is to put it into the namespace of
>>> the controller. Without it, controlling resource sizes in namespace
>>> 'foobar' has to be done from init_net, which is just wrong.
>
>you need to look at how netdevsim creates a device per netdevice.

That means one devlink instance for each netdevsim device, doesn't it?

^ permalink raw reply

* Re: [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink
From: David Ahern @ 2018-03-23 15:03 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski,
	David Ahern
In-Reply-To: <20180323150149.GD2125@nanopsycho>

On 3/23/18 9:01 AM, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 03:31:02PM CET, dsa@cumulusnetworks.com wrote:
>> On 3/23/18 12:50 AM, Jiri Pirko wrote:
>>>> +void nsim_devlink_setup(struct netdevsim *ns)
>>>> +{
>>>> +	struct net *net = dev_net(ns->netdev);
>>>> +	bool *reg_devlink = net_generic(net, nsim_devlink_id);
>>>> +	struct devlink *devlink;
>>>> +	int err = -ENOMEM;
>>>> +
>>>> +	/* only one device per namespace controls devlink */
>>>> +	if (!*reg_devlink) {
>>>> +		ns->devlink = NULL;
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	devlink = devlink_alloc(&nsim_devlink_ops, 0);
>>>> +	if (!devlink)
>>>> +		return;
>>>> +
>>>> +	devlink_net_set(devlink, net);
>>>> +	err = devlink_register(devlink, &ns->dev);
>>>
>>> This reg_devlink construct looks odd. Why don't you leave the devlink
>>> instance in init_ns?
>>
>> It is a per-network namespace resource controller. Since struct devlink
> 
> Wait a second. What do you mean by "per-network namespace"? Devlink
> instance is always associated with one physical device. Like an ASIC.
> 
> 
>> has a net entry, the simplest design is to put it into the namespace of
>> the controller. Without it, controlling resource sizes in namespace
>> 'foobar' has to be done from init_net, which is just wrong.

you need to look at how netdevsim creates a device per netdevice.

^ permalink raw reply


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