Messages in this thread | | | Date | Fri, 15 Apr 2022 11:58:43 +0800 | Subject | Re: [PATCH 3/3] vdpa/mlx5: Add RX MAC VLAN filter support | From | Jason Wang <> |
| |
在 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@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);
| |