On 12/30/14, 12:40 AM, Arad, Ronen wrote:

quoted

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
Behalf Of roopa@cumulusnetworks.com
Sent: Monday, December 29, 2014 11:05 PM
To: netdev@vger.kernel.org; shemminger@vyatta.com; vyasevic@redhat.com
Cc: Roopa Prabhu; Wilson kok
Subject: [PATCH 3/6] bridge: modify IFLA_AF_SPEC parser to parse
IFLA_BRIDGE_VLAN_RANGE_INFO

From: Roopa Prabhu <redacted>

This patch modifies br_afspec to parse incoming IFLA_BRIDGE_VLAN_RANGE_INFO

Signed-off-by: Wilson kok <redacted>
Signed-off-by: Roopa Prabhu <redacted>
---
net/bridge/br_netlink.c |   70 +++++++++++++++++++++++++++++++++-------------
-
1 file changed, 49 insertions(+), 21 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index e7d1fc0..4c47ba0 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c

@@ -227,48 +227,76 @@ static const struct nla_policy

ifla_br_policy[IFLA_MAX+1] = {
			 .len = sizeof(struct bridge_vlan_range_info), },
};

+static int br_afspec_vlan_add(struct net_bridge *br,
+			      struct net_bridge_port *p,
+			      u16 vid, u16 flags)
+{
+	int err = 0;
+
+	if (p) {
+		err = nbp_vlan_add(p, vid, flags);
+		if (err)
+			return err;
+
+		if (flags & BRIDGE_VLAN_INFO_MASTER)
+			err = br_vlan_add(p->br, vid, flags);
+	} else {
+		err = br_vlan_add(br, vid, flags);
+	}
+
+	return err;
+}
+
+static void br_afspec_vlan_del(struct net_bridge *br,
+			       struct net_bridge_port *p,
+			       u16 vid, u16 flags)
+{
+	if (p) {
+		nbp_vlan_delete(p, vid);
+		if (flags & BRIDGE_VLAN_INFO_MASTER)
+			br_vlan_delete(p->br, vid);
+	} else {
+		br_vlan_delete(br, vid);
+	}
+}
+
static int br_afspec(struct net_bridge *br,
		     struct net_bridge_port *p,
		     struct nlattr *af_spec,
		     int cmd)
{
-	struct bridge_vlan_info *vinfo;
-	int err = 0;
+	struct bridge_vlan_range_info *vinfo;
	struct nlattr *attr;
	int err = 0;
	int rem;
	u16 vid;

	nla_for_each_nested(attr, af_spec, rem) {
-		if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
+		if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO &&
+		    nla_type(attr) != IFLA_BRIDGE_VLAN_RANGE_INFO)
			continue;
-
		vinfo = nla_data(attr);
-		if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK)
+
+		if (nla_type(attr) == IFLA_BRIDGE_VLAN_INFO)
+			vinfo->vid_end = vinfo->vid;
+
+		if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK ||
+		    vinfo->vid_end >= VLAN_VID_MASK ||
+		    vinfo->vid > vinfo->vid_end)
			return -EINVAL;

		switch (cmd) {
		case RTM_SETLINK:
-			if (p) {
-				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
+			for (vid = vinfo->vid; vid <= vinfo->vid_end; vid++) {
+				err = br_afspec_vlan_add(br, p, vid,
+							 vinfo->flags);

vinfo->flags could have BRIDGE_VLAN_INFO_PVID set. It is really a port property and there could only be a single PVID for a port. The loop will make the port pvid set, in turn, to each vid in the range until it finally set to vid_end.
This could be avoided by turning off this flag for all but the last vid in range:

				err = br_afspec_vlan_addr(br, p, vid,
                                                     ((vid == vinfo->vid_end)
                                                      ? vinfo->flags
                                                      : vinfo->flags & ~BRIDGE_VLAN_INFO_PVID));
Another alternative could be to add explicit pvid field to bridge_vlan_range_info and disallow PVID flag.
This allows for setting the port pvid to any vid in the range.

If both alternatives seem somewhat complex we could just disallow PVID flag in IFLA_BRIDGE_VLAN_RANGE_INFO and allow it only in IFLA_BRIDGE_VLAN_INFO.

I believe it is a user error to set PVID flag on a range of vlans using 
IFLA_BRIDGE_VLAN_RANGE_INFO
or for that matter using multiple IFLA_BRIDGE_VLAN_INFO with pvid flag 
set. Today, the last one probably sticks.
I will check current behavior and mimic that or return EINVAL when 
multiple attributes come in with the PVID flag.

quoted

				if (err)
					break;
-
-				if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
-					err = br_vlan_add(p->br, vinfo->vid,
-							  vinfo->flags);
-			} else
-				err = br_vlan_add(br, vinfo->vid, vinfo->flags);
-
+			}
			break;
-
		case RTM_DELLINK:
-			if (p) {
-				nbp_vlan_delete(p, vinfo->vid);
-				if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
-					br_vlan_delete(p->br, vinfo->vid);
-			} else
-				br_vlan_delete(br, vinfo->vid);
+			for (vid = vinfo->vid; vid <= vinfo->vid_end; vid++)
+				br_afspec_vlan_del(br, p, vid, vinfo->flags);
			break;
		}
	}
--
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html