On Mon, Jun 10, 2024 at 09:44:33AM +0200, Michal Swiatkowski wrote:
> It is needed because subfunction port representor shouldn't configure
> the source VSI during representor creation.
> 
> Move the code to separate function and call it only in case the VF port
> representor is being created.
> 
> Signed-off-by: Michal Swiatkowski <michal.swiatkow...@linux.intel.com>

Hi Michal,

The nit below notwithstanding, this looks good to me.

Reviewed-by: Simon Horman <ho...@kernel.org>

> ---
>  drivers/net/ethernet/intel/ice/ice_eswitch.c | 55 ++++++++++++++------
>  drivers/net/ethernet/intel/ice/ice_eswitch.h | 10 ++++
>  drivers/net/ethernet/intel/ice/ice_repr.c    |  7 +++
>  3 files changed, 57 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c 
> b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> index 7b57a6561a5a..3f73f46111fc 100644
> --- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> @@ -117,17 +117,10 @@ static int ice_eswitch_setup_repr(struct ice_pf *pf, 
> struct ice_repr *repr)
>       struct ice_vsi *vsi = repr->src_vsi;
>       struct metadata_dst *dst;
>  
> -     ice_remove_vsi_fltr(&pf->hw, vsi->idx);
>       repr->dst = metadata_dst_alloc(0, METADATA_HW_PORT_MUX,
>                                      GFP_KERNEL);
>       if (!repr->dst)
> -             goto err_add_mac_fltr;
> -
> -     if (ice_vsi_update_security(vsi, ice_vsi_ctx_clear_antispoof))
> -             goto err_dst_free;
> -
> -     if (ice_vsi_add_vlan_zero(vsi))
> -             goto err_update_security;
> +             return -ENOMEM;
>  
>       netif_keep_dst(uplink_vsi->netdev);
>  
> @@ -136,16 +129,48 @@ static int ice_eswitch_setup_repr(struct ice_pf *pf, 
> struct ice_repr *repr)
>       dst->u.port_info.lower_dev = uplink_vsi->netdev;
>  
>       return 0;
> +}
>  
> -err_update_security:
> +/**
> + * ice_eswitch_cfg_vsi - configure VSI to work in slow-path
> + * @vsi: VSI structure of representee
> + * @mac: representee MAC
> + *
> + * Return: 0 on success, non-zero on error.
> + */
> +int ice_eswitch_cfg_vsi(struct ice_vsi *vsi, const u8 *mac)
> +{
> +     int err;
> +
> +     ice_remove_vsi_fltr(&vsi->back->hw, vsi->idx);
> +
> +     err = ice_vsi_update_security(vsi, ice_vsi_ctx_clear_antispoof);
> +     if (err)
> +             goto err_update_security;
> +
> +     err = ice_vsi_add_vlan_zero(vsi);
> +     if (err)
> +             goto err_vlan_zero;
> +
> +     return 0;
> +
> +err_vlan_zero:
>       ice_vsi_update_security(vsi, ice_vsi_ctx_set_antispoof);

nit: Please consider continuing the practice, that is used for the labels
     removed by this patch, of naming labels after what they do rather
     than what jumps to them.

> -err_dst_free:
> -     metadata_dst_free(repr->dst);
> -     repr->dst = NULL;
> -err_add_mac_fltr:
> -     ice_fltr_add_mac_and_broadcast(vsi, repr->parent_mac, ICE_FWD_TO_VSI);
> +err_update_security:
> +     ice_fltr_add_mac_and_broadcast(vsi, mac, ICE_FWD_TO_VSI);
>  
> -     return -ENODEV;
> +     return err;
> +}
> +

...

Reply via email to