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.
Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
Signed-off-by: Eli Cohen <[email protected]>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 90913365def4..dd29fdfc24ed 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1472,7 +1472,7 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8 *mac,
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;
--
2.35.1
Hi Eli,
It's not for this patch but something related, so just a friendly
heads-up. I haven't validated the VLAN tagging behavior yet for mlx5
vdpa, but from my quick read of the code it doesn't seem it
differentiates the case with and without VIRTIO_NET_F_CTRL_VLAN, to be
compatible/compliant with what's been implemented in QEMU software (a
spec addendum was filed as requested by Michael):
https://github.com/oasis-tcs/virtio-spec/issues/147
- when VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
all VLANs filtered (meaning only untagged traffic can be received,
and traffic with VLAN tag will be dropped).
- when VIRTIO_NET_F_CTRL_VLAN is not negotiated, all traffic including
untagged and tagged can be received.
Can you please help check if we need further fix in terms of VLAN tagging?
Thanks,
-Siwei
On 10/18/2022 4:12 AM, Eli Cohen 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.
>
> Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
>
> Signed-off-by: Eli Cohen <[email protected]>
Reviewed-by: Si-Wei Liu <[email protected]>
> ---
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 90913365def4..dd29fdfc24ed 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1472,7 +1472,7 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8 *mac,
> 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;
> From: Si-Wei Liu <[email protected]>
> Sent: Tuesday, 18 October 2022 22:21
> To: Eli Cohen <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH 1/4] vdpa/mlx5: Fix rule forwarding VLAN to TIR
>
> Hi Eli,
>
> It's not for this patch but something related, so just a friendly
> heads-up. I haven't validated the VLAN tagging behavior yet for mlx5
> vdpa, but from my quick read of the code it doesn't seem it
> differentiates the case with and without VIRTIO_NET_F_CTRL_VLAN, to be
> compatible/compliant with what's been implemented in QEMU software (a
> spec addendum was filed as requested by Michael):
>
> https://github.com/oasis-tcs/virtio-spec/issues/147
>
> - when VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
> all VLANs filtered (meaning only untagged traffic can be received,
> and traffic with VLAN tag will be dropped).
>
> - when VIRTIO_NET_F_CTRL_VLAN is not negotiated, all traffic including
> untagged and tagged can be received.
>
> Can you please help check if we need further fix in terms of VLAN tagging?
>
Sure. It's broken today. I will fix this to conform to the above requirements and send V1.
> Thanks,
> -Siwei
>
>
> On 10/18/2022 4:12 AM, Eli Cohen 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.
> >
> > Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
> >
> > Signed-off-by: Eli Cohen <[email protected]>
> Reviewed-by: Si-Wei Liu <[email protected]>
>
> > ---
> > drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > index 90913365def4..dd29fdfc24ed 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -1472,7 +1472,7 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct
> mlx5_vdpa_net *ndev, u8 *mac,
> > 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;
On Wed, Oct 19, 2022 at 05:31:48AM +0000, Eli Cohen wrote:
> > From: Si-Wei Liu <[email protected]>
> > Sent: Tuesday, 18 October 2022 22:21
> > To: Eli Cohen <[email protected]>; [email protected]; [email protected];
> > [email protected]; [email protected]
> > Cc: [email protected]; [email protected]
> > Subject: Re: [PATCH 1/4] vdpa/mlx5: Fix rule forwarding VLAN to TIR
> >
> > Hi Eli,
> >
> > It's not for this patch but something related, so just a friendly
> > heads-up. I haven't validated the VLAN tagging behavior yet for mlx5
> > vdpa, but from my quick read of the code it doesn't seem it
> > differentiates the case with and without VIRTIO_NET_F_CTRL_VLAN, to be
> > compatible/compliant with what's been implemented in QEMU software (a
> > spec addendum was filed as requested by Michael):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/147
> >
> > - when VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
> > all VLANs filtered (meaning only untagged traffic can be received,
> > and traffic with VLAN tag will be dropped).
> >
> > - when VIRTIO_NET_F_CTRL_VLAN is not negotiated, all traffic including
> > untagged and tagged can be received.
> >
> > Can you please help check if we need further fix in terms of VLAN tagging?
> >
>
> Sure. It's broken today. I will fix this to conform to the above requirements and send V1.
Did you mean v2?
> > Thanks,
> > -Siwei
> >
> >
> > On 10/18/2022 4:12 AM, Eli Cohen 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.
> > >
> > > Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
> > >
> > > Signed-off-by: Eli Cohen <[email protected]>
> > Reviewed-by: Si-Wei Liu <[email protected]>
> >
> > > ---
> > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > index 90913365def4..dd29fdfc24ed 100644
> > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > @@ -1472,7 +1472,7 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct
> > mlx5_vdpa_net *ndev, u8 *mac,
> > > 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;
>
> From: Michael S. Tsirkin <[email protected]>
> Sent: Wednesday, 19 October 2022 8:34
> To: Eli Cohen <[email protected]>
> Cc: Si-Wei Liu <[email protected]>; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 1/4] vdpa/mlx5: Fix rule forwarding VLAN to TIR
>
> On Wed, Oct 19, 2022 at 05:31:48AM +0000, Eli Cohen wrote:
> > > From: Si-Wei Liu <[email protected]>
> > > Sent: Tuesday, 18 October 2022 22:21
> > > To: Eli Cohen <[email protected]>; [email protected]; [email protected];
> > > [email protected]; [email protected]
> > > Cc: [email protected]; [email protected]
> > > Subject: Re: [PATCH 1/4] vdpa/mlx5: Fix rule forwarding VLAN to TIR
> > >
> > > Hi Eli,
> > >
> > > It's not for this patch but something related, so just a friendly
> > > heads-up. I haven't validated the VLAN tagging behavior yet for mlx5
> > > vdpa, but from my quick read of the code it doesn't seem it
> > > differentiates the case with and without VIRTIO_NET_F_CTRL_VLAN, to be
> > > compatible/compliant with what's been implemented in QEMU software (a
> > > spec addendum was filed as requested by Michael):
> > >
> > > https://github.com/oasis-tcs/virtio-spec/issues/147
> > >
> > > - when VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
> > > all VLANs filtered (meaning only untagged traffic can be received,
> > > and traffic with VLAN tag will be dropped).
> > >
> > > - when VIRTIO_NET_F_CTRL_VLAN is not negotiated, all traffic including
> > > untagged and tagged can be received.
> > >
> > > Can you please help check if we need further fix in terms of VLAN tagging?
> > >
> >
> > Sure. It's broken today. I will fix this to conform to the above requirements
> and send V1.
>
> Did you mean v2?
>
I count from 0 and have been following this scheme but I can make it v2 if that's the norm.
> > > Thanks,
> > > -Siwei
> > >
> > >
> > > On 10/18/2022 4:12 AM, Eli Cohen 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.
> > > >
> > > > Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
> > > >
> > > > Signed-off-by: Eli Cohen <[email protected]>
> > > Reviewed-by: Si-Wei Liu <[email protected]>
> > >
> > > > ---
> > > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > index 90913365def4..dd29fdfc24ed 100644
> > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > @@ -1472,7 +1472,7 @@ static int
> mlx5_vdpa_add_mac_vlan_rules(struct
> > > mlx5_vdpa_net *ndev, u8 *mac,
> > > > 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;
> >
On Wed, Oct 19, 2022 at 05:36:17AM +0000, Eli Cohen wrote:
> > From: Michael S. Tsirkin <[email protected]>
> > Sent: Wednesday, 19 October 2022 8:34
> > To: Eli Cohen <[email protected]>
> > Cc: Si-Wei Liu <[email protected]>; [email protected]; linux-
> > [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH 1/4] vdpa/mlx5: Fix rule forwarding VLAN to TIR
> >
> > On Wed, Oct 19, 2022 at 05:31:48AM +0000, Eli Cohen wrote:
> > > > From: Si-Wei Liu <[email protected]>
> > > > Sent: Tuesday, 18 October 2022 22:21
> > > > To: Eli Cohen <[email protected]>; [email protected]; [email protected];
> > > > [email protected]; [email protected]
> > > > Cc: [email protected]; [email protected]
> > > > Subject: Re: [PATCH 1/4] vdpa/mlx5: Fix rule forwarding VLAN to TIR
> > > >
> > > > Hi Eli,
> > > >
> > > > It's not for this patch but something related, so just a friendly
> > > > heads-up. I haven't validated the VLAN tagging behavior yet for mlx5
> > > > vdpa, but from my quick read of the code it doesn't seem it
> > > > differentiates the case with and without VIRTIO_NET_F_CTRL_VLAN, to be
> > > > compatible/compliant with what's been implemented in QEMU software (a
> > > > spec addendum was filed as requested by Michael):
> > > >
> > > > https://github.com/oasis-tcs/virtio-spec/issues/147
> > > >
> > > > - when VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
> > > > all VLANs filtered (meaning only untagged traffic can be received,
> > > > and traffic with VLAN tag will be dropped).
> > > >
> > > > - when VIRTIO_NET_F_CTRL_VLAN is not negotiated, all traffic including
> > > > untagged and tagged can be received.
> > > >
> > > > Can you please help check if we need further fix in terms of VLAN tagging?
> > > >
> > >
> > > Sure. It's broken today. I will fix this to conform to the above requirements
> > and send V1.
> >
> > Did you mean v2?
> >
>
> I count from 0 and have been following this scheme but I can make it v2 if that's the norm.
Yes, most people seem to count patches from 1 so [PATCH] is followed by
[PATCH v2]. I don't know why. But it doesn't matter much - I just
wanted to understand whether you will be sending a new version of this
patchset. I know now.
> > > > Thanks,
> > > > -Siwei
> > > >
> > > >
> > > > On 10/18/2022 4:12 AM, Eli Cohen 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.
> > > > >
> > > > > Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
> > > > >
> > > > > Signed-off-by: Eli Cohen <[email protected]>
> > > > Reviewed-by: Si-Wei Liu <[email protected]>
> > > >
> > > > > ---
> > > > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > index 90913365def4..dd29fdfc24ed 100644
> > > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > @@ -1472,7 +1472,7 @@ static int
> > mlx5_vdpa_add_mac_vlan_rules(struct
> > > > mlx5_vdpa_net *ndev, u8 *mac,
> > > > > 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;
> > >
>