diff mbox series

[net-next,v2,11/15] nfp: move devlink port type set after netdev registration

Message ID 20190322165636.1725-12-jiri@resnulli.us
State Superseded
Delegated to: David Miller
Headers show
Series devlink: small spring cleanup | expand

Commit Message

Jiri Pirko March 22, 2019, 4:56 p.m. UTC 
From: Jiri Pirko <jiri@mellanox.com>

Similar to other driver, move the port type set after netdev registration
is done. Along with that, clear the type before unregistration.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v1->v2:
- new patch
---
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c  | 11 ++++++++++-
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c |  4 ++++
 drivers/net/ethernet/netronome/nfp/nfp_port.h     |  2 ++
 3 files changed, 16 insertions(+), 1 deletion(-)

Comments

Jakub Kicinski March 23, 2019, 8:31 p.m. UTC | #1 
On Fri, 22 Mar 2019 17:56:32 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Similar to other driver, move the port type set after netdev registration
> is done. Along with that, clear the type before unregistration.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> index e9eca99cf493..cb59a18ec6a6 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> @@ -362,7 +362,6 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
>  	if (ret)
>  		return ret;
>  
> -	devlink_port_type_eth_set(&port->dl_port, port->netdev);
>  	devlink_port_attrs_set(&port->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
>  			       eth_port.label_port, eth_port.is_split,
>  			       eth_port.label_subport);
> @@ -377,6 +376,16 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
>  	devlink_port_unregister(&port->dl_port);
>  }
>  
> +void nfp_devlink_port_type_eth_set(struct nfp_port *port)
> +{
> +	devlink_port_type_eth_set(&port->dl_port, port->netdev);
> +}
> +
> +void nfp_devlink_port_type_clear(struct nfp_port *port)
> +{
> +	devlink_port_type_clear(&port->dl_port);
> +}
> +
>  struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
>  {
>  	struct nfp_app *app;
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> index 08f5fdbd8e41..e29744f2af02 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> @@ -169,6 +169,8 @@ nfp_net_pf_init_vnic(struct nfp_pf *pf, struct nfp_net *nn, unsigned int id)
>  		if (err)
>  			goto err_devlink_port_clean;
>  	}
> +	if (nn->port)
> +		nfp_devlink_port_type_eth_set(nn->port);
>  
>  	return 0;
>  
> @@ -218,6 +220,8 @@ nfp_net_pf_alloc_vnics(struct nfp_pf *pf, void __iomem *ctrl_bar,
>  
>  static void nfp_net_pf_clean_vnic(struct nfp_pf *pf, struct nfp_net *nn)
>  {
> +	if (nn->port)
> +		nfp_devlink_port_type_clear(nn->port);
>  	if (nfp_net_is_data_vnic(nn))
>  		nfp_app_vnic_clean(pf->app, nn);

Sorry for not noticing right away, I thought you just split my patch in
two, but it seems you reordered the app_vnic_init/clean calls with the
type setting.  I'd rather those calls happened on fully initialized
netdev (and port), could you perhaps just take this patch into the
series:

https://git.kernel.org/pub/scm/linux/kernel/git/kuba/linux.git/commit/?h=devlink-pci-ports&id=1debfdef5aef369412e72f370d11120d309a6a74

>  	if (nn->port)
Jiri Pirko March 23, 2019, 10:47 p.m. UTC | #2 
Sat, Mar 23, 2019 at 09:31:02PM CET, jakub.kicinski@netronome.com wrote:
>On Fri, 22 Mar 2019 17:56:32 +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Similar to other driver, move the port type set after netdev registration
>> is done. Along with that, clear the type before unregistration.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>> index e9eca99cf493..cb59a18ec6a6 100644
>> --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>> +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>> @@ -362,7 +362,6 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
>>  	if (ret)
>>  		return ret;
>>  
>> -	devlink_port_type_eth_set(&port->dl_port, port->netdev);
>>  	devlink_port_attrs_set(&port->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
>>  			       eth_port.label_port, eth_port.is_split,
>>  			       eth_port.label_subport);
>> @@ -377,6 +376,16 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
>>  	devlink_port_unregister(&port->dl_port);
>>  }
>>  
>> +void nfp_devlink_port_type_eth_set(struct nfp_port *port)
>> +{
>> +	devlink_port_type_eth_set(&port->dl_port, port->netdev);
>> +}
>> +
>> +void nfp_devlink_port_type_clear(struct nfp_port *port)
>> +{
>> +	devlink_port_type_clear(&port->dl_port);
>> +}
>> +
>>  struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
>>  {
>>  	struct nfp_app *app;
>> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
>> index 08f5fdbd8e41..e29744f2af02 100644
>> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
>> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
>> @@ -169,6 +169,8 @@ nfp_net_pf_init_vnic(struct nfp_pf *pf, struct nfp_net *nn, unsigned int id)
>>  		if (err)
>>  			goto err_devlink_port_clean;
>>  	}
>> +	if (nn->port)
>> +		nfp_devlink_port_type_eth_set(nn->port);
>>  
>>  	return 0;
>>  
>> @@ -218,6 +220,8 @@ nfp_net_pf_alloc_vnics(struct nfp_pf *pf, void __iomem *ctrl_bar,
>>  
>>  static void nfp_net_pf_clean_vnic(struct nfp_pf *pf, struct nfp_net *nn)
>>  {
>> +	if (nn->port)
>> +		nfp_devlink_port_type_clear(nn->port);
>>  	if (nfp_net_is_data_vnic(nn))
>>  		nfp_app_vnic_clean(pf->app, nn);
>
>Sorry for not noticing right away, I thought you just split my patch in
>two, but it seems you reordered the app_vnic_init/clean calls with the

I did the patch from scratch.


>type setting.  I'd rather those calls happened on fully initialized
>netdev (and port), could you perhaps just take this patch into the

I don't get it, you are commenting cleanup part. On init, the type set
happens on fully initialized devlink_port and netdev.


>series:
>
>https://git.kernel.org/pub/scm/linux/kernel/git/kuba/linux.git/commit/?h=devlink-pci-ports&id=1debfdef5aef369412e72f370d11120d309a6a74
>
>>  	if (nn->port)
Jakub Kicinski March 24, 2019, 3:04 a.m. UTC | #3 
On Sat, 23 Mar 2019 23:47:50 +0100, Jiri Pirko wrote:
> >type setting.  I'd rather those calls happened on fully initialized
> >netdev (and port), could you perhaps just take this patch into the  
> 
> I don't get it, you are commenting cleanup part. On init, the type set
> happens on fully initialized devlink_port and netdev.

I'm talking about internal nfp API calls, nfp_app_vnic_init() and
nfp_app_vnic_clean() should be called while vNIC is completely
initialized and registered from stack's PoV.  netdev, and devlink 
port fully initialized and set up.  I don't think it matters much, 
just an imperfection.
Jiri Pirko March 24, 2019, 8:01 a.m. UTC | #4 
Sun, Mar 24, 2019 at 04:04:05AM CET, jakub.kicinski@netronome.com wrote:
>On Sat, 23 Mar 2019 23:47:50 +0100, Jiri Pirko wrote:
>> >type setting.  I'd rather those calls happened on fully initialized
>> >netdev (and port), could you perhaps just take this patch into the  
>> 
>> I don't get it, you are commenting cleanup part. On init, the type set
>> happens on fully initialized devlink_port and netdev.
>
>I'm talking about internal nfp API calls, nfp_app_vnic_init() and
>nfp_app_vnic_clean() should be called while vNIC is completely
>initialized and registered from stack's PoV.  netdev, and devlink 
>port fully initialized and set up.  I don't think it matters much, 
>just an imperfection.

Oh, I get it. Will change the order. Thanks.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index e9eca99cf493..cb59a18ec6a6 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -362,7 +362,6 @@  int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 	if (ret)
 		return ret;
 
-	devlink_port_type_eth_set(&port->dl_port, port->netdev);
 	devlink_port_attrs_set(&port->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
 			       eth_port.label_port, eth_port.is_split,
 			       eth_port.label_subport);
@@ -377,6 +376,16 @@  void nfp_devlink_port_unregister(struct nfp_port *port)
 	devlink_port_unregister(&port->dl_port);
 }
 
+void nfp_devlink_port_type_eth_set(struct nfp_port *port)
+{
+	devlink_port_type_eth_set(&port->dl_port, port->netdev);
+}
+
+void nfp_devlink_port_type_clear(struct nfp_port *port)
+{
+	devlink_port_type_clear(&port->dl_port);
+}
+
 struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
 {
 	struct nfp_app *app;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 08f5fdbd8e41..e29744f2af02 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -169,6 +169,8 @@  nfp_net_pf_init_vnic(struct nfp_pf *pf, struct nfp_net *nn, unsigned int id)
 		if (err)
 			goto err_devlink_port_clean;
 	}
+	if (nn->port)
+		nfp_devlink_port_type_eth_set(nn->port);
 
 	return 0;
 
@@ -218,6 +220,8 @@  nfp_net_pf_alloc_vnics(struct nfp_pf *pf, void __iomem *ctrl_bar,
 
 static void nfp_net_pf_clean_vnic(struct nfp_pf *pf, struct nfp_net *nn)
 {
+	if (nn->port)
+		nfp_devlink_port_type_clear(nn->port);
 	if (nfp_net_is_data_vnic(nn))
 		nfp_app_vnic_clean(pf->app, nn);
 	if (nn->port)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.h b/drivers/net/ethernet/netronome/nfp/nfp_port.h
index 90ae053f5c07..d7fd203bb180 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_port.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.h
@@ -131,6 +131,8 @@  int nfp_net_refresh_port_table_sync(struct nfp_pf *pf);
 
 int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port);
 void nfp_devlink_port_unregister(struct nfp_port *port);
+void nfp_devlink_port_type_eth_set(struct nfp_port *port);
+void nfp_devlink_port_type_clear(struct nfp_port *port);
 
 /**
  * Mac stats (0x0000 - 0x0200)