| Message ID | 20231127060512.1283336-1-sachin.bahadur@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [iwl-net,v4] ice: Block PF reinit if attached to bond | expand |
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 > >
> 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, 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 >> > >> >
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 >>>> >>>> >
> 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 > >> > > >> >
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 >> >> > >> >> >
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: