[PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[Hong Zhiguo]

When neither IFF_BRIDGE_PORT or IFF_BRIDGE_PORT is set,
and afspec == NULL but  protinfo != NULL, we run into
"if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p);" with
random value in ret.

Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
---
 net/bridge/br_netlink.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 84c3b7d..b96e02e 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -357,7 +357,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
 	struct nlattr *afspec;
 	struct net_bridge_port *p;
 	struct nlattr *tb[IFLA_BRPORT_MAX + 1];
-	int err;
+	int err = 0;
 
 	ifm = nlmsg_data(nlh);
 
@@ -370,7 +370,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
 	/* We want to accept dev as bridge itself if the AF_SPEC
 	 * is set to see if someone is setting vlan info on the brigde
 	 */
-	if (!p && ((dev->priv_flags & IFF_EBRIDGE) && !afspec))
+	if (!p && !afspec)
 		return -EINVAL;
 
 	if (p && protinfo) {
-- 
1.7.10.4

Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[Sergei Shtylyov]

Hello.

On 23-03-2013 9:14, Hong Zhiguo wrote:

> When neither IFF_BRIDGE_PORT or IFF_BRIDGE_PORT is set,

    That is the same flag repeated twice.

> and afspec == NULL but  protinfo != NULL, we run into
> "if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p);" with
> random value in ret.

> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>

WBR, Sergei

[PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[Hong Zhiguo]

When neither IFF_BRIDGE nor IFF_BRIDGE_PORT is set,
and afspec == NULL but  protinfo != NULL, we run into
"if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p);" with
random value in ret.

Thanks to Sergei for pointing out the error in commit comments.

Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
---
 net/bridge/br_netlink.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 84c3b7d..b96e02e 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -357,7 +357,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
 	struct nlattr *afspec;
 	struct net_bridge_port *p;
 	struct nlattr *tb[IFLA_BRPORT_MAX + 1];
-	int err;
+	int err = 0;
 
 	ifm = nlmsg_data(nlh);
 
@@ -370,7 +370,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
 	/* We want to accept dev as bridge itself if the AF_SPEC
 	 * is set to see if someone is setting vlan info on the brigde
 	 */
-	if (!p && ((dev->priv_flags & IFF_EBRIDGE) && !afspec))
+	if (!p && !afspec)
 		return -EINVAL;
 
 	if (p && protinfo) {
-- 
1.7.10.4

Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[Sergei Shtylyov]

Hello.

On 03/24/2013 04:26 PM, Hong Zhiguo wrote:

> When neither IFF_BRIDGE nor IFF_BRIDGE_PORT is set,
> and afspec == NULL but  protinfo != NULL, we run into
> "if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p);" with
> random value in ret.
>
> Thanks to Sergei for pointing out the error in commit comments.
>
> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>

     For the future, if you post the revised version of the patch, you
should indicate in the subject it like this: [PATCH v2].

WBR, Sergei

Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sun, 24 Mar 2013 17:34:47 +0300

> Hello.
> 
> On 03/24/2013 04:26 PM, Hong Zhiguo wrote:
> 
>> When neither IFF_BRIDGE nor IFF_BRIDGE_PORT is set,
>> and afspec == NULL but  protinfo != NULL, we run into
>> "if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p);" with
>> random value in ret.
>>
>> Thanks to Sergei for pointing out the error in commit comments.
>>
>> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
> 
>     For the future, if you post the revised version of the patch, you
> should indicate in the subject it like this: [PATCH v2].

I'm disappointed that you have enough energy to point out such a lower
priority omission, but you lack the time for something more important,
which is giving this patch your explicit ACK if it fixes all of the
issues you pointed out to him.

Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[Sergei Shtylyov]

Hello.

On 25-03-2013 1:15, David Miller wrote:

>>> When neither IFF_BRIDGE nor IFF_BRIDGE_PORT is set,
>>> and afspec == NULL but  protinfo != NULL, we run into
>>> "if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p);" with
>>> random value in ret.

>>> Thanks to Sergei for pointing out the error in commit comments.

>>> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>

>>      For the future, if you post the revised version of the patch, you
>> should indicate in the subject it like this: [PATCH v2].

> I'm disappointed that you have enough energy to point out such a lower
> priority omission, but you lack the time for something more important,
> which is giving this patch your explicit ACK if it fixes all of the
> issues you pointed out to him.

    Sorry, I don't usuallly ACK patches in an area I'm not closely familiar 
with. Though I can add:

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

WBR, Sergei

Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Mon, 25 Mar 2013 19:32:14 +0400

>    Sorry, I don't usuallly ACK patches in an area I'm not closely
>    familiar with. Though I can add:

If you feel confident enough to ask for corrections, you better be
ready to ACK the result when your requests have been met.

You can't hold other people accountable yet take none of the
same responsibility yourself.

Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[Sergei Shtylyov]

Hello.

On 25-03-2013 20:08, David Miller wrote:

>>     Sorry, I don't usuallly ACK patches in an area I'm not closely
>>     familiar with. Though I can add:

> If you feel confident enough to ask for corrections, you better be
> ready to ACK the result when your requests have been met.

   Come on, I only asked to correct an evident cut and paste typo in the 
description. That's too little a correction to ACK the whole patch. To ACK it, 
I should know well the code it's patching and be confident that the whole 
patch is correct. I have neither knowledge.

> You can't hold other people accountable yet take none of the
> same responsibility yourself.

    I don't hold anyone accountable, I just provide my comments which might as 
well be ignored by the submitter/maintainer.

WBR, Sergei

Re: [PATCH net-next] bridge: avoid br_ifinfo_notify when nothing changed

[David Miller]

From: Hong Zhiguo <honkiko@gmail.com>
Date: Sun, 24 Mar 2013 21:26:47 +0800

> When neither IFF_BRIDGE nor IFF_BRIDGE_PORT is set,
> and afspec == NULL but  protinfo != NULL, we run into
> "if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p);" with
> random value in ret.
> 
> Thanks to Sergei for pointing out the error in commit comments.
> 
> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>

Applied.