Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2] drivers/isdn: checkng length to be sure not memory overflow
From: David Miller @ 2013-03-08  5:36 UTC (permalink / raw)
  To: gang.chen; +Cc: jslaby, jkosina, isdn, gregkh, alan, netdev
In-Reply-To: <51396845.3050600@asianux.com>

From: Chen Gang <gang.chen@asianux.com>
Date: Fri, 08 Mar 2013 12:25:41 +0800

> 
>   sizeof (cmd.parm.cmsg.para) is 50 (MAX_CAPI_PARA_LEN).
>   sizeof (cmd.parm) is 80+, but less than 100.
>   strlen(msg) may be more than 80+ (Modem-Commandbuffer, less than 255).
>     isdn_tty_send_msg is called by isdn_tty_parse_at
>     the relative parameter is m->mdmcmd (atemu *m)
>     the relative command may be "+M..."
> 
>   so need check the length to be sure not memory overflow.
>     cmd.parm is a union, and need keep original valid buffer length no touch
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net/rds: zero last byte for strncpy
From: David Miller @ 2013-03-08  5:36 UTC (permalink / raw)
  To: gang.chen
  Cc: bhutchings, David.Laight, venkat.x.venkatsubra, rds-devel, netdev
In-Reply-To: <513958C4.1020404@asianux.com>

From: Chen Gang <gang.chen@asianux.com>
Date: Fri, 08 Mar 2013 11:19:32 +0800

> 
>   for NUL terminated string, need be always sure '\0' in the end.
> 
> additional info:
>   strncpy will pads with zeroes to the end of the given buffer.
>   should initialise every bit of memory that is going to be copied to userland
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

Applied.

^ permalink raw reply

* Re: [PATCH net 0/2] bnx2x: Link fixes
From: David Miller @ 2013-03-08  5:36 UTC (permalink / raw)
  To: yanivr; +Cc: netdev
In-Reply-To: <1362698854-6786-1-git-send-email-yanivr@broadcom.com>

From: "Yaniv Rosner" <yanivr@broadcom.com>
Date: Fri, 8 Mar 2013 01:27:32 +0200

> The following patch series has two small link fixes.
> Please consider applying it to net.

All applied, thanks.

^ permalink raw reply

* [PATCH v2] drivers/isdn: checkng length to be sure not memory overflow
From: Chen Gang @ 2013-03-08  4:25 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Jiri Kosina, isdn, Greg KH, alan, netdev
In-Reply-To: <513813F5.5040806@asianux.com>


  sizeof (cmd.parm.cmsg.para) is 50 (MAX_CAPI_PARA_LEN).
  sizeof (cmd.parm) is 80+, but less than 100.
  strlen(msg) may be more than 80+ (Modem-Commandbuffer, less than 255).
    isdn_tty_send_msg is called by isdn_tty_parse_at
    the relative parameter is m->mdmcmd (atemu *m)
    the relative command may be "+M..."

  so need check the length to be sure not memory overflow.
    cmd.parm is a union, and need keep original valid buffer length no touch


Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 drivers/isdn/i4l/isdn_tty.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index d8a7d83..ebaebdf 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -902,7 +902,9 @@ isdn_tty_send_msg(modem_info *info, atemu *m, char *msg)
 	int j;
 	int l;
 
-	l = strlen(msg);
+	l = min(strlen(msg), sizeof(cmd.parm) - sizeof(cmd.parm.cmsg)
+		+ sizeof(cmd.parm.cmsg.para) - 2);
+
 	if (!l) {
 		isdn_tty_modem_result(RESULT_ERROR, info);
 		return;
-- 
1.7.7.6

^ permalink raw reply related

* Re: [Patch net] bridge: do not expire mdb entry when bridge still uses it
From: Herbert Xu @ 2013-03-08  4:08 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, bridge, Stephen Hemminger, David S. Miller, Adam Baker
In-Reply-To: <1362715613.9773.29.camel@cr0>

On Fri, Mar 08, 2013 at 12:06:53PM +0800, Cong Wang wrote:
> On Fri, 2013-03-08 at 10:47 +0800, Herbert Xu wrote:
> > On Fri, Mar 08, 2013 at 10:44:06AM +0800, Cong Wang wrote:
> > >
> > > This is a good question. It is due to inside br_handle_frame_finish() we
> > > use 'skb2' to decide if we deliver packets to bridge itself, and in this
> > > case 'skb2' is non-NULL only when:
> > > 
> > >                 if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
> > >                         if ((mdst && mdst->mglist) ||
> > >                             br_multicast_is_router(br))
> > >                                 skb2 = skb; //        <======== HERE
> > >                         br_multicast_forward(mdst, skb, skb2);
> > > 
> > > For other ports, br_multicast_forward() will always forward 'skb' to
> > > them.
> > 
> > Yes but the point is that other ports should also expire after the
> > timeout and thus be removed from mdst.  So why is this only happening
> > for the bridge aka mdst->mglist?
> 
> The mdst may be removed, however the BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)
> is still set, because in br_multicast_ipv4_rcv():

But those packets are only meant for routers, which is why we
have the br_multicast_is_router test above.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [Patch net] bridge: do not expire mdb entry when bridge still uses it
From: Cong Wang @ 2013-03-08  4:06 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Stephen Hemminger, netdev, bridge, David S. Miller, Adam Baker
In-Reply-To: <20130308024726.GA27319@gondor.apana.org.au>

On Fri, 2013-03-08 at 10:47 +0800, Herbert Xu wrote:
> On Fri, Mar 08, 2013 at 10:44:06AM +0800, Cong Wang wrote:
> >
> > This is a good question. It is due to inside br_handle_frame_finish() we
> > use 'skb2' to decide if we deliver packets to bridge itself, and in this
> > case 'skb2' is non-NULL only when:
> > 
> >                 if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
> >                         if ((mdst && mdst->mglist) ||
> >                             br_multicast_is_router(br))
> >                                 skb2 = skb; //        <======== HERE
> >                         br_multicast_forward(mdst, skb, skb2);
> > 
> > For other ports, br_multicast_forward() will always forward 'skb' to
> > them.
> 
> Yes but the point is that other ports should also expire after the
> timeout and thus be removed from mdst.  So why is this only happening
> for the bridge aka mdst->mglist?

The mdst may be removed, however the BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)
is still set, because in br_multicast_ipv4_rcv():

        if (iph->protocol != IPPROTO_IGMP) {
                if ((iph->daddr & IGMP_LOCAL_GROUP_MASK) !=
IGMP_LOCAL_GROUP)
                        BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
                return 0;
        }

^ permalink raw reply

