| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 1 Jan 2015 11:34:45 -0800
From: Scott Feldman <sfeldma@...il.com>
To: "Arad, Ronen" <ronen.arad@...el.com>
Cc: "roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"hemminger@...tta.com" <hemminger@...tta.com>,
"vyasevic@...hat.com" <vyasevic@...hat.com>,
"wkok@...ulusnetworks.com" <wkok@...ulusnetworks.com>
Subject: Re: [PATCH net-next v2 2/2] bridge: modify bridge af spec parser to
accomodate vlan list and ranges
On Thu, Jan 1, 2015 at 12:54 AM, Arad, Ronen <ronen.arad@...el.com> wrote:
>
>
>>-----Original Message-----
>>From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org] On
>>Behalf Of roopa@...ulusnetworks.com
>>Sent: Wednesday, December 31, 2014 6:49 PM
>>To: netdev@...r.kernel.org; hemminger@...tta.com; vyasevic@...hat.com
>>Cc: sfeldma@...il.com; wkok@...ulusnetworks.com; Roopa Prabhu
>>Subject: [PATCH net-next v2 2/2] bridge: modify bridge af spec parser to
>>accomodate vlan list and ranges
>>
>>From: Roopa Prabhu <roopa@...ulusnetworks.com>
>>
>>This patch modifies br_afspec to parse incoming IFLA_BRIDGE_VLAN_INFO_LIST
>>
>>Signed-off-by: Roopa Prabhu <roopa@...ulusnetworks.com>
>>---
>> net/bridge/br_netlink.c | 115 ++++++++++++++++++++++++++++++++++------------
>>-
>> 1 file changed, 85 insertions(+), 30 deletions(-)
>>
>>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>index 492ef6a..bcba9d2 100644
>>--- a/net/bridge/br_netlink.c
>>+++ b/net/bridge/br_netlink.c
>>@@ -226,53 +226,108 @@ static const struct nla_policy
>>ifla_br_policy[IFLA_MAX+1] = {
>> [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
>> };
>>
>>+static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
>>+ int cmd, struct bridge_vlan_info *vinfo)
>>+{
>>+ int err = 0;
>>+
>>+ switch (cmd) {
>>+ case RTM_SETLINK:
>>+ if (p) {
>>+ err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>>+ 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);
>>+ }
>>+ break;
>>+ }
>>+
>>+ return err;
>>+}
>>+
>> static int br_afspec(struct net_bridge *br,
>> struct net_bridge_port *p,
>> struct nlattr *af_spec,
>> int cmd)
>> {
>> struct nlattr *tb[IFLA_BRIDGE_MAX+1];
>>+ struct nlattr *attr;
>> int err = 0;
>>+ int rem;
>>
>> err = nla_parse_nested(tb, IFLA_BRIDGE_MAX, af_spec, ifla_br_policy);
>> if (err)
>> return err;
>>
>> if (tb[IFLA_BRIDGE_VLAN_INFO]) {
>>- struct bridge_vlan_info *vinfo;
>>-
>>- vinfo = nla_data(tb[IFLA_BRIDGE_VLAN_INFO]);
>>-
>>- if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK)
>>- return -EINVAL;
>>-
>>- switch (cmd) {
>>- case RTM_SETLINK:
>>- if (p) {
>>- err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>>- 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);
>>- break;
>>+ attr = tb[IFLA_BRIDGE_VLAN_INFO];
>>+ if (nla_len(attr) != sizeof(struct bridge_vlan_info))
>>+ goto err_inval;
>>+
>>+ err = br_vlan_info(br, p, cmd,
>>+ (struct bridge_vlan_info *)nla_data(attr));
>>+
>>+ } else if (tb[IFLA_BRIDGE_VLAN_INFO_LIST]) {
>>+ struct bridge_vlan_info *vinfo_start = NULL;
>>+ struct bridge_vlan_info *vinfo = NULL;
>>+
>>+ nla_for_each_nested(attr, tb[IFLA_BRIDGE_VLAN_INFO_LIST], rem) {
>>+ if (nla_len(attr) != sizeof(struct bridge_vlan_info) ||
>>+ nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
>>+ goto err_inval;
>>+ vinfo = nla_data(attr);
>>+ if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_START) {
>>+ if (vinfo_start)
>>+ goto err_inval;
>>+ vinfo_start = vinfo;
>>+ continue;
>>+ }
>>+
>>+ if (vinfo_start) {
>>+ int v;
>>+
>>+ if (!(vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END))
>>+ goto err_inval;
>>+
>>+ if (vinfo->vid < vinfo_start->vid)
>
> This check rejects inverted range. However it allows the RANGE_START and
> RANGE_END vinfos to have the same vid. Isn't it inconsistent with the rejection
> of a single vinfo with both RANGE_START and RANGE_END set?
Allowing both START and END to be set on single vinfo might simplify
the encoding of LIST, so maybe it should be allowed.
Roopa, I know you dropped the subsequent notification patches from the
set, but I suspect now with these new START/END markers, the
notification algorithm can be very close to the original loop, without
having to make copies of the vlan_bitmap and untagged_bitmap. Using
both START/END on a single vinfo will keep the loop simple for adding
single vids that are not in a range.
(hmmm...START/STOP or BEGIN/END? Seems START/END is mixing the two
concepts...BEGIN/END seems best)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists