[v4] ice: Block PF reinit if attached to bond

[PATCH iwl-net v4] ice: Block PF reinit if attached to bond

Posted by Sachin Bahadur 2 years ago

PF interface part of Bond should not allow driver reinit via devlink. Bond
config will be lost due to PF reinit. PF needs to be re-added to Bond
after PF reinit. ice_devlink_reload_down is called before PF driver reinit.
If PF is attached to bond, ice_devlink_reload_down returns error.

Fixes: trailer
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Sachin Bahadur <sachin.bahadur@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_devlink.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index f4e24d11ebd0..5fe88e949b09 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -457,6 +457,10 @@ ice_devlink_reload_down(struct devlink *devlink, bool netns_change,
 					   "Remove all VFs before doing reinit\n");
 			return -EOPNOTSUPP;
 		}
+		if (pf->lag && pf->lag->bonded) {
+			NL_SET_ERR_MSG_MOD(extack, "Remove all associated Bonds before doing reinit");
+			return -EBUSY;
+		}
 		ice_unload(pf);
 		return 0;
 	case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
--
2.25.1

Re: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond

Posted by Jiri Pirko 2 years ago

Mon, Nov 27, 2023 at 07:05:12AM CET, sachin.bahadur@intel.com wrote:
>PF interface part of Bond should not allow driver reinit via devlink. Bond
>config will be lost due to PF reinit. PF needs to be re-added to Bond
>after PF reinit. ice_devlink_reload_down is called before PF driver reinit.
>If PF is attached to bond, ice_devlink_reload_down returns error.
>
>Fixes: trailer
>Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
>Signed-off-by: Sachin Bahadur <sachin.bahadur@intel.com>
>---
> drivers/net/ethernet/intel/ice/ice_devlink.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
>index f4e24d11ebd0..5fe88e949b09 100644
>--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>@@ -457,6 +457,10 @@ ice_devlink_reload_down(struct devlink *devlink, bool netns_change,
> 					   "Remove all VFs before doing reinit\n");
> 			return -EOPNOTSUPP;
> 		}
>+		if (pf->lag && pf->lag->bonded) {
>+			NL_SET_ERR_MSG_MOD(extack, "Remove all associated Bonds before doing reinit");

Nack. Remove the netdev during re-init, that would solve your issue.
Looks like some checks are needed to be added in devlink code to make
sure drivers behave properly. I'm on in.



>+			return -EBUSY;
>+		}
> 		ice_unload(pf);
> 		return 0;
> 	case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
>--
>2.25.1
>
>

RE: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond

Posted by Bahadur, Sachin 2 years ago

 
> Nack. Remove the netdev during re-init, that would solve your issue.
> Looks like some checks are needed to be added in devlink code to make sure
> drivers behave properly. I'm on in.

Sure. This fix should apply to all drivers. Adding it in devlink makes more
sense. I am not a devlink expert, so I hope you or someone else can
help with it.

> 
> 
> >+			return -EBUSY;
> >+		}
> > 		ice_unload(pf);
> > 		return 0;
> > 	case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
> >--
> >2.25.1
> >
> >

Re: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond

Posted by Jiri Pirko 2 years ago

Mon, Nov 27, 2023 at 05:23:55PM CET, sachin.bahadur@intel.com wrote:
> 
>> Nack. Remove the netdev during re-init, that would solve your issue.
>> Looks like some checks are needed to be added in devlink code to make sure
>> drivers behave properly. I'm on in.
>
>Sure. This fix should apply to all drivers. Adding it in devlink makes more
>sense. I am not a devlink expert, so I hope you or someone else can
>help with it.

No, you misunderstood. I'll just add a check-warn in devlink for case
when port exists during reload. You need to fix it in your driver.

>
>> 
>> 
>> >+			return -EBUSY;
>> >+		}
>> > 		ice_unload(pf);
>> > 		return 0;
>> > 	case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
>> >--
>> >2.25.1
>> >
>> >

RE: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond

Posted by Bahadur, Sachin 2 years ago


> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Monday, November 27, 2023 11:38 PM
> To: Bahadur, Sachin <sachin.bahadur@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond
> 
> Mon, Nov 27, 2023 at 05:23:55PM CET, sachin.bahadur@intel.com wrote:
> >
> >> Nack. Remove the netdev during re-init, that would solve your issue.
> >> Looks like some checks are needed to be added in devlink code to make
> >> sure drivers behave properly. I'm on in.
> >
> >Sure. This fix should apply to all drivers. Adding it in devlink makes
> >more sense. I am not a devlink expert, so I hope you or someone else
> >can help with it.
> 
> No, you misunderstood. I'll just add a check-warn in devlink for case when port
> exists during reload. You need to fix it in your driver.


What should be fixed in my driver. Can you clarify ? 
And are suggesting I add the check-warn in devlink code ?


> >
> >>
> >>
> >> >+			return -EBUSY;
> >> >+		}
> >> > 		ice_unload(pf);
> >> > 		return 0;
> >> > 	case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
> >> >--
> >> >2.25.1
> >> >
> >> >

Re: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond

Posted by Jiri Pirko 2 years ago

Tue, Nov 28, 2023 at 06:45:47PM CET, sachin.bahadur@intel.com wrote:
>
>
>> From: Jiri Pirko <jiri@resnulli.us>
>> Sent: Monday, November 27, 2023 11:38 PM
>> To: Bahadur, Sachin <sachin.bahadur@intel.com>
>> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond
>> 
>> Mon, Nov 27, 2023 at 05:23:55PM CET, sachin.bahadur@intel.com wrote:
>> >
>> >> Nack. Remove the netdev during re-init, that would solve your issue.
>> >> Looks like some checks are needed to be added in devlink code to make
>> >> sure drivers behave properly. I'm on in.
>> >
>> >Sure. This fix should apply to all drivers. Adding it in devlink makes
>> >more sense. I am not a devlink expert, so I hope you or someone else
>> >can help with it.
>> 
>> No, you misunderstood. I'll just add a check-warn in devlink for case when port
>> exists during reload. You need to fix it in your driver.
>
>
>What should be fixed in my driver. Can you clarify ? 
>And are suggesting I add the check-warn in devlink code ?

Remove the netdev during re-init.


>
>
>> >
>> >>
>> >>
>> >> >+			return -EBUSY;
>> >> >+		}
>> >> > 		ice_unload(pf);
>> >> > 		return 0;
>> >> > 	case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
>> >> >--
>> >> >2.25.1
>> >> >
>> >> >

Re: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond

Posted by Przemek Kitszel 2 years ago

On 11/28/23 08:37, Jiri Pirko wrote:
> Mon, Nov 27, 2023 at 05:23:55PM CET, sachin.bahadur@intel.com wrote:
>>
>>> Nack. Remove the netdev during re-init, that would solve your issue.
>>> Looks like some checks are needed to be added in devlink code to make sure
>>> drivers behave properly. I'm on in.
>>
>> Sure. This fix should apply to all drivers. Adding it in devlink makes more
>> sense. I am not a devlink expert, so I hope you or someone else can
>> help with it.
> 
> No, you misunderstood. I'll just add a check-warn in devlink for case
> when port exists during reload. You need to fix it in your driver.

Having a message in log that reminds devs would be useful, thanks!

> 
>>
>>>
>>>
>>>> +			return -EBUSY;
>>>> +		}
>>>> 		ice_unload(pf);
>>>> 		return 0;
>>>> 	case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
>>>> --
>>>> 2.25.1
>>>>
>>>>
>