* Re: [PATCH net-next] tg3: Update link_up flag for phylib devices
From: David Miller @ 2013-03-08  3:29 UTC (permalink / raw)
  To: nsujir; +Cc: netdev, hirofumi, mchan
In-Reply-To: <1362710727-15438-1-git-send-email-nsujir@broadcom.com>

From: "Nithin Nayak Sujir" <nsujir@broadcom.com>
Date: Thu, 7 Mar 2013 18:45:27 -0800

> Commit f4a46d1f46a8fece34edd2023e054072b02e110d introduced a bug where
> the ifconfig stats would remain 0 for phylib devices. This is due to
> tp->link_up flag never becoming true causing tg3_periodic_fetch_stats()
> to return.

This is a non-trivial regression, therefore shouldn't this change
be targetted at 'net' instead of 'net-next'?

^ permalink raw reply

* [PATCH] netfilter: register pernet subsystem before register L4 proto
From: Gao feng @ 2013-03-08  3:20 UTC (permalink / raw)
  To: netdev, netfilter-devel; +Cc: pablo, ebiederm, Gao feng, Alexey Dobriyan

In commit c296bb4d5d417d466c9bcc8afef68a3db5449a64
"netfilter: nf_conntrack: refactor l4proto support for netns",
we register l4proto gre/dccp/udplite/sctp before register
pernet subsystem,it's a wrong order.

We should register pernet subsystem before register L4proto,
since after register L4proto, init_conntrack may try to access
the resources which allocated in register_pernet_subsys.

Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/netfilter/nf_conntrack_proto_dccp.c    | 12 ++++++------
 net/netfilter/nf_conntrack_proto_gre.c     | 12 ++++++------
 net/netfilter/nf_conntrack_proto_sctp.c    | 12 ++++++------
 net/netfilter/nf_conntrack_proto_udplite.c | 12 ++++++------
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index 432f957..ba65b20 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -969,6 +969,10 @@ static int __init nf_conntrack_proto_dccp_init(void)
 {
 	int ret;
 
+	ret = register_pernet_subsys(&dccp_net_ops);
+	if (ret < 0)
+		goto out_pernet;
+
 	ret = nf_ct_l4proto_register(&dccp_proto4);
 	if (ret < 0)
 		goto out_dccp4;
@@ -977,16 +981,12 @@ static int __init nf_conntrack_proto_dccp_init(void)
 	if (ret < 0)
 		goto out_dccp6;
 
-	ret = register_pernet_subsys(&dccp_net_ops);
-	if (ret < 0)
-		goto out_pernet;
-
 	return 0;
-out_pernet:
-	nf_ct_l4proto_unregister(&dccp_proto6);
 out_dccp6:
 	nf_ct_l4proto_unregister(&dccp_proto4);
 out_dccp4:
+	unregister_pernet_subsys(&dccp_net_ops);
+out_pernet:
 	return ret;
 }
 
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index bd7d01d..155ce9f 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -420,18 +420,18 @@ static int __init nf_ct_proto_gre_init(void)
 {
 	int ret;
 
-	ret = nf_ct_l4proto_register(&nf_conntrack_l4proto_gre4);
-	if (ret < 0)
-		goto out_gre4;
-
 	ret = register_pernet_subsys(&proto_gre_net_ops);
 	if (ret < 0)
 		goto out_pernet;
 
+	ret = nf_ct_l4proto_register(&nf_conntrack_l4proto_gre4);
+	if (ret < 0)
+		goto out_gre4;
+
 	return 0;
-out_pernet:
-	nf_ct_l4proto_unregister(&nf_conntrack_l4proto_gre4);
 out_gre4:
+	unregister_pernet_subsys(&proto_gre_net_ops);
+out_pernet:
 	return ret;
 }
 
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 480f616..ec83536 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -888,6 +888,10 @@ static int __init nf_conntrack_proto_sctp_init(void)
 {
 	int ret;
 
+	ret = register_pernet_subsys(&sctp_net_ops);
+	if (ret < 0)
+		goto out_pernet;
+
 	ret = nf_ct_l4proto_register(&nf_conntrack_l4proto_sctp4);
 	if (ret < 0)
 		goto out_sctp4;
@@ -896,16 +900,12 @@ static int __init nf_conntrack_proto_sctp_init(void)
 	if (ret < 0)
 		goto out_sctp6;
 
-	ret = register_pernet_subsys(&sctp_net_ops);
-	if (ret < 0)
-		goto out_pernet;
-
 	return 0;
-out_pernet:
-	nf_ct_l4proto_unregister(&nf_conntrack_l4proto_sctp6);
 out_sctp6:
 	nf_ct_l4proto_unregister(&nf_conntrack_l4proto_sctp4);
 out_sctp4:
+	unregister_pernet_subsys(&sctp_net_ops);
+out_pernet:
 	return ret;
 }
 
diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
index 1574895..ca969f6 100644
--- a/net/netfilter/nf_conntrack_proto_udplite.c
+++ b/net/netfilter/nf_conntrack_proto_udplite.c
@@ -371,6 +371,10 @@ static int __init nf_conntrack_proto_udplite_init(void)
 {
 	int ret;
 
+	ret = register_pernet_subsys(&udplite_net_ops);
+	if (ret < 0)
+		goto out_pernet;
+
 	ret = nf_ct_l4proto_register(&nf_conntrack_l4proto_udplite4);
 	if (ret < 0)
 		goto out_udplite4;
@@ -379,16 +383,12 @@ static int __init nf_conntrack_proto_udplite_init(void)
 	if (ret < 0)
 		goto out_udplite6;
 
-	ret = register_pernet_subsys(&udplite_net_ops);
-	if (ret < 0)
-		goto out_pernet;
-
 	return 0;
-out_pernet:
-	nf_ct_l4proto_unregister(&nf_conntrack_l4proto_udplite6);
 out_udplite6:
 	nf_ct_l4proto_unregister(&nf_conntrack_l4proto_udplite4);
 out_udplite4:
+	unregister_pernet_subsys(&udplite_net_ops);
+out_pernet:
 	return ret;
 }
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH] net/rds: zero last byte for strncpy
From: Chen Gang @ 2013-03-08  3:19 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: David Laight, venkat.x.venkatsubra, David Miller, rds-devel,
	netdev
In-Reply-To: <1362502844.2791.32.camel@bwh-desktop.uk.solarflarecom.com>


  for NUL terminated string, need be always sure '\0' in the end.

additional info:
  strncpy will pads with zeroes to the end of the given buffer.
  should initialise every bit of memory that is going to be copied to userland


