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 <[email protected]>
Signed-off-by: Sachin Bahadur <[email protected]>
---
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
Mon, Nov 27, 2023 at 07:05:12AM CET, [email protected] 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 <[email protected]>
>Signed-off-by: Sachin Bahadur <[email protected]>
>---
> 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
>
>
> 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
> >
> >
Mon, Nov 27, 2023 at 05:23:55PM CET, [email protected] 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
>> >
>> >
On 11/28/23 08:37, Jiri Pirko wrote:
> Mon, Nov 27, 2023 at 05:23:55PM CET, [email protected] 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
>>>>
>>>>
>
> From: Jiri Pirko <[email protected]>
> Sent: Monday, November 27, 2023 11:38 PM
> To: Bahadur, Sachin <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond
>
> Mon, Nov 27, 2023 at 05:23:55PM CET, [email protected] 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
> >> >
> >> >
Tue, Nov 28, 2023 at 06:45:47PM CET, [email protected] wrote:
>
>
>> From: Jiri Pirko <[email protected]>
>> Sent: Monday, November 27, 2023 11:38 PM
>> To: Bahadur, Sachin <[email protected]>
>> Cc: [email protected]; [email protected]; linux-
>> [email protected]
>> Subject: Re: [PATCH iwl-net v4] ice: Block PF reinit if attached to bond
>>
>> Mon, Nov 27, 2023 at 05:23:55PM CET, [email protected] 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
>> >> >
>> >> >