RE: [PATCH 1/7] vdpa/mlx5: Fix rule forwarding VLAN to TIR

[Eli Cohen]

> -----Original Message-----
> From: Michael S. Tsirkin <mst@xxxxxxxxxx>
> Sent: Monday, 14 November 2022 10:07
> To: Eli Cohen <elic@xxxxxxxxxx>
> Cc: Jason Wang <jasowang@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx; si-wei.liu@xxxxxxxxxx;
> eperezma@xxxxxxxxxx; lulu@xxxxxxxxxx
> Subject: Re: [PATCH 1/7] vdpa/mlx5: Fix rule forwarding VLAN to TIR
> 
> On Mon, Nov 14, 2022 at 06:44:30AM +0000, Eli Cohen wrote:
> > > From: Jason Wang <jasowang@xxxxxxxxxx>
> > > Sent: Monday, 14 November 2022 6:39
> > > To: Eli Cohen <elic@xxxxxxxxxx>
> > > Cc: mst@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> virtualization@lists.linux-
> > > foundation.org; si-wei.liu@xxxxxxxxxx; eperezma@xxxxxxxxxx;
> > > lulu@xxxxxxxxxx
> > > Subject: Re: [PATCH 1/7] vdpa/mlx5: Fix rule forwarding VLAN to TIR
> > >
> > > On Sun, Nov 13, 2022 at 9:45 PM Eli Cohen <elic@xxxxxxxxxx> wrote:
> > > >
> > > > Set the VLAN id to the header values field instead of overwriting the
> > > > headers criteria field.
> > > >
> > > > Before this fix, VLAN filtering would not really work and tagged packets
> > > > would be forwarded unfiltered to the TIR.
> > > >
> > > > In addition modify the logic so that VLAN filtering is enforced only
> > > > when VIRTIO_NET_F_CTRL_VLAN is negotiated. When not negotiated, all
> > > > incoming traffic is accepted as long as it is targeting the net device's
> > > > MAC address.
> > > >
> > > > Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
> > > >
> > > > Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
> > > > ---
> > > >  drivers/vdpa/mlx5/net/mlx5_vnet.c | 11 ++++++++---
> > > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > index 90913365def4..ea95081eca0c 100644
> > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > @@ -1468,11 +1468,13 @@ static int
> mlx5_vdpa_add_mac_vlan_rules(struct
> > > mlx5_vdpa_net *ndev, u8 *mac,
> > > >         dmac_v = MLX5_ADDR_OF(fte_match_param, headers_v,
> > > outer_headers.dmac_47_16);
> > > >         eth_broadcast_addr(dmac_c);
> > > >         ether_addr_copy(dmac_v, mac);
> > > > -       MLX5_SET(fte_match_set_lyr_2_4, headers_c, cvlan_tag, 1);
> > > > +       if (ndev->mvdev.actual_features & VIRTIO_NET_F_CTRL_VLAN) {
> > > > +               MLX5_SET(fte_match_set_lyr_2_4, headers_c, cvlan_tag, 1);
> > > > +               MLX5_SET_TO_ONES(fte_match_set_lyr_2_4, headers_c,
> first_vid);
> > > > +       }
> > > >         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);
> > > > +               MLX5_SET(fte_match_set_lyr_2_4, headers_v, first_vid, vid);
> > > >         }
> > > >         flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> > > >         dest.type = MLX5_FLOW_DESTINATION_TYPE_TIR;
> > > > @@ -1821,6 +1823,9 @@ static virtio_net_ctrl_ack
> handle_ctrl_vlan(struct
> > > mlx5_vdpa_dev *mvdev, u8 cmd)
> > > >         size_t read;
> > > >         u16 id;
> > > >
> > > > +       if (!(ndev->mvdev.actual_features &
> > > BIT_ULL(VIRTIO_NET_F_CTRL_VLAN)))
> > > > +               return status;
> > >
> > > Nit: this seems unrelated to the patch.
> > >
> > Will put in another patch
> 
> 
> OK since you are doing a new version, please split fixes and
> features in two patchsets. Thanks!

Sure
> 
> > > Other than this.
> > >
> > > Acked-by: Jason Wang <jasowang@xxxxxxxxxx>
> > >
> > > > +
> > > >         switch (cmd) {
> > > >         case VIRTIO_NET_CTRL_VLAN_ADD:
> > > >                 read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan,
> > > sizeof(vlan));
> > > > --
> > > > 2.38.1
> > > >
> >