Netdev List
 help / color / mirror / Atom feed
* VLAN HW accel, performance advantage?
From: Joakim Tjernlund @ 2013-09-03  9:05 UTC (permalink / raw)
  To: netdev

I am considering impl. VLAN HW acceleration in Freescales ucc_geth driver 
to improve
performance, primarily for bridged interfaces.
 I am hoping the community has some insights as to what performance gains 
one
could expect?

   Jocke

^ permalink raw reply

* [patch] qlcnic: remove a stray semicolon
From: Dan Carpenter @ 2013-09-03  9:13 UTC (permalink / raw)
  To: Himanshu Madhani
  Cc: Rajesh Borundia, Shahed Shaikh, Jitendra Kalsaria, Sony Chacko,
	Sucheta Chakraborty, linux-driver, netdev, kernel-janitors

Just remove a small semicolon.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 332aa71..4d7ad00 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -1709,7 +1709,7 @@ qlcnic_set_dump(struct net_device *netdev, struct ethtool_dump *val)
 		qlcnic_dev_request_reset(adapter, val->flag);
 		adapter->flags &= ~QLCNIC_FW_RESET_OWNER;
 		break;
-;
+
 	case QLCNIC_SET_QUIESCENT:
 	case QLCNIC_RESET_QUIESCENT:
 		state = QLC_SHARED_REG_RD32(adapter, QLCNIC_CRB_DEV_STATE);

^ permalink raw reply related

* [patch] x25: add a sanity check parsing X.25 facilities
From: Dan Carpenter @ 2013-09-03  9:03 UTC (permalink / raw)
  To: Andrew Hendry; +Cc: David S. Miller, linux-x25, netdev, kernel-janitors

This was found with a manual audit and I don't have a reproducer.  We
limit ->calling_len and ->called_len when we get them from
copy_from_user() in x25_ioctl() so when they come from skb->data then
we should cap them there as well.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Possibly should be applied to -stable.  My guess is that this leads to
memory corruption, but I don't know.

diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
index 66c63873..b825325 100644
--- a/net/x25/x25_facilities.c
+++ b/net/x25/x25_facilities.c
@@ -156,6 +156,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 			case X25_FAC_CALLING_AE:
 				if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
 					return -1;
+				if (p[2] > X25_MAX_AE_LEN)
+					return -1;
 				dte_facs->calling_len = p[2];
 				memcpy(dte_facs->calling_ae, &p[3], p[1] - 1);
 				*vc_fac_mask |= X25_MASK_CALLING_AE;
@@ -163,6 +165,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 			case X25_FAC_CALLED_AE:
 				if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
 					return -1;
+				if (p[2] > X25_MAX_AE_LEN)
+					return -1;
 				dte_facs->called_len = p[2];
 				memcpy(dte_facs->called_ae, &p[3], p[1] - 1);
 				*vc_fac_mask |= X25_MASK_CALLED_AE;

^ permalink raw reply related

* [patch] caif: add a sanity check to the tty name
From: Dan Carpenter @ 2013-09-03  9:02 UTC (permalink / raw)
  To: Dmitry Tarnyagin; +Cc: netdev, kernel-janitors

"tty->name" and "name" are a 64 character buffers.  My static checker
complains because we add the "cf" on the front so it look like we are
copying a 66 character string into a 64 character buffer.

Also if the name is larger than IFNAMSIZ (16) it triggers a BUG_ON()
inside the call to alloc_netdev().

This is all under CAP_SYS_ADMIN so it's not a security fix, it just adds
a little robustness.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 34dea95..88a6a58 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -347,7 +347,9 @@ static int ldisc_open(struct tty_struct *tty)
 	/* release devices to avoid name collision */
 	ser_release(NULL);
 
-	sprintf(name, "cf%s", tty->name);
+	result = snprintf(name, sizeof(name), "cf%s", tty->name);
+	if (result >= IFNAMSIZ)
+		return -EINVAL;
 	dev = alloc_netdev(sizeof(*ser), name, caifdev_setup);
 	if (!dev)
 		return -ENOMEM;

^ permalink raw reply related

* Re: Is fallback vhost_net to qemu for live migrate available?
From: Michael S. Tsirkin @ 2013-09-03  8:55 UTC (permalink / raw)
  To: Wei Liu
  Cc: Qin Chuanyu, Anthony Liguori, jasowang, KVM list, netdev,
	qianhuibin, xen-devel@lists.xen.org, wangfuhai, likunyun,
	liuyongan, liuyingdong
In-Reply-To: <20130903084047.GF14104@zion.uk.xensource.com>

On Tue, Sep 03, 2013 at 09:40:48AM +0100, Wei Liu wrote:
> On Tue, Sep 03, 2013 at 09:28:11AM +0800, Qin Chuanyu wrote:
> > On 2013/9/2 15:57, Wei Liu wrote:
> > >On Sat, Aug 31, 2013 at 12:45:11PM +0800, Qin Chuanyu wrote:
> > >>On 2013/8/30 0:08, Anthony Liguori wrote:
> > >>>Hi Qin,
> > >>
> > >>>>By change the memory copy and notify mechanism ,currently virtio-net with
> > >>>>vhost_net could run on Xen with good performance。
> > >>>
> > >>>I think the key in doing this would be to implement a property
> > >>>ioeventfd and irqfd interface in the driver domain kernel.  Just
> > >>>hacking vhost_net with Xen specific knowledge would be pretty nasty
> > >>>IMHO.
> > >>>
> > >>Yes, I add a kernel module which persist virtio-net pio_addr and
> > >>msix address as what kvm module did. Guest wake up vhost thread by
> > >>adding a hook func in evtchn_interrupt.
> > >>
> > >>>Did you modify the front end driver to do grant table mapping or is
> > >>>this all being done by mapping the domain's memory?
> > >>>
> > >>There is nothing changed in front end driver. Currently I use
> > >>alloc_vm_area to get address space, and map the domain's memory as
> > >>what what qemu did.
> > >>
> > >
> > >You mean you're using xc_map_foreign_range and friends in the backend to
> > >map guest memory? That's not very desirable as it violates Xen's
> > >security model. It would not be too hard to pass grant references
> > >instead of guest physical memory address IMHO.
> > >
> > In fact, I did what virtio-net have done in Qemu. I think security
> > is a pseudo question because Dom0 is under control.
> > 
> 
> Consider that you might have driver domains. Not every domain is under
> control or trusted.

I don't see anything that will prevent using driver domains here.

> Also consider that security model like XSM can be
> used to audit operations to enhance security so your foreign mapping
> approach might not always work.

It could be nice to have as an option, sure.
XSM is disabled by default though so I don't think lack of support for
that makes it a prototype.

> In short term foreign mapping can save you some time implementing the
> prototype.
> In long term using grant table is the proper way to go. And
> IMHO the benifit outweights the cost.
> 
> Wei.

I'm guessing direct access could be quite a bit faster.
But someone would have to implement your idea in order to
do a cost/benefit analysis.

> > Host could access memory of guest in KVM much easier than Xen,
> > but I hadn't heard someone said KVM is un-secret.
> > 
> > Regards
> > Qin chuanyu
> > 
> > 
> > --
> > 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

* Re: Is fallback vhost_net to qemu for live migrate available?
From: Wei Liu @ 2013-09-03  8:40 UTC (permalink / raw)
  To: Qin Chuanyu
  Cc: Wei Liu, Anthony Liguori, Michael S. Tsirkin, jasowang, KVM list,
	netdev, qianhuibin, xen-devel@lists.xen.org, wangfuhai, likunyun,
	liuyongan, liuyingdong
In-Reply-To: <52253B2B.6050909@huawei.com>

On Tue, Sep 03, 2013 at 09:28:11AM +0800, Qin Chuanyu wrote:
> On 2013/9/2 15:57, Wei Liu wrote:
> >On Sat, Aug 31, 2013 at 12:45:11PM +0800, Qin Chuanyu wrote:
> >>On 2013/8/30 0:08, Anthony Liguori wrote:
> >>>Hi Qin,
> >>
> >>>>By change the memory copy and notify mechanism ,currently virtio-net with
> >>>>vhost_net could run on Xen with good performance。
> >>>
> >>>I think the key in doing this would be to implement a property
> >>>ioeventfd and irqfd interface in the driver domain kernel.  Just
> >>>hacking vhost_net with Xen specific knowledge would be pretty nasty
> >>>IMHO.
> >>>
> >>Yes, I add a kernel module which persist virtio-net pio_addr and
> >>msix address as what kvm module did. Guest wake up vhost thread by
> >>adding a hook func in evtchn_interrupt.
> >>
> >>>Did you modify the front end driver to do grant table mapping or is
> >>>this all being done by mapping the domain's memory?
> >>>
> >>There is nothing changed in front end driver. Currently I use
> >>alloc_vm_area to get address space, and map the domain's memory as
> >>what what qemu did.
> >>
> >
> >You mean you're using xc_map_foreign_range and friends in the backend to
> >map guest memory? That's not very desirable as it violates Xen's
> >security model. It would not be too hard to pass grant references
> >instead of guest physical memory address IMHO.
> >
> In fact, I did what virtio-net have done in Qemu. I think security
> is a pseudo question because Dom0 is under control.
> 

Consider that you might have driver domains. Not every domain is under
control or trusted. Also consider that security model like XSM can be
used to audit operations to enhance security so your foreign mapping
approach might not always work.

In short term foreign mapping can save you some time implementing the
prototype. In long term using grant table is the proper way to go. And
IMHO the benifit outweights the cost.

Wei.

> Host could access memory of guest in KVM much easier than Xen,
> but I hadn't heard someone said KVM is un-secret.
> 
> Regards
> Qin chuanyu
> 
> 
> --
> 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

* Re: [PATCH RFC net-next 01/21] net: add neighbour_dev_list to save only neighbours
From: Veaceslav Falico @ 2013-09-03  8:34 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, David S. Miller, Eric Dumazet, Alexander Duyck, Cong Wang
In-Reply-To: <20130903082943.GD1437@minipsycho.brq.redhat.com>

On Tue, Sep 03, 2013 at 10:29:43AM +0200, Jiri Pirko wrote:
>Mon, Sep 02, 2013 at 11:39:05PM CEST, vfalico@redhat.com wrote:
...snip...
>>-	struct list_head	upper_dev_list; /* List of upper devices */
>>-	struct list_head	lower_dev_list;
>>+
>>+	/* directly linked devices, like slaves for bonding */
>>+	struct {
>>+		struct list_head upper;
>>+		struct list_head lower;
>>+	} neighbour_dev_list ;
>>+
>>+	/* all linked devices, *including* neighbours */
>>+	struct {
>>+		struct list_head upper;
>>+		struct list_head lower;
>>+	} all_dev_list ;
>
>
>I think there is need for some naming consistency for functions and
>macros handling these lists.
>
>I propose:
>dev_list (drop the "neighbour") and all_dev_list

Agreed.

>
>and exported functions and macros called like:
>netdev_lower_get_next_priv()
>netdev_lower_for_each_priv()
>netdev_all_upper_get_next_rcu()
>etc...

Yep, will try to convert and see how it goes.

Thanks a lot!

^ permalink raw reply

* Re: [PATCH RFC net-next 01/21] net: add neighbour_dev_list to save only neighbours
From: Jiri Pirko @ 2013-09-03  8:29 UTC (permalink / raw)
  To: Veaceslav Falico
  Cc: netdev, David S. Miller, Eric Dumazet, Alexander Duyck, Cong Wang
In-Reply-To: <1378157965-17537-2-git-send-email-vfalico@redhat.com>

Mon, Sep 02, 2013 at 11:39:05PM CEST, vfalico@redhat.com wrote:
>Currently, we distinguish neighbours (first-level linked devices) from
>non-neighbours by the neighbour bool in the netdev_adjacent. This could be
>quite time-consuming in case we would like to traverse *only* through
>neighbours - cause we'd have to traverse through all devices and check for
>this flag, and in a (quite common) scenario where we have lots of vlans on
>top of bridge, which is on top of a bond - the bonding would have to go
>through all those vlans to get its upper neighbour linked devices.
>
>This situation is really unpleasant, cause there are already a lot of cases
>when a device with slaves needs to go through them in hot path.
>
>To fix this, introduce a new upper/lower device lists structure -
>neighbour_dev_list, which contains only the neighbours. It works always in
>pair with the all_device_list structure (renamed from upper/lower_dev_list),
>i.e. both of them contain the same links, only that all_dev_list contains
>also non-neighbour device links. It's really a small change visible,
>currently, only for __netdev_adjacent_dev_insert/remove(), and doesn't
>change the main linked logic at all.
>
>Also, add some comments a fix a name collision in
>netdev_for_each_upper_dev_rcu().
>
>CC: "David S. Miller" <davem@davemloft.net>
>CC: Eric Dumazet <edumazet@google.com>
>CC: Jiri Pirko <jiri@resnulli.us>
>CC: Alexander Duyck <alexander.h.duyck@intel.com>
>CC: Cong Wang <amwang@redhat.com>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>---
> include/linux/netdevice.h |  24 ++++---
> net/core/dev.c            | 157 ++++++++++++++++++++++++++++++++++------------
> 2 files changed, 134 insertions(+), 47 deletions(-)
>
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index 3ad49b8..df50548 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -1124,8 +1124,18 @@ struct net_device {
> 	struct list_head	dev_list;
> 	struct list_head	napi_list;
> 	struct list_head	unreg_list;
>-	struct list_head	upper_dev_list; /* List of upper devices */
>-	struct list_head	lower_dev_list;
>+
>+	/* directly linked devices, like slaves for bonding */
>+	struct {
>+		struct list_head upper;
>+		struct list_head lower;
>+	} neighbour_dev_list ;
>+
>+	/* all linked devices, *including* neighbours */
>+	struct {
>+		struct list_head upper;
>+		struct list_head lower;
>+	} all_dev_list ;


I think there is need for some naming consistency for functions and
macros handling these lists.

I propose:
dev_list (drop the "neighbour") and all_dev_list

and exported functions and macros called like:
netdev_lower_get_next_priv()
netdev_lower_for_each_priv()
netdev_all_upper_get_next_rcu()
etc...

> 
> 
> 	/* currently active device features */
>@@ -2772,11 +2782,11 @@ extern struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
> 							struct list_head **iter);
> 
> /* iterate through upper list, must be called under RCU read lock */
>-#define netdev_for_each_upper_dev_rcu(dev, upper, iter) \
>-	for (iter = &(dev)->upper_dev_list, \
>-	     upper = netdev_upper_get_next_dev_rcu(dev, &(iter)); \
>-	     upper; \
>-	     upper = netdev_upper_get_next_dev_rcu(dev, &(iter)))
>+#define netdev_for_each_upper_dev_rcu(dev, updev, iter) \
>+	for (iter = &(dev)->all_dev_list.upper, \
>+	     updev= netdev_upper_get_next_dev_rcu(dev, &(iter)); \
>+	     updev; \
>+	     updev = netdev_upper_get_next_dev_rcu(dev, &(iter)))
> 
> extern struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
> extern struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev);
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 743620e..9e4eb40 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -4373,9 +4373,6 @@ struct netdev_adjacent {
> 	/* upper master flag, there can only be one master device per list */
> 	bool master;
> 
>-	/* indicates that this dev is our first-level lower/upper device */
>-	bool neighbour;
>-
> 	/* counter for the number of times this device was added to us */
> 	u16 ref_nr;
> 
>@@ -4385,12 +4382,17 @@ struct netdev_adjacent {
> 
> static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
> 						 struct net_device *adj_dev,
>-						 bool upper)
>+						 bool upper, bool neighbour)
> {
> 	struct netdev_adjacent *adj;
> 	struct list_head *dev_list;
> 
>-	dev_list = upper ? &dev->upper_dev_list : &dev->lower_dev_list;
>+	if (neighbour)
>+		dev_list = upper ? &dev->neighbour_dev_list.upper :
>+				   &dev->neighbour_dev_list.lower;
>+	else
>+		dev_list = upper ? &dev->all_dev_list.upper :
>+				   &dev->all_dev_list.lower;
> 
> 	list_for_each_entry(adj, dev_list, list) {
> 		if (adj->dev == adj_dev)
>@@ -4402,13 +4404,25 @@ static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
> static inline struct netdev_adjacent *__netdev_find_upper(struct net_device *dev,
> 							  struct net_device *udev)
> {
>-	return __netdev_find_adj(dev, udev, true);
>+	return __netdev_find_adj(dev, udev, true, false);
> }
> 
> static inline struct netdev_adjacent *__netdev_find_lower(struct net_device *dev,
> 							  struct net_device *ldev)
> {
>-	return __netdev_find_adj(dev, ldev, false);
>+	return __netdev_find_adj(dev, ldev, false, false);
>+}
>+
>+static inline struct netdev_adjacent *__netdev_find_upper_neighbour(struct net_device *dev,
>+								    struct net_device *udev)
>+{
>+	return __netdev_find_adj(dev, udev, true, true);
>+}
>+
>+static inline struct netdev_adjacent *__netdev_find_lower_neighbour(struct net_device *dev,
>+								    struct net_device *ldev)
>+{
>+	return __netdev_find_adj(dev, ldev, false, true);
> }
> 
> /**
>@@ -4440,7 +4454,7 @@ bool netdev_has_any_upper_dev(struct net_device *dev)
> {
> 	ASSERT_RTNL();
> 
>-	return !list_empty(&dev->upper_dev_list);
>+	return !list_empty(&dev->all_dev_list.upper);
> }
> EXPORT_SYMBOL(netdev_has_any_upper_dev);
> 
>@@ -4457,10 +4471,10 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
> 
> 	ASSERT_RTNL();
> 
>-	if (list_empty(&dev->upper_dev_list))
>+	if (list_empty(&dev->neighbour_dev_list.upper))
> 		return NULL;
> 
>-	upper = list_first_entry(&dev->upper_dev_list,
>+	upper = list_first_entry(&dev->neighbour_dev_list.upper,
> 				 struct netdev_adjacent, list);
> 	if (likely(upper->master))
> 		return upper->dev;
>@@ -4484,7 +4498,7 @@ struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
> 
> 	upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
> 
>-	if (&upper->list == &dev->upper_dev_list)
>+	if (&upper->list == &dev->all_dev_list.upper)
> 		return NULL;
> 
> 	*iter = &upper->list;
>@@ -4504,7 +4518,7 @@ struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev)
> {
> 	struct netdev_adjacent *upper;
> 
>-	upper = list_first_or_null_rcu(&dev->upper_dev_list,
>+	upper = list_first_or_null_rcu(&dev->neighbour_dev_list.upper,
> 				       struct netdev_adjacent, list);
> 	if (upper && likely(upper->master))
> 		return upper->dev;
>@@ -4517,11 +4531,12 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
> 					bool neighbour, bool master,
> 					bool upper)
> {
>-	struct netdev_adjacent *adj;
>+	struct netdev_adjacent *adj, *neigh = NULL;
> 
>-	adj = __netdev_find_adj(dev, adj_dev, upper);
>+	adj = __netdev_find_adj(dev, adj_dev, upper, false);
> 
> 	if (adj) {
>+		/* we cannot insert a neighbour device twice */
> 		BUG_ON(neighbour);
> 		adj->ref_nr++;
> 		return 0;
>@@ -4533,24 +4548,50 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
> 
> 	adj->dev = adj_dev;
> 	adj->master = master;
>-	adj->neighbour = neighbour;
> 	adj->ref_nr = 1;
>-
> 	dev_hold(adj_dev);
>+
>+	if (neighbour) {
>+		neigh = kmalloc(sizeof(*neigh), GFP_KERNEL);
>+		if (!neigh) {
>+			kfree(adj);
>+			return -ENOMEM;
>+		}
>+		neigh->dev = adj_dev;
>+		neigh->master = master;
>+		neigh->ref_nr = 1;
>+		dev_hold(adj_dev);
>+	}
>+
> 	pr_debug("dev_hold for %s, because of %s link added from %s to %s\n",
> 		 adj_dev->name, upper ? "upper" : "lower", dev->name,
> 		 adj_dev->name);
>+	if (neigh)
>+		pr_debug("dev_hold for %s, because of %s link added from %s"
>+			 " to %s (neighbour)\n",
>+			 adj_dev->name, upper ? "upper" : "lower", dev->name,
>+			 adj_dev->name);
> 
> 	if (!upper) {
>-		list_add_tail_rcu(&adj->list, &dev->lower_dev_list);
>+		if (neigh)
>+			list_add_tail_rcu(&neigh->list,
>+					  &dev->neighbour_dev_list.lower);
>+		list_add_tail_rcu(&adj->list, &dev->all_dev_list.lower);
> 		return 0;
> 	}
> 
> 	/* Ensure that master upper link is always the first item in list. */
>-	if (master)
>-		list_add_rcu(&adj->list, &dev->upper_dev_list);
>-	else
>-		list_add_tail_rcu(&adj->list, &dev->upper_dev_list);
>+	if (master) {
>+		if (neigh)
>+			list_add_rcu(&neigh->list,
>+				     &dev->neighbour_dev_list.upper);
>+		list_add_rcu(&adj->list, &dev->all_dev_list.upper);
>+	} else {
>+		if (neigh)
>+			list_add_tail_rcu(&neigh->list,
>+					  &dev->neighbour_dev_list.upper);
>+		list_add_tail_rcu(&adj->list, &dev->all_dev_list.upper);
>+	}
> 
> 	return 0;
> }
>@@ -4574,17 +4615,36 @@ static inline int __netdev_lower_dev_insert(struct net_device *dev,
> void __netdev_adjacent_dev_remove(struct net_device *dev,
> 				  struct net_device *adj_dev, bool upper)
> {
>-	struct netdev_adjacent *adj;
>+	struct netdev_adjacent *adj, *neighbour;
> 
>-	if (upper)
>+	if (upper) {
> 		adj = __netdev_find_upper(dev, adj_dev);
>-	else
>+		neighbour = __netdev_find_upper_neighbour(dev, adj_dev);
>+	} else {
> 		adj = __netdev_find_lower(dev, adj_dev);
>+		neighbour = __netdev_find_lower_neighbour(dev, adj_dev);
>+	}
> 
>-	if (!adj)
>+	if (!adj) {
>+		pr_err("tried to remove %s device %s from %s\n",
>+		       upper ? "upper" : "lower", dev->name, adj_dev->name);
> 		BUG();
>+	}
> 
> 	if (adj->ref_nr > 1) {
>+		pr_debug("rec_cnt-- for link to %s, because of %s link removed"
>+			 " from %s to %s, remains %d\n",
>+			 adj_dev->name, upper ? "upper" : "lower", dev->name,
>+			 adj_dev->name, adj->ref_nr-1);
>+		if (neighbour) {
>+			pr_debug("rec_cnt-- for link to %s, because of %s link"
>+				 " removed from %s to %s, remain %d (neigh)\n",
>+				 adj_dev->name, upper ? "upper" : "lower",
>+				 dev->name, adj_dev->name,
>+				 neighbour->ref_nr-1);
>+			BUG_ON(adj->ref_nr != neighbour->ref_nr);
>+			neighbour->ref_nr--;
>+		}
> 		adj->ref_nr--;
> 		return;
> 	}
>@@ -4595,6 +4655,15 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
> 		 adj_dev->name);
> 	dev_put(adj_dev);
> 	kfree_rcu(adj, rcu);
>+	if (neighbour) {
>+		pr_debug("dev_put for %s, because of %s link removed from %s"
>+			 " to %s (neighbour)\n",
>+			 adj_dev->name, upper ? "upper" : "lower", dev->name,
>+			 adj_dev->name);
>+		list_del_rcu(&neighbour->list);
>+		dev_put(adj_dev);
>+		kfree_rcu(neighbour, rcu);
>+	}
> }
> 
> static inline void __netdev_upper_dev_remove(struct net_device *dev,
>@@ -4675,12 +4744,14 @@ static int __netdev_upper_dev_link(struct net_device *dev,
> 		return ret;
> 
> 	/* Now that we linked these devs, make all the upper_dev's
>-	 * upper_dev_list visible to every dev's lower_dev_list and vice
>+	 * all_dev_list.upper visible to every dev's all_dev_list.lower an
> 	 * versa, and don't forget the devices itself. All of these
> 	 * links are non-neighbours.
> 	 */
>-	list_for_each_entry(i, &dev->lower_dev_list, list) {
>-		list_for_each_entry(j, &upper_dev->upper_dev_list, list) {
>+	list_for_each_entry(i, &dev->all_dev_list.lower, list) {
>+		list_for_each_entry(j, &upper_dev->all_dev_list.upper, list) {
>+			pr_debug("Interlinking %s with %s, non-neighbour\n",
>+				 i->dev->name, j->dev->name);
> 			ret = __netdev_adjacent_dev_link(i->dev, j->dev);
> 			if (ret)
> 				goto rollback_mesh;
>@@ -4688,14 +4759,18 @@ static int __netdev_upper_dev_link(struct net_device *dev,
> 	}
> 
> 	/* add dev to every upper_dev's upper device */
>-	list_for_each_entry(i, &upper_dev->upper_dev_list, list) {
>+	list_for_each_entry(i, &upper_dev->all_dev_list.upper, list) {
>+		pr_debug("linking %s's upper device %s with %s\n", upper_dev->name,
>+			 i->dev->name, dev->name);
> 		ret = __netdev_adjacent_dev_link(dev, i->dev);
> 		if (ret)
> 			goto rollback_upper_mesh;
> 	}
> 
> 	/* add upper_dev to every dev's lower device */
>-	list_for_each_entry(i, &dev->lower_dev_list, list) {
>+	list_for_each_entry(i, &dev->all_dev_list.lower, list) {
>+		pr_debug("linking %s's lower device %s with %s\n", dev->name,
>+			 i->dev->name, upper_dev->name);
> 		ret = __netdev_adjacent_dev_link(i->dev, upper_dev);
> 		if (ret)
> 			goto rollback_lower_mesh;
>@@ -4706,7 +4781,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
> 
> rollback_lower_mesh:
> 	to_i = i;
>-	list_for_each_entry(i, &dev->lower_dev_list, list) {
>+	list_for_each_entry(i, &dev->all_dev_list.lower, list) {
> 		if (i == to_i)
> 			break;
> 		__netdev_adjacent_dev_unlink(i->dev, upper_dev);
>@@ -4716,7 +4791,7 @@ rollback_lower_mesh:
> 
> rollback_upper_mesh:
> 	to_i = i;
>-	list_for_each_entry(i, &upper_dev->upper_dev_list, list) {
>+	list_for_each_entry(i, &upper_dev->all_dev_list.upper, list) {
> 		if (i == to_i)
> 			break;
> 		__netdev_adjacent_dev_unlink(dev, i->dev);
>@@ -4727,8 +4802,8 @@ rollback_upper_mesh:
> rollback_mesh:
> 	to_i = i;
> 	to_j = j;
>-	list_for_each_entry(i, &dev->lower_dev_list, list) {
>-		list_for_each_entry(j, &upper_dev->upper_dev_list, list) {
>+	list_for_each_entry(i, &dev->all_dev_list.lower, list) {
>+		list_for_each_entry(j, &upper_dev->all_dev_list.upper, list) {
> 			if (i == to_i && j == to_j)
> 				break;
> 			__netdev_adjacent_dev_unlink(i->dev, j->dev);
>@@ -4797,17 +4872,17 @@ void netdev_upper_dev_unlink(struct net_device *dev,
> 	 * devices from all upper_dev's upper devices and vice
> 	 * versa, to maintain the graph relationship.
> 	 */
>-	list_for_each_entry(i, &dev->lower_dev_list, list)
>-		list_for_each_entry(j, &upper_dev->upper_dev_list, list)
>+	list_for_each_entry(i, &dev->all_dev_list.lower, list)
>+		list_for_each_entry(j, &upper_dev->all_dev_list.upper, list)
> 			__netdev_adjacent_dev_unlink(i->dev, j->dev);
> 
> 	/* remove also the devices itself from lower/upper device
> 	 * list
> 	 */
>-	list_for_each_entry(i, &dev->lower_dev_list, list)
>+	list_for_each_entry(i, &dev->all_dev_list.lower, list)
> 		__netdev_adjacent_dev_unlink(i->dev, upper_dev);
> 
>-	list_for_each_entry(i, &upper_dev->upper_dev_list, list)
>+	list_for_each_entry(i, &upper_dev->all_dev_list.upper, list)
> 		__netdev_adjacent_dev_unlink(dev, i->dev);
> 
> 	call_netdevice_notifiers(NETDEV_CHANGEUPPER, dev);
>@@ -6069,8 +6144,10 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> 	INIT_LIST_HEAD(&dev->napi_list);
> 	INIT_LIST_HEAD(&dev->unreg_list);
> 	INIT_LIST_HEAD(&dev->link_watch_list);
>-	INIT_LIST_HEAD(&dev->upper_dev_list);
>-	INIT_LIST_HEAD(&dev->lower_dev_list);
>+	INIT_LIST_HEAD(&dev->neighbour_dev_list.upper);
>+	INIT_LIST_HEAD(&dev->neighbour_dev_list.lower);
>+	INIT_LIST_HEAD(&dev->all_dev_list.upper);
>+	INIT_LIST_HEAD(&dev->all_dev_list.lower);
> 	dev->priv_flags = IFF_XMIT_DST_RELEASE;
> 	setup(dev);
> 
>-- 
>1.8.4
>

^ permalink raw reply

* Re: [PATCH RFC net-next 18/21] net: add a function to get the next/prev private
From: Jiri Pirko @ 2013-09-03  8:10 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: netdev, David S. Miller, Eric Dumazet, Alexander Duyck
In-Reply-To: <1378157965-17537-19-git-send-email-vfalico@redhat.com>

Mon, Sep 02, 2013 at 11:39:22PM CEST, vfalico@redhat.com wrote:
>The boolean flag specifies which direction to go.
>
>CC: "David S. Miller" <davem@davemloft.net>
>CC: Eric Dumazet <edumazet@google.com>
>CC: Jiri Pirko <jiri@resnulli.us>
>CC: Alexander Duyck <alexander.h.duyck@intel.com>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>---
> include/linux/netdevice.h |  2 ++
> net/core/dev.c            | 24 ++++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index bde2244..a112ccc 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -2835,6 +2835,8 @@ extern void *netdev_lower_dev_get_private_rcu(struct net_device *dev,
> 					      struct net_device *lower_dev);
> extern void *netdev_lower_dev_get_private(struct net_device *dev,
> 					  struct net_device *lower_dev);
>+extern void *netdev_lower_dev_get_next_private(struct net_device *dev,
>+					       void *private, bool prev);
> extern int skb_checksum_help(struct sk_buff *skb);
> extern struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
> 	netdev_features_t features, bool tx_path);
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 6df11a0..c694059 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -5163,6 +5163,30 @@ void *netdev_lower_dev_get_private(struct net_device *dev,
> }
> EXPORT_SYMBOL(netdev_lower_dev_get_private);
> 
>+extern void *netdev_lower_dev_get_next_private(struct net_device *dev,
>+					       void *private, bool prev)

                                                             ^^^^^^^^^^^
				Please rather do this in 2 functions
				Next should return next, not previous...

>+{
>+	struct netdev_adjacent *lower;
>+
>+	list_for_each_entry(lower, &dev->neighbour_dev_list.lower, list)
>+		if (lower->private == private)
>+			break;
>+
>+	if (&lower->list == &dev->neighbour_dev_list.lower)
>+		return NULL;
>+
>+	lower = prev ? list_entry(lower->list.prev, struct netdev_adjacent, list) :
>+		       list_entry(lower->list.next, struct netdev_adjacent, list);
>+	if (&lower->list != &dev->neighbour_dev_list.lower)
>+		return lower->private;
>+
>+	lower = prev ? list_entry(lower->list.prev, struct netdev_adjacent, list) :
>+		       list_entry(lower->list.next, struct netdev_adjacent, list);
>+
>+	return lower->private;
>+}
>+EXPORT_SYMBOL(netdev_lower_dev_get_next_private);
>+
> static void dev_change_rx_flags(struct net_device *dev, int flags)
> {
> 	const struct net_device_ops *ops = dev->netdev_ops;
>-- 
>1.8.4
>

^ permalink raw reply

* Re: [PATCH RFC net-next 06/21] net: create sysfs symlinks for neighbour devices
From: Veaceslav Falico @ 2013-09-03  8:05 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller,
	Eric Dumazet, Alexander Duyck
In-Reply-To: <20130903074853.GB1437@minipsycho.brq.redhat.com>

On Tue, Sep 03, 2013 at 09:48:53AM +0200, Jiri Pirko wrote:
>Mon, Sep 02, 2013 at 11:39:10PM CEST, vfalico@redhat.com wrote:
>>Also, remove the same functionality from bonding - it will be already done
>>for any device that links to its lower/upper neighbour.
>>
>>The links will be created for dev's kobject, and will look like
>>slave_eth0 for lower device eth0 and upper_bridge0 for upper device
>>bridge0.
>
>The pair should be either slave/master or lower/upper. It's undesirable
>to mix these two. Maybe for compatibility reasons we should leave
>current bonding sysfs files and create proper new generic ones?

I completely agree with the slave/master vs lower/upper logic, and you've
correctly understood why I've chosen this path.

I'm ok with any approach, but it looks weird. I've googled a bit and saw
that people use the slave_eth0...

So dunno what's best option here, really.

>
>I think there should be directories under netdev dir like:
>/sys/class/net/em1-
>                  |-upper-
>                         |-bond0(link)
>                         |-vlanx(link)
>/sys/class/net/bond0-
>                    |-lower-
>                           |-em1(link)
>                           |-em2(link)
>
>
>>
>>CC: Jay Vosburgh <fubar@us.ibm.com>
>>CC: Andy Gospodarek <andy@greyhouse.net>
>>CC: "David S. Miller" <davem@davemloft.net>
>>CC: Eric Dumazet <edumazet@google.com>
>>CC: Jiri Pirko <jiri@resnulli.us>
>>CC: Alexander Duyck <alexander.h.duyck@intel.com>
>>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>>---
>> drivers/net/bonding/bond_main.c  | 11 +----------
>> drivers/net/bonding/bond_sysfs.c | 25 -------------------------
>> drivers/net/bonding/bonding.h    |  2 --
>> net/core/dev.c                   | 29 +++++++++++++++++++++++++++++
>> 4 files changed, 30 insertions(+), 37 deletions(-)
>>
>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>index c50679f..b0b753d 100644
>>--- a/drivers/net/bonding/bond_main.c
>>+++ b/drivers/net/bonding/bond_main.c
>>@@ -1635,15 +1635,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>>
>> 	read_unlock(&bond->lock);
>>
>>-	res = bond_create_slave_symlinks(bond_dev, slave_dev);
>>-	if (res)
>>-		goto err_detach;
>>-
>> 	res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
>> 					 new_slave);
>> 	if (res) {
>> 		pr_debug("Error %d calling netdev_rx_handler_register\n", res);
>>-		goto err_dest_symlinks;
>>+		goto err_detach;
>> 	}
>>
>> 	pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
>>@@ -1655,9 +1651,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>> 	return 0;
>>
>> /* Undo stages on error */
>>-err_dest_symlinks:
>>-	bond_destroy_slave_symlinks(bond_dev, slave_dev);
>>-
>> err_detach:
>> 	if (!USES_PRIMARY(bond->params.mode))
>> 		bond_hw_addr_flush(bond_dev, slave_dev);
>>@@ -1856,8 +1849,6 @@ static int __bond_release_one(struct net_device *bond_dev,
>> 			bond_dev->name, slave_dev->name, bond_dev->name);
>>
>> 	/* must do this from outside any spinlocks */
>>-	bond_destroy_slave_symlinks(bond_dev, slave_dev);
>>-
>> 	vlan_vids_del_by_dev(slave_dev, bond_dev);
>>
>> 	/* If the mode USES_PRIMARY, then this cases was handled above by
>>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>index 5715277..7b962a2 100644
>>--- a/drivers/net/bonding/bond_sysfs.c
>>+++ b/drivers/net/bonding/bond_sysfs.c
>>@@ -168,31 +168,6 @@ static const struct class_attribute class_attr_bonding_masters = {
>> 	.namespace = bonding_namespace,
>> };
>>
>>-int bond_create_slave_symlinks(struct net_device *master,
>>-			       struct net_device *slave)
>>-{
>>-	char linkname[IFNAMSIZ+7];
>>-	int ret = 0;
>>-
>>-	/* create a link from the master to the slave */
>>-	sprintf(linkname, "slave_%s", slave->name);
>>-	ret = sysfs_create_link(&(master->dev.kobj), &(slave->dev.kobj),
>>-				linkname);
>>-
>>-	return ret;
>>-
>>-}
>>-
>>-void bond_destroy_slave_symlinks(struct net_device *master,
>>-				 struct net_device *slave)
>>-{
>>-	char linkname[IFNAMSIZ+7];
>>-
>>-	sprintf(linkname, "slave_%s", slave->name);
>>-	sysfs_remove_link(&(master->dev.kobj), linkname);
>>-}
>>-
>>-
>> /*
>>  * Show the slaves in the current bond.
>>  */
>>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>>index 4abc925..f7a167d 100644
>>--- a/drivers/net/bonding/bonding.h
>>+++ b/drivers/net/bonding/bonding.h
>>@@ -455,8 +455,6 @@ int bond_create(struct net *net, const char *name);
>> int bond_create_sysfs(struct bond_net *net);
>> void bond_destroy_sysfs(struct bond_net *net);
>> void bond_prepare_sysfs_group(struct bonding *bond);
>>-int bond_create_slave_symlinks(struct net_device *master, struct net_device *slave);
>>-void bond_destroy_slave_symlinks(struct net_device *master, struct net_device *slave);
>> int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev);
>> int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
>> void bond_mii_monitor(struct work_struct *);
>>diff --git a/net/core/dev.c b/net/core/dev.c
>>index 7eecf35..1c3e98d 100644
>>--- a/net/core/dev.c
>>+++ b/net/core/dev.c
>>@@ -4580,6 +4580,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
>> 					bool upper, void *private)
>> {
>> 	struct netdev_adjacent *adj, *neigh = NULL;
>>+	char linkname[IFNAMSIZ+7];
>> 	int ret;
>>
>> 	adj = __netdev_find_adj(dev, adj_dev, upper, false);
>>@@ -4628,6 +4629,16 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
>> 			 */
>> 			if (master)
>> 				neigh->private = private;
>>+
>>+			sprintf(linkname, "slave_%s", adj_dev->name);
>>+			ret = sysfs_create_link(&(dev->dev.kobj),
>>+						&(adj_dev->dev.kobj),
>>+						linkname);
>>+			if (ret) {
>>+				kfree(neigh);
>>+				kfree(adj);
>>+				return ret;
>>+			}
>> 			list_add_tail_rcu(&neigh->list,
>> 					  &dev->neighbour_dev_list.lower);
>> 		}
>>@@ -4635,10 +4646,24 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
>> 		return 0;
>> 	}
>>
>>+	/* it's upper neighbour, we don't care if it's master or not */
>>+	if (neigh) {
>>+		sprintf(linkname, "upper_%s", adj_dev->name);
>>+		ret = sysfs_create_link(&(dev->dev.kobj), &(adj_dev->dev.kobj),
>>+					linkname);
>>+		if (ret) {
>>+			kfree(neigh);
>>+			kfree(adj);
>>+			return ret;
>>+		}
>>+	}
>>+
>> 	/* Ensure that master upper link is always the first item in list. */
>> 	if (master) {
>> 		ret = sysfs_create_link(&(dev->dev.kobj), &(adj_dev->dev.kobj), "master");
>> 		if (ret) {
>>+			if (neigh)
>>+				sysfs_remove_link(&(dev->dev.kobj), linkname);
>> 			kfree(neigh);
>> 			kfree(adj);
>> 			return ret;
>>@@ -4678,6 +4703,7 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
>> 				  struct net_device *adj_dev, bool upper)
>> {
>> 	struct netdev_adjacent *adj, *neighbour;
>>+	char linkname[IFNAMSIZ+7];
>>
>> 	if (upper) {
>> 		adj = __netdev_find_upper(dev, adj_dev);
>>@@ -4725,6 +4751,9 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
>> 		list_del_rcu(&neighbour->list);
>> 		if (neighbour->master && upper)
>> 			sysfs_remove_link(&(dev->dev.kobj), "master");
>>+		sprintf(linkname, "%s_%s", upper ? "upper" : "slave",
>>+			adj_dev->name);
>>+		sysfs_remove_link(&(dev->dev.kobj), linkname);
>> 		dev_put(adj_dev);
>> 		kfree_rcu(neighbour, rcu);
>> 	}
>>--
>>1.8.4
>>

^ permalink raw reply

* Re: [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
From: Michele Baldessari @ 2013-09-03  8:04 UTC (permalink / raw)
  To: netdev, Hideaki YOSHIFUJI, David S. Miller, Sasha Levin
In-Reply-To: <20130903001331.GA8820@order.stressinduktion.org>

Hi Hannes,

On Tue, Sep 03, 2013 at 02:13:31AM +0200, Hannes Frederic Sowa wrote:
> On Mon, Sep 02, 2013 at 10:31:28PM +0100, Michele Baldessari wrote:
> > with the latest linux master git tree from Linus
> > (248d296d6d9df384996c2ed95676b367d876d48c - 2 Sep) I can reproduceably oops 
> > the kernel with the following commands:
> > ip addrlabel flush
> > ip addrlabel add prefix ::1/128              label 0
> > ip addrlabel add prefix ::/0                 label 1
> 
> Thanks for the report! This patch should fix this issue:
> 
> [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
> 
> Commit b67bfe0d42cac56c512dd5da4b1b347a23f4b70a ("hlist: drop
> the node parameter from iterators") changed the behavior of
> hlist_for_each_entry_safe to leave the p argument NULL.
> 
> Fix this up by tracking the last argument.
> 
> Reported-by: Michele Baldessari <michele@acksyn.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks for the patch, fixes it for me. 

Tested-by: Michele Baldessari <michele@acksyn.org>

^ permalink raw reply

* [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>

Document force_mld_version parameter in ip-sysctl.txt.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 Documentation/networking/ip-sysctl.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 1cb3aeb..a46d785 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1358,6 +1358,11 @@ mldv2_unsolicited_report_interval - INTEGER
 	MLDv2 report retransmit will take place.
 	Default: 1000 (1 second)
 
+force_mld_version - INTEGER
+	0 - (default) No enforcement of a MLD version, MLDv1 fallback allowed
+	1 - Enforce to use MLD version 1
+	2 - Enforce to use MLD version 2
+
 suppress_frag_ndisc - INTEGER
 	Control RFC 6980 (Security Implications of IPv6 Fragmentation
 	with IPv6 Neighbor Discovery) behavior:
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>

We already have mld_{gq,ifc,dad}_start_timer() functions, so introduce
mld_{gq,ifc,dad}_stop_timer() functions to reduce code size and make it
more readable.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index b01aa32..a43028d 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1006,6 +1006,13 @@ static void mld_gq_start_timer(struct inet6_dev *idev)
 		in6_dev_hold(idev);
 }
 
+static void mld_gq_stop_timer(struct inet6_dev *idev)
+{
+	idev->mc_gq_running = 0;
+	if (del_timer(&idev->mc_gq_timer))
+		__in6_dev_put(idev);
+}
+
 static void mld_ifc_start_timer(struct inet6_dev *idev, unsigned long delay)
 {
 	unsigned long tv = net_random() % delay;
@@ -1014,6 +1021,13 @@ static void mld_ifc_start_timer(struct inet6_dev *idev, unsigned long delay)
 		in6_dev_hold(idev);
 }
 
+static void mld_ifc_stop_timer(struct inet6_dev *idev)
+{
+	idev->mc_ifc_count = 0;
+	if (del_timer(&idev->mc_ifc_timer))
+		__in6_dev_put(idev);
+}
+
 static void mld_dad_start_timer(struct inet6_dev *idev, unsigned long delay)
 {
 	unsigned long tv = net_random() % delay;
@@ -1022,6 +1036,12 @@ static void mld_dad_start_timer(struct inet6_dev *idev, unsigned long delay)
 		in6_dev_hold(idev);
 }
 
+static void mld_dad_stop_timer(struct inet6_dev *idev)
+{
+	if (del_timer(&idev->mc_dad_timer))
+		__in6_dev_put(idev);
+}
+
 /*
  *	IGMP handling (alias multicast ICMPv6 messages)
  */
@@ -1223,15 +1243,9 @@ static int mld_process_v1(struct inet6_dev *idev, struct mld_msg *mld,
 	mld_set_v1_mode(idev);
 
 	/* cancel MLDv2 report timer */
-	idev->mc_gq_running = 0;
-	if (del_timer(&idev->mc_gq_timer))
-		__in6_dev_put(idev);
-
+	mld_gq_stop_timer(idev);
 	/* cancel the interface change timer */
-	idev->mc_ifc_count = 0;
-	if (del_timer(&idev->mc_ifc_timer))
-		__in6_dev_put(idev);
-
+	mld_ifc_stop_timer(idev);
 	/* clear deleted report items */
 	mld_clear_delrec(idev);
 
@@ -2423,14 +2437,9 @@ void ipv6_mc_down(struct inet6_dev *idev)
 	/* Withdraw multicast list */
 
 	read_lock_bh(&idev->lock);
-	idev->mc_ifc_count = 0;
-	if (del_timer(&idev->mc_ifc_timer))
-		__in6_dev_put(idev);
-	idev->mc_gq_running = 0;
-	if (del_timer(&idev->mc_gq_timer))
-		__in6_dev_put(idev);
-	if (del_timer(&idev->mc_dad_timer))
-		__in6_dev_put(idev);
+	mld_ifc_stop_timer(idev);
+	mld_gq_stop_timer(idev);
+	mld_dad_stop_timer(idev);
 
 	for (i = idev->mc_list; i; i=i->next)
 		igmp6_group_dropped(i);
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>

Make igmp6_event_query() a bit easier to read by refactoring code
parts into mld_process_v1() and mld_process_v2().

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 89 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 56 insertions(+), 33 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 75a4324..b01aa32 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1207,6 +1207,55 @@ static void mld_update_qri(struct inet6_dev *idev,
 	idev->mc_qri = msecs_to_jiffies(mldv2_mrc(mlh2));
 }
 
+static int mld_process_v1(struct inet6_dev *idev, struct mld_msg *mld,
+			  unsigned long *max_delay)
+{
+	unsigned long mldv1_md;
+
+	/* Ignore v1 queries */
+	if (mld_in_v2_mode_only(idev))
+		return -EINVAL;
+
+	/* MLDv1 router present */
+	mldv1_md = ntohs(mld->mld_maxdelay);
+	*max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
+
+	mld_set_v1_mode(idev);
+
+	/* cancel MLDv2 report timer */
+	idev->mc_gq_running = 0;
+	if (del_timer(&idev->mc_gq_timer))
+		__in6_dev_put(idev);
+
+	/* cancel the interface change timer */
+	idev->mc_ifc_count = 0;
+	if (del_timer(&idev->mc_ifc_timer))
+		__in6_dev_put(idev);
+
+	/* clear deleted report items */
+	mld_clear_delrec(idev);
+
+	return 0;
+}
+
+static int mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
+			  unsigned long *max_delay)
+{
+	/* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
+	if (mld_in_v1_mode(idev))
+		return -EINVAL;
+
+	*max_delay = max(msecs_to_jiffies(mldv2_mrc(mld)), 1UL);
+
+	mld_update_qrv(idev, mld);
+	mld_update_qi(idev, mld);
+	mld_update_qri(idev, mld);
+
+	idev->mc_maxdelay = *max_delay;
+
+	return 0;
+}
+
 /* called with rcu_read_lock() */
 int igmp6_event_query(struct sk_buff *skb)
 {
@@ -1218,7 +1267,7 @@ int igmp6_event_query(struct sk_buff *skb)
 	struct mld_msg *mld;
 	int group_type;
 	int mark = 0;
-	int len;
+	int len, err;
 
 	if (!pskb_may_pull(skb, sizeof(struct in6_addr)))
 		return -EINVAL;
@@ -1244,47 +1293,21 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	if (len == MLD_V1_QUERY_LEN) {
-		unsigned long mldv1_md;
-
-		/* Ignore v1 queries */
-		if (mld_in_v2_mode_only(idev))
-			return 0;
-
-		/* MLDv1 router present */
-		mldv1_md = ntohs(mld->mld_maxdelay);
-		max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
-
-		mld_set_v1_mode(idev);
-
-		/* cancel MLDv2 report timer */
-		idev->mc_gq_running = 0;
-		if (del_timer(&idev->mc_gq_timer))
-			__in6_dev_put(idev);
-
-		/* cancel the interface change timer */
-		idev->mc_ifc_count = 0;
-		if (del_timer(&idev->mc_ifc_timer))
-			__in6_dev_put(idev);
-		/* clear deleted report items */
-		mld_clear_delrec(idev);
+		err = mld_process_v1(idev, mld, &max_delay);
+		if (err < 0)
+			return err;
 	} else if (len >= MLD_V2_QUERY_LEN_MIN) {
 		int srcs_offset = sizeof(struct mld2_query) -
 				  sizeof(struct icmp6hdr);
 
-		/* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
-		if (mld_in_v1_mode(idev))
-			return 0;
 		if (!pskb_may_pull(skb, srcs_offset))
 			return -EINVAL;
 
 		mlh2 = (struct mld2_query *)skb_transport_header(skb);
 
-		max_delay = max(msecs_to_jiffies(mldv2_mrc(mlh2)), 1UL);
-		idev->mc_maxdelay = max_delay;
-
-		mld_update_qrv(idev, mlh2);
-		mld_update_qi(idev, mlh2);
-		mld_update_qri(idev, mlh2);
+		err = mld_process_v2(idev, mlh2, &max_delay);
+		if (err < 0)
+			return err;
 
 		if (group_type == IPV6_ADDR_ANY) { /* general query */
 			if (mlh2->mld2q_nsrcs)
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>

Similarly as we do in MLDv2 queries, set a forged MLDv1 query with
0 ms mld_maxdelay to minimum timer shot time of 1 jiffies. This is
eventually done in igmp6_group_queried() anyway, so we can simplify
a check there.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 02cd0c5..75a4324 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1040,12 +1040,9 @@ static void igmp6_group_queried(struct ifmcaddr6 *ma, unsigned long resptime)
 		delay = ma->mca_timer.expires - jiffies;
 	}
 
-	if (delay >= resptime) {
-		if (resptime)
-			delay = net_random() % resptime;
-		else
-			delay = 1;
-	}
+	if (delay >= resptime)
+		delay = net_random() % resptime;
+
 	ma->mca_timer.expires = jiffies + delay;
 	if (!mod_timer(&ma->mca_timer, jiffies + delay))
 		atomic_inc(&ma->mca_refcnt);
@@ -1247,12 +1244,15 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	if (len == MLD_V1_QUERY_LEN) {
+		unsigned long mldv1_md;
+
 		/* Ignore v1 queries */
 		if (mld_in_v2_mode_only(idev))
 			return 0;
 
 		/* MLDv1 router present */
-		max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
+		mldv1_md = ntohs(mld->mld_maxdelay);
+		max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
 
 		mld_set_v1_mode(idev);
 
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>

RFC3810, 10. Security Considerations says under subsection 10.1.
Query Message:

  A forged Version 1 Query message will put MLDv2 listeners on that
  link in MLDv1 Host Compatibility Mode. This scenario can be avoided
  by providing MLDv2 hosts with a configuration option to ignore
  Version 1 messages completely.

Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:

  echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 005b22f..02cd0c5 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1112,9 +1112,21 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
 	return true;
 }
 
+static bool mld_in_v2_mode_only(const struct inet6_dev *idev)
+{
+	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 2;
+}
+
+static bool mld_in_v1_mode_only(const struct inet6_dev *idev)
+{
+	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1;
+}
+
 static bool mld_in_v1_mode(const struct inet6_dev *idev)
 {
-	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1)
+	if (mld_in_v2_mode_only(idev))
+		return false;
+	if (mld_in_v1_mode_only(idev))
 		return true;
 	if (idev->cnf.force_mld_version == 1)
 		return true;
@@ -1223,7 +1235,6 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	idev = __in6_dev_get(skb->dev);
-
 	if (idev == NULL)
 		return 0;
 
@@ -1236,6 +1247,10 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	if (len == MLD_V1_QUERY_LEN) {
+		/* Ignore v1 queries */
+		if (mld_in_v2_mode_only(idev))
+			return 0;
+
 		/* MLDv1 router present */
 		max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
 
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>

Get rid of MLDV2_MRC and use our new macros for mantisse and
exponent to calculate Maximum Response Delay out of the Maximum
Response Code.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 This will give a merge conflict in net/bridge/br_multicast.c with net tree. You can
 resolve it by simply taking this version as we already do msecs_to_jiffies() here.

 include/net/mld.h         | 28 +++++++++++++++++++---------
 net/bridge/br_multicast.c |  3 ++-
 net/ipv6/mcast.c          | 18 ++----------------
 3 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/include/net/mld.h b/include/net/mld.h
index 2b5421f..faa1d16 100644
--- a/include/net/mld.h
+++ b/include/net/mld.h
@@ -63,15 +63,6 @@ struct mld2_query {
 #define mld2q_mrc		mld2q_hdr.icmp6_maxdelay
 #define mld2q_resv1		mld2q_hdr.icmp6_dataun.un_data16[1]
 
-/* Max Response Code, TODO: transform this to use the below */
-#define MLDV2_MASK(value, nb) ((nb)>=32 ? (value) : ((1<<(nb))-1) & (value))
-#define MLDV2_EXP(thresh, nbmant, nbexp, value) \
-	((value) < (thresh) ? (value) : \
-	((MLDV2_MASK(value, nbmant) | (1<<(nbmant))) << \
-	(MLDV2_MASK((value) >> (nbmant), nbexp) + (nbexp))))
-
-#define MLDV2_MRC(value) MLDV2_EXP(0x8000, 12, 3, value)
-
 /* RFC3810, 5.1.3. Maximum Response Code:
  *
  * If Maximum Response Code >= 32768, Maximum Response Code represents a
@@ -97,4 +88,23 @@ struct mld2_query {
 #define MLDV2_QQIC_EXP(value)	(((value) >> 4) & 0x07)
 #define MLDV2_QQIC_MAN(value)	((value) & 0x0f)
 
+static inline unsigned long mldv2_mrc(const struct mld2_query *mlh2)
+{
+	/* RFC3810, 5.1.3. Maximum Response Code */
+	unsigned long ret, mc_mrc = ntohs(mlh2->mld2q_mrc);
+
+	if (mc_mrc < 32768) {
+		ret = mc_mrc;
+	} else {
+		unsigned long mc_man, mc_exp;
+
+		mc_exp = MLDV2_MRC_EXP(mc_mrc);
+		mc_man = MLDV2_MRC_MAN(mc_mrc);
+
+		ret = (mc_man | 0x1000) << (mc_exp + 3);
+	}
+
+	return ret;
+}
+
 #endif
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 08e576a..4accd0d 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1203,7 +1203,8 @@ static int br_ip6_multicast_query(struct net_bridge *br,
 		mld2q = (struct mld2_query *)icmp6_hdr(skb);
 		if (!mld2q->mld2q_nsrcs)
 			group = &mld2q->mld2q_mca;
-		max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
+
+		max_delay = max(msecs_to_jiffies(mldv2_mrc(mld2q)), 1UL);
 	}
 
 	br_multicast_query_received(br, port, !ipv6_addr_any(&ip6h->saddr),
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index f86e26b..005b22f 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1195,20 +1195,7 @@ static void mld_update_qri(struct inet6_dev *idev,
 	 *  - 5.1.3. Maximum Response Code
 	 *  - 9.3. Query Response Interval
 	 */
-	unsigned long mc_qri, mc_mrc = ntohs(mlh2->mld2q_mrc);
-
-	if (mc_mrc < 32768) {
-		mc_qri = mc_mrc;
-	} else {
-		unsigned long mc_man, mc_exp;
-
-		mc_exp = MLDV2_MRC_EXP(mc_mrc);
-		mc_man = MLDV2_MRC_MAN(mc_mrc);
-
-		mc_qri = (mc_man | 0x1000) << (mc_exp + 3);
-	}
-
-	idev->mc_qri = msecs_to_jiffies(mc_qri);
+	idev->mc_qri = msecs_to_jiffies(mldv2_mrc(mlh2));
 }
 
 /* called with rcu_read_lock() */
@@ -1277,8 +1264,7 @@ int igmp6_event_query(struct sk_buff *skb)
 
 		mlh2 = (struct mld2_query *)skb_transport_header(skb);
 
-		max_delay = max(msecs_to_jiffies(MLDV2_MRC(ntohs(mlh2->mld2q_mrc))), 1UL);
-
+		max_delay = max(msecs_to_jiffies(mldv2_mrc(mlh2)), 1UL);
 		idev->mc_maxdelay = max_delay;
 
 		mld_update_qrv(idev, mlh2);
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, David Stevens, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>

i) RFC3810, 9.2. Query Interval [QI] says:

   The Query Interval variable denotes the interval between General
   Queries sent by the Querier. Default value: 125 seconds. [...]

ii) RFC3810, 9.3. Query Response Interval [QRI] says:

  The Maximum Response Delay used to calculate the Maximum Response
  Code inserted into the periodic General Queries. Default value:
  10000 (10 seconds) [...] The number of seconds represented by the
  [Query Response Interval] must be less than the [Query Interval].

iii) RFC3810, 9.12. Older Version Querier Present Timeout [OVQPT] says:

  The Older Version Querier Present Timeout is the time-out for
  transitioning a host back to MLDv2 Host Compatibility Mode. When an
  MLDv1 query is received, MLDv2 hosts set their Older Version Querier
  Present Timer to [Older Version Querier Present Timeout].

  This value MUST be ([Robustness Variable] times (the [Query Interval]
  in the last Query received)) plus ([Query Response Interval]).

Hence, on *default* the timeout results in:

  [RV] = 2, [QI] = 125sec, [QRI] = 10sec
  [OVQPT] = [RV] * [QI] + [QRI] = 260sec

Having that said, we currently calculate [OVQPT] (here given as 'switchback'
variable) as ...

  switchback = (idev->mc_qrv + 1) * max_delay

RFC3810, 9.12. says "the [Query Interval] in the last Query received". In
section "9.14. Configuring timers", it is said:

  This section is meant to provide advice to network administrators on
  how to tune these settings to their network. Ambitious router
  implementations might tune these settings dynamically based upon
  changing characteristics of the network. [...]

iv) RFC38010, 9.14.2. Query Interval:

  The overall level of periodic MLD traffic is inversely proportional
  to the Query Interval. A longer Query Interval results in a lower
  overall level of MLD traffic. The value of the Query Interval MUST
  be equal to or greater than the Maximum Response Delay used to
  calculate the Maximum Response Code inserted in General Query
  messages.

I assume that was why switchback is calculated as is (3 * max_delay), although
this setting seems to be meant for routers only to configure their [QI]
interval for non-default intervals. So usage here like this is clearly wrong.

Concluding, the current behaviour in IPv6's multicast code is not conform
to the RFC as switch back is calculated wrongly. That is, it has a too small
value, so MLDv2 hosts switch back again to MLDv2 way too early, i.e. ~30secs
instead of ~260secs on default.

Hence, introduce necessary helper functions and fix this up properly as it
should be.

Introduced in 06da92283 ("[IPV6]: Add MLDv2 support."). Credits to Hannes
Frederic Sowa who also had a hand in this as well. Also thanks to Hangbin Liu
who did initial testing.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: David Stevens <dlstevens@us.ibm.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 06da92283 is linux-history tree.

 include/net/if_inet6.h |   9 +++-
 include/net/mld.h      |  27 +++++++++++-
 net/ipv6/mcast.c       | 116 ++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 143 insertions(+), 9 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 736b5fb..02ef772 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -171,12 +171,17 @@ struct inet6_dev {
 	struct ifmcaddr6	*mc_list;
 	struct ifmcaddr6	*mc_tomb;
 	spinlock_t		mc_lock;
-	unsigned char		mc_qrv;
+
+	unsigned char		mc_qrv;		/* Query Robustness Variable */
 	unsigned char		mc_gq_running;
 	unsigned char		mc_ifc_count;
 	unsigned char		mc_dad_count;
-	unsigned long		mc_v1_seen;
+
+	unsigned long		mc_v1_seen;	/* Max time we stay in MLDv1 mode */
+	unsigned long		mc_qi;		/* Query Interval */
+	unsigned long		mc_qri;		/* Query Response Interval */
 	unsigned long		mc_maxdelay;
+
 	struct timer_list	mc_gq_timer;	/* general query timer */
 	struct timer_list	mc_ifc_timer;	/* interface change timer */
 	struct timer_list	mc_dad_timer;	/* dad complete mc timer */
diff --git a/include/net/mld.h b/include/net/mld.h
index 467143c..2b5421f 100644
--- a/include/net/mld.h
+++ b/include/net/mld.h
@@ -63,7 +63,7 @@ struct mld2_query {
 #define mld2q_mrc		mld2q_hdr.icmp6_maxdelay
 #define mld2q_resv1		mld2q_hdr.icmp6_dataun.un_data16[1]
 
-/* Max Response Code */
+/* Max Response Code, TODO: transform this to use the below */
 #define MLDV2_MASK(value, nb) ((nb)>=32 ? (value) : ((1<<(nb))-1) & (value))
 #define MLDV2_EXP(thresh, nbmant, nbexp, value) \
 	((value) < (thresh) ? (value) : \
@@ -72,4 +72,29 @@ struct mld2_query {
 
 #define MLDV2_MRC(value) MLDV2_EXP(0x8000, 12, 3, value)
 
+/* RFC3810, 5.1.3. Maximum Response Code:
+ *
+ * If Maximum Response Code >= 32768, Maximum Response Code represents a
+ * floating-point value as follows:
+ *
+ *  0 1 2 3 4 5 6 7 8 9 A B C D E F
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |1| exp |          mant         |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
+#define MLDV2_MRC_EXP(value)	(((value) >> 12) & 0x0007)
+#define MLDV2_MRC_MAN(value)	((value) & 0x0fff)
+
+/* RFC3810, 5.1.9. QQIC (Querier's Query Interval Code):
+ *
+ * If QQIC >= 128, QQIC represents a floating-point value as follows:
+ *
+ *  0 1 2 3 4 5 6 7
+ * +-+-+-+-+-+-+-+-+
+ * |1| exp | mant  |
+ * +-+-+-+-+-+-+-+-+
+ */
+#define MLDV2_QQIC_EXP(value)	(((value) >> 4) & 0x07)
+#define MLDV2_QQIC_MAN(value)	((value) & 0x0f)
+
 #endif
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 98ead2b..8992ff2 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -108,6 +108,10 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 			    struct inet6_dev *idev);
 
 #define MLD_QRV_DEFAULT		2
+/* RFC3810, 9.2. Query Interval */
+#define MLD_QI_DEFAULT		(125 * HZ)
+/* RFC3810, 9.3. Query Response Interval */
+#define MLD_QRI_DEFAULT		(10 * HZ)
 
 /* RFC3810, 8.1 Query Version Distinctions */
 #define MLD_V1_QUERY_LEN	24
@@ -1112,6 +1116,93 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
 	return true;
 }
 
+static void mld_set_v1_mode(struct inet6_dev *idev)
+{
+	/* RFC3810, relevant sections:
+	 *  - 9.1. Robustness Variable
+	 *  - 9.2. Query Interval
+	 *  - 9.3. Query Response Interval
+	 *  - 9.12. Older Version Querier Present Timeout
+	 */
+	unsigned long switchback;
+
+	switchback = (idev->mc_qrv * idev->mc_qi) + idev->mc_qri;
+
+	idev->mc_v1_seen = jiffies + switchback;
+}
+
+static void mld_update_qrv(struct inet6_dev *idev,
+			   const struct mld2_query *mlh2)
+{
+	/* RFC3810, relevant sections:
+	 *  - 5.1.8. QRV (Querier's Robustness Variable)
+	 *  - 9.1. Robustness Variable
+	 */
+
+	/* The value of the Robustness Variable MUST NOT be zero,
+	 * and SHOULD NOT be one. Catch this here if we ever run
+	 * into such a case in future.
+	 */
+	WARN_ON(idev->mc_qrv == 0);
+
+	if (mlh2->mld2q_qrv > 0)
+		idev->mc_qrv = mlh2->mld2q_qrv;
+
+	if (unlikely(idev->mc_qrv < 2)) {
+		net_warn_ratelimited("IPv6: MLD: clamping QRV from %u to %u!\n",
+				     idev->mc_qrv, MLD_QRV_DEFAULT);
+		idev->mc_qrv = MLD_QRV_DEFAULT;
+	}
+}
+
+static void mld_update_qi(struct inet6_dev *idev,
+			  const struct mld2_query *mlh2)
+{
+	/* RFC3810, relevant sections:
+	 *  - 5.1.9. QQIC (Querier's Query Interval Code)
+	 *  - 9.2. Query Interval
+	 *  - 9.12. Older Version Querier Present Timeout
+	 *    (the [Query Interval] in the last Query received)
+	 */
+	unsigned long mc_qqi;
+
+	if (mlh2->mld2q_qqic < 128) {
+		mc_qqi = mlh2->mld2q_qqic;
+	} else {
+		unsigned long mc_man, mc_exp;
+
+		mc_exp = MLDV2_QQIC_EXP(mlh2->mld2q_qqic);
+		mc_man = MLDV2_QQIC_MAN(mlh2->mld2q_qqic);
+
+		mc_qqi = (mc_man | 0x10) << (mc_exp + 3);
+	}
+
+	idev->mc_qi = mc_qqi * HZ;
+}
+
+static void mld_update_qri(struct inet6_dev *idev,
+			   const struct mld2_query *mlh2)
+{
+	/* RFC3810, relevant sections:
+	 *  - 5.1.3. Maximum Response Code
+	 *  - 9.3. Query Response Interval
+	 */
+	unsigned long mc_qri, mc_mrc = ntohs(mlh2->mld2q_mrc);
+
+	if (mc_mrc < 32768) {
+		mc_qri = mc_mrc;
+	} else {
+		unsigned long mc_man, mc_exp;
+
+		mc_exp = MLDV2_MRC_EXP(mc_mrc);
+		mc_man = MLDV2_MRC_MAN(mc_mrc);
+
+		mc_qri = (mc_man | 0x1000) << (mc_exp + 3);
+	}
+
+	idev->mc_qri = msecs_to_jiffies(mc_qri);
+}
+
 /* called with rcu_read_lock() */
 int igmp6_event_query(struct sk_buff *skb)
 {
@@ -1150,12 +1241,15 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	if (len == MLD_V1_QUERY_LEN) {
-		int switchback;
 		/* MLDv1 router present */
-
 		max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
-		switchback = (idev->mc_qrv + 1) * max_delay;
-		idev->mc_v1_seen = jiffies + switchback;
+
+		mld_set_v1_mode(idev);
+
+		/* cancel MLDv2 report timer */
+		idev->mc_gq_running = 0;
+		if (del_timer(&idev->mc_gq_timer))
+			__in6_dev_put(idev);
 
 		/* cancel the interface change timer */
 		idev->mc_ifc_count = 0;
@@ -1166,6 +1260,10 @@ int igmp6_event_query(struct sk_buff *skb)
 	} else if (len >= MLD_V2_QUERY_LEN_MIN) {
 		int srcs_offset = sizeof(struct mld2_query) -
 				  sizeof(struct icmp6hdr);
+
+		/* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
+		if (MLD_V1_SEEN(idev))
+			return 0;
 		if (!pskb_may_pull(skb, srcs_offset))
 			return -EINVAL;
 
@@ -1175,8 +1273,10 @@ int igmp6_event_query(struct sk_buff *skb)
 
 		idev->mc_maxdelay = max_delay;
 
-		if (mlh2->mld2q_qrv)
-			idev->mc_qrv = mlh2->mld2q_qrv;
+		mld_update_qrv(idev, mlh2);
+		mld_update_qi(idev, mlh2);
+		mld_update_qri(idev, mlh2);
+
 		if (group_type == IPV6_ADDR_ANY) { /* general query */
 			if (mlh2->mld2q_nsrcs)
 				return -EINVAL; /* no sources allowed */
@@ -2337,7 +2437,11 @@ void ipv6_mc_init_dev(struct inet6_dev *idev)
 			(unsigned long)idev);
 	setup_timer(&idev->mc_dad_timer, mld_dad_timer_expire,
 		    (unsigned long)idev);
+
 	idev->mc_qrv = MLD_QRV_DEFAULT;
+	idev->mc_qi = MLD_QI_DEFAULT;
+	idev->mc_qri = MLD_QRI_DEFAULT;
+
 	idev->mc_maxdelay = unsolicited_report_interval(idev);
 	idev->mc_v1_seen = 0;
 	write_unlock_bh(&idev->lock);
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>

Replace the macro with a function to make it more readable. GCC will
eventually decide whether to inline this or not (also, that's not
fast-path anyway).

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 8992ff2..f86e26b 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -95,6 +95,7 @@ static void mld_ifc_event(struct inet6_dev *idev);
 static void mld_add_delrec(struct inet6_dev *idev, struct ifmcaddr6 *pmc);
 static void mld_del_delrec(struct inet6_dev *idev, const struct in6_addr *addr);
 static void mld_clear_delrec(struct inet6_dev *idev);
+static bool mld_in_v1_mode(const struct inet6_dev *idev);
 static int sf_setstate(struct ifmcaddr6 *pmc);
 static void sf_markstate(struct ifmcaddr6 *pmc);
 static void ip6_mc_clear_src(struct ifmcaddr6 *pmc);
@@ -117,11 +118,6 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 #define MLD_V1_QUERY_LEN	24
 #define MLD_V2_QUERY_LEN_MIN	28
 
-#define MLD_V1_SEEN(idev) (dev_net((idev)->dev)->ipv6.devconf_all->force_mld_version == 1 || \
-		(idev)->cnf.force_mld_version == 1 || \
-		((idev)->mc_v1_seen && \
-		time_before(jiffies, (idev)->mc_v1_seen)))
-
 #define IPV6_MLD_MAX_MSF	64
 
 int sysctl_mld_max_msf __read_mostly = IPV6_MLD_MAX_MSF;
@@ -139,7 +135,7 @@ static int unsolicited_report_interval(struct inet6_dev *idev)
 {
 	int iv;
 
-	if (MLD_V1_SEEN(idev))
+	if (mld_in_v1_mode(idev))
 		iv = idev->cnf.mldv1_unsolicited_report_interval;
 	else
 		iv = idev->cnf.mldv2_unsolicited_report_interval;
@@ -695,7 +691,7 @@ static void igmp6_group_added(struct ifmcaddr6 *mc)
 	if (!(dev->flags & IFF_UP) || (mc->mca_flags & MAF_NOREPORT))
 		return;
 
-	if (MLD_V1_SEEN(mc->idev)) {
+	if (mld_in_v1_mode(mc->idev)) {
 		igmp6_join_group(mc);
 		return;
 	}
@@ -1116,6 +1112,18 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
 	return true;
 }
 
+static bool mld_in_v1_mode(const struct inet6_dev *idev)
+{
+	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1)
+		return true;
+	if (idev->cnf.force_mld_version == 1)
+		return true;
+	if (idev->mc_v1_seen && time_before(jiffies, idev->mc_v1_seen))
+		return true;
+
+	return false;
+}
+
 static void mld_set_v1_mode(struct inet6_dev *idev)
 {
 	/* RFC3810, relevant sections:
@@ -1262,7 +1270,7 @@ int igmp6_event_query(struct sk_buff *skb)
 				  sizeof(struct icmp6hdr);
 
 		/* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
-		if (MLD_V1_SEEN(idev))
+		if (mld_in_v1_mode(idev))
 			return 0;
 		if (!pskb_may_pull(skb, srcs_offset))
 			return -EINVAL;
@@ -1942,7 +1950,7 @@ err_out:
 
 static void mld_resend_report(struct inet6_dev *idev)
 {
-	if (MLD_V1_SEEN(idev)) {
+	if (mld_in_v1_mode(idev)) {
 		struct ifmcaddr6 *mcaddr;
 		read_lock_bh(&idev->lock);
 		for (mcaddr = idev->mc_list; mcaddr; mcaddr = mcaddr->next) {
@@ -2006,7 +2014,7 @@ static int ip6_mc_del1_src(struct ifmcaddr6 *pmc, int sfmode,
 		else
 			pmc->mca_sources = psf->sf_next;
 		if (psf->sf_oldin && !(pmc->mca_flags & MAF_NOREPORT) &&
-		    !MLD_V1_SEEN(idev)) {
+		    !mld_in_v1_mode(idev)) {
 			psf->sf_crcount = idev->mc_qrv;
 			psf->sf_next = pmc->mca_tomb;
 			pmc->mca_tomb = psf;
@@ -2306,7 +2314,7 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 
 static void igmp6_leave_group(struct ifmcaddr6 *ma)
 {
-	if (MLD_V1_SEEN(ma->idev)) {
+	if (mld_in_v1_mode(ma->idev)) {
 		if (ma->mca_flags & MAF_LAST_REPORTER)
 			igmp6_send(&ma->mca_addr, ma->idev->dev,
 				ICMPV6_MGM_REDUCTION);
@@ -2340,7 +2348,7 @@ static void mld_ifc_timer_expire(unsigned long data)
 
 static void mld_ifc_event(struct inet6_dev *idev)
 {
-	if (MLD_V1_SEEN(idev))
+	if (mld_in_v1_mode(idev))
 		return;
 	idev->mc_ifc_count = idev->mc_qrv;
 	mld_ifc_start_timer(idev, 1);
@@ -2351,7 +2359,7 @@ static void igmp6_timer_handler(unsigned long data)
 {
 	struct ifmcaddr6 *ma = (struct ifmcaddr6 *) data;
 
-	if (MLD_V1_SEEN(ma->idev))
+	if (mld_in_v1_mode(ma->idev))
 		igmp6_send(&ma->mca_addr, ma->idev->dev, ICMPV6_MGM_REPORT);
 	else
 		mld_send_report(ma->idev, ma);
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 0/8] IPv6 MLD updates
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw)
  To: davem; +Cc: netdev

This set contains the non-RFC version of the previous submission entitled
"[RFC PATCH net-next 0/7] IPv6 MLD updates". Most importantly it contains
a fix for MLDv1/v2 switchback timeout where hosts currently are switching
back from v1 compat mode to normal v2 too early (i.e. switchback time was
<= 30secs instead of >= 260secs on default), and the set also contains a
patch that allows for v2-only mode as per RFC recommendation. The rest is
related to cleanups that make the code more readable resp. maintainable.

Changes from RFC to non-RFC:

We ignore v2 messages now when in v1 compat mode, otherwise report timers
are reset and triggered, also stop current v2 report timer in case it is
currently running and we received a v1 query; use WARN_ON instead of BUG_ON
for RV of 0 (patch1).

Two more patches have been added that makes to code more readable, that is
"net: ipv6: mld: refactor query processing into v1/v2 functions" and
"net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions", the
sysctl patch "net: ipv6: mld: restrict min/max of sysctl force_mld_version"
has been dropped as extra1 and extra2 vars are overwritten with idev and net
anyway when addrconf sysctl is registered, hence dropped for now. The rest is
unchanged, only adapted to take changes into account.

Thanks!

Daniel Borkmann (8):
  net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
  net: ipv6: mld: clean up MLD_V1_SEEN macro
  net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
  net: ipv6: mld: implement RFC3810 MLDv2 mode only
  net: ipv6: mld: similarly to MLDv2 have min max_delay of 1
  net: ipv6: mld: refactor query processing into v1/v2 functions
  net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions
  net: ipv6: mld: document force_mld_version in ip-sysctl.txt

 Documentation/networking/ip-sysctl.txt |   5 +
 include/net/if_inet6.h                 |   9 +-
 include/net/mld.h                      |  51 +++++--
 net/bridge/br_multicast.c              |   3 +-
 net/ipv6/mcast.c                       | 237 ++++++++++++++++++++++++++-------
 5 files changed, 248 insertions(+), 57 deletions(-)

-- 
1.7.11.7

^ permalink raw reply

* Re: BQL support in gianfar causes network hiccup
From: Per Dalén @ 2013-09-03  7:55 UTC (permalink / raw)
  To: Claudiu Manoil; +Cc: netdev
In-Reply-To: <52258A60.8040305@freescale.com>

On 09/03/2013 09:06 AM, Claudiu Manoil wrote:
> On 9/2/2013 7:50 PM, Per Dalén wrote:
>> On 09/02/2013 05:35 PM, Claudiu Manoil wrote:
>>> On 9/2/2013 5:37 PM, Per Dalén wrote:
>>>>> The proposed fix is currently under review:
>>>>> http://patchwork.ozlabs.org/patch/271242/
>>>>> "gianfar: Fix reported number of sent bytes to BQL"
>>>>>
>>>>> Does this one work for you? You might need to pull
>>>>> one recent gianfar clean-up patch from net-next in order
>>>>> to apply this one without incidents.
>>>>>
>>>>
>>>> No, still the same error:
>>>>
>>>> NETDEV WATCHDOG: eth0 (fsl-gianfar): transmit queue 0 timed out
>>>> ------------[ cut here ]------------
>>>> WARNING: at net/sched/sch_generic.c:256
>>>> Modules linked in:
>>>> NIP: c039ce0c LR: c039ce0c CTR: c02ef578
>>>> ...
>>>> 4e800421 80fe0244 4bffff40 7fc3f378 4bfea9d1 7fc4f378 7c651b78 3c60c055
>>>> 7fe6fb78 38635304 4cc63182 480fcddd <0fe00000> 39200001 993c0a3c
>>>> 4bffffb4
>>>> ---[ end trace 5f5e1e3c30024010 ]---
>>>>
>>>>
>>>
>>> Tried to reproduce the issue with a recent net-next kernel (Linux
>>> p2020rdb 3.11.0-rc6) + BQL fix patch (http://patchwork.ozlabs.org/patc
>>> /271242/), but the iperf test finished without incidents (see log
>>> below).
>>> Will try if the problem is apparent without the fix patch, on the same
>>> net-next kernel (3.11.0-rc6).
>>>
>>
>> I was able to reproduce it on our card and the P2020RDB using 3.11.0-rc7
>> + David Miller's -next networking tree and your patch
>> (http://patchwork.ozlabs.org/patch/271242/).
>>
>> root@p2020rdb:~# iperf -s
>
> Ok, I see, iperf -s on P2020. This way I was able to get the tx timeout
> too.  With iperf -c on P2020 it doesn't come up.  Now it'll be
> interesting to find out what BQL/ BQL integration in gianfar has to do
> with this.

Yes, it's weird. The only reason I removed the BQL commit 
(d8a0f1b0af67679bba886784de10d8c21acc4e0e) was because the error Tino 
Keitel had was the similar as mine.

>
> Thanks.
> claudiu
>
>

Thanks,
Per

^ permalink raw reply

* Re: [PATCH RFC net-next 06/21] net: create sysfs symlinks for neighbour devices
From: Jiri Pirko @ 2013-09-03  7:48 UTC (permalink / raw)
  To: Veaceslav Falico
  Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller,
	Eric Dumazet, Alexander Duyck
In-Reply-To: <1378157965-17537-7-git-send-email-vfalico@redhat.com>

Mon, Sep 02, 2013 at 11:39:10PM CEST, vfalico@redhat.com wrote:
>Also, remove the same functionality from bonding - it will be already done
>for any device that links to its lower/upper neighbour.
>
>The links will be created for dev's kobject, and will look like
>slave_eth0 for lower device eth0 and upper_bridge0 for upper device
>bridge0.

The pair should be either slave/master or lower/upper. It's undesirable
to mix these two. Maybe for compatibility reasons we should leave
current bonding sysfs files and create proper new generic ones?

I think there should be directories under netdev dir like:
/sys/class/net/em1-
                  |-upper-
                         |-bond0(link)
                         |-vlanx(link)
/sys/class/net/bond0-
                    |-lower-
                           |-em1(link)
                           |-em2(link)
           

>
>CC: Jay Vosburgh <fubar@us.ibm.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>CC: "David S. Miller" <davem@davemloft.net>
>CC: Eric Dumazet <edumazet@google.com>
>CC: Jiri Pirko <jiri@resnulli.us>
>CC: Alexander Duyck <alexander.h.duyck@intel.com>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>---
> drivers/net/bonding/bond_main.c  | 11 +----------
> drivers/net/bonding/bond_sysfs.c | 25 -------------------------
> drivers/net/bonding/bonding.h    |  2 --
> net/core/dev.c                   | 29 +++++++++++++++++++++++++++++
> 4 files changed, 30 insertions(+), 37 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index c50679f..b0b753d 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1635,15 +1635,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> 
> 	read_unlock(&bond->lock);
> 
>-	res = bond_create_slave_symlinks(bond_dev, slave_dev);
>-	if (res)
>-		goto err_detach;
>-
> 	res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
> 					 new_slave);
> 	if (res) {
> 		pr_debug("Error %d calling netdev_rx_handler_register\n", res);
>-		goto err_dest_symlinks;
>+		goto err_detach;
> 	}
> 
> 	pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
>@@ -1655,9 +1651,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> 	return 0;
> 
> /* Undo stages on error */
>-err_dest_symlinks:
>-	bond_destroy_slave_symlinks(bond_dev, slave_dev);
>-
> err_detach:
> 	if (!USES_PRIMARY(bond->params.mode))
> 		bond_hw_addr_flush(bond_dev, slave_dev);
>@@ -1856,8 +1849,6 @@ static int __bond_release_one(struct net_device *bond_dev,
> 			bond_dev->name, slave_dev->name, bond_dev->name);
> 
> 	/* must do this from outside any spinlocks */
>-	bond_destroy_slave_symlinks(bond_dev, slave_dev);
>-
> 	vlan_vids_del_by_dev(slave_dev, bond_dev);
> 
> 	/* If the mode USES_PRIMARY, then this cases was handled above by
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 5715277..7b962a2 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -168,31 +168,6 @@ static const struct class_attribute class_attr_bonding_masters = {
> 	.namespace = bonding_namespace,
> };
> 
>-int bond_create_slave_symlinks(struct net_device *master,
>-			       struct net_device *slave)
>-{
>-	char linkname[IFNAMSIZ+7];
>-	int ret = 0;
>-
>-	/* create a link from the master to the slave */
>-	sprintf(linkname, "slave_%s", slave->name);
>-	ret = sysfs_create_link(&(master->dev.kobj), &(slave->dev.kobj),
>-				linkname);
>-
>-	return ret;
>-
>-}
>-
>-void bond_destroy_slave_symlinks(struct net_device *master,
>-				 struct net_device *slave)
>-{
>-	char linkname[IFNAMSIZ+7];
>-
>-	sprintf(linkname, "slave_%s", slave->name);
>-	sysfs_remove_link(&(master->dev.kobj), linkname);
>-}
>-
>-
> /*
>  * Show the slaves in the current bond.
>  */
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 4abc925..f7a167d 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -455,8 +455,6 @@ int bond_create(struct net *net, const char *name);
> int bond_create_sysfs(struct bond_net *net);
> void bond_destroy_sysfs(struct bond_net *net);
> void bond_prepare_sysfs_group(struct bonding *bond);
>-int bond_create_slave_symlinks(struct net_device *master, struct net_device *slave);
>-void bond_destroy_slave_symlinks(struct net_device *master, struct net_device *slave);
> int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev);
> int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
> void bond_mii_monitor(struct work_struct *);
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 7eecf35..1c3e98d 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -4580,6 +4580,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
> 					bool upper, void *private)
> {
> 	struct netdev_adjacent *adj, *neigh = NULL;
>+	char linkname[IFNAMSIZ+7];
> 	int ret;
> 
> 	adj = __netdev_find_adj(dev, adj_dev, upper, false);
>@@ -4628,6 +4629,16 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
> 			 */
> 			if (master)
> 				neigh->private = private;
>+
>+			sprintf(linkname, "slave_%s", adj_dev->name);
>+			ret = sysfs_create_link(&(dev->dev.kobj),
>+						&(adj_dev->dev.kobj),
>+						linkname);
>+			if (ret) {
>+				kfree(neigh);
>+				kfree(adj);
>+				return ret;
>+			}
> 			list_add_tail_rcu(&neigh->list,
> 					  &dev->neighbour_dev_list.lower);
> 		}
>@@ -4635,10 +4646,24 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
> 		return 0;
> 	}
> 
>+	/* it's upper neighbour, we don't care if it's master or not */
>+	if (neigh) {
>+		sprintf(linkname, "upper_%s", adj_dev->name);
>+		ret = sysfs_create_link(&(dev->dev.kobj), &(adj_dev->dev.kobj),
>+					linkname);
>+		if (ret) {
>+			kfree(neigh);
>+			kfree(adj);
>+			return ret;
>+		}
>+	}
>+
> 	/* Ensure that master upper link is always the first item in list. */
> 	if (master) {
> 		ret = sysfs_create_link(&(dev->dev.kobj), &(adj_dev->dev.kobj), "master");
> 		if (ret) {
>+			if (neigh)
>+				sysfs_remove_link(&(dev->dev.kobj), linkname);
> 			kfree(neigh);
> 			kfree(adj);
> 			return ret;
>@@ -4678,6 +4703,7 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
> 				  struct net_device *adj_dev, bool upper)
> {
> 	struct netdev_adjacent *adj, *neighbour;
>+	char linkname[IFNAMSIZ+7];
> 
> 	if (upper) {
> 		adj = __netdev_find_upper(dev, adj_dev);
>@@ -4725,6 +4751,9 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
> 		list_del_rcu(&neighbour->list);
> 		if (neighbour->master && upper)
> 			sysfs_remove_link(&(dev->dev.kobj), "master");
>+		sprintf(linkname, "%s_%s", upper ? "upper" : "slave",
>+			adj_dev->name);
>+		sysfs_remove_link(&(dev->dev.kobj), linkname);
> 		dev_put(adj_dev);
> 		kfree_rcu(neighbour, rcu);
> 	}
>-- 
>1.8.4
>

^ permalink raw reply

* Re: [PATCH v2 net-next 0/9] set addr_assign_type when inheriting a dev_addr
From: Bjørn Mork @ 2013-09-03  7:38 UTC (permalink / raw)
  To: David Miller
  Cc: devel, jiri, libertas-dev, j, netdev, linux-wireless, linville,
	stephen, forest, gregkh, kaber, dan.carpenter
In-Reply-To: <20130831.225057.1874268263797742541.davem@davemloft.net>

David Miller <davem@davemloft.net> writes:

> From: Bjørn Mork <bjorn@mork.no>
> Date: Fri, 30 Aug 2013 18:08:43 +0200
>
>> Copying the dev_addr from a parent device is an operation
>> common to a number of drivers. The addr_assign_type should
>> be updated accordingly, either by reusing the value from
>> the source device or explicitly indicating that the address
>> is stolen by setting addr_assign_type to NET_ADDR_STOLEN.
>> 
>> This patch set adds a helper copying both the dev_addr and
>> the addr_assign_type, and use this helper in drivers which
>> don't currently set the addr_assign_type. Using NET_ADDR_STOLEN
>> might be more appropriate in some of these cases.  Please
>> let me know, and I'll update the patch accordingly.
>> 
>> Changes in v2:
>>  - assuming addr_len == ETH_ALEN to allow optimized memcpy
>>  - dropped the vt6656 patch due to addr_len being unset in that driver
>
> Looks good, series applied, thanks.

Thanks.  But it doesn't look like it ended up in net-next?  Or am I
missing something (again)?


Bjørn
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply

* Re: Networking problem with 3.11-rc6+
From: Chris Clayton @ 2013-09-03  7:35 UTC (permalink / raw)
  To: Francois Romieu; +Cc: Chris Clayton, LKML, netdev
In-Reply-To: <20130820215419.GA29560@electric-eye.fr.zoreil.com>



On 08/20/13 22:54, Francois Romieu wrote:
> Chris Clayton <chris2553@gmail.com> :
> [...]
>> [    0.207094] acpi PNP0A08:00: ACPI _OSC support notification failed, disabling PCIe ASPM
>> [    0.207155] acpi PNP0A08:00: Unable to request _OSC control (_OSC support mask: 0x08)
> [...]
>> [    5.311191] r8169 0000:07:00.0: can't disable ASPM; OS doesn't have ASPM control
> [...]
>> [   65.181715] [<f8e38000>] rtl8169_interrupt [r8169]
>> [   65.181717] Disabling IRQ #17
>>
>> Let me know if I can provide any additional information, although to
>> be honest, the boot completed and the KDE desktop started up OK, so
>> there may not be much else I can provide unless I find that the
>> problem is repeatable.
>
> Please don't hesitate if it happens again, even if it can't be reproduced
> on demand.
>

I've booted the kernel numerous times since the incident reported above, 
but haven't encountered the problem again. If it does happen again, are 
there any files from /sys or /proc that might help diagnostics?

Chris

^ permalink raw reply

* [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code
From: Yijing Wang @ 2013-09-03  7:35 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Gavin Shan, Bjorn Helgaas,
	James E.J. Bottomley, David S. Miller
  Cc: e1000-devel, linux-pci, Hanjun Guo, linux-kernel, netdev,
	Yijing Wang
In-Reply-To: <1378193715-25328-1-git-send-email-wangyijing@huawei.com>

use pcie_capability_read_word() to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Cc: e1000-devel@lists.sourceforge.net
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bad8f14..bfa0b06 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -152,7 +152,6 @@ MODULE_VERSION(DRV_VERSION);
 static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
 					  u32 reg, u16 *value)
 {
-	int pos = 0;
 	struct pci_dev *parent_dev;
 	struct pci_bus *parent_bus;
 
@@ -164,11 +163,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
 	if (!parent_dev)
 		return -1;
 
-	pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
-	if (!pos)
+	if (!pci_is_pcie(parent_dev))
 		return -1;
 
-	pci_read_config_word(parent_dev, pos + reg, value);
+	pcie_capability_read_word(parent_dev, reg, value);
 	return 0;
 }
 
-- 
1.7.1



------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ 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