* [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan
@ 2019-02-21 17:54 Stephen Hemminger
2019-02-21 18:34 ` Stefano Brivio
2019-02-22 4:58 ` Moshe Shemesh
0 siblings, 2 replies; 5+ messages in thread
From: Stephen Hemminger @ 2019-02-21 17:54 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: netdev, Stephen Hemminger, Stephen Hemminger
The netlink API for IFLA_VF_VLAN_LIST allows multiple VLAN tags to
be passed (and the message was validated) but only the first VLAN
tag was being passed to the device. Change to iterate over each tag received.
Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/core/rtnetlink.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a51cab95ba64..3a9ec988ae21 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2207,11 +2207,10 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
if (tb[IFLA_VF_VLAN_LIST]) {
struct ifla_vf_vlan_info *ivvl[MAX_VLAN_LIST_LEN];
struct nlattr *attr;
- int rem, len = 0;
+ int i, rem, len = 0;
- err = -EOPNOTSUPP;
if (!ops->ndo_set_vf_vlan)
- return err;
+ return -EOPNOTSUPP;
nla_for_each_nested(attr, tb[IFLA_VF_VLAN_LIST], rem) {
if (nla_type(attr) != IFLA_VF_VLAN_INFO ||
@@ -2224,13 +2223,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
len++;
}
- if (len == 0)
- return -EINVAL;
- err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
- ivvl[0]->qos, ivvl[0]->vlan_proto);
- if (err < 0)
- return err;
+ err = -EINVAL; /* empty list error */
+ for (i = 0; i < len; i++) {
+ err = ops->ndo_set_vf_vlan(dev, ivvl[i]->vf,
+ ivvl[i]->vlan, ivvl[i]->qos,
+ ivvl[i]->vlan_proto);
+ if (err < 0)
+ return err;
+ }
}
if (tb[IFLA_VF_TX_RATE]) {
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan
2019-02-21 17:54 [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan Stephen Hemminger
@ 2019-02-21 18:34 ` Stefano Brivio
2019-02-21 18:52 ` Stephen Hemminger
2019-02-22 4:58 ` Moshe Shemesh
1 sibling, 1 reply; 5+ messages in thread
From: Stefano Brivio @ 2019-02-21 18:34 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Moshe Shemesh, netdev, Stephen Hemminger
On Thu, 21 Feb 2019 09:54:36 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:
> @@ -2224,13 +2223,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
>
> len++;
> }
> - if (len == 0)
> - return -EINVAL;
>
> - err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
> - ivvl[0]->qos, ivvl[0]->vlan_proto);
> - if (err < 0)
> - return err;
> + err = -EINVAL; /* empty list error */
> + for (i = 0; i < len; i++) {
> + err = ops->ndo_set_vf_vlan(dev, ivvl[i]->vf,
> + ivvl[i]->vlan, ivvl[i]->qos,
> + ivvl[i]->vlan_proto);
> + if (err < 0)
> + return err;
> + }
I think the:
if (err < 0)
return err;
should be outside the loop (with a "break;" inside), otherwise you won't
return anymore if len == 0.
--
Stefano
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan
2019-02-21 18:34 ` Stefano Brivio
@ 2019-02-21 18:52 ` Stephen Hemminger
0 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2019-02-21 18:52 UTC (permalink / raw)
To: Stefano Brivio; +Cc: Moshe Shemesh, netdev, Stephen Hemminger
On Thu, 21 Feb 2019 19:34:57 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:
> On Thu, 21 Feb 2019 09:54:36 -0800
> Stephen Hemminger <stephen@networkplumber.org> wrote:
>
> > @@ -2224,13 +2223,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> >
> > len++;
> > }
> > - if (len == 0)
> > - return -EINVAL;
> >
> > - err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
> > - ivvl[0]->qos, ivvl[0]->vlan_proto);
> > - if (err < 0)
> > - return err;
> > + err = -EINVAL; /* empty list error */
> > + for (i = 0; i < len; i++) {
> > + err = ops->ndo_set_vf_vlan(dev, ivvl[i]->vf,
> > + ivvl[i]->vlan, ivvl[i]->qos,
> > + ivvl[i]->vlan_proto);
> > + if (err < 0)
> > + return err;
> > + }
>
> I think the:
>
> if (err < 0)
> return err;
>
> should be outside the loop (with a "break;" inside), otherwise you won't
> return anymore if len == 0.
>
Your right with empty list it would fall through and look at other attributes
which could overwrite err.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan
2019-02-21 17:54 [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan Stephen Hemminger
2019-02-21 18:34 ` Stefano Brivio
@ 2019-02-22 4:58 ` Moshe Shemesh
2019-02-22 18:09 ` Stephen Hemminger
1 sibling, 1 reply; 5+ messages in thread
From: Moshe Shemesh @ 2019-02-22 4:58 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Stephen Hemminger
On 2/21/2019 7:54 PM, Stephen Hemminger wrote:
> The netlink API for IFLA_VF_VLAN_LIST allows multiple VLAN tags to
> be passed (and the message was validated) but only the first VLAN
> tag was being passed to the device. Change to iterate over each tag received.
>
> Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> net/core/rtnetlink.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index a51cab95ba64..3a9ec988ae21 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2207,11 +2207,10 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> if (tb[IFLA_VF_VLAN_LIST]) {
> struct ifla_vf_vlan_info *ivvl[MAX_VLAN_LIST_LEN];
> struct nlattr *attr;
> - int rem, len = 0;
> + int i, rem, len = 0;
>
> - err = -EOPNOTSUPP;
> if (!ops->ndo_set_vf_vlan)
> - return err;
> + return -EOPNOTSUPP;
>
> nla_for_each_nested(attr, tb[IFLA_VF_VLAN_LIST], rem) {
> if (nla_type(attr) != IFLA_VF_VLAN_INFO ||
> @@ -2224,13 +2223,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
>
> len++;
> }
> - if (len == 0)
> - return -EINVAL;
>
> - err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
> - ivvl[0]->qos, ivvl[0]->vlan_proto);
> - if (err < 0)
> - return err;
> + err = -EINVAL; /* empty list error */
> + for (i = 0; i < len; i++) {
> + err = ops->ndo_set_vf_vlan(dev, ivvl[i]->vf,
> + ivvl[i]->vlan, ivvl[i]->qos,
> + ivvl[i]->vlan_proto);
Doing that each vlan will just overwrite the vf vlan configuration set
by its preceding one.
Note #define MAX_VLAN_LIST_LEN 1
The point here was that I had to add the rtnl interface to set vf vlan
with option to set vf vlan protocol. While doing that I was asked to add
option to get a list of vlans from user to support QinQ, so once it will
be needed there won't be a need to add another rtnl interface.
The driver which will support setting double vlan or more per vf will
change MAX_VLAN_LIST_LEN and change the ndo function.
> + if (err < 0)
> + return err;
> + }
> }
>
> if (tb[IFLA_VF_TX_RATE]) {
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan
2019-02-22 4:58 ` Moshe Shemesh
@ 2019-02-22 18:09 ` Stephen Hemminger
0 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2019-02-22 18:09 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: netdev, Stephen Hemminger
On Fri, 22 Feb 2019 04:58:06 +0000
Moshe Shemesh <moshe@mellanox.com> wrote:
> On 2/21/2019 7:54 PM, Stephen Hemminger wrote:
> > The netlink API for IFLA_VF_VLAN_LIST allows multiple VLAN tags to
> > be passed (and the message was validated) but only the first VLAN
> > tag was being passed to the device. Change to iterate over each tag received.
> >
> > Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > net/core/rtnetlink.c | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index a51cab95ba64..3a9ec988ae21 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -2207,11 +2207,10 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> > if (tb[IFLA_VF_VLAN_LIST]) {
> > struct ifla_vf_vlan_info *ivvl[MAX_VLAN_LIST_LEN];
> > struct nlattr *attr;
> > - int rem, len = 0;
> > + int i, rem, len = 0;
> >
> > - err = -EOPNOTSUPP;
> > if (!ops->ndo_set_vf_vlan)
> > - return err;
> > + return -EOPNOTSUPP;
> >
> > nla_for_each_nested(attr, tb[IFLA_VF_VLAN_LIST], rem) {
> > if (nla_type(attr) != IFLA_VF_VLAN_INFO ||
> > @@ -2224,13 +2223,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> >
> > len++;
> > }
> > - if (len == 0)
> > - return -EINVAL;
> >
> > - err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
> > - ivvl[0]->qos, ivvl[0]->vlan_proto);
> > - if (err < 0)
> > - return err;
> > + err = -EINVAL; /* empty list error */
> > + for (i = 0; i < len; i++) {
> > + err = ops->ndo_set_vf_vlan(dev, ivvl[i]->vf,
> > + ivvl[i]->vlan, ivvl[i]->qos,
> > + ivvl[i]->vlan_proto);
>
> Doing that each vlan will just overwrite the vf vlan configuration set
> by its preceding one.
> Note #define MAX_VLAN_LIST_LEN 1
> The point here was that I had to add the rtnl interface to set vf vlan
> with option to set vf vlan protocol. While doing that I was asked to add
> option to get a list of vlans from user to support QinQ, so once it will
> be needed there won't be a need to add another rtnl interface.
>
> The driver which will support setting double vlan or more per vf will
> change MAX_VLAN_LIST_LEN and change the ndo function.
>
OK, then the original code kind of makes sense now. Maybe the whole loop
could be eliminated.
I expected that the vlan list was for allowing multiple vlan's to go to
a single VF but it is really a future (unlikely to ever be implemented)
support for QinQ.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-22 18:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-21 17:54 [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan Stephen Hemminger
2019-02-21 18:34 ` Stefano Brivio
2019-02-21 18:52 ` Stephen Hemminger
2019-02-22 4:58 ` Moshe Shemesh
2019-02-22 18:09 ` Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).