diff mbox series

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

Message ID 20221113134442.152695-2-elic@nvidia.com
State New
Headers
Series vdpa/mlx5: Add debugfs subtree and fixes |

Commit Message

Eli Cohen Nov. 13, 2022, 1:44 p.m. UTC 
  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@nvidia.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Jason Wang Nov. 14, 2022, 4:38 a.m. UTC | #1 
On Sun, Nov 13, 2022 at 9:45 PM Eli Cohen <elic@nvidia.com> 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@nvidia.com>
> ---
>  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.

Other than this.

Acked-by: Jason Wang <jasowang@redhat.com>

> +
>         switch (cmd) {
>         case VIRTIO_NET_CTRL_VLAN_ADD:
>                 read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan, sizeof(vlan));
> --
> 2.38.1
>
Eli Cohen Nov. 14, 2022, 6:44 a.m. UTC | #2 
> From: Jason Wang <jasowang@redhat.com>
> Sent: Monday, 14 November 2022 6:39
> To: Eli Cohen <elic@nvidia.com>
> Cc: mst@redhat.com; linux-kernel@vger.kernel.org; virtualization@lists.linux-
> foundation.org; si-wei.liu@oracle.com; eperezma@redhat.com;
> lulu@redhat.com
> 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@nvidia.com> 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@nvidia.com>
> > ---
> >  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

> Other than this.
> 
> Acked-by: Jason Wang <jasowang@redhat.com>
> 
> > +
> >         switch (cmd) {
> >         case VIRTIO_NET_CTRL_VLAN_ADD:
> >                 read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan,
> sizeof(vlan));
> > --
> > 2.38.1
> >
Michael S. Tsirkin Nov. 14, 2022, 8:07 a.m. UTC | #3 
On Mon, Nov 14, 2022 at 06:44:30AM +0000, Eli Cohen wrote:
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Monday, 14 November 2022 6:39
> > To: Eli Cohen <elic@nvidia.com>
> > Cc: mst@redhat.com; linux-kernel@vger.kernel.org; virtualization@lists.linux-
> > foundation.org; si-wei.liu@oracle.com; eperezma@redhat.com;
> > lulu@redhat.com
> > 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@nvidia.com> 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@nvidia.com>
> > > ---
> > >  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!

> > Other than this.
> > 
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > 
> > > +
> > >         switch (cmd) {
> > >         case VIRTIO_NET_CTRL_VLAN_ADD:
> > >                 read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan,
> > sizeof(vlan));
> > > --
> > > 2.38.1
> > >
>
Eli Cohen Nov. 14, 2022, 8:58 a.m. UTC | #4 
> -----Original Message-----
> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, 14 November 2022 10:07
> To: Eli Cohen <elic@nvidia.com>
> Cc: Jason Wang <jasowang@redhat.com>; linux-kernel@vger.kernel.org;
> virtualization@lists.linux-foundation.org; si-wei.liu@oracle.com;
> eperezma@redhat.com; lulu@redhat.com
> 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@redhat.com>
> > > Sent: Monday, 14 November 2022 6:39
> > > To: Eli Cohen <elic@nvidia.com>
> > > Cc: mst@redhat.com; linux-kernel@vger.kernel.org;
> virtualization@lists.linux-
> > > foundation.org; si-wei.liu@oracle.com; eperezma@redhat.com;
> > > lulu@redhat.com
> > > 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@nvidia.com> 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@nvidia.com>
> > > > ---
> > > >  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@redhat.com>
> > >
> > > > +
> > > >         switch (cmd) {
> > > >         case VIRTIO_NET_CTRL_VLAN_ADD:
> > > >                 read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan,
> > > sizeof(vlan));
> > > > --
> > > > 2.38.1
> > > >
> >
diff mbox series

Patch

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;
+
 	switch (cmd) {
 	case VIRTIO_NET_CTRL_VLAN_ADD:
 		read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan, sizeof(vlan));