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: Sat, 06 Dec 2014 00:03:30 -0800 From: Roopa Prabhu <roopa@...ulusnetworks.com> To: Scott Feldman <sfeldma@...il.com> CC: Jiří Pírko <jiri@...nulli.us>, Jamal Hadi Salim <jhs@...atatu.com>, Benjamin LaHaise <bcrl@...ck.org>, Thomas Graf <tgraf@...g.ch>, john fastabend <john.fastabend@...il.com>, "stephen@...workplumber.org" <stephen@...workplumber.org>, John Linville <linville@...driver.com>, "vyasevic@...hat.com" <vyasevic@...hat.com>, Netdev <netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>, shm@...ulusnetworks.com, Andy Gospodarek <gospo@...ulusnetworks.com> Subject: Re: [PATCH v2 iproute] bridge link: add option 'self' On 12/5/14, 9:36 PM, Scott Feldman wrote: > On Fri, Dec 5, 2014 at 4:59 PM, <roopa@...ulusnetworks.com> wrote: >> From: Roopa Prabhu <roopa@...ulusnetworks.com> >> >> Currently self is set internally only if hwmode is set. >> This makes it necessary for the hw to have a mode. >> There is no hwmode really required to go to hardware. So, introduce >> self for anybody who wants to target hardware. >> >> v1 -> v2 >> - fix a few bugs. Initialize flags to zero: this was required to >> keep the current behaviour unchanged. >> >> Signed-off-by: Roopa Prabhu <roopa@...ulusnetworks.com> >> --- >> bridge/link.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/bridge/link.c b/bridge/link.c >> index 90d9e7f..b8b8675 100644 >> --- a/bridge/link.c >> +++ b/bridge/link.c >> @@ -261,7 +261,7 @@ static int brlink_modify(int argc, char **argv) >> __s16 priority = -1; >> __s8 state = -1; >> __s16 mode = -1; >> - __u16 flags = BRIDGE_FLAGS_MASTER; >> + __u16 flags = 0; > I don't follow why this change is necessary. Seems defaulting to > MASTER unless SELF is set due to "self" or "hwmode" would be more > correct (and expected). That flags initialization was always a no-op. IFLA_BRIDGE_FLAGS was always set only with the 'self' flag on. And i don't want to change that unnecessarily with my patch. Setting it to master now means we will always include IFLA_AF_SPEC with IFLA_BRIDGE_FLAGS in every 'bridge setlink' msg. Which will be a change from the current behavior. And i dont intend to introduce it with this patch. Kernel has always treated no flags as 'master', so we are good from kernel perspective. > >> struct rtattr *nest; >> >> memset(&req, 0, sizeof(req)); >> @@ -321,6 +321,8 @@ static int brlink_modify(int argc, char **argv) >> "\"veb\".\n"); >> exit(-1); >> } >> + } else if (strcmp(*argv, "self") == 0) { >> + flags = BRIDGE_FLAGS_SELF; >> } else { >> usage(); >> } >> @@ -375,10 +377,11 @@ static int brlink_modify(int argc, char **argv) >> * devices so far. Thus we only need to include the flags attribute >> * if we are setting the hw mode. >> */ > The comment above needs to be updated to reflect the new logic. ok, will do, Thanks > >> - if (mode >= 0) { >> + if (mode >= 0 || flags > 0) { >> nest = addattr_nest(&req.n, sizeof(req), IFLA_AF_SPEC); >> >> - addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags); >> + if (flags > 0) >> + addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags); >> >> if (mode >= 0) >> addattr16(&req.n, sizeof(req), IFLA_BRIDGE_MODE, mode); >> -- >> 1.7.10.4 >> > -- > 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 -- 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