Signed-off-by: Chen Gang <gang.chen@asianux.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 net/rds/stats.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/rds/stats.c b/net/rds/stats.c
index 7be790d..73be187 100644
--- a/net/rds/stats.c
+++ b/net/rds/stats.c
@@ -87,6 +87,7 @@ void rds_stats_info_copy(struct rds_info_iterator *iter,
 	for (i = 0; i < nr; i++) {
 		BUG_ON(strlen(names[i]) >= sizeof(ctr.name));
 		strncpy(ctr.name, names[i], sizeof(ctr.name) - 1);
+		ctr.name[sizeof(ctr.name) - 1] = '\0';
 		ctr.value = values[i];
 
 		rds_info_copy(iter, &ctr, sizeof(ctr));
-- 
1.7.7.6

^ permalink raw reply related

* drivers/vhost: sizing of ubuf_info and heads
From: Rusty Russell @ 2013-03-08  2:57 UTC (permalink / raw)
  To: mst; +Cc: netdev, virtualization

Hi Michael,

        I'm a bit confused about why ubuf_info and heads are UIO_MAXIOV
length arrays, rather than being the size of the ring?  In particular,
this is suspicious:

linux/drivers/vhost/net.c:342:	struct ubuf_info *ubuf = &vq->ubuf_info[head];

And it seems to assume we trust head: a malicious guest could put the
same head entry in the ring twice, and we will get two callbacks on the
same value.  I don't know what that will do, but I'm not sure it's
harmless.

Thanks,
Rusty.

^ permalink raw reply

* Re: [Patch net] bridge: do not expire mdb entry when bridge still uses it
From: Herbert Xu @ 2013-03-08  2:47 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, bridge, Stephen Hemminger, David S. Miller, Adam Baker
In-Reply-To: <1362710646.9773.25.camel@cr0>

On Fri, Mar 08, 2013 at 10:44:06AM +0800, Cong Wang wrote:
>
> This is a good question. It is due to inside br_handle_frame_finish() we
> use 'skb2' to decide if we deliver packets to bridge itself, and in this
> case 'skb2' is non-NULL only when:
> 
>                 if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
>                         if ((mdst && mdst->mglist) ||
>                             br_multicast_is_router(br))
>                                 skb2 = skb; //        <======== HERE
>                         br_multicast_forward(mdst, skb, skb2);
> 
> For other ports, br_multicast_forward() will always forward 'skb' to
> them.

Yes but the point is that other ports should also expire after the
timeout and thus be removed from mdst.  So why is this only happening
for the bridge aka mdst->mglist?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH net-next] tg3: Update link_up flag for phylib devices
From: Nithin Nayak Sujir @ 2013-03-08  2:45 UTC (permalink / raw)
  To: davem; +Cc: netdev, hirofumi, Nithin Nayak Sujir, Michael Chan

Commit f4a46d1f46a8fece34edd2023e054072b02e110d introduced a bug where
the ifconfig stats would remain 0 for phylib devices. This is due to
tp->link_up flag never becoming true causing tg3_periodic_fetch_stats()
to return.

The link_up flag was being updated in tg3_test_and_report_link_chg()
after setting up the phy. This function however, is not called for
phylib devices since the driver does not do the phy setup.

This patch moves the link_up flag update into the common
tg3_link_report() function that gets called for phylib devices as well
for non phylib devices when the link state changes.

To avoid updating link_up twice, we replace tg3_carrier_...() calls that
are followed by tg3_link_report(), with netif_carrier_...(). We can then
remove the unused tg3_carrier_on() function.

Reported-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 2b2bee6..0c1a2ef 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -1870,6 +1870,8 @@ static void tg3_link_report(struct tg3 *tp)
 
 		tg3_ump_link_report(tp);
 	}
+
+	tp->link_up = netif_carrier_ok(tp->dev);
 }
 
 static u16 tg3_advert_flowctrl_1000X(u8 flow_ctrl)
@@ -2523,12 +2525,6 @@ static int tg3_phy_reset_5703_4_5(struct tg3 *tp)
 	return err;
 }
 
