2022-11-13 14:08:26

by Eli Cohen

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

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

2022-11-14 05:14:10

by Jason Wang

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

On Sun, Nov 13, 2022 at 9:45 PM Eli Cohen <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>

> +
> switch (cmd) {
> case VIRTIO_NET_CTRL_VLAN_ADD:
> read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan, sizeof(vlan));
> --
> 2.38.1
>

2022-11-14 07:10:23

by Eli Cohen

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

> From: Jason Wang <[email protected]>
> Sent: Monday, 14 November 2022 6:39
> To: Eli Cohen <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]
> foundation.org; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/7] vdpa/mlx5: Fix rule forwarding VLAN to TIR
>
> On Sun, Nov 13, 2022 at 9:45 PM Eli Cohen <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
> > +
> > switch (cmd) {
> > case VIRTIO_NET_CTRL_VLAN_ADD:
> > read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan,
> sizeof(vlan));
> > --
> > 2.38.1
> >

2022-11-14 08:55:25

by Michael S. Tsirkin

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 <[email protected]>
> > Sent: Monday, 14 November 2022 6:39
> > To: Eli Cohen <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected]
> > foundation.org; [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [PATCH 1/7] vdpa/mlx5: Fix rule forwarding VLAN to TIR
> >
> > On Sun, Nov 13, 2022 at 9:45 PM Eli Cohen <[email protected]> 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 <[email protected]>
> > > ---
> > > 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 <[email protected]>
> >
> > > +
> > > switch (cmd) {
> > > case VIRTIO_NET_CTRL_VLAN_ADD:
> > > read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan,
> > sizeof(vlan));
> > > --
> > > 2.38.1
> > >
>

2022-11-14 09:44:07

by Eli Cohen

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

> -----Original Message-----
> From: Michael S. Tsirkin <[email protected]>
> Sent: Monday, 14 November 2022 10:07
> To: Eli Cohen <[email protected]>
> Cc: Jason Wang <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> 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 <[email protected]>
> > > Sent: Monday, 14 November 2022 6:39
> > > To: Eli Cohen <[email protected]>
> > > Cc: [email protected]; [email protected];
> [email protected]
> > > foundation.org; [email protected]; [email protected];
> > > [email protected]
> > > Subject: Re: [PATCH 1/7] vdpa/mlx5: Fix rule forwarding VLAN to TIR
> > >
> > > On Sun, Nov 13, 2022 at 9:45 PM Eli Cohen <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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 <[email protected]>
> > >
> > > > +
> > > > switch (cmd) {
> > > > case VIRTIO_NET_CTRL_VLAN_ADD:
> > > > read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, &vlan,
> > > sizeof(vlan));
> > > > --
> > > > 2.38.1
> > > >
> >