* [Bridge] [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc @ 2014-06-05 11:53 ` Toshiaki Makita 0 siblings, 0 replies; 14+ messages in thread From: Toshiaki Makita @ 2014-06-05 11:53 UTC (permalink / raw) To: David S . Miller, Stephen Hemminger Cc: Vlad Yasevich, netdev, David Laight, bridge br_manage_promisc() incorrectly expects br_auto_port() to return only 0 or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> --- net/bridge/br_if.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index a08d2b8..3eca3fd 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -153,7 +153,8 @@ void br_manage_promisc(struct net_bridge *br) * This lets us disable promiscuous mode and write * this config to hw. */ - if (br->auto_cnt <= br_auto_port(p)) + if (br->auto_cnt == 0 || + (br->auto_cnt == 1 && br_auto_port(p))) br_port_clear_promisc(p); else br_port_set_promisc(p); -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc @ 2014-06-05 11:53 ` Toshiaki Makita 0 siblings, 0 replies; 14+ messages in thread From: Toshiaki Makita @ 2014-06-05 11:53 UTC (permalink / raw) To: David S . Miller, Stephen Hemminger Cc: Vlad Yasevich, netdev, David Laight, bridge br_manage_promisc() incorrectly expects br_auto_port() to return only 0 or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> --- net/bridge/br_if.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index a08d2b8..3eca3fd 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -153,7 +153,8 @@ void br_manage_promisc(struct net_bridge *br) * This lets us disable promiscuous mode and write * this config to hw. */ - if (br->auto_cnt <= br_auto_port(p)) + if (br->auto_cnt == 0 || + (br->auto_cnt == 1 && br_auto_port(p))) br_port_clear_promisc(p); else br_port_set_promisc(p); -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Bridge] [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc 2014-06-05 11:53 ` Toshiaki Makita @ 2014-06-05 12:30 ` Michael S. Tsirkin -1 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2014-06-05 12:30 UTC (permalink / raw) To: Toshiaki Makita Cc: Vlad Yasevich, netdev, bridge, Stephen Hemminger, David Laight, David S . Miller On Thu, Jun 05, 2014 at 08:53:32PM +0900, Toshiaki Makita wrote: > br_manage_promisc() incorrectly expects br_auto_port() to return only 0 > or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. > > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> > --- > net/bridge/br_if.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c > index a08d2b8..3eca3fd 100644 > --- a/net/bridge/br_if.c > +++ b/net/bridge/br_if.c > @@ -153,7 +153,8 @@ void br_manage_promisc(struct net_bridge *br) > * This lets us disable promiscuous mode and write > * this config to hw. > */ > - if (br->auto_cnt <= br_auto_port(p)) > + if (br->auto_cnt == 0 || > + (br->auto_cnt == 1 && br_auto_port(p))) > br_port_clear_promisc(p); > else > br_port_set_promisc(p); It's all a nasty side-effect of using macros IMHO. How about we just make these inline functions returning bool? The bugfix will fall out naturally. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Warning: untested. diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 53d6e32..5818dd2 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -200,8 +200,15 @@ struct net_bridge_port #endif }; -#define br_auto_port(p) ((p)->flags & BR_AUTO_MASK) -#define br_promisc_port(p) ((p)->flags & BR_PROMISC) +static inline bool br_auto_port(struct net_bridge_port *p) +{ + return p->flags & BR_AUTO_MASK; +} + +static inline bool br_promisc_port(struct net_bridge_port *p) +{ + return p->flags & BR_PROMISC; +} #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT) ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc @ 2014-06-05 12:30 ` Michael S. Tsirkin 0 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2014-06-05 12:30 UTC (permalink / raw) To: Toshiaki Makita Cc: Vlad Yasevich, netdev, bridge, Stephen Hemminger, David Laight, David S . Miller On Thu, Jun 05, 2014 at 08:53:32PM +0900, Toshiaki Makita wrote: > br_manage_promisc() incorrectly expects br_auto_port() to return only 0 > or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. > > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> > --- > net/bridge/br_if.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c > index a08d2b8..3eca3fd 100644 > --- a/net/bridge/br_if.c > +++ b/net/bridge/br_if.c > @@ -153,7 +153,8 @@ void br_manage_promisc(struct net_bridge *br) > * This lets us disable promiscuous mode and write > * this config to hw. > */ > - if (br->auto_cnt <= br_auto_port(p)) > + if (br->auto_cnt == 0 || > + (br->auto_cnt == 1 && br_auto_port(p))) > br_port_clear_promisc(p); > else > br_port_set_promisc(p); It's all a nasty side-effect of using macros IMHO. How about we just make these inline functions returning bool? The bugfix will fall out naturally. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Warning: untested. diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 53d6e32..5818dd2 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -200,8 +200,15 @@ struct net_bridge_port #endif }; -#define br_auto_port(p) ((p)->flags & BR_AUTO_MASK) -#define br_promisc_port(p) ((p)->flags & BR_PROMISC) +static inline bool br_auto_port(struct net_bridge_port *p) +{ + return p->flags & BR_AUTO_MASK; +} + +static inline bool br_promisc_port(struct net_bridge_port *p) +{ + return p->flags & BR_PROMISC; +} #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT) ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Bridge] [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc 2014-06-05 12:30 ` Michael S. Tsirkin @ 2014-06-05 13:01 ` Toshiaki Makita -1 siblings, 0 replies; 14+ messages in thread From: Toshiaki Makita @ 2014-06-05 13:01 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Vlad Yasevich, netdev, bridge, Stephen Hemminger, David Laight, David S . Miller (2014/06/05 21:30), Michael S. Tsirkin wrote: > On Thu, Jun 05, 2014 at 08:53:32PM +0900, Toshiaki Makita wrote: >> br_manage_promisc() incorrectly expects br_auto_port() to return only 0 >> or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. >> >> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> >> --- >> net/bridge/br_if.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c >> index a08d2b8..3eca3fd 100644 >> --- a/net/bridge/br_if.c >> +++ b/net/bridge/br_if.c >> @@ -153,7 +153,8 @@ void br_manage_promisc(struct net_bridge *br) >> * This lets us disable promiscuous mode and write >> * this config to hw. >> */ >> - if (br->auto_cnt <= br_auto_port(p)) >> + if (br->auto_cnt == 0 || >> + (br->auto_cnt == 1 && br_auto_port(p))) >> br_port_clear_promisc(p); >> else >> br_port_set_promisc(p); > > It's all a nasty side-effect of using macros IMHO. > > How about we just make these inline functions returning bool? > > The bugfix will fall out naturally. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > Warning: untested. > > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 53d6e32..5818dd2 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -200,8 +200,15 @@ struct net_bridge_port > #endif > }; > > -#define br_auto_port(p) ((p)->flags & BR_AUTO_MASK) > -#define br_promisc_port(p) ((p)->flags & BR_PROMISC) > +static inline bool br_auto_port(struct net_bridge_port *p) > +{ > + return p->flags & BR_AUTO_MASK; > +} > + > +static inline bool br_promisc_port(struct net_bridge_port *p) > +{ > + return p->flags & BR_PROMISC; > +} > > #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT) This also looks good. IMHO, the caller side should not assume these macros (or inline functions) return boolean value. There exists similar macro such as br_port_exists() that doesn't return boolean. Ohterwise, we should change all macros into boolean functions, but it might affect performance a little if such a macro is used in fast path? (I'm worried about the cost of casting non-zero values into 1.) Thanks, Toshiaki Makita ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc @ 2014-06-05 13:01 ` Toshiaki Makita 0 siblings, 0 replies; 14+ messages in thread From: Toshiaki Makita @ 2014-06-05 13:01 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Vlad Yasevich, netdev, bridge, Stephen Hemminger, David Laight, David S . Miller (2014/06/05 21:30), Michael S. Tsirkin wrote: > On Thu, Jun 05, 2014 at 08:53:32PM +0900, Toshiaki Makita wrote: >> br_manage_promisc() incorrectly expects br_auto_port() to return only 0 >> or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. >> >> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> >> --- >> net/bridge/br_if.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c >> index a08d2b8..3eca3fd 100644 >> --- a/net/bridge/br_if.c >> +++ b/net/bridge/br_if.c >> @@ -153,7 +153,8 @@ void br_manage_promisc(struct net_bridge *br) >> * This lets us disable promiscuous mode and write >> * this config to hw. >> */ >> - if (br->auto_cnt <= br_auto_port(p)) >> + if (br->auto_cnt == 0 || >> + (br->auto_cnt == 1 && br_auto_port(p))) >> br_port_clear_promisc(p); >> else >> br_port_set_promisc(p); > > It's all a nasty side-effect of using macros IMHO. > > How about we just make these inline functions returning bool? > > The bugfix will fall out naturally. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > Warning: untested. > > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 53d6e32..5818dd2 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -200,8 +200,15 @@ struct net_bridge_port > #endif > }; > > -#define br_auto_port(p) ((p)->flags & BR_AUTO_MASK) > -#define br_promisc_port(p) ((p)->flags & BR_PROMISC) > +static inline bool br_auto_port(struct net_bridge_port *p) > +{ > + return p->flags & BR_AUTO_MASK; > +} > + > +static inline bool br_promisc_port(struct net_bridge_port *p) > +{ > + return p->flags & BR_PROMISC; > +} > > #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT) This also looks good. IMHO, the caller side should not assume these macros (or inline functions) return boolean value. There exists similar macro such as br_port_exists() that doesn't return boolean. Ohterwise, we should change all macros into boolean functions, but it might affect performance a little if such a macro is used in fast path? (I'm worried about the cost of casting non-zero values into 1.) Thanks, Toshiaki Makita ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bridge] [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc 2014-06-05 13:01 ` Toshiaki Makita @ 2014-06-05 14:57 ` Vlad Yasevich -1 siblings, 0 replies; 14+ messages in thread From: Vlad Yasevich @ 2014-06-05 14:57 UTC (permalink / raw) To: Toshiaki Makita, Michael S. Tsirkin Cc: Vlad Yasevich, netdev, bridge, Stephen Hemminger, David Laight, David S . Miller On 06/05/2014 09:01 AM, Toshiaki Makita wrote: > (2014/06/05 21:30), Michael S. Tsirkin wrote: >> On Thu, Jun 05, 2014 at 08:53:32PM +0900, Toshiaki Makita wrote: >>> br_manage_promisc() incorrectly expects br_auto_port() to return only 0 >>> or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. >>> >>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> >>> --- >>> net/bridge/br_if.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c >>> index a08d2b8..3eca3fd 100644 >>> --- a/net/bridge/br_if.c >>> +++ b/net/bridge/br_if.c >>> @@ -153,7 +153,8 @@ void br_manage_promisc(struct net_bridge *br) >>> * This lets us disable promiscuous mode and write >>> * this config to hw. >>> */ >>> - if (br->auto_cnt <= br_auto_port(p)) >>> + if (br->auto_cnt == 0 || >>> + (br->auto_cnt == 1 && br_auto_port(p))) >>> br_port_clear_promisc(p); >>> else >>> br_port_set_promisc(p); >> >> It's all a nasty side-effect of using macros IMHO. >> >> How about we just make these inline functions returning bool? >> >> The bugfix will fall out naturally. >> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> >> Warning: untested. >> >> >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >> index 53d6e32..5818dd2 100644 >> --- a/net/bridge/br_private.h >> +++ b/net/bridge/br_private.h >> @@ -200,8 +200,15 @@ struct net_bridge_port >> #endif >> }; >> >> -#define br_auto_port(p) ((p)->flags & BR_AUTO_MASK) >> -#define br_promisc_port(p) ((p)->flags & BR_PROMISC) >> +static inline bool br_auto_port(struct net_bridge_port *p) >> +{ >> + return p->flags & BR_AUTO_MASK; >> +} >> + >> +static inline bool br_promisc_port(struct net_bridge_port *p) >> +{ >> + return p->flags & BR_PROMISC; >> +} >> >> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT) > > This also looks good. > > IMHO, the caller side should not assume these macros (or inline > functions) return boolean value. There exists similar macro such as > br_port_exists() that doesn't return boolean. > > Ohterwise, we should change all macros into boolean functions, but it > might affect performance a little if such a macro is used in fast path? > (I'm worried about the cost of casting non-zero values into 1.) The following works correctly for me: #define br_auto_port(p) !!((p)->flags & BR_AUTO_MASK) Small test shows: printf("%d\n", br_auto_port(0x20); 1 <-- correct. learning is set. printf("%d\n", br_auto_port(0x40); 1 <-- correct. flooding is set. printf("%d\n", br_auto_port(0x60); 1 <-- correct, both are set. printf("%d\n", br_auto_port(0x08); 0 <-- correct. neither are set. -vlad > > Thanks, > Toshiaki Makita > -- > 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc @ 2014-06-05 14:57 ` Vlad Yasevich 0 siblings, 0 replies; 14+ messages in thread From: Vlad Yasevich @ 2014-06-05 14:57 UTC (permalink / raw) To: Toshiaki Makita, Michael S. Tsirkin Cc: David S . Miller, Stephen Hemminger, David Laight, Vlad Yasevich, netdev, bridge On 06/05/2014 09:01 AM, Toshiaki Makita wrote: > (2014/06/05 21:30), Michael S. Tsirkin wrote: >> On Thu, Jun 05, 2014 at 08:53:32PM +0900, Toshiaki Makita wrote: >>> br_manage_promisc() incorrectly expects br_auto_port() to return only 0 >>> or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. >>> >>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> >>> --- >>> net/bridge/br_if.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c >>> index a08d2b8..3eca3fd 100644 >>> --- a/net/bridge/br_if.c >>> +++ b/net/bridge/br_if.c >>> @@ -153,7 +153,8 @@ void br_manage_promisc(struct net_bridge *br) >>> * This lets us disable promiscuous mode and write >>> * this config to hw. >>> */ >>> - if (br->auto_cnt <= br_auto_port(p)) >>> + if (br->auto_cnt == 0 || >>> + (br->auto_cnt == 1 && br_auto_port(p))) >>> br_port_clear_promisc(p); >>> else >>> br_port_set_promisc(p); >> >> It's all a nasty side-effect of using macros IMHO. >> >> How about we just make these inline functions returning bool? >> >> The bugfix will fall out naturally. >> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> >> Warning: untested. >> >> >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >> index 53d6e32..5818dd2 100644 >> --- a/net/bridge/br_private.h >> +++ b/net/bridge/br_private.h >> @@ -200,8 +200,15 @@ struct net_bridge_port >> #endif >> }; >> >> -#define br_auto_port(p) ((p)->flags & BR_AUTO_MASK) >> -#define br_promisc_port(p) ((p)->flags & BR_PROMISC) >> +static inline bool br_auto_port(struct net_bridge_port *p) >> +{ >> + return p->flags & BR_AUTO_MASK; >> +} >> + >> +static inline bool br_promisc_port(struct net_bridge_port *p) >> +{ >> + return p->flags & BR_PROMISC; >> +} >> >> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT) > > This also looks good. > > IMHO, the caller side should not assume these macros (or inline > functions) return boolean value. There exists similar macro such as > br_port_exists() that doesn't return boolean. > > Ohterwise, we should change all macros into boolean functions, but it > might affect performance a little if such a macro is used in fast path? > (I'm worried about the cost of casting non-zero values into 1.) The following works correctly for me: #define br_auto_port(p) !!((p)->flags & BR_AUTO_MASK) Small test shows: printf("%d\n", br_auto_port(0x20); 1 <-- correct. learning is set. printf("%d\n", br_auto_port(0x40); 1 <-- correct. flooding is set. printf("%d\n", br_auto_port(0x60); 1 <-- correct, both are set. printf("%d\n", br_auto_port(0x08); 0 <-- correct. neither are set. -vlad > > Thanks, > Toshiaki Makita > -- > 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bridge] [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc 2014-06-05 11:53 ` Toshiaki Makita @ 2014-06-05 22:21 ` David Miller -1 siblings, 0 replies; 14+ messages in thread From: David Miller @ 2014-06-05 22:21 UTC (permalink / raw) To: makita.toshiaki; +Cc: stephen, vyasevic, David.Laight, bridge, netdev From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> Date: Thu, 5 Jun 2014 20:53:32 +0900 > br_manage_promisc() incorrectly expects br_auto_port() to return only 0 > or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. > > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> I'm applying this as-is for now, even though I saw the other suggestions in this thread (which BTW didn't get picked up by patchwork, maybe some of you dropped the Message-Id in your replies by accident). ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc @ 2014-06-05 22:21 ` David Miller 0 siblings, 0 replies; 14+ messages in thread From: David Miller @ 2014-06-05 22:21 UTC (permalink / raw) To: makita.toshiaki; +Cc: stephen, David.Laight, vyasevic, netdev, bridge From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> Date: Thu, 5 Jun 2014 20:53:32 +0900 > br_manage_promisc() incorrectly expects br_auto_port() to return only 0 > or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. > > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> I'm applying this as-is for now, even though I saw the other suggestions in this thread (which BTW didn't get picked up by patchwork, maybe some of you dropped the Message-Id in your replies by accident). ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bridge] [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc 2014-06-05 22:21 ` David Miller @ 2014-06-06 8:48 ` David Laight -1 siblings, 0 replies; 14+ messages in thread From: David Laight @ 2014-06-06 8:48 UTC (permalink / raw) To: 'David Miller', makita.toshiaki@lab.ntt.co.jp Cc: stephen@networkplumber.org, vyasevic@redhat.com, bridge@lists.linux-foundation.org, netdev@vger.kernel.org From: David Miller [mailto:davem@davemloft.net] > > br_manage_promisc() incorrectly expects br_auto_port() to return only 0 > > or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. > > > > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> > > I'm applying this as-is for now, even though I saw the other > suggestions in this thread (which BTW didn't get picked up by > patchwork, maybe some of you dropped the Message-Id in your replies by > accident). I don't have a problem with that. The condition looks odd, but it is enabling promiscuous mode if any other ports are in 'auto' mode. Possibly the comment above made that clear, but it was truncated in the diffs. David ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc @ 2014-06-06 8:48 ` David Laight 0 siblings, 0 replies; 14+ messages in thread From: David Laight @ 2014-06-06 8:48 UTC (permalink / raw) To: 'David Miller', makita.toshiaki@lab.ntt.co.jp Cc: stephen@networkplumber.org, vyasevic@redhat.com, netdev@vger.kernel.org, bridge@lists.linux-foundation.org From: David Miller [mailto:davem@davemloft.net] > > br_manage_promisc() incorrectly expects br_auto_port() to return only 0 > > or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. > > > > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> > > I'm applying this as-is for now, even though I saw the other > suggestions in this thread (which BTW didn't get picked up by > patchwork, maybe some of you dropped the Message-Id in your replies by > accident). I don't have a problem with that. The condition looks odd, but it is enabling promiscuous mode if any other ports are in 'auto' mode. Possibly the comment above made that clear, but it was truncated in the diffs. David ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bridge] [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc 2014-06-06 8:48 ` David Laight @ 2014-06-06 14:13 ` Vlad Yasevich -1 siblings, 0 replies; 14+ messages in thread From: Vlad Yasevich @ 2014-06-06 14:13 UTC (permalink / raw) To: David Laight, 'David Miller', makita.toshiaki@lab.ntt.co.jp Cc: stephen@networkplumber.org, vyasevic@redhat.com, bridge@lists.linux-foundation.org, netdev@vger.kernel.org On 06/06/2014 04:48 AM, David Laight wrote: > From: David Miller [mailto:davem@davemloft.net] >>> br_manage_promisc() incorrectly expects br_auto_port() to return only 0 >>> or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. >>> >>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> >> >> I'm applying this as-is for now, even though I saw the other >> suggestions in this thread (which BTW didn't get picked up by >> patchwork, maybe some of you dropped the Message-Id in your replies by >> accident). > > I don't have a problem with that. > The condition looks odd, but it is enabling promiscuous mode > if any other ports are in 'auto' mode. No, the condition is correct and explicit. The cases are: 0 auto ports == all ports are statically configured and non-promisc. 1 auto port == only this port can be non-promisc. all others promisc. > 1 auto port == all ports promisc. -vlad > Possibly the comment above made that clear, but it was truncated > in the diffs. > > David > > > > -- > 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc @ 2014-06-06 14:13 ` Vlad Yasevich 0 siblings, 0 replies; 14+ messages in thread From: Vlad Yasevich @ 2014-06-06 14:13 UTC (permalink / raw) To: David Laight, 'David Miller', makita.toshiaki@lab.ntt.co.jp Cc: stephen@networkplumber.org, vyasevic@redhat.com, bridge@lists.linux-foundation.org, netdev@vger.kernel.org On 06/06/2014 04:48 AM, David Laight wrote: > From: David Miller [mailto:davem@davemloft.net] >>> br_manage_promisc() incorrectly expects br_auto_port() to return only 0 >>> or 1, while it actually returns flags, i.e., a subset of BR_AUTO_MASK. >>> >>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> >> >> I'm applying this as-is for now, even though I saw the other >> suggestions in this thread (which BTW didn't get picked up by >> patchwork, maybe some of you dropped the Message-Id in your replies by >> accident). > > I don't have a problem with that. > The condition looks odd, but it is enabling promiscuous mode > if any other ports are in 'auto' mode. No, the condition is correct and explicit. The cases are: 0 auto ports == all ports are statically configured and non-promisc. 1 auto port == only this port can be non-promisc. all others promisc. > 1 auto port == all ports promisc. -vlad > Possibly the comment above made that clear, but it was truncated > in the diffs. > > David > > > > -- > 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-06-06 14:13 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-06-05 11:53 [Bridge] [PATCH v2 net-next] bridge: Fix incorrect judgment of promisc Toshiaki Makita 2014-06-05 11:53 ` Toshiaki Makita 2014-06-05 12:30 ` [Bridge] " Michael S. Tsirkin 2014-06-05 12:30 ` Michael S. Tsirkin 2014-06-05 13:01 ` [Bridge] " Toshiaki Makita 2014-06-05 13:01 ` Toshiaki Makita 2014-06-05 14:57 ` [Bridge] " Vlad Yasevich 2014-06-05 14:57 ` Vlad Yasevich 2014-06-05 22:21 ` [Bridge] " David Miller 2014-06-05 22:21 ` David Miller 2014-06-06 8:48 ` [Bridge] " David Laight 2014-06-06 8:48 ` David Laight 2014-06-06 14:13 ` [Bridge] " Vlad Yasevich 2014-06-06 14:13 ` Vlad Yasevich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.