? 2022/4/11 20:29, Eli Cohen ??:> Support HW offloaded filtering of MAC/VLAN packets.
> To allow that, we add a handler to handle VLAN configurations coming
> through the control VQ. Two operations are supported.
>
> 1. Adding VLAN - in this case, an entry will be added to the RX flow
> table that will allow the combination of the MAC/VLAN to be
> forwarded to the TIR.
> 2. Removing VLAN - will remove the entry from the flow table,
> effectively blocking such packets from going through.
>
> Currently the control VQ does not propagate changes to the MAC of the
> VLAN device so we always use the MAC of the parent device.
>
> Examples:
> 1. Create vlan device:
> $ ip link add link ens1 name ens1.8 type vlan id 8
>
> Signed-off-by: Eli Cohen <elic at nvidia.com>
> ---
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 274 +++++++++++++++++++++++-------
> 1 file changed, 216 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 5aa6220c7129..f81f9a213ed2 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -48,6 +48,8 @@ MODULE_LICENSE("Dual BSD/GPL");
>
> #define MLX5_FEATURE(_mvdev, _feature) (!!((_mvdev)->actual_features
& BIT_ULL(_feature)))
>
> +#define MLX5V_UNTAGGED 0x1000
> +
> struct mlx5_vdpa_net_resources {
> u32 tisn;
> u32 tdn;
> @@ -143,6 +145,8 @@ static bool is_index_valid(struct mlx5_vdpa_dev *mvdev,
u16 idx)
> return idx <= mvdev->max_idx;
> }
>
> +#define MLX5V_MACVLAN_SIZE 256
> +
> struct mlx5_vdpa_net {
> struct mlx5_vdpa_dev mvdev;
> struct mlx5_vdpa_net_resources res;
> @@ -156,14 +160,20 @@ struct mlx5_vdpa_net {
> */
> struct mutex reslock;
> struct mlx5_flow_table *rxft;
> - struct mlx5_flow_handle *rx_rule_ucast;
> - struct mlx5_flow_handle *rx_rule_mcast;
> bool setup;
> u32 cur_num_vqs;
> u32 rqt_size;
> struct notifier_block nb;
> struct vdpa_callback config_cb;
> struct mlx5_vdpa_wq_ent cvq_ent;
> + struct hlist_head macvlan_hash[MLX5V_MACVLAN_SIZE];
> +};
> +
> +struct macvlan_node {
> + struct hlist_node hlist;
> + struct mlx5_flow_handle *ucast_rule;
> + struct mlx5_flow_handle *mcast_rule;
> + u64 macvlan;
> };
>
> static void free_resources(struct mlx5_vdpa_net *ndev);
> @@ -1346,12 +1356,17 @@ static void destroy_tir(struct mlx5_vdpa_net *ndev)
> mlx5_vdpa_destroy_tir(&ndev->mvdev, ndev->res.tirn);
> }
>
> -static int add_fwd_to_tir(struct mlx5_vdpa_net *ndev)
> +#define MAX_STEERING_ENT 0x8000
> +#define MAX_STEERING_GROUPS 2
> +
> +static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8
*mac,
> + u16 vid, bool tagged,
> + struct mlx5_flow_handle **ucast,
> + struct mlx5_flow_handle **mcast)
> {
> struct mlx5_flow_destination dest = {};
> - struct mlx5_flow_table_attr ft_attr = {};
> struct mlx5_flow_act flow_act = {};
> - struct mlx5_flow_namespace *ns;
> + struct mlx5_flow_handle *rule;
> struct mlx5_flow_spec *spec;
> void *headers_c;
> void *headers_v;
> @@ -1364,74 +1379,178 @@ static int add_fwd_to_tir(struct mlx5_vdpa_net
*ndev)
> return -ENOMEM;
>
> spec->match_criteria_enable = MLX5_MATCH_OUTER_HEADERS;
> - ft_attr.max_fte = 2;
> - ft_attr.autogroup.max_num_groups = 2;
> -
> - ns = mlx5_get_flow_namespace(ndev->mvdev.mdev,
MLX5_FLOW_NAMESPACE_BYPASS);
> - if (!ns) {
> - mlx5_vdpa_warn(&ndev->mvdev, "failed to get flow
namespace\n");
> - err = -EOPNOTSUPP;
> - goto err_ns;
> - }
> -
> - ndev->rxft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
> - if (IS_ERR(ndev->rxft)) {
> - err = PTR_ERR(ndev->rxft);
> - goto err_ns;
> - }
> -
> headers_c = MLX5_ADDR_OF(fte_match_param, spec->match_criteria,
outer_headers);
> - dmac_c = MLX5_ADDR_OF(fte_match_param, headers_c,
outer_headers.dmac_47_16);
> - memset(dmac_c, 0xff, ETH_ALEN);
> headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value,
outer_headers);
> + dmac_c = MLX5_ADDR_OF(fte_match_param, headers_c,
outer_headers.dmac_47_16);
> dmac_v = MLX5_ADDR_OF(fte_match_param, headers_v,
outer_headers.dmac_47_16);
> - ether_addr_copy(dmac_v, ndev->config.mac);
> -
> + memset(dmac_c, 0xff, ETH_ALEN);
> + ether_addr_copy(dmac_v, mac);
> + MLX5_SET(fte_match_set_lyr_2_4, headers_c, cvlan_tag, 1);
> + if (tagged) {
> + MLX5_SET(fte_match_set_lyr_2_4, headers_v, cvlan_tag, 1);
> + MLX5_SET_TO_ONES(fte_match_set_lyr_2_4, headers_c, first_vid);
> + MLX5_SET(fte_match_set_lyr_2_4, headers_c, first_vid, vid);
> + }
> flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> dest.type = MLX5_FLOW_DESTINATION_TYPE_TIR;
> dest.tir_num = ndev->res.tirn;
> - ndev->rx_rule_ucast = mlx5_add_flow_rules(ndev->rxft, spec,
&flow_act, &dest, 1);
> + rule = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, &dest,
1);
> + if (IS_ERR(rule))
> + return PTR_ERR(rule);
>
> - if (IS_ERR(ndev->rx_rule_ucast)) {
> - err = PTR_ERR(ndev->rx_rule_ucast);
> - ndev->rx_rule_ucast = NULL;
> - goto err_rule_ucast;
> - }
> + *ucast = rule;
>
> memset(dmac_c, 0, ETH_ALEN);
> memset(dmac_v, 0, ETH_ALEN);
> dmac_c[0] = 1;
> dmac_v[0] = 1;
> - flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> - ndev->rx_rule_mcast = mlx5_add_flow_rules(ndev->rxft, spec,
&flow_act, &dest, 1);
> - if (IS_ERR(ndev->rx_rule_mcast)) {
> - err = PTR_ERR(ndev->rx_rule_mcast);
> - ndev->rx_rule_mcast = NULL;
> - goto err_rule_mcast;
> + rule = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, &dest,
1);
> + kvfree(spec);
> + if (IS_ERR(rule)) {
> + err = PTR_ERR(rule);
> + goto err_mcast;
> }
>
> - kvfree(spec);
> + *mcast = rule;
> return 0;
>
> -err_rule_mcast:
> - mlx5_del_flow_rules(ndev->rx_rule_ucast);
> - ndev->rx_rule_ucast = NULL;
> -err_rule_ucast:
> - mlx5_destroy_flow_table(ndev->rxft);
> -err_ns:
> - kvfree(spec);
> +err_mcast:
> + mlx5_del_flow_rules(*ucast);
> + return err;
> +}
> +
> +static void mlx5_vdpa_del_mac_vlan_rules(struct mlx5_vdpa_net *ndev,
> + struct mlx5_flow_handle *ucast,
> + struct mlx5_flow_handle *mcast)
> +{
> + mlx5_del_flow_rules(ucast);
> + mlx5_del_flow_rules(mcast);
> +}
> +
> +static u64 search_val(u8 *mac, u16 vlan, bool tagged)
> +{
> + u64 val;
> +
> + if (!tagged)
> + vlan = MLX5V_UNTAGGED;
> +
> + val = (u64)vlan << 48 |
> + (u64)mac[0] << 40 |
> + (u64)mac[1] << 32 |
> + (u64)mac[2] << 24 |
> + (u64)mac[3] << 16 |
> + (u64)mac[4] << 8 |
> + (u64)mac[5];
> +
> + return val;
> +}
> +
> +static struct macvlan_node *mac_vlan_lookup(struct mlx5_vdpa_net *ndev,
u64 value)
> +{
> + struct macvlan_node *pos;
> + u32 idx;
> +
> + idx = hash_64(value, 8); // tbd 8
> + hlist_for_each_entry(pos, &ndev->macvlan_hash[idx], hlist) {
> + if (pos->macvlan == value)
> + return pos;
> + }
> + return NULL;
> +}
> +
> +static int mac_vlan_add(struct mlx5_vdpa_net *ndev, u8 *mac, u16 vlan,
bool tagged) // vlan -> vid
> +{
I guess checkpatch may not be happy with such kind of comment.
> + struct macvlan_node *ptr;
> + u64 val;
> + u32 idx;
> + int err;
> +
> + val = search_val(mac, vlan, tagged);
> + if (mac_vlan_lookup(ndev, val))
> + return -EEXIST;
> +
> + ptr = kzalloc(sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return -ENOMEM;
> +
> + err = mlx5_vdpa_add_mac_vlan_rules(ndev, ndev->config.mac, vlan,
tagged,
> + &ptr->ucast_rule, &ptr->mcast_rule);
> + if (err)
> + goto err_add;
> +
> + ptr->macvlan = val;
> + idx = hash_64(val, 8);
> + hlist_add_head(&ptr->hlist, &ndev->macvlan_hash[idx]);
> + return 0;
> +
> +err_add:
> + kfree(ptr);
> return err;
> }
>
> -static void remove_fwd_to_tir(struct mlx5_vdpa_net *ndev)
> +static void mac_vlan_del(struct mlx5_vdpa_net *ndev, u8 *mac, u16 vlan,
bool tagged)
> {
> - if (!ndev->rx_rule_ucast)
> + struct macvlan_node *ptr;
> +
> + ptr = mac_vlan_lookup(ndev, search_val(mac, vlan, tagged));
> + if (!ptr)
> return;
>
> - mlx5_del_flow_rules(ndev->rx_rule_mcast);
> - ndev->rx_rule_mcast = NULL;
> - mlx5_del_flow_rules(ndev->rx_rule_ucast);
> - ndev->rx_rule_ucast = NULL;
> + hlist_del(&ptr->hlist);
> + mlx5_vdpa_del_mac_vlan_rules(ndev, ptr->ucast_rule,
ptr->mcast_rule);
> + kfree(ptr);
> +}
> +
> +static void clear_mac_vlan_table(struct mlx5_vdpa_net *ndev)
> +{
> + struct macvlan_node *pos;
> + struct hlist_node *n;
> + int i;
> +
> + for (i = 0; i < MLX5V_MACVLAN_SIZE; i++) {
> + hlist_for_each_entry_safe(pos, n, &ndev->macvlan_hash[i], hlist)
{
> + hlist_del(&pos->hlist);
> + mlx5_vdpa_del_mac_vlan_rules(ndev, pos->ucast_rule,
pos->mcast_rule);
> + kfree(pos);
> + }
> + }
> +}
> +
> +static int setup_steering(struct mlx5_vdpa_net *ndev)
> +{
> + struct mlx5_flow_table_attr ft_attr = {};
> + struct mlx5_flow_namespace *ns;
> + int err;
> +
> + ft_attr.max_fte = MAX_STEERING_ENT;
> + ft_attr.autogroup.max_num_groups = MAX_STEERING_GROUPS;
> +
> + ns = mlx5_get_flow_namespace(ndev->mvdev.mdev,
MLX5_FLOW_NAMESPACE_BYPASS);
> + if (!ns) {
> + mlx5_vdpa_warn(&ndev->mvdev, "failed to get flow
namespace\n");
> + return -EOPNOTSUPP;
> + }
> +
> + ndev->rxft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
> + if (IS_ERR(ndev->rxft)) {
> + mlx5_vdpa_warn(&ndev->mvdev, "failed to create flow
table\n");
> + return PTR_ERR(ndev->rxft);
> + }
> +
> + err = mac_vlan_add(ndev, ndev->config.mac, 0, false);
> + if (err)
> + goto err_add;
> +
> + return 0;
> +
> +err_add:
> + mlx5_destroy_flow_table(ndev->rxft);
> + return err;
> +}
> +
> +static void teardown_steering(struct mlx5_vdpa_net *ndev)
> +{
> + clear_mac_vlan_table(ndev);
> mlx5_destroy_flow_table(ndev->rxft);
> }
>
> @@ -1482,9 +1601,9 @@ static virtio_net_ctrl_ack handle_ctrl_mac(struct
mlx5_vdpa_dev *mvdev, u8 cmd)
>
> /* Need recreate the flow table entry, so that the packet could forward
back
> */
> - remove_fwd_to_tir(ndev);
> + mac_vlan_del(ndev, ndev->config.mac, 0, false);
>
> - if (add_fwd_to_tir(ndev)) {
> + if (mac_vlan_add(ndev, ndev->config.mac, 0, false)) {
> mlx5_vdpa_warn(mvdev, "failed to insert forward rules, try to
restore\n");
>
> /* Although it hardly run here, we still need double check */
> @@ -1508,7 +1627,7 @@ static virtio_net_ctrl_ack handle_ctrl_mac(struct
mlx5_vdpa_dev *mvdev, u8 cmd)
>
> memcpy(ndev->config.mac, mac_back, ETH_ALEN);
>
> - if (add_fwd_to_tir(ndev))
> + if (mac_vlan_add(ndev, ndev->config.mac, 0, false))
> mlx5_vdpa_warn(mvdev, "restore forward rules failed: insert
forward rules failed\n");
>
> break;
> @@ -1610,6 +1729,42 @@ static virtio_net_ctrl_ack handle_ctrl_mq(struct
mlx5_vdpa_dev *mvdev, u8 cmd)
> return status;
> }
>
> +static virtio_net_ctrl_ack handle_ctrl_vlan(struct mlx5_vdpa_dev *mvdev,
u8 cmd)
> +{
> + struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> + virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> + struct mlx5_control_vq *cvq = &mvdev->cvq;
> + struct virtio_net_ctrl_vlan vlan;
> + size_t read;
> + u16 id;
> +
> + switch (cmd) {
> + case VIRTIO_NET_CTRL_VLAN_ADD:
> + read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov,
(void *)&vlan, sizeof(vlan));
> + if (read != sizeof(vlan))
> + break;
> +
> + id = mlx5vdpa16_to_cpu(mvdev, vlan.id);
> + if (mac_vlan_add(ndev, ndev->config.mac, id, true))
> + break;
This may work now but I wonder if we had the plan to support
VIRTIO_NET_F_CTRL_RX?
if $mac is not in $mac_table
??? drop;
if $vlan is not in $vlan_table
??? drop;
With that features we probably requires the dedicated vlan filters
without a mac? Or do we want to a $mac * $vlans rules?
If yes, maybe it's better to decouple vlan filters from mac now.
Thanks
> +
> + status = VIRTIO_NET_OK;
> + break;
> + case VIRTIO_NET_CTRL_VLAN_DEL:
> + read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov,
(void *)&vlan, sizeof(vlan));
> + if (read != sizeof(vlan))
> + break;
> +
> + id = mlx5vdpa16_to_cpu(mvdev, vlan.id);
> + mac_vlan_del(ndev, ndev->config.mac, id, true);
> + break;
> + default:
> + break;
> +}
> +
> +return status;
> +}
> +
> static void mlx5_cvq_kick_handler(struct work_struct *work)
> {
> virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> @@ -1654,7 +1809,9 @@ static void mlx5_cvq_kick_handler(struct work_struct
*work)
> case VIRTIO_NET_CTRL_MQ:
> status = handle_ctrl_mq(mvdev, ctrl.cmd);
> break;
> -
> + case VIRTIO_NET_CTRL_VLAN:
> + status = handle_ctrl_vlan(mvdev, ctrl.cmd);
> + break;
> default:
> break;
> }
> @@ -1913,6 +2070,7 @@ static u64 get_supported_features(struct
mlx5_core_dev *mdev)
> mlx_vdpa_features |= BIT_ULL(VIRTIO_NET_F_MQ);
> mlx_vdpa_features |= BIT_ULL(VIRTIO_NET_F_STATUS);
> mlx_vdpa_features |= BIT_ULL(VIRTIO_NET_F_MTU);
> + mlx_vdpa_features |= BIT_ULL(VIRTIO_NET_F_CTRL_VLAN);
>
> return mlx_vdpa_features;
> }
> @@ -2198,9 +2356,9 @@ static int setup_driver(struct mlx5_vdpa_dev *mvdev)
> goto err_tir;
> }
>
> - err = add_fwd_to_tir(ndev);
> + err = setup_steering(ndev);
> if (err) {
> - mlx5_vdpa_warn(mvdev, "add_fwd_to_tir\n");
> + mlx5_vdpa_warn(mvdev, "setup_steering\n");
> goto err_fwd;
> }
> ndev->setup = true;
> @@ -2226,7 +2384,7 @@ static void teardown_driver(struct mlx5_vdpa_net
*ndev)
> if (!ndev->setup)
> return;
>
> - remove_fwd_to_tir(ndev);
> + teardown_steering(ndev);
> destroy_tir(ndev);
> destroy_rqt(ndev);
> teardown_virtqueues(ndev);