* [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
@ 2022-06-17 17:50 Jun Zhang
2022-06-20 5:32 ` Michal Swiatkowski
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jun Zhang @ 2022-06-17 17:50 UTC (permalink / raw)
To: intel-wired-lan; +Cc: Kiran Patil
From: Kiran Patil <kiran.patil@intel.com>
Before allowing tc-filter using dest MAC, VLAN - check to make
sure there is basic active filter using specified dest MAC and
likewise for VLAN.
This check is must to allow only legit filter via tc-filter
code path with or without ADQ.
Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
Signed-off-by: Kiran Patil <kiran.patil@intel.com>
Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 62 ++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 57c51a15bcbc..287c3e4bf8af 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -3558,6 +3558,48 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
return ret;
}
+/**
+ * iavf_is_vlan_tc_filter_allowed - allowed to add tc-filter using VLAN
+ * @adapter: board private structure
+ * @vlan: VLAN to verify
+ *
+ * Using specified "vlan" ID, there must be active VLAN filter in VF's
+ * MAC-VLAN filter list.
+ */
+static bool
+iavf_is_vlan_tc_filter_allowed(struct iavf_adapter *adapter, u16 vlan)
+{
+ struct iavf_vlan_filter *f;
+ bool allowed;
+
+ spin_lock_bh(&adapter->mac_vlan_list_lock);
+ f = iavf_find_vlan(adapter, IAVF_VLAN(vlan, ETH_P_8021Q));
+ allowed = (f && f->add && !f->remove);
+ spin_unlock_bh(&adapter->mac_vlan_list_lock);
+ return allowed;
+}
+
+/**
+ * iavf_is_mac_tc_filter_allowed - allowed to add tc-filter using MAC addr
+ * @adapter: board private structure
+ * @macaddr: MAC address
+ *
+ * Using specified MAC address, there must be active MAC filter in VF's
+ * MAC-VLAN filter list.
+ */
+static bool
+iavf_is_mac_tc_filter_allowed(struct iavf_adapter *adapter, const u8 *macaddr)
+{
+ struct iavf_mac_filter *f;
+ bool allowed;
+
+ spin_lock_bh(&adapter->mac_vlan_list_lock);
+ f = iavf_find_filter(adapter, macaddr);
+ allowed = (f && f->add && !f->is_new_mac && !f->remove);
+ spin_unlock_bh(&adapter->mac_vlan_list_lock);
+ return allowed;
+}
+
/**
* iavf_parse_cls_flower - Parse tc flower filters provided by kernel
* @adapter: board private structure
@@ -3651,7 +3693,15 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
}
}
- if (!is_zero_ether_addr(match.key->dst))
+ if (!is_zero_ether_addr(match.key->dst)) {
+ if (!iavf_is_mac_tc_filter_allowed(adapter,
+ match.key->dst)) {
+ dev_err(&adapter->pdev->dev,
+ "Dest MAC %pM doesn't belong to this VF\n",
+ match.mask->dst);
+ return -EINVAL;
+ }
+
if (is_valid_ether_addr(match.key->dst) ||
is_multicast_ether_addr(match.key->dst)) {
/* set the mask if a valid dst_mac address */
@@ -3660,6 +3710,7 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
ether_addr_copy(vf->data.tcp_spec.dst_mac,
match.key->dst);
}
+ }
if (!is_zero_ether_addr(match.key->src))
if (is_valid_ether_addr(match.key->src) ||
@@ -3677,6 +3728,8 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
flow_rule_match_vlan(rule, &match);
if (match.mask->vlan_id) {
+ u16 vlan = match.key->vlan_id & VLAN_VID_MASK;
+
if (match.mask->vlan_id == VLAN_VID_MASK) {
field_flags |= IAVF_CLOUD_FIELD_IVLAN;
} else {
@@ -3684,6 +3737,13 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
match.mask->vlan_id);
return -EINVAL;
}
+
+ if (!iavf_is_vlan_tc_filter_allowed(adapter, vlan)) {
+ dev_err(&adapter->pdev->dev,
+ "VLAN %u doesn't belong to this VF\n",
+ vlan);
+ return -EINVAL;
+ }
}
vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
vf->data.tcp_spec.vlan_id = cpu_to_be16(match.key->vlan_id);
--
2.35.3
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
2022-06-17 17:50 [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path Jun Zhang
@ 2022-06-20 5:32 ` Michal Swiatkowski
2022-06-21 20:21 ` Zhang, Xuejun
2022-08-02 15:30 ` Sreenivas, Bharathi
2022-08-02 16:04 ` Sreenivas, Bharathi
2 siblings, 1 reply; 9+ messages in thread
From: Michal Swiatkowski @ 2022-06-20 5:32 UTC (permalink / raw)
To: Jun Zhang; +Cc: intel-wired-lan, Kiran Patil
On Fri, Jun 17, 2022 at 01:50:00PM -0400, Jun Zhang wrote:
> From: Kiran Patil <kiran.patil@intel.com>
>
> Before allowing tc-filter using dest MAC, VLAN - check to make
> sure there is basic active filter using specified dest MAC and
> likewise for VLAN.
>
> This check is must to allow only legit filter via tc-filter
> code path with or without ADQ.
>
> Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 62 ++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 57c51a15bcbc..287c3e4bf8af 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -3558,6 +3558,48 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> return ret;
> }
>
> +/**
> + * iavf_is_vlan_tc_filter_allowed - allowed to add tc-filter using VLAN
> + * @adapter: board private structure
> + * @vlan: VLAN to verify
> + *
> + * Using specified "vlan" ID, there must be active VLAN filter in VF's
> + * MAC-VLAN filter list.
> + */
> +static bool
> +iavf_is_vlan_tc_filter_allowed(struct iavf_adapter *adapter, u16 vlan)
> +{
> + struct iavf_vlan_filter *f;
> + bool allowed;
> +
> + spin_lock_bh(&adapter->mac_vlan_list_lock);
Why do we need lock here?
> + f = iavf_find_vlan(adapter, IAVF_VLAN(vlan, ETH_P_8021Q));
> + allowed = (f && f->add && !f->remove);
> + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> + return allowed;
> +}
> +
> +/**
> + * iavf_is_mac_tc_filter_allowed - allowed to add tc-filter using MAC addr
> + * @adapter: board private structure
> + * @macaddr: MAC address
> + *
> + * Using specified MAC address, there must be active MAC filter in VF's
> + * MAC-VLAN filter list.
> + */
> +static bool
> +iavf_is_mac_tc_filter_allowed(struct iavf_adapter *adapter, const u8 *macaddr)
> +{
> + struct iavf_mac_filter *f;
> + bool allowed;
> +
> + spin_lock_bh(&adapter->mac_vlan_list_lock);
> + f = iavf_find_filter(adapter, macaddr);
> + allowed = (f && f->add && !f->is_new_mac && !f->remove);
> + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> + return allowed;
> +}
> +
> /**
> * iavf_parse_cls_flower - Parse tc flower filters provided by kernel
> * @adapter: board private structure
> @@ -3651,7 +3693,15 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> }
> }
>
> - if (!is_zero_ether_addr(match.key->dst))
> + if (!is_zero_ether_addr(match.key->dst)) {
> + if (!iavf_is_mac_tc_filter_allowed(adapter,
> + match.key->dst)) {
> + dev_err(&adapter->pdev->dev,
> + "Dest MAC %pM doesn't belong to this VF\n",
> + match.mask->dst);
> + return -EINVAL;
> + }
> +
> if (is_valid_ether_addr(match.key->dst) ||
> is_multicast_ether_addr(match.key->dst)) {
> /* set the mask if a valid dst_mac address */
> @@ -3660,6 +3710,7 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> ether_addr_copy(vf->data.tcp_spec.dst_mac,
> match.key->dst);
> }
> + }
>
> if (!is_zero_ether_addr(match.key->src))
> if (is_valid_ether_addr(match.key->src) ||
> @@ -3677,6 +3728,8 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
>
> flow_rule_match_vlan(rule, &match);
> if (match.mask->vlan_id) {
> + u16 vlan = match.key->vlan_id & VLAN_VID_MASK;
> +
> if (match.mask->vlan_id == VLAN_VID_MASK) {
> field_flags |= IAVF_CLOUD_FIELD_IVLAN;
> } else {
> @@ -3684,6 +3737,13 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> match.mask->vlan_id);
> return -EINVAL;
> }
> +
> + if (!iavf_is_vlan_tc_filter_allowed(adapter, vlan)) {
> + dev_err(&adapter->pdev->dev,
> + "VLAN %u doesn't belong to this VF\n",
> + vlan);
> + return -EINVAL;
> + }
> }
> vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
> vf->data.tcp_spec.vlan_id = cpu_to_be16(match.key->vlan_id);
> --
> 2.35.3
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
2022-06-20 5:32 ` Michal Swiatkowski
@ 2022-06-21 20:21 ` Zhang, Xuejun
2022-06-22 1:31 ` Michal Swiatkowski
0 siblings, 1 reply; 9+ messages in thread
From: Zhang, Xuejun @ 2022-06-21 20:21 UTC (permalink / raw)
To: Michal Swiatkowski; +Cc: intel-wired-lan, Kiran Patil
Hi Michal,
Pls add your comments whenever you have a chance.
Regards,
Jun
-----Original Message-----
From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Sent: Sunday, June 19, 2022 10:33 PM
To: Zhang, Xuejun <xuejun.zhang@intel.com>
Cc: intel-wired-lan@lists.osuosl.org; Kiran Patil <kiran.patil@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
On Fri, Jun 17, 2022 at 01:50:00PM -0400, Jun Zhang wrote:
> From: Kiran Patil <kiran.patil@intel.com>
>
> Before allowing tc-filter using dest MAC, VLAN - check to make sure
> there is basic active filter using specified dest MAC and likewise for
> VLAN.
>
> This check is must to allow only legit filter via tc-filter code path
> with or without ADQ.
>
> Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 62
> ++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 57c51a15bcbc..287c3e4bf8af 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -3558,6 +3558,48 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> return ret;
> }
>
> +/**
> + * iavf_is_vlan_tc_filter_allowed - allowed to add tc-filter using
> +VLAN
> + * @adapter: board private structure
> + * @vlan: VLAN to verify
> + *
> + * Using specified "vlan" ID, there must be active VLAN filter in
> +VF's
> + * MAC-VLAN filter list.
> + */
> +static bool
> +iavf_is_vlan_tc_filter_allowed(struct iavf_adapter *adapter, u16
> +vlan) {
> + struct iavf_vlan_filter *f;
> + bool allowed;
> +
> + spin_lock_bh(&adapter->mac_vlan_list_lock);
Why do we need lock here?
> + f = iavf_find_vlan(adapter, IAVF_VLAN(vlan, ETH_P_8021Q));
> + allowed = (f && f->add && !f->remove);
> + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> + return allowed;
> +}
> +
> +/**
> + * iavf_is_mac_tc_filter_allowed - allowed to add tc-filter using MAC
> +addr
> + * @adapter: board private structure
> + * @macaddr: MAC address
> + *
> + * Using specified MAC address, there must be active MAC filter in
> +VF's
> + * MAC-VLAN filter list.
> + */
> +static bool
> +iavf_is_mac_tc_filter_allowed(struct iavf_adapter *adapter, const u8
> +*macaddr) {
> + struct iavf_mac_filter *f;
> + bool allowed;
> +
> + spin_lock_bh(&adapter->mac_vlan_list_lock);
> + f = iavf_find_filter(adapter, macaddr);
> + allowed = (f && f->add && !f->is_new_mac && !f->remove);
> + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> + return allowed;
> +}
> +
> /**
> * iavf_parse_cls_flower - Parse tc flower filters provided by kernel
> * @adapter: board private structure
> @@ -3651,7 +3693,15 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> }
> }
>
> - if (!is_zero_ether_addr(match.key->dst))
> + if (!is_zero_ether_addr(match.key->dst)) {
> + if (!iavf_is_mac_tc_filter_allowed(adapter,
> + match.key->dst)) {
> + dev_err(&adapter->pdev->dev,
> + "Dest MAC %pM doesn't belong to this VF\n",
> + match.mask->dst);
> + return -EINVAL;
> + }
> +
> if (is_valid_ether_addr(match.key->dst) ||
> is_multicast_ether_addr(match.key->dst)) {
> /* set the mask if a valid dst_mac address */ @@ -3660,6 +3710,7
> @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> ether_addr_copy(vf->data.tcp_spec.dst_mac,
> match.key->dst);
> }
> + }
>
> if (!is_zero_ether_addr(match.key->src))
> if (is_valid_ether_addr(match.key->src) || @@ -3677,6 +3728,8 @@
> static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
>
> flow_rule_match_vlan(rule, &match);
> if (match.mask->vlan_id) {
> + u16 vlan = match.key->vlan_id & VLAN_VID_MASK;
> +
> if (match.mask->vlan_id == VLAN_VID_MASK) {
> field_flags |= IAVF_CLOUD_FIELD_IVLAN;
> } else {
> @@ -3684,6 +3737,13 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> match.mask->vlan_id);
> return -EINVAL;
> }
> +
> + if (!iavf_is_vlan_tc_filter_allowed(adapter, vlan)) {
> + dev_err(&adapter->pdev->dev,
> + "VLAN %u doesn't belong to this VF\n",
> + vlan);
> + return -EINVAL;
> + }
> }
> vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
> vf->data.tcp_spec.vlan_id = cpu_to_be16(match.key->vlan_id);
> --
> 2.35.3
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
2022-06-21 20:21 ` Zhang, Xuejun
@ 2022-06-22 1:31 ` Michal Swiatkowski
2022-06-22 22:31 ` Zhang, Xuejun
0 siblings, 1 reply; 9+ messages in thread
From: Michal Swiatkowski @ 2022-06-22 1:31 UTC (permalink / raw)
To: Zhang, Xuejun; +Cc: intel-wired-lan, Kiran Patil
On Tue, Jun 21, 2022 at 08:21:42PM +0000, Zhang, Xuejun wrote:
> Hi Michal,
Hi Jun
>
> Pls add your comments whenever you have a chance.
Sorry, I am confused, comments about what? Did I miss replay to my
previous one? I didn't receive any replay to my question about why we
need this spin lock when there is only check for filter.
Please resend it to me if You can.
Regards
Michal
>
> Regards,
>
> Jun
>
> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Sent: Sunday, June 19, 2022 10:33 PM
> To: Zhang, Xuejun <xuejun.zhang@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Kiran Patil <kiran.patil@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
>
> On Fri, Jun 17, 2022 at 01:50:00PM -0400, Jun Zhang wrote:
> > From: Kiran Patil <kiran.patil@intel.com>
> >
> > Before allowing tc-filter using dest MAC, VLAN - check to make sure
> > there is basic active filter using specified dest MAC and likewise for
> > VLAN.
> >
> > This check is must to allow only legit filter via tc-filter code path
> > with or without ADQ.
> >
> > Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> > Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> > Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
> > ---
> > drivers/net/ethernet/intel/iavf/iavf_main.c | 62
> > ++++++++++++++++++++-
> > 1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > index 57c51a15bcbc..287c3e4bf8af 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > @@ -3558,6 +3558,48 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> > return ret;
> > }
> >
> > +/**
> > + * iavf_is_vlan_tc_filter_allowed - allowed to add tc-filter using
> > +VLAN
> > + * @adapter: board private structure
> > + * @vlan: VLAN to verify
> > + *
> > + * Using specified "vlan" ID, there must be active VLAN filter in
> > +VF's
> > + * MAC-VLAN filter list.
> > + */
> > +static bool
> > +iavf_is_vlan_tc_filter_allowed(struct iavf_adapter *adapter, u16
> > +vlan) {
> > + struct iavf_vlan_filter *f;
> > + bool allowed;
> > +
> > + spin_lock_bh(&adapter->mac_vlan_list_lock);
> Why do we need lock here?
>
> > + f = iavf_find_vlan(adapter, IAVF_VLAN(vlan, ETH_P_8021Q));
> > + allowed = (f && f->add && !f->remove);
> > + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> > + return allowed;
> > +}
> > +
> > +/**
> > + * iavf_is_mac_tc_filter_allowed - allowed to add tc-filter using MAC
> > +addr
> > + * @adapter: board private structure
> > + * @macaddr: MAC address
> > + *
> > + * Using specified MAC address, there must be active MAC filter in
> > +VF's
> > + * MAC-VLAN filter list.
> > + */
> > +static bool
> > +iavf_is_mac_tc_filter_allowed(struct iavf_adapter *adapter, const u8
> > +*macaddr) {
> > + struct iavf_mac_filter *f;
> > + bool allowed;
> > +
> > + spin_lock_bh(&adapter->mac_vlan_list_lock);
> > + f = iavf_find_filter(adapter, macaddr);
> > + allowed = (f && f->add && !f->is_new_mac && !f->remove);
> > + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> > + return allowed;
> > +}
> > +
> > /**
> > * iavf_parse_cls_flower - Parse tc flower filters provided by kernel
> > * @adapter: board private structure
> > @@ -3651,7 +3693,15 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > }
> > }
> >
> > - if (!is_zero_ether_addr(match.key->dst))
> > + if (!is_zero_ether_addr(match.key->dst)) {
> > + if (!iavf_is_mac_tc_filter_allowed(adapter,
> > + match.key->dst)) {
> > + dev_err(&adapter->pdev->dev,
> > + "Dest MAC %pM doesn't belong to this VF\n",
> > + match.mask->dst);
> > + return -EINVAL;
> > + }
> > +
> > if (is_valid_ether_addr(match.key->dst) ||
> > is_multicast_ether_addr(match.key->dst)) {
> > /* set the mask if a valid dst_mac address */ @@ -3660,6 +3710,7
> > @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > ether_addr_copy(vf->data.tcp_spec.dst_mac,
> > match.key->dst);
> > }
> > + }
> >
> > if (!is_zero_ether_addr(match.key->src))
> > if (is_valid_ether_addr(match.key->src) || @@ -3677,6 +3728,8 @@
> > static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> >
> > flow_rule_match_vlan(rule, &match);
> > if (match.mask->vlan_id) {
> > + u16 vlan = match.key->vlan_id & VLAN_VID_MASK;
> > +
> > if (match.mask->vlan_id == VLAN_VID_MASK) {
> > field_flags |= IAVF_CLOUD_FIELD_IVLAN;
> > } else {
> > @@ -3684,6 +3737,13 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > match.mask->vlan_id);
> > return -EINVAL;
> > }
> > +
> > + if (!iavf_is_vlan_tc_filter_allowed(adapter, vlan)) {
> > + dev_err(&adapter->pdev->dev,
> > + "VLAN %u doesn't belong to this VF\n",
> > + vlan);
> > + return -EINVAL;
> > + }
> > }
> > vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
> > vf->data.tcp_spec.vlan_id = cpu_to_be16(match.key->vlan_id);
> > --
> > 2.35.3
> >
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan@osuosl.org
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
2022-06-22 1:31 ` Michal Swiatkowski
@ 2022-06-22 22:31 ` Zhang, Xuejun
2022-06-28 1:21 ` Michal Swiatkowski
0 siblings, 1 reply; 9+ messages in thread
From: Zhang, Xuejun @ 2022-06-22 22:31 UTC (permalink / raw)
To: Michal Swiatkowski; +Cc: intel-wired-lan, Kiran Patil
Hi Michal,
Thanks for the reply.
The lock is access lock for both mac_filter_list & vlan_filter_list. Access to anyone of the two filter lists requires holding the lock per current code design.
Regards
Jun
-----Original Message-----
From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Sent: Tuesday, June 21, 2022 6:31 PM
To: Zhang, Xuejun <xuejun.zhang@intel.com>
Cc: intel-wired-lan@lists.osuosl.org; Kiran Patil <kiran.patil@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
On Tue, Jun 21, 2022 at 08:21:42PM +0000, Zhang, Xuejun wrote:
> Hi Michal,
Hi Jun
>
> Pls add your comments whenever you have a chance.
Sorry, I am confused, comments about what? Did I miss replay to my previous one? I didn't receive any replay to my question about why we need this spin lock when there is only check for filter.
Please resend it to me if You can.
Regards
Michal
>
> Regards,
>
> Jun
>
> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Sent: Sunday, June 19, 2022 10:33 PM
> To: Zhang, Xuejun <xuejun.zhang@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Kiran Patil
> <kiran.patil@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC
> and VLAN from tc-filter code path
>
> On Fri, Jun 17, 2022 at 01:50:00PM -0400, Jun Zhang wrote:
> > From: Kiran Patil <kiran.patil@intel.com>
> >
> > Before allowing tc-filter using dest MAC, VLAN - check to make sure
> > there is basic active filter using specified dest MAC and likewise
> > for VLAN.
> >
> > This check is must to allow only legit filter via tc-filter code
> > path with or without ADQ.
> >
> > Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> > Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> > Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
> > ---
> > drivers/net/ethernet/intel/iavf/iavf_main.c | 62
> > ++++++++++++++++++++-
> > 1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > index 57c51a15bcbc..287c3e4bf8af 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > @@ -3558,6 +3558,48 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> > return ret;
> > }
> >
> > +/**
> > + * iavf_is_vlan_tc_filter_allowed - allowed to add tc-filter using
> > +VLAN
> > + * @adapter: board private structure
> > + * @vlan: VLAN to verify
> > + *
> > + * Using specified "vlan" ID, there must be active VLAN filter in
> > +VF's
> > + * MAC-VLAN filter list.
> > + */
> > +static bool
> > +iavf_is_vlan_tc_filter_allowed(struct iavf_adapter *adapter, u16
> > +vlan) {
> > + struct iavf_vlan_filter *f;
> > + bool allowed;
> > +
> > + spin_lock_bh(&adapter->mac_vlan_list_lock);
> Why do we need lock here?
>
> > + f = iavf_find_vlan(adapter, IAVF_VLAN(vlan, ETH_P_8021Q));
> > + allowed = (f && f->add && !f->remove);
> > + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> > + return allowed;
> > +}
> > +
> > +/**
> > + * iavf_is_mac_tc_filter_allowed - allowed to add tc-filter using
> > +MAC addr
> > + * @adapter: board private structure
> > + * @macaddr: MAC address
> > + *
> > + * Using specified MAC address, there must be active MAC filter in
> > +VF's
> > + * MAC-VLAN filter list.
> > + */
> > +static bool
> > +iavf_is_mac_tc_filter_allowed(struct iavf_adapter *adapter, const
> > +u8
> > +*macaddr) {
> > + struct iavf_mac_filter *f;
> > + bool allowed;
> > +
> > + spin_lock_bh(&adapter->mac_vlan_list_lock);
> > + f = iavf_find_filter(adapter, macaddr);
> > + allowed = (f && f->add && !f->is_new_mac && !f->remove);
> > + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> > + return allowed;
> > +}
> > +
> > /**
> > * iavf_parse_cls_flower - Parse tc flower filters provided by kernel
> > * @adapter: board private structure @@ -3651,7 +3693,15 @@ static
> > int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > }
> > }
> >
> > - if (!is_zero_ether_addr(match.key->dst))
> > + if (!is_zero_ether_addr(match.key->dst)) {
> > + if (!iavf_is_mac_tc_filter_allowed(adapter,
> > + match.key->dst)) {
> > + dev_err(&adapter->pdev->dev,
> > + "Dest MAC %pM doesn't belong to this VF\n",
> > + match.mask->dst);
> > + return -EINVAL;
> > + }
> > +
> > if (is_valid_ether_addr(match.key->dst) ||
> > is_multicast_ether_addr(match.key->dst)) {
> > /* set the mask if a valid dst_mac address */ @@ -3660,6
> > +3710,7 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > ether_addr_copy(vf->data.tcp_spec.dst_mac,
> > match.key->dst);
> > }
> > + }
> >
> > if (!is_zero_ether_addr(match.key->src))
> > if (is_valid_ether_addr(match.key->src) || @@ -3677,6 +3728,8 @@
> > static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> >
> > flow_rule_match_vlan(rule, &match);
> > if (match.mask->vlan_id) {
> > + u16 vlan = match.key->vlan_id & VLAN_VID_MASK;
> > +
> > if (match.mask->vlan_id == VLAN_VID_MASK) {
> > field_flags |= IAVF_CLOUD_FIELD_IVLAN;
> > } else {
> > @@ -3684,6 +3737,13 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > match.mask->vlan_id);
> > return -EINVAL;
> > }
> > +
> > + if (!iavf_is_vlan_tc_filter_allowed(adapter, vlan)) {
> > + dev_err(&adapter->pdev->dev,
> > + "VLAN %u doesn't belong to this VF\n",
> > + vlan);
> > + return -EINVAL;
> > + }
> > }
> > vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
> > vf->data.tcp_spec.vlan_id = cpu_to_be16(match.key->vlan_id);
> > --
> > 2.35.3
> >
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan@osuosl.org
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
2022-06-22 22:31 ` Zhang, Xuejun
@ 2022-06-28 1:21 ` Michal Swiatkowski
2022-06-28 17:15 ` Maciej Fijalkowski
0 siblings, 1 reply; 9+ messages in thread
From: Michal Swiatkowski @ 2022-06-28 1:21 UTC (permalink / raw)
To: Zhang, Xuejun; +Cc: intel-wired-lan, Kiran Patil
On Wed, Jun 22, 2022 at 10:31:42PM +0000, Zhang, Xuejun wrote:
> Hi Michal,
>
> Thanks for the reply.
>
> The lock is access lock for both mac_filter_list & vlan_filter_list. Access to anyone of the two filter lists requires holding the lock per current code design.
Ok, thanks for explanation.
>
> Regards
>
> Jun
>
> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Sent: Tuesday, June 21, 2022 6:31 PM
> To: Zhang, Xuejun <xuejun.zhang@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Kiran Patil <kiran.patil@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
>
> On Tue, Jun 21, 2022 at 08:21:42PM +0000, Zhang, Xuejun wrote:
> > Hi Michal,
>
> Hi Jun
> >
> > Pls add your comments whenever you have a chance.
>
> Sorry, I am confused, comments about what? Did I miss replay to my previous one? I didn't receive any replay to my question about why we need this spin lock when there is only check for filter.
>
> Please resend it to me if You can.
>
> Regards
> Michal
>
> >
> > Regards,
> >
> > Jun
> >
> > -----Original Message-----
> > From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > Sent: Sunday, June 19, 2022 10:33 PM
> > To: Zhang, Xuejun <xuejun.zhang@intel.com>
> > Cc: intel-wired-lan@lists.osuosl.org; Kiran Patil
> > <kiran.patil@intel.com>
> > Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC
> > and VLAN from tc-filter code path
> >
> > On Fri, Jun 17, 2022 at 01:50:00PM -0400, Jun Zhang wrote:
> > > From: Kiran Patil <kiran.patil@intel.com>
> > >
> > > Before allowing tc-filter using dest MAC, VLAN - check to make sure
> > > there is basic active filter using specified dest MAC and likewise
> > > for VLAN.
> > >
> > > This check is must to allow only legit filter via tc-filter code
> > > path with or without ADQ.
> > >
> > > Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> > > Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> > > Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
> > > ---
> > > drivers/net/ethernet/intel/iavf/iavf_main.c | 62
> > > ++++++++++++++++++++-
> > > 1 file changed, 61 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > > b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > > index 57c51a15bcbc..287c3e4bf8af 100644
> > > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > > @@ -3558,6 +3558,48 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> > > return ret;
> > > }
> > >
> > > +/**
> > > + * iavf_is_vlan_tc_filter_allowed - allowed to add tc-filter using
> > > +VLAN
> > > + * @adapter: board private structure
> > > + * @vlan: VLAN to verify
> > > + *
> > > + * Using specified "vlan" ID, there must be active VLAN filter in
> > > +VF's
> > > + * MAC-VLAN filter list.
> > > + */
> > > +static bool
> > > +iavf_is_vlan_tc_filter_allowed(struct iavf_adapter *adapter, u16
> > > +vlan) {
> > > + struct iavf_vlan_filter *f;
> > > + bool allowed;
> > > +
> > > + spin_lock_bh(&adapter->mac_vlan_list_lock);
> > Why do we need lock here?
> >
> > > + f = iavf_find_vlan(adapter, IAVF_VLAN(vlan, ETH_P_8021Q));
> > > + allowed = (f && f->add && !f->remove);
> > > + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> > > + return allowed;
> > > +}
> > > +
> > > +/**
> > > + * iavf_is_mac_tc_filter_allowed - allowed to add tc-filter using
> > > +MAC addr
> > > + * @adapter: board private structure
> > > + * @macaddr: MAC address
> > > + *
> > > + * Using specified MAC address, there must be active MAC filter in
> > > +VF's
> > > + * MAC-VLAN filter list.
> > > + */
> > > +static bool
> > > +iavf_is_mac_tc_filter_allowed(struct iavf_adapter *adapter, const
> > > +u8
> > > +*macaddr) {
> > > + struct iavf_mac_filter *f;
> > > + bool allowed;
> > > +
> > > + spin_lock_bh(&adapter->mac_vlan_list_lock);
> > > + f = iavf_find_filter(adapter, macaddr);
> > > + allowed = (f && f->add && !f->is_new_mac && !f->remove);
> > > + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> > > + return allowed;
> > > +}
> > > +
> > > /**
> > > * iavf_parse_cls_flower - Parse tc flower filters provided by kernel
> > > * @adapter: board private structure @@ -3651,7 +3693,15 @@ static
> > > int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > > }
> > > }
> > >
> > > - if (!is_zero_ether_addr(match.key->dst))
> > > + if (!is_zero_ether_addr(match.key->dst)) {
> > > + if (!iavf_is_mac_tc_filter_allowed(adapter,
> > > + match.key->dst)) {
> > > + dev_err(&adapter->pdev->dev,
> > > + "Dest MAC %pM doesn't belong to this VF\n",
> > > + match.mask->dst);
> > > + return -EINVAL;
> > > + }
> > > +
> > > if (is_valid_ether_addr(match.key->dst) ||
> > > is_multicast_ether_addr(match.key->dst)) {
> > > /* set the mask if a valid dst_mac address */ @@ -3660,6
> > > +3710,7 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > > ether_addr_copy(vf->data.tcp_spec.dst_mac,
> > > match.key->dst);
> > > }
> > > + }
> > >
> > > if (!is_zero_ether_addr(match.key->src))
> > > if (is_valid_ether_addr(match.key->src) || @@ -3677,6 +3728,8 @@
> > > static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > >
> > > flow_rule_match_vlan(rule, &match);
> > > if (match.mask->vlan_id) {
> > > + u16 vlan = match.key->vlan_id & VLAN_VID_MASK;
> > > +
> > > if (match.mask->vlan_id == VLAN_VID_MASK) {
> > > field_flags |= IAVF_CLOUD_FIELD_IVLAN;
> > > } else {
> > > @@ -3684,6 +3737,13 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > > match.mask->vlan_id);
> > > return -EINVAL;
> > > }
> > > +
> > > + if (!iavf_is_vlan_tc_filter_allowed(adapter, vlan)) {
> > > + dev_err(&adapter->pdev->dev,
> > > + "VLAN %u doesn't belong to this VF\n",
> > > + vlan);
> > > + return -EINVAL;
> > > + }
> > > }
> > > vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
> > > vf->data.tcp_spec.vlan_id = cpu_to_be16(match.key->vlan_id);
> > > --
> > > 2.35.3
> > >
> > > _______________________________________________
> > > Intel-wired-lan mailing list
> > > Intel-wired-lan@osuosl.org
> > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
2022-06-28 1:21 ` Michal Swiatkowski
@ 2022-06-28 17:15 ` Maciej Fijalkowski
0 siblings, 0 replies; 9+ messages in thread
From: Maciej Fijalkowski @ 2022-06-28 17:15 UTC (permalink / raw)
To: Michal Swiatkowski; +Cc: intel-wired-lan, Kiran Patil
On Mon, Jun 27, 2022 at 09:21:59PM -0400, Michal Swiatkowski wrote:
> On Wed, Jun 22, 2022 at 10:31:42PM +0000, Zhang, Xuejun wrote:
> > Hi Michal,
> >
> > Thanks for the reply.
> >
> > The lock is access lock for both mac_filter_list & vlan_filter_list. Access to anyone of the two filter lists requires holding the lock per current code design.
>
> Ok, thanks for explanation.
This doesn't explain why spin_lock_bh() is used and what contexts are
being protected.
>
> >
> > Regards
> >
> > Jun
> >
> > -----Original Message-----
> > From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > Sent: Tuesday, June 21, 2022 6:31 PM
> > To: Zhang, Xuejun <xuejun.zhang@intel.com>
> > Cc: intel-wired-lan@lists.osuosl.org; Kiran Patil <kiran.patil@intel.com>
> > Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
> >
> > On Tue, Jun 21, 2022 at 08:21:42PM +0000, Zhang, Xuejun wrote:
> > > Hi Michal,
> >
> > Hi Jun
> > >
> > > Pls add your comments whenever you have a chance.
> >
> > Sorry, I am confused, comments about what? Did I miss replay to my previous one? I didn't receive any replay to my question about why we need this spin lock when there is only check for filter.
> >
> > Please resend it to me if You can.
> >
> > Regards
> > Michal
> >
> > >
> > > Regards,
> > >
> > > Jun
> > >
> > > -----Original Message-----
> > > From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > > Sent: Sunday, June 19, 2022 10:33 PM
> > > To: Zhang, Xuejun <xuejun.zhang@intel.com>
> > > Cc: intel-wired-lan@lists.osuosl.org; Kiran Patil
> > > <kiran.patil@intel.com>
> > > Subject: Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC
> > > and VLAN from tc-filter code path
> > >
> > > On Fri, Jun 17, 2022 at 01:50:00PM -0400, Jun Zhang wrote:
> > > > From: Kiran Patil <kiran.patil@intel.com>
> > > >
> > > > Before allowing tc-filter using dest MAC, VLAN - check to make sure
> > > > there is basic active filter using specified dest MAC and likewise
> > > > for VLAN.
> > > >
> > > > This check is must to allow only legit filter via tc-filter code
> > > > path with or without ADQ.
> > > >
> > > > Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> > > > Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> > > > Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
> > > > ---
> > > > drivers/net/ethernet/intel/iavf/iavf_main.c | 62
> > > > ++++++++++++++++++++-
> > > > 1 file changed, 61 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > > > b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > > > index 57c51a15bcbc..287c3e4bf8af 100644
> > > > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > > > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > > > @@ -3558,6 +3558,48 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
> > > > return ret;
> > > > }
> > > >
> > > > +/**
> > > > + * iavf_is_vlan_tc_filter_allowed - allowed to add tc-filter using
> > > > +VLAN
> > > > + * @adapter: board private structure
> > > > + * @vlan: VLAN to verify
> > > > + *
> > > > + * Using specified "vlan" ID, there must be active VLAN filter in
> > > > +VF's
> > > > + * MAC-VLAN filter list.
> > > > + */
> > > > +static bool
> > > > +iavf_is_vlan_tc_filter_allowed(struct iavf_adapter *adapter, u16
> > > > +vlan) {
> > > > + struct iavf_vlan_filter *f;
> > > > + bool allowed;
> > > > +
> > > > + spin_lock_bh(&adapter->mac_vlan_list_lock);
> > > Why do we need lock here?
> > >
> > > > + f = iavf_find_vlan(adapter, IAVF_VLAN(vlan, ETH_P_8021Q));
> > > > + allowed = (f && f->add && !f->remove);
> > > > + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> > > > + return allowed;
> > > > +}
> > > > +
> > > > +/**
> > > > + * iavf_is_mac_tc_filter_allowed - allowed to add tc-filter using
> > > > +MAC addr
> > > > + * @adapter: board private structure
> > > > + * @macaddr: MAC address
> > > > + *
> > > > + * Using specified MAC address, there must be active MAC filter in
> > > > +VF's
> > > > + * MAC-VLAN filter list.
> > > > + */
> > > > +static bool
> > > > +iavf_is_mac_tc_filter_allowed(struct iavf_adapter *adapter, const
> > > > +u8
> > > > +*macaddr) {
> > > > + struct iavf_mac_filter *f;
> > > > + bool allowed;
> > > > +
> > > > + spin_lock_bh(&adapter->mac_vlan_list_lock);
> > > > + f = iavf_find_filter(adapter, macaddr);
> > > > + allowed = (f && f->add && !f->is_new_mac && !f->remove);
> > > > + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> > > > + return allowed;
> > > > +}
> > > > +
> > > > /**
> > > > * iavf_parse_cls_flower - Parse tc flower filters provided by kernel
> > > > * @adapter: board private structure @@ -3651,7 +3693,15 @@ static
> > > > int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > > > }
> > > > }
> > > >
> > > > - if (!is_zero_ether_addr(match.key->dst))
> > > > + if (!is_zero_ether_addr(match.key->dst)) {
> > > > + if (!iavf_is_mac_tc_filter_allowed(adapter,
> > > > + match.key->dst)) {
> > > > + dev_err(&adapter->pdev->dev,
> > > > + "Dest MAC %pM doesn't belong to this VF\n",
> > > > + match.mask->dst);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > if (is_valid_ether_addr(match.key->dst) ||
> > > > is_multicast_ether_addr(match.key->dst)) {
> > > > /* set the mask if a valid dst_mac address */ @@ -3660,6
> > > > +3710,7 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > > > ether_addr_copy(vf->data.tcp_spec.dst_mac,
> > > > match.key->dst);
> > > > }
> > > > + }
> > > >
> > > > if (!is_zero_ether_addr(match.key->src))
> > > > if (is_valid_ether_addr(match.key->src) || @@ -3677,6 +3728,8 @@
> > > > static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > > >
> > > > flow_rule_match_vlan(rule, &match);
> > > > if (match.mask->vlan_id) {
> > > > + u16 vlan = match.key->vlan_id & VLAN_VID_MASK;
> > > > +
> > > > if (match.mask->vlan_id == VLAN_VID_MASK) {
> > > > field_flags |= IAVF_CLOUD_FIELD_IVLAN;
> > > > } else {
> > > > @@ -3684,6 +3737,13 @@ static int iavf_parse_cls_flower(struct iavf_adapter *adapter,
> > > > match.mask->vlan_id);
> > > > return -EINVAL;
> > > > }
> > > > +
> > > > + if (!iavf_is_vlan_tc_filter_allowed(adapter, vlan)) {
> > > > + dev_err(&adapter->pdev->dev,
> > > > + "VLAN %u doesn't belong to this VF\n",
> > > > + vlan);
> > > > + return -EINVAL;
> > > > + }
> > > > }
> > > > vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
> > > > vf->data.tcp_spec.vlan_id = cpu_to_be16(match.key->vlan_id);
> > > > --
> > > > 2.35.3
> > > >
> > > > _______________________________________________
> > > > Intel-wired-lan mailing list
> > > > Intel-wired-lan@osuosl.org
> > > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
2022-06-17 17:50 [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path Jun Zhang
2022-06-20 5:32 ` Michal Swiatkowski
@ 2022-08-02 15:30 ` Sreenivas, Bharathi
2022-08-02 16:04 ` Sreenivas, Bharathi
2 siblings, 0 replies; 9+ messages in thread
From: Sreenivas, Bharathi @ 2022-08-02 15:30 UTC (permalink / raw)
To: intel-wired-lan
Hi All,
This patch test is failing.
Please refer to the below HSD_ES for more details:
https://hsdes.intel.com/appstore/article/#/16017335057
Thanks,
Bharathi
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jun
> Zhang
> Sent: Friday, June 17, 2022 11:20 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Kiran Patil <kiran.patil@intel.com>
> Subject: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN
> from tc-filter code path
>
> From: Kiran Patil <kiran.patil@intel.com>
>
> Before allowing tc-filter using dest MAC, VLAN - check to make sure there is
> basic active filter using specified dest MAC and likewise for VLAN.
>
> This check is must to allow only legit filter via tc-filter code path with or
> without ADQ.
>
> Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 62 ++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 57c51a15bcbc..287c3e4bf8af 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -3558,6 +3558,48 @@ static int __iavf_setup_tc(struct net_device
> *netdev, void *type_data)
> return ret;
> }
>
> +/**
> + * iavf_is_vlan_tc_filter_allowed - allowed to add tc-filter using VLAN
> + * @adapter: board private structure
> + * @vlan: VLAN to verify
> + *
> + * Using specified "vlan" ID, there must be active VLAN filter in VF's
> + * MAC-VLAN filter list.
> + */
> +static bool
> +iavf_is_vlan_tc_filter_allowed(struct iavf_adapter *adapter, u16 vlan)
> +{
> + struct iavf_vlan_filter *f;
> + bool allowed;
> +
> + spin_lock_bh(&adapter->mac_vlan_list_lock);
> + f = iavf_find_vlan(adapter, IAVF_VLAN(vlan, ETH_P_8021Q));
> + allowed = (f && f->add && !f->remove);
> + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> + return allowed;
> +}
> +
> +/**
> + * iavf_is_mac_tc_filter_allowed - allowed to add tc-filter using MAC
> +addr
> + * @adapter: board private structure
> + * @macaddr: MAC address
> + *
> + * Using specified MAC address, there must be active MAC filter in VF's
> + * MAC-VLAN filter list.
> + */
> +static bool
> +iavf_is_mac_tc_filter_allowed(struct iavf_adapter *adapter, const u8
> +*macaddr) {
> + struct iavf_mac_filter *f;
> + bool allowed;
> +
> + spin_lock_bh(&adapter->mac_vlan_list_lock);
> + f = iavf_find_filter(adapter, macaddr);
> + allowed = (f && f->add && !f->is_new_mac && !f->remove);
> + spin_unlock_bh(&adapter->mac_vlan_list_lock);
> + return allowed;
> +}
> +
> /**
> * iavf_parse_cls_flower - Parse tc flower filters provided by kernel
> * @adapter: board private structure
> @@ -3651,7 +3693,15 @@ static int iavf_parse_cls_flower(struct iavf_adapter
> *adapter,
> }
> }
>
> - if (!is_zero_ether_addr(match.key->dst))
> + if (!is_zero_ether_addr(match.key->dst)) {
> + if (!iavf_is_mac_tc_filter_allowed(adapter,
> + match.key->dst)) {
> + dev_err(&adapter->pdev->dev,
> + "Dest MAC %pM doesn't belong to
> this VF\n",
> + match.mask->dst);
> + return -EINVAL;
> + }
> +
> if (is_valid_ether_addr(match.key->dst) ||
> is_multicast_ether_addr(match.key->dst)) {
> /* set the mask if a valid dst_mac address */
> @@ -3660,6 +3710,7 @@ static int iavf_parse_cls_flower(struct iavf_adapter
> *adapter,
> ether_addr_copy(vf->data.tcp_spec.dst_mac,
> match.key->dst);
> }
> + }
>
> if (!is_zero_ether_addr(match.key->src))
> if (is_valid_ether_addr(match.key->src) || @@ -
> 3677,6 +3728,8 @@ static int iavf_parse_cls_flower(struct iavf_adapter
> *adapter,
>
> flow_rule_match_vlan(rule, &match);
> if (match.mask->vlan_id) {
> + u16 vlan = match.key->vlan_id & VLAN_VID_MASK;
> +
> if (match.mask->vlan_id == VLAN_VID_MASK) {
> field_flags |= IAVF_CLOUD_FIELD_IVLAN;
> } else {
> @@ -3684,6 +3737,13 @@ static int iavf_parse_cls_flower(struct iavf_adapter
> *adapter,
> match.mask->vlan_id);
> return -EINVAL;
> }
> +
> + if (!iavf_is_vlan_tc_filter_allowed(adapter, vlan)) {
> + dev_err(&adapter->pdev->dev,
> + "VLAN %u doesn't belong to this
> VF\n",
> + vlan);
> + return -EINVAL;
> + }
> }
> vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
> vf->data.tcp_spec.vlan_id = cpu_to_be16(match.key->vlan_id);
> --
> 2.35.3
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path
2022-06-17 17:50 [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path Jun Zhang
2022-06-20 5:32 ` Michal Swiatkowski
2022-08-02 15:30 ` Sreenivas, Bharathi
@ 2022-08-02 16:04 ` Sreenivas, Bharathi
2 siblings, 0 replies; 9+ messages in thread
From: Sreenivas, Bharathi @ 2022-08-02 16:04 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jun
> Zhang
> Sent: Friday, June 17, 2022 11:20 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Kiran Patil <kiran.patil@intel.com>
> Subject: [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN
> from tc-filter code path
>
> From: Kiran Patil <kiran.patil@intel.com>
>
> Before allowing tc-filter using dest MAC, VLAN - check to make sure there is
> basic active filter using specified dest MAC and likewise for VLAN.
>
> This check is must to allow only legit filter via tc-filter code path with or
> without ADQ.
>
> Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> Signed-off-by: Jun Zhang <xuejun.zhang@intel.com>
> ---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 62 ++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
The patch test is failing while adding a filter with MAC address on VF of a Fortville card.
#tc filter add dev ens803f0v0 protocol ip parent ffff: prio 1 flower ip_proto tcp dst_mac 6e:a5:60:1f:d6:54 dst_port 5555 skip_sw hw_tc 1
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Dmesg Error Log:
+20.796049] iavf 000:86:02.0: Dest MAC ff:ff:ff:ff:ff:ff:ff:ff doesn't belong to this VF
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-08-02 16:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17 17:50 [Intel-wired-lan] [PATCH net v1] iavf: validate dest MAC and VLAN from tc-filter code path Jun Zhang
2022-06-20 5:32 ` Michal Swiatkowski
2022-06-21 20:21 ` Zhang, Xuejun
2022-06-22 1:31 ` Michal Swiatkowski
2022-06-22 22:31 ` Zhang, Xuejun
2022-06-28 1:21 ` Michal Swiatkowski
2022-06-28 17:15 ` Maciej Fijalkowski
2022-08-02 15:30 ` Sreenivas, Bharathi
2022-08-02 16:04 ` Sreenivas, Bharathi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.