-static void tg3_carrier_on(struct tg3 *tp)
-{
-	netif_carrier_on(tp->dev);
-	tp->link_up = true;
-}
-
 static void tg3_carrier_off(struct tg3 *tp)
 {
 	netif_carrier_off(tp->dev);
@@ -2554,7 +2550,7 @@ static int tg3_phy_reset(struct tg3 *tp)
 		return -EBUSY;
 
 	if (netif_running(tp->dev) && tp->link_up) {
-		tg3_carrier_off(tp);
+		netif_carrier_off(tp->dev);
 		tg3_link_report(tp);
 	}
 
@@ -4403,9 +4399,9 @@ static bool tg3_test_and_report_link_chg(struct tg3 *tp, int curr_link_up)
 {
 	if (curr_link_up != tp->link_up) {
 		if (curr_link_up) {
-			tg3_carrier_on(tp);
+			netif_carrier_on(tp->dev);
 		} else {
-			tg3_carrier_off(tp);
+			netif_carrier_off(tp->dev);
 			if (tp->phy_flags & TG3_PHYFLG_MII_SERDES)
 				tp->phy_flags &= ~TG3_PHYFLG_PARALLEL_DETECT;
 		}
-- 
1.8.1.4

^ permalink raw reply related

* Re: [Patch net] bridge: do not expire mdb entry when bridge still uses it
From: Cong Wang @ 2013-03-08  2:44 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Stephen Hemminger, netdev, bridge, David S. Miller, Adam Baker
In-Reply-To: <20130308023151.GA27155@gondor.apana.org.au>

On Fri, 2013-03-08 at 10:31 +0800, Herbert Xu wrote:
> On Fri, Mar 08, 2013 at 10:07:03AM +0800, Cong Wang wrote:
> > From: Cong Wang <amwang@redhat.com>
> > 
> > This is a long-standing bug and reported several times:
> > https://bugzilla.redhat.com/show_bug.cgi?id=880035
> > http://marc.info/?l=linux-netdev&m=136164389416341&w=2
> > 
> > This bug can be observed in virt environment, when a KVM guest
> > communicates with the host via multicast. After some time (should
> > be 260 sec, I didn't measure), the multicast traffic suddenly
> > terminates.
> > 
> > This is due to the mdb entry for bridge itself expires automatically,
> > it should not expire as long as the bridge still generates multicast
> > traffic. It should expire when the bridge leaves the multicast group,
> > OR when there is no multicast traffic on this bridge.
> > 
> > I fix this by adding another bool which is set when there is
> > multicast traffic goes to the bridge, cleared in the expire timer and
> > when IGMP leave is received. I ran omping for 15 minutes, everything
> > looks good now.
> 
> I gather from the bugzilla entry that this happens where there is
> no querier in the network.
> 
> So my question is why does this only affect the bridge port and
> not other ports on the bridge?
> 

This is a good question. It is due to inside br_handle_frame_finish() we
use 'skb2' to decide if we deliver packets to bridge itself, and in this
case 'skb2' is non-NULL only when:

                if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
                        if ((mdst && mdst->mglist) ||
                            br_multicast_is_router(br))
                                skb2 = skb; //        <======== HERE
                        br_multicast_forward(mdst, skb, skb2);

For other ports, br_multicast_forward() will always forward 'skb' to
them.

Thanks.

^ permalink raw reply

* Re: [Patch net] bridge: do not expire mdb entry when bridge still uses it
From: Herbert Xu @ 2013-03-08  2:31 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, bridge, Stephen Hemminger, David S. Miller, Adam Baker
In-Reply-To: <1362708423-23932-1-git-send-email-amwang@redhat.com>

On Fri, Mar 08, 2013 at 10:07:03AM +0800, Cong Wang wrote:
> From: Cong Wang <amwang@redhat.com>
> 
> This is a long-standing bug and reported several times:
> https://bugzilla.redhat.com/show_bug.cgi?id=880035
> http://marc.info/?l=linux-netdev&m=136164389416341&w=2
> 
> This bug can be observed in virt environment, when a KVM guest
> communicates with the host via multicast. After some time (should
> be 260 sec, I didn't measure), the multicast traffic suddenly
> terminates.
> 
> This is due to the mdb entry for bridge itself expires automatically,
> it should not expire as long as the bridge still generates multicast
> traffic. It should expire when the bridge leaves the multicast group,
> OR when there is no multicast traffic on this bridge.
> 
> I fix this by adding another bool which is set when there is
> multicast traffic goes to the bridge, cleared in the expire timer and
> when IGMP leave is received. I ran omping for 15 minutes, everything
> looks good now.

I gather from the bugzilla entry that this happens where there is
no querier in the network.

So my question is why does this only affect the bridge port and
not other ports on the bridge?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] ipv6 addrconf: Fix addrconf_ifid_eui64 of 802.15.4 devices
From: YOSHIFUJI Hideaki @ 2013-03-08  2:21 UTC (permalink / raw)
  To: Alexander Aring; +Cc: davem, kuznet, jmorris, linux-zigbee-devel, netdev
In-Reply-To: <20130307204120.GA3808@x61s.8.8.8.8>

Hi,

Alexander Aring wrote:
> Hi,
> 
> I don't know if this is a right solution for that, because other
> functions like addrconf_ifid_infiniband to manipulate eui after memcpy,
> too.
> 
> But this patch solves my problems with ieee802154 stack.

What kind of problems do you have?

--yoshfuji

^ permalink raw reply

* Re: [PATCH net-next v3 1/4] ipv6: introdcue __ipv6_addr_needs_scope_id and ipv6_iface_scope_id helper functions
From: YOSHIFUJI Hideaki @ 2013-03-08  2:21 UTC (permalink / raw)
  To: Hannes Frederic Sowa, David Miller, netdev
In-Reply-To: <20130307210529.GA23621@order.stressinduktion.org>

Hannes Frederic Sowa wrote:
> __ipv6_addr_needs_scope_id checks if an ipv6 address needs to supply
> a 'sin6_scope_id != 0'. 'sin6_scope_id != 0' was enforced in case
> of link-local addresses. To support interface-local multicast these
> checks had to be enhanced and are now consolidated into these new helper
> functions.
> 
> v2:
> a) migrated to struct ipv6_addr_props
> 
> v3:
> a) reverted changes for ipv6_addr_props
> b) test for address type instead of comparing scope
> 
> Suggested-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  include/net/ipv6.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 64d12e7..e11d304 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -320,6 +320,18 @@ static inline int ipv6_addr_src_scope(const struct in6_addr *addr)
>  	return __ipv6_addr_src_scope(__ipv6_addr_type(addr));
>  }
>  
> +static inline bool __ipv6_addr_needs_scope_id(int type)
> +{
> +	return type & IPV6_ADDR_LINKLOCAL ||
> +	       (type & IPV6_ADDR_MULTICAST &&
> +		(type & (IPV6_ADDR_LOOPBACK|IPV6_ADDR_LINKLOCAL)));
> +}
> +
> +static inline __u32 ipv6_iface_scope_id(const struct in6_addr *addr, int iface)
> +{
> +	return __ipv6_addr_needs_scope_id(__ipv6_addr_type(addr)) ? iface : 0;
> +}
> +
>  static inline int ipv6_addr_cmp(const struct in6_addr *a1, const struct in6_addr *a2)
>  {
>  	return memcmp(a1, a2, sizeof(struct in6_addr));
> 

Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

^ permalink raw reply

* Re: [PATCH net-next v3 2/4] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id
From: YOSHIFUJI Hideaki @ 2013-03-08  2:21 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: YOSHIFUJI Hideaki
In-Reply-To: <20130307210533.GA23676@order.stressinduktion.org>

Hannes Frederic Sowa wrote:
> This patch requires multicast interface-scoped addresses to supply a
> sin6_scope_id. Because the sin6_scope_id is now also correctly used
> in case of interface-scoped multicast traffic this enables one to use
> interface scoped addresses over interfaces which are not targeted by the
> default multicast route (the route has to be put there manually, though).
> 
> getsockname() and getpeername() now return the correct sin6_scope_id in
> case of interface-local mc addresses.
> 
> v2:
> a) rebased ontop of patch 1/4 (now uses ipv6_addr_props)
> 
> v3:
> a) reverted changes for ipv6_addr_props
> 
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/ipv6/af_inet6.c              |  6 +++---
>  net/ipv6/datagram.c              | 16 +++++++++-------
>  net/ipv6/icmp.c                  |  2 +-
>  net/ipv6/inet6_connection_sock.c |  6 ++----
>  net/ipv6/raw.c                   |  9 ++++-----
>  net/ipv6/udp.c                   | 13 +++++++------
>  6 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index 6b793bf..f56277f 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>  			struct net_device *dev = NULL;
>  
>  			rcu_read_lock();
> -			if (addr_type & IPV6_ADDR_LINKLOCAL) {
> +			if (__ipv6_addr_needs_scope_id(addr_type)) {
>  				if (addr_len >= sizeof(struct sockaddr_in6) &&
>  				    addr->sin6_scope_id) {
>  					/* Override any existing binding, if another one
> @@ -471,8 +471,8 @@ int inet6_getname(struct socket *sock, struct sockaddr *uaddr,
>  
>  		sin->sin6_port = inet->inet_sport;
>  	}
> -	if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
> -		sin->sin6_scope_id = sk->sk_bound_dev_if;
> +	sin->sin6_scope_id = ipv6_iface_scope_id(&sin->sin6_addr,
> +						 sk->sk_bound_dev_if);
>  	*uaddr_len = sizeof(*sin);
>  	return 0;
>  }
> diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
> index f5a5478..b55e70a 100644
> --- a/net/ipv6/datagram.c
> +++ b/net/ipv6/datagram.c
> @@ -124,7 +124,7 @@ ipv4_connected:
>  		goto out;
>  	}
>  
> -	if (addr_type&IPV6_ADDR_LINKLOCAL) {
> +	if (__ipv6_addr_needs_scope_id(addr_type)) {
>  		if (addr_len >= sizeof(struct sockaddr_in6) &&
>  		    usin->sin6_scope_id) {
>  			if (sk->sk_bound_dev_if &&
> @@ -355,18 +355,19 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len)
>  		sin->sin6_family = AF_INET6;
>  		sin->sin6_flowinfo = 0;
>  		sin->sin6_port = serr->port;
> -		sin->sin6_scope_id = 0;
>  		if (skb->protocol == htons(ETH_P_IPV6)) {
>  			const struct ipv6hdr *ip6h = container_of((struct in6_addr *)(nh + serr->addr_offset),
>  								  struct ipv6hdr, daddr);
>  			sin->sin6_addr = ip6h->daddr;
>  			if (np->sndflow)
>  				sin->sin6_flowinfo = ip6_flowinfo(ip6h);
> -			if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
> -				sin->sin6_scope_id = IP6CB(skb)->iif;
> +			sin->sin6_scope_id =
> +				ipv6_iface_scope_id(&sin->sin6_addr,
> +						    IP6CB(skb)->iif);
>  		} else {
>  			ipv6_addr_set_v4mapped(*(__be32 *)(nh + serr->addr_offset),
>  					       &sin->sin6_addr);
> +			sin->sin6_scope_id = 0;
>  		}
>  	}
>  
> @@ -376,18 +377,19 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len)
>  	if (serr->ee.ee_origin != SO_EE_ORIGIN_LOCAL) {
>  		sin->sin6_family = AF_INET6;
>  		sin->sin6_flowinfo = 0;
> -		sin->sin6_scope_id = 0;
>  		if (skb->protocol == htons(ETH_P_IPV6)) {
>  			sin->sin6_addr = ipv6_hdr(skb)->saddr;
>  			if (np->rxopt.all)
>  				ip6_datagram_recv_ctl(sk, msg, skb);
> -			if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
> -				sin->sin6_scope_id = IP6CB(skb)->iif;
> +			sin->sin6_scope_id =
> +				ipv6_iface_scope_id(&sin->sin6_addr,
> +						    IP6CB(skb)->iif);
>  		} else {
>  			struct inet_sock *inet = inet_sk(sk);
>  
>  			ipv6_addr_set_v4mapped(ip_hdr(skb)->saddr,
>  					       &sin->sin6_addr);
> +			sin->sin6_scope_id = 0;
>  			if (inet->cmsg_flags)
>  				ip_cmsg_recv(msg, skb);
>  		}
> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index fff5bdd..71b900c 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -434,7 +434,7 @@ void icmpv6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info)
>  	 *	Source addr check
>  	 */
>  
> -	if (addr_type & IPV6_ADDR_LINKLOCAL)
> +	if (__ipv6_addr_needs_scope_id(addr_type))
>  		iif = skb->dev->ifindex;
>  
>  	/*
> diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
> index 5f25510..e4311cb 100644
> --- a/net/ipv6/inet6_connection_sock.c
> +++ b/net/ipv6/inet6_connection_sock.c
> @@ -173,10 +173,8 @@ void inet6_csk_addr2sockaddr(struct sock *sk, struct sockaddr * uaddr)
>  	sin6->sin6_port	= inet_sk(sk)->inet_dport;
>  	/* We do not store received flowlabel for TCP */
>  	sin6->sin6_flowinfo = 0;
> -	sin6->sin6_scope_id = 0;
> -	if (sk->sk_bound_dev_if &&
> -	    ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL)
> -		sin6->sin6_scope_id = sk->sk_bound_dev_if;
> +	sin6->sin6_scope_id = ipv6_iface_scope_id(&sin6->sin6_addr,
> +						  sk->sk_bound_dev_if);
>  }
>  
>  EXPORT_SYMBOL_GPL(inet6_csk_addr2sockaddr);
> diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
> index 330b5e7..eedff8c 100644
> --- a/net/ipv6/raw.c
> +++ b/net/ipv6/raw.c
> @@ -263,7 +263,7 @@ static int rawv6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
>  	if (addr_type != IPV6_ADDR_ANY) {
>  		struct net_device *dev = NULL;
>  
> -		if (addr_type & IPV6_ADDR_LINKLOCAL) {
> +		if (__ipv6_addr_needs_scope_id(addr_type)) {
>  			if (addr_len >= sizeof(struct sockaddr_in6) &&
>  			    addr->sin6_scope_id) {
>  				/* Override any existing binding, if another
> @@ -498,9 +498,8 @@ static int rawv6_recvmsg(struct kiocb *iocb, struct sock *sk,
>  		sin6->sin6_port = 0;
>  		sin6->sin6_addr = ipv6_hdr(skb)->saddr;
>  		sin6->sin6_flowinfo = 0;
> -		sin6->sin6_scope_id = 0;
> -		if (ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL)
> -			sin6->sin6_scope_id = IP6CB(skb)->iif;
> +		sin6->sin6_scope_id = ipv6_iface_scope_id(&sin6->sin6_addr,
> +							  IP6CB(skb)->iif);
>  	}
>  
>  	sock_recv_ts_and_drops(msg, sk, skb);
> @@ -802,7 +801,7 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
>  
>  		if (addr_len >= sizeof(struct sockaddr_in6) &&
>  		    sin6->sin6_scope_id &&
> -		    ipv6_addr_type(daddr)&IPV6_ADDR_LINKLOCAL)
> +		    __ipv6_addr_needs_scope_id(__ipv6_addr_type(daddr)))
>  			fl6.flowi6_oif = sin6->sin6_scope_id;
>  	} else {
>  		if (sk->sk_state != TCP_ESTABLISHED)
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 599e1ba6..3ed57ec 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -450,15 +450,16 @@ try_again:
>  		sin6->sin6_family = AF_INET6;
>  		sin6->sin6_port = udp_hdr(skb)->source;
>  		sin6->sin6_flowinfo = 0;
> -		sin6->sin6_scope_id = 0;
>  
> -		if (is_udp4)
> +		if (is_udp4) {
>  			ipv6_addr_set_v4mapped(ip_hdr(skb)->saddr,
>  					       &sin6->sin6_addr);
> -		else {
> +			sin6->sin6_scope_id = 0;
> +		} else {
>  			sin6->sin6_addr = ipv6_hdr(skb)->saddr;
> -			if (ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL)
> -				sin6->sin6_scope_id = IP6CB(skb)->iif;
> +			sin6->sin6_scope_id =
> +				ipv6_iface_scope_id(&sin6->sin6_addr,
> +						    IP6CB(skb)->iif);
>  		}
>  
>  	}
> @@ -1118,7 +1119,7 @@ do_udp_sendmsg:
>  
>  		if (addr_len >= sizeof(struct sockaddr_in6) &&
>  		    sin6->sin6_scope_id &&
> -		    ipv6_addr_type(daddr)&IPV6_ADDR_LINKLOCAL)
> +		    __ipv6_addr_needs_scope_id(__ipv6_addr_type(daddr)))
>  			fl6.flowi6_oif = sin6->sin6_scope_id;
>  	} else {
>  		if (sk->sk_state != TCP_ESTABLISHED)
> 

Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

^ permalink raw reply

* Re: [PATCH net-next v3 3/4] ipv6: stop multicast forwarding to process interface scoped addresses
From: YOSHIFUJI Hideaki @ 2013-03-08  2:19 UTC (permalink / raw)
  To: netdev, David Miller, Hannes Frederic Sowa; +Cc: YOSHIFUJI Hideaki
In-Reply-To: <20130307210537.GA23729@order.stressinduktion.org>

Hannes Frederic Sowa wrote:
> v2:
> a) used struct ipv6_addr_props
> 
> v3:
> a) reverted changes for ipv6_addr_props
> 
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/ipv6/ip6_input.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index b1876e5..b73bcc2 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -281,7 +281,7 @@ int ip6_mc_input(struct sk_buff *skb)
>  	 *      IPv6 multicast router mode is now supported ;)
>  	 */
>  	if (dev_net(skb->dev)->ipv6.devconf_all->mc_forwarding &&
> -	    !(ipv6_addr_type(&hdr->daddr) & IPV6_ADDR_LINKLOCAL) &&
> +	    !__ipv6_addr_needs_scope_id((ipv6_addr_type(&hdr->daddr))) &&
>  	    likely(!(IP6CB(skb)->flags & IP6SKB_FORWARDED))) {
>  		/*
>  		 * Okay, we try to forward - split and duplicate
> 

Well, I think __ipv6_addr_needs_scope_id() should be used just for
filling "sin6_scope_id", not for forwarding decision.

Since we always have multicast address, so 
            !(ipv6_addr_type(&hdr->daddr) & (IPV6_ADDR_LOOPBACK|IPV6_ADDR_LINKLOCAL))
is just fine for disallow "forwarding" multicast for interface-local addresses.

--yoshfuji

^ permalink raw reply

* Re: [PATCH net-next v3 4/4] ipv6: report sin6_scope_id if sockopt RECVORIGDSTADDR is set
From: YOSHIFUJI Hideaki @ 2013-03-08  2:09 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: YOSHIFUJI Hideaki
In-Reply-To: <20130307210540.GA23781@order.stressinduktion.org>

Hannes Frederic Sowa wrote:
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/ipv6/datagram.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
> index b55e70a..4b56cbb 100644
> --- a/net/ipv6/datagram.c
> +++ b/net/ipv6/datagram.c
> @@ -594,7 +594,9 @@ int ip6_datagram_recv_ctl(struct sock *sk, struct msghdr *msg,
>  			sin6.sin6_addr = ipv6_hdr(skb)->daddr;
>  			sin6.sin6_port = ports[1];
>  			sin6.sin6_flowinfo = 0;
> -			sin6.sin6_scope_id = 0;
> +			sin6.sin6_scope_id =
> +				ipv6_iface_scope_id(&ipv6_hdr(skb)->daddr,
> +						    opt->iif);
>  
>  			put_cmsg(msg, SOL_IPV6, IPV6_ORIGDSTADDR, sizeof(sin6), &sin6);
>  		}
> 

Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

^ permalink raw reply

* [Patch net] bridge: do not expire mdb entry when bridge still uses it
From: Cong Wang @ 2013-03-08  2:07 UTC (permalink / raw)
  To: netdev
  Cc: Cong Wang, bridge, Adam Baker, Stephen Hemminger, Herbert Xu,
	David S. Miller

From: Cong Wang <amwang@redhat.com>

This is a long-standing bug and reported several times:
https://bugzilla.redhat.com/show_bug.cgi?id=880035
http://marc.info/?l=linux-netdev&m=136164389416341&w=2

This bug can be observed in virt environment, when a KVM guest
communicates with the host via multicast. After some time (should
be 260 sec, I didn't measure), the multicast traffic suddenly
terminates.

This is due to the mdb entry for bridge itself expires automatically,
it should not expire as long as the bridge still generates multicast
traffic. It should expire when the bridge leaves the multicast group,
OR when there is no multicast traffic on this bridge.

I fix this by adding another bool which is set when there is
multicast traffic goes to the bridge, cleared in the expire timer and
when IGMP leave is received. I ran omping for 15 minutes, everything
looks good now.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Adam Baker <linux@baker-net.org.uk>
Signed-off-by: Cong Wang <amwang@redhat.com>

---
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 828e2bc..c706619 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -99,9 +99,11 @@ int br_handle_frame_finish(struct sk_buff *skb)
 	else if (is_multicast_ether_addr(dest)) {
 		mdst = br_mdb_get(br, skb, vid);
 		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
-			if ((mdst && mdst->mglist) ||
-			    br_multicast_is_router(br))
+			bool to_br = mdst && mdst->mglist;
+			if (to_br || br_multicast_is_router(br))
 				skb2 = skb;
+			if (to_br)
+				mdst->busy = true;
 			br_multicast_forward(mdst, skb, skb2);
 			skb = NULL;
 			if (!skb2)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 923fbea..4b12a0d 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -228,11 +228,16 @@ static void br_multicast_group_expired(unsigned long data)
 	if (!netif_running(br->dev) || timer_pending(&mp->timer))
 		goto out;
 
-	mp->mglist = false;
-
 	if (mp->ports)
 		goto out;
 
+	if (mp->mglist && mp->busy) {
+		mp->busy = false;
+		goto out;
+	}
+
+	mp->mglist = false;
+
 	mdb = mlock_dereference(br->mdb, br);
 
 	hlist_del_rcu(&mp->hlist[mdb->ver]);
@@ -1280,6 +1285,7 @@ static void br_multicast_leave_group(struct net_bridge *br,
 			mod_timer(&mp->timer, time);
 		}
 
+		mp->busy = false;
 		goto out;
 	}
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 3cbf5be..bb9a3e4 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -112,6 +112,7 @@ struct net_bridge_mdb_entry
 	struct timer_list		timer;
 	struct br_ip			addr;
 	bool				mglist;
+	bool				busy; /* update this only when mglist == true */
 };
 
 struct net_bridge_mdb_htable

^ permalink raw reply related

* Re: re L4 conntracking netns conversion
From: Gao feng @ 2013-03-08  1:56 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Alexey Dobriyan, Pablo Neira Ayuso, netdev, netfilter-devel
In-Reply-To: <87sj46ptgc.fsf@xmission.com>

On 2013/03/08 09:32, Eric W. Biederman wrote:
> Gao feng <gaofeng@cn.fujitsu.com> writes:
> 
>> On 2013/03/07 19:50, Alexey Dobriyan wrote:
>>> Lots of netns changes!
>>>
>>> I can't verify right now, but unless I'm not mistaken,
>>> every L4 protocol conversion is buggy/oopsable/remotely ddosable
>>> because per-netns stuff is initialized after protocol is hooked into
>>> master dispatcher.
>>>
>>
>> Doesn't we do register_pernet_subsys before we register hooks and l4proto?
>> Sorry I don't quite understand what you mean. :(
> 
>>> See c296bb4d5d417d466c9bcc8afef68a3db5449a64.
> 
> The registration in the referenced commit has register_pernet_subsys
> happening after nf_ct_l4_proto_register.  The unregistration is also
> happening in that order so something seems fishy.  If there is
> an ordering dependency between the two unregistration should happen
> in the opposite order of registration.
> 

Yes, we have the incorrect order when registering l4proto_sctp/gre/dccp/udplite.

> However, I don't know the code well enough to know if it is a problem or
> not.
> 

Had better to fix this problem, Since the l4proto may access the memory before
register_pernet_subsys allocates it.

Thanks

^ permalink raw reply

* [RFC net-next 11/11] firewire net: Release broadcast/fifo resources on ifdown.
From: YOSHIFUJI Hideaki @ 2013-03-08  1:45 UTC (permalink / raw)
  To: stefanr, linux1394-devel; +Cc: netdev

Since those resources are allocated on ifup, relsase them on ifdown.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 drivers/firewire/net.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index ca41446..c1898ad 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1295,9 +1295,12 @@ out:
 /* ifdown */
 static int fwnet_stop(struct net_device *net)
 {
+	struct fwnet_device *dev = netdev_priv(net);
+
 	netif_stop_queue(net);
 
-	/* Deallocate iso context for use by other applications? */
+	fwnet_broadcast_stop(dev);
+	fwnet_fifo_stop(dev);
 
 	return 0;
 }
@@ -1638,9 +1641,6 @@ static int fwnet_remove(struct device *_dev)
 	if (list_empty(&dev->peer_list)) {
 		unregister_netdev(net);
 
-		fwnet_fifo_stop(dev);
-		fwnet_broadcast_stop(dev);
-
 		for (i = 0; dev->queued_datagrams && i < 5; i++)
 			ssleep(1);
 		WARN_ON(dev->queued_datagrams);
-- 
1.7.9.5

^ permalink raw reply related

* [RFC net-next 10/11] firewire net: Introduce fwnet_broadcast_stop() to destroy broadcast resources.
From: YOSHIFUJI Hideaki @ 2013-03-08  1:45 UTC (permalink / raw)
  To: stefanr, linux1394-devel; +Cc: netdev

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 drivers/firewire/net.c |   68 +++++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 32 deletions(-)

diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 21210fb..ca41446 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1146,6 +1146,32 @@ static int fwnet_fifo_start(struct fwnet_device *dev)
 	return 0;
 }
 
+static void __fwnet_broadcast_stop(struct fwnet_device *dev)
+{
+	unsigned u;
+
+	if (dev->broadcast_state != FWNET_BROADCAST_ERROR) {
+		for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++)
+			kunmap(dev->broadcast_rcv_buffer.pages[u]);
+		fw_iso_buffer_destroy(&dev->broadcast_rcv_buffer, dev->card);
+	}
+	if (dev->broadcast_rcv_context) {
+		fw_iso_context_destroy(dev->broadcast_rcv_context);
+		dev->broadcast_rcv_context = NULL;
+	}
+	kfree(dev->broadcast_rcv_buffer_ptrs);
+	dev->broadcast_rcv_buffer_ptrs = NULL;
+	dev->broadcast_state = FWNET_BROADCAST_ERROR;
+}
+
+static void fwnet_broadcast_stop(struct fwnet_device *dev)
+{
+	if (dev->broadcast_state == FWNET_BROADCAST_ERROR)
+		return;
+	fw_iso_context_stop(dev->broadcast_rcv_context);
+	__fwnet_broadcast_stop(dev);
+}
+
 static int fwnet_broadcast_start(struct fwnet_device *dev)
 {
 	struct fw_iso_context *context;
@@ -1166,7 +1192,7 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 	ptrptr = kmalloc(sizeof(void *) * num_packets, GFP_KERNEL);
 	if (!ptrptr) {
 		retval = -ENOMEM;
-		goto failed_ptrs_alloc;
+		goto failed;
 	}
 	dev->broadcast_rcv_buffer_ptrs = ptrptr;
 
@@ -1176,13 +1202,15 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 					fwnet_receive_broadcast, dev);
 	if (IS_ERR(context)) {
 		retval = PTR_ERR(context);
-		goto failed_context_create;
+		goto failed;
 	}
 
 	retval = fw_iso_buffer_init(&dev->broadcast_rcv_buffer, dev->card,
 				    FWNET_ISO_PAGE_COUNT, DMA_FROM_DEVICE);
 	if (retval < 0)
-		goto failed_buffer_init;
+		goto failed;
+
+	dev->broadcast_state = FWNET_BROADCAST_STOPPED;
 
 	for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++) {
 		void *ptr;
@@ -1206,7 +1234,7 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 		retval = fw_iso_context_queue(context, &packet,
 				&dev->broadcast_rcv_buffer, offset);
 		if (retval < 0)
-			goto failed_rcv_queue;
+			goto failed;
 
 		offset += max_receive;
 	}
@@ -1216,7 +1244,7 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 	retval = fw_iso_context_start(context, -1, 0,
 			FW_ISO_CONTEXT_MATCH_ALL_TAGS); /* ??? sync */
 	if (retval < 0)
-		goto failed_rcv_queue;
+		goto failed;
 
 	/* FIXME: adjust it according to the min. speed of all known peers? */
 	dev->broadcast_xmt_max_payload = IEEE1394_MAX_PAYLOAD_S100
@@ -1225,18 +1253,8 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 
 	return 0;
 
- failed_rcv_queue:
-	for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++)
-		kunmap(dev->broadcast_rcv_buffer.pages[u]);
-	fw_iso_buffer_destroy(&dev->broadcast_rcv_buffer, dev->card);
- failed_buffer_init:
-	fw_iso_context_destroy(context);
-	dev->broadcast_rcv_context = NULL;
- failed_context_create:
-	kfree(dev->broadcast_rcv_buffer_ptrs);
-	dev->broadcast_rcv_buffer_ptrs = NULL;
- failed_ptrs_alloc:
-
+ failed:
+	__fwnet_broadcast_stop(dev);
 	return retval;
 }
 
@@ -1621,22 +1639,8 @@ static int fwnet_remove(struct device *_dev)
 		unregister_netdev(net);
 
 		fwnet_fifo_stop(dev);
-		if (dev->broadcast_rcv_context) {
-			unsigned u;
-
-			fw_iso_context_stop(dev->broadcast_rcv_context);
+		fwnet_broadcast_stop(dev);
 
-			for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++)
-				kunmap(dev->broadcast_rcv_buffer.pages[u]);
-
-			fw_iso_buffer_destroy(&dev->broadcast_rcv_buffer,
-					      dev->card);
-			fw_iso_context_destroy(dev->broadcast_rcv_context);
-			dev->broadcast_rcv_context = NULL;
-			kfree(dev->broadcast_rcv_buffer_ptrs);
-			dev->broadcast_rcv_buffer_ptrs = NULL;
-			dev->broadcast_state = FWNET_BROADCAST_ERROR;
-		}
 		for (i = 0; dev->queued_datagrams && i < 5; i++)
 			ssleep(1);
 		WARN_ON(dev->queued_datagrams);
-- 
1.7.9.5

^ permalink raw reply related

* [RFC net-next 09/11] firewire net: Allocate dev->broadcast_rcv_buffer_ptrs early.
From: YOSHIFUJI Hideaki @ 2013-03-08  1:43 UTC (permalink / raw)
  To: stefanr, linux1394-devel; +Cc: netdev

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 drivers/firewire/net.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 0dc2fdf..21210fb 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1163,6 +1163,13 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 	max_receive = 1U << (dev->card->max_receive + 1);
 	num_packets = (FWNET_ISO_PAGE_COUNT * PAGE_SIZE) / max_receive;
 
+	ptrptr = kmalloc(sizeof(void *) * num_packets, GFP_KERNEL);
+	if (!ptrptr) {
+		retval = -ENOMEM;
+		goto failed_ptrs_alloc;
+	}
+	dev->broadcast_rcv_buffer_ptrs = ptrptr;
+
 	context = fw_iso_context_create(dev->card, FW_ISO_CONTEXT_RECEIVE,
 					IEEE1394_BROADCAST_CHANNEL,
 					dev->card->link_speed, 8,
@@ -1177,13 +1184,6 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 	if (retval < 0)
 		goto failed_buffer_init;
 
-	ptrptr = kmalloc(sizeof(void *) * num_packets, GFP_KERNEL);
-	if (!ptrptr) {
-		retval = -ENOMEM;
-		goto failed_ptrs_alloc;
-	}
-
-	dev->broadcast_rcv_buffer_ptrs = ptrptr;
 	for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++) {
 		void *ptr;
 		unsigned v;
@@ -1226,16 +1226,16 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 	return 0;
 
  failed_rcv_queue:
-	kfree(dev->broadcast_rcv_buffer_ptrs);
-	dev->broadcast_rcv_buffer_ptrs = NULL;
 	for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++)
 		kunmap(dev->broadcast_rcv_buffer.pages[u]);
- failed_ptrs_alloc:
 	fw_iso_buffer_destroy(&dev->broadcast_rcv_buffer, dev->card);
  failed_buffer_init:
 	fw_iso_context_destroy(context);
 	dev->broadcast_rcv_context = NULL;
  failed_context_create:
+	kfree(dev->broadcast_rcv_buffer_ptrs);
+	dev->broadcast_rcv_buffer_ptrs = NULL;
+ failed_ptrs_alloc:
 
 	return retval;
 }
