Date:   Thu, 13 Apr 2023 15:48:04 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Emeel Hakim <ehakim@...dia.com>, <davem@...emloft.net>,
        <kuba@...nel.org>, <pabeni@...hat.com>, <edumazet@...gle.com>,
        <sd@...asysnail.net>
CC:     <netdev@...r.kernel.org>, <leon@...nel.org>
Subject: Re: [PATCH net-next v5 0/5] Support MACsec VLAN



On 4/13/2023 3:56 AM, Emeel Hakim wrote:
> Dear maintainers,
> 
> This patch series introduces support for hardware (HW) offload MACsec
> devices with VLAN configuration. The patches address both scenarios
> where the VLAN header is both the inner and outer header for MACsec.
> 
> The changes include:
> 
> 1. Adding MACsec offload operation for VLAN.
> 2. Considering VLAN when accessing MACsec net device.
> 3. Currently offloading MACsec when it's configured over VLAN with
> current MACsec TX steering rules would wrongly insert the MACsec sec tag
> after inserting the VLAN header. This resulted in an ETHERNET | SECTAG |
> VLAN packet when ETHERNET | VLAN | SECTAG is configured. The patche
> handles this issue when configuring steering rules.
> 4. Adding MACsec rx_handler change support in case of a marked skb and a
> mismatch on the dst MAC address.
> 
> Please review these changes and let me know if you have any feedback or
> concerns.
> 
> Updates since v1:
> - Consult vlan_features when adding NETIF_F_HW_MACSEC.
> - Allow grep for the functions.
> - Add helper function to get the macsec operation to allow the compiler
>   to make some choice.
> 
> Updates since v2:
> - Don't use macros to allow direct navigattion from mdo functions to its
>   implementation.
> - Make the vlan_get_macsec_ops argument a const.
> - Check if the specific mdo function is available before calling it.
> - Enable NETIF_F_HW_MACSEC by default when the lower device has it enabled
>   and in case the lower device currently has NETIF_F_HW_MACSEC but disabled
>   let the new vlan device also have it disabled.
> 
> Updates since v3:
> - Split patch ("vlan: Add MACsec offload operations for VLAN interface")
>   to prevent mixing generic vlan code changes with driver changes.
> - Add mdo_open, stop and stats to support drivers which have those.
> - Don't fail if macsec offload operations are available but a specific
>   function is not, to support drivers which does not implement all
>   macsec offload operations.
> - Don't call find_rx_sc twice in the same loop, instead save the result
>   in a parameter and re-use it.
> - Completely remove _BUILD_VLAN_MACSEC_MDO macro, to prevent returning
>   from a macro.
> - Reorder the functions inside struct macsec_ops to match the struct
>   decleration.
>   
>  Updates since v4:
>  - Change subject line of ("macsec: Add MACsec rx_handler change support") and adapt commit message.
>  - Don't separate the new check in patch ("macsec: Add MACsec rx_handler change support")
>    from the previous if/else if.
>  - Drop"_found" from the parameter naming "rx_sc_found" and move the definition to
>    the relevant block.
>  - Remove "{}" since not needed around a single line.
> 
> Emeel Hakim (5):
>   vlan: Add MACsec offload operations for VLAN interface
>   net/mlx5: Enable MACsec offload feature for VLAN interface
>   net/mlx5: Support MACsec over VLAN
>   net/mlx5: Consider VLAN interface in MACsec TX steering rules
>   macsec: Don't rely solely on the dst MAC address to identify
>     destination MACsec device
> 

For the whole series:

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>

>  .../mellanox/mlx5/core/en_accel/macsec.c      |  42 +--
>  .../mellanox/mlx5/core/en_accel/macsec_fs.c   |   7 +
>  .../net/ethernet/mellanox/mlx5/core/en_main.c |   1 +
>  drivers/net/macsec.c                          |  13 +-
>  net/8021q/vlan_dev.c                          | 242 ++++++++++++++++++
>  5 files changed, 287 insertions(+), 18 deletions(-)
> 

Powered by blists - more mailing lists