[net-next] bnxt_en: Allow to set switchdev mode without existing VFs
Commit Message
Remove an inability of bnxt_en driver to set eswitch to switchdev
mode without existing VFs by:
1. Allow to set switchdev mode in bnxt_dl_eswitch_mode_set() so
representors are created only when num_vfs > 0 otherwise just
set bp->eswitch_mode
2. Do not automatically change bp->eswitch_mode during
bnxt_vf_reps_create() and bnxt_vf_reps_destroy() calls so
the eswitch mode is managed only by an user by devlink.
Just set temporarily bp->eswitch_mode to legacy to avoid
re-opening of representors during destroy.
3. Create representors in bnxt_sriov_enable() if current eswitch
mode is switchdev one
Tested by this sequence:
1. Set PF interface up
2. Set PF's eswitch mode to switchdev
3. Created N VFs
4. Checked that N representors were created
5. Set eswitch mode to legacy
6. Checked that representors were deleted
7. Set eswitch mode back to switchdev
8. Checked that representros were re-created
9. Deleted all VFs
10. Checked that all representors were deleted as well
11. Checked that current eswitch mode is still switchdev
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
.../net/ethernet/broadcom/bnxt/bnxt_sriov.c | 16 ++++++++++
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 29 ++++++++++++-------
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 6 ++++
3 files changed, 41 insertions(+), 10 deletions(-)
Comments
On Thu, Apr 06, 2023 at 03:04:55PM +0200, Ivan Vecera wrote:
> Remove an inability of bnxt_en driver to set eswitch to switchdev
> mode without existing VFs by:
>
> 1. Allow to set switchdev mode in bnxt_dl_eswitch_mode_set() so
> representors are created only when num_vfs > 0 otherwise just
> set bp->eswitch_mode
> 2. Do not automatically change bp->eswitch_mode during
> bnxt_vf_reps_create() and bnxt_vf_reps_destroy() calls so
> the eswitch mode is managed only by an user by devlink.
> Just set temporarily bp->eswitch_mode to legacy to avoid
> re-opening of representors during destroy.
> 3. Create representors in bnxt_sriov_enable() if current eswitch
> mode is switchdev one
>
> Tested by this sequence:
> 1. Set PF interface up
> 2. Set PF's eswitch mode to switchdev
> 3. Created N VFs
> 4. Checked that N representors were created
> 5. Set eswitch mode to legacy
> 6. Checked that representors were deleted
> 7. Set eswitch mode back to switchdev
> 8. Checked that representros were re-created
Why do you think that this last item is the right behavior?
IMHO all configurations which were done after you switched mode
should be cleared and not recreated while toggling.
Thanks
> 9. Deleted all VFs
> 10. Checked that all representors were deleted as well
> 11. Checked that current eswitch mode is still switchdev
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
On 09. 04. 23 11:02, Leon Romanovsky wrote:
> On Thu, Apr 06, 2023 at 03:04:55PM +0200, Ivan Vecera wrote:
>> Remove an inability of bnxt_en driver to set eswitch to switchdev
>> mode without existing VFs by:
>>
>> 1. Allow to set switchdev mode in bnxt_dl_eswitch_mode_set() so
>> representors are created only when num_vfs > 0 otherwise just
>> set bp->eswitch_mode
>> 2. Do not automatically change bp->eswitch_mode during
>> bnxt_vf_reps_create() and bnxt_vf_reps_destroy() calls so
>> the eswitch mode is managed only by an user by devlink.
>> Just set temporarily bp->eswitch_mode to legacy to avoid
>> re-opening of representors during destroy.
>> 3. Create representors in bnxt_sriov_enable() if current eswitch
>> mode is switchdev one
>>
>> Tested by this sequence:
>> 1. Set PF interface up
>> 2. Set PF's eswitch mode to switchdev
>> 3. Created N VFs
>> 4. Checked that N representors were created
>> 5. Set eswitch mode to legacy
>> 6. Checked that representors were deleted
>> 7. Set eswitch mode back to switchdev
>> 8. Checked that representros were re-created
>
> Why do you think that this last item is the right behavior?
> IMHO all configurations which were done after you switched mode
> should be cleared and not recreated while toggling.
No, I mean that if I switch back to switchdev mode then representors
should be created again for existing virtual functions not that
representors should be restored with their existing state...
So the point 8 should say:
"8. Checked that representors exist again for VFs"
Ivan
On Thu, 2023-04-06 at 15:04 +0200, Ivan Vecera wrote:
> Remove an inability of bnxt_en driver to set eswitch to switchdev
> mode without existing VFs by:
>
> 1. Allow to set switchdev mode in bnxt_dl_eswitch_mode_set() so
> representors are created only when num_vfs > 0 otherwise just
> set bp->eswitch_mode
> 2. Do not automatically change bp->eswitch_mode during
> bnxt_vf_reps_create() and bnxt_vf_reps_destroy() calls so
> the eswitch mode is managed only by an user by devlink.
> Just set temporarily bp->eswitch_mode to legacy to avoid
> re-opening of representors during destroy.
> 3. Create representors in bnxt_sriov_enable() if current eswitch
> mode is switchdev one
>
> Tested by this sequence:
> 1. Set PF interface up
> 2. Set PF's eswitch mode to switchdev
> 3. Created N VFs
> 4. Checked that N representors were created
> 5. Set eswitch mode to legacy
> 6. Checked that representors were deleted
> 7. Set eswitch mode back to switchdev
> 8. Checked that representros were re-created
Could you please update the commit message and re-post?
Thanks!
Paolo
On 11. 04. 23 12:22, Paolo Abeni wrote:
> On Thu, 2023-04-06 at 15:04 +0200, Ivan Vecera wrote:
>> Remove an inability of bnxt_en driver to set eswitch to switchdev
>> mode without existing VFs by:
>>
>> 1. Allow to set switchdev mode in bnxt_dl_eswitch_mode_set() so
>> representors are created only when num_vfs > 0 otherwise just
>> set bp->eswitch_mode
>> 2. Do not automatically change bp->eswitch_mode during
>> bnxt_vf_reps_create() and bnxt_vf_reps_destroy() calls so
>> the eswitch mode is managed only by an user by devlink.
>> Just set temporarily bp->eswitch_mode to legacy to avoid
>> re-opening of representors during destroy.
>> 3. Create representors in bnxt_sriov_enable() if current eswitch
>> mode is switchdev one
>>
>> Tested by this sequence:
>> 1. Set PF interface up
>> 2. Set PF's eswitch mode to switchdev
>> 3. Created N VFs
>> 4. Checked that N representors were created
>> 5. Set eswitch mode to legacy
>> 6. Checked that representors were deleted
>> 7. Set eswitch mode back to switchdev
>> 8. Checked that representros were re-created
>
> Could you please update the commit message and re-post?
>
> Thanks!
of course..
I.
@@ -825,8 +825,24 @@ static int bnxt_sriov_enable(struct bnxt *bp, int *num_vfs)
if (rc)
goto err_out2;
+ if (bp->eswitch_mode != DEVLINK_ESWITCH_MODE_SWITCHDEV)
+ return 0;
+
+ /* Create representors for VFs in switchdev mode */
+ devl_lock(bp->dl);
+ rc = bnxt_vf_reps_create(bp);
+ devl_unlock(bp->dl);
+ if (rc) {
+ netdev_info(bp->dev, "Cannot enable VFS as representors cannot be created\n");
+ goto err_out3;
+ }
+
return 0;
+err_out3:
+ /* Disable SR-IOV */
+ pci_disable_sriov(bp->pdev);
+
err_out2:
/* Free the resources reserved for various VF's */
bnxt_hwrm_func_vf_resource_free(bp, *num_vfs);
@@ -356,10 +356,15 @@ void bnxt_vf_reps_destroy(struct bnxt *bp)
/* un-publish cfa_code_map so that RX path can't see it anymore */
kfree(bp->cfa_code_map);
bp->cfa_code_map = NULL;
- bp->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
- if (closed)
+ if (closed) {
+ /* Temporarily set legacy mode to avoid re-opening
+ * representors and restore switchdev mode after that.
+ */
+ bp->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
bnxt_open_nic(bp, false, false);
+ bp->eswitch_mode = DEVLINK_ESWITCH_MODE_SWITCHDEV;
+ }
rtnl_unlock();
/* Need to call vf_reps_destroy() outside of rntl_lock
@@ -482,7 +487,7 @@ static void bnxt_vf_rep_netdev_init(struct bnxt *bp, struct bnxt_vf_rep *vf_rep,
dev->min_mtu = ETH_ZLEN;
}
-static int bnxt_vf_reps_create(struct bnxt *bp)
+int bnxt_vf_reps_create(struct bnxt *bp)
{
u16 *cfa_code_map = NULL, num_vfs = pci_num_vf(bp->pdev);
struct bnxt_vf_rep *vf_rep;
@@ -535,7 +540,6 @@ static int bnxt_vf_reps_create(struct bnxt *bp)
/* publish cfa_code_map only after all VF-reps have been initialized */
bp->cfa_code_map = cfa_code_map;
- bp->eswitch_mode = DEVLINK_ESWITCH_MODE_SWITCHDEV;
netif_keep_dst(bp->dev);
return 0;
@@ -559,6 +563,7 @@ int bnxt_dl_eswitch_mode_set(struct devlink *devlink, u16 mode,
struct netlink_ext_ack *extack)
{
struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+ int ret = 0;
if (bp->eswitch_mode == mode) {
netdev_info(bp->dev, "already in %s eswitch mode\n",
@@ -570,7 +575,7 @@ int bnxt_dl_eswitch_mode_set(struct devlink *devlink, u16 mode,
switch (mode) {
case DEVLINK_ESWITCH_MODE_LEGACY:
bnxt_vf_reps_destroy(bp);
- return 0;
+ break;
case DEVLINK_ESWITCH_MODE_SWITCHDEV:
if (bp->hwrm_spec_code < 0x10803) {
@@ -578,15 +583,19 @@ int bnxt_dl_eswitch_mode_set(struct devlink *devlink, u16 mode,
return -ENOTSUPP;
}
- if (pci_num_vf(bp->pdev) == 0) {
- netdev_info(bp->dev, "Enable VFs before setting switchdev mode\n");
- return -EPERM;
- }
- return bnxt_vf_reps_create(bp);
+ /* Create representors for existing VFs */
+ if (pci_num_vf(bp->pdev) > 0)
+ ret = bnxt_vf_reps_create(bp);
+ break;
default:
return -EINVAL;
}
+
+ if (!ret)
+ bp->eswitch_mode = mode;
+
+ return ret;
}
#endif
@@ -14,6 +14,7 @@
#define MAX_CFA_CODE 65536
+int bnxt_vf_reps_create(struct bnxt *bp);
void bnxt_vf_reps_destroy(struct bnxt *bp);
void bnxt_vf_reps_close(struct bnxt *bp);
void bnxt_vf_reps_open(struct bnxt *bp);
@@ -37,6 +38,11 @@ int bnxt_dl_eswitch_mode_set(struct devlink *devlink, u16 mode,
#else
+static inline int bnxt_vf_reps_create(struct bnxt *bp)
+{
+ return 0;
+}
+
static inline void bnxt_vf_reps_close(struct bnxt *bp)
{
}