@@ -1626,8 +1626,6 @@ static int fwnet_remove(struct device *_dev)
 
 			fw_iso_context_stop(dev->broadcast_rcv_context);
 
-			kfree(dev->broadcast_rcv_buffer_ptrs);
-			dev->broadcast_rcv_buffer_ptrs = NULL;
 			for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++)
 				kunmap(dev->broadcast_rcv_buffer.pages[u]);
 
@@ -1635,6 +1633,8 @@ static int fwnet_remove(struct device *_dev)
 					      dev->card);
 			fw_iso_context_destroy(dev->broadcast_rcv_context);
 			dev->broadcast_rcv_context = NULL;
+			kfree(dev->broadcast_rcv_buffer_ptrs);
+			dev->broadcast_rcv_buffer_ptrs = NULL;
 			dev->broadcast_state = FWNET_BROADCAST_ERROR;
 		}
 		for (i = 0; dev->queued_datagrams && i < 5; i++)
-- 
1.7.9.5

^ permalink raw reply related

* [RFC net-next 08/11] firewire net: Fix leakage of kmap for broadcast receive buffer.
From: YOSHIFUJI Hideaki @ 2013-03-08  1:43 UTC (permalink / raw)
  To: stefanr, linux1394-devel; +Cc: netdev

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 drivers/firewire/net.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index d8cb6ac..0dc2fdf 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1228,6 +1228,8 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
  failed_rcv_queue:
 	kfree(dev->broadcast_rcv_buffer_ptrs);
 	dev->broadcast_rcv_buffer_ptrs = NULL;
+	for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++)
+		kunmap(dev->broadcast_rcv_buffer.pages[u]);
  failed_ptrs_alloc:
 	fw_iso_buffer_destroy(&dev->broadcast_rcv_buffer, dev->card);
  failed_buffer_init:
@@ -1620,10 +1622,15 @@ static int fwnet_remove(struct device *_dev)
 
 		fwnet_fifo_stop(dev);
 		if (dev->broadcast_rcv_context) {
+			unsigned u;
+
 			fw_iso_context_stop(dev->broadcast_rcv_context);
 
 			kfree(dev->broadcast_rcv_buffer_ptrs);
 			dev->broadcast_rcv_buffer_ptrs = NULL;
+			for (u = 0; u < FWNET_ISO_PAGE_COUNT; u++)
+				kunmap(dev->broadcast_rcv_buffer.pages[u]);
+
 			fw_iso_buffer_destroy(&dev->broadcast_rcv_buffer,
 					      dev->card);
 			fw_iso_context_destroy(dev->broadcast_rcv_context);
-- 
1.7.9.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox