netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI
@ 2015-04-28 16:33 Nicolas Dichtel
  2015-04-28 16:33 ` [PATCH net 1/3] bridge/mdb: remove wrong use of NLM_F_MULTI Nicolas Dichtel
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Nicolas Dichtel @ 2015-04-28 16:33 UTC (permalink / raw)
  To: netdev; +Cc: davem, dmitry.tarnyaginuug.ch


The first three patches fix a wrong use of this flag.

Because it's not the first time that this kind of bug happens, it could be
good to find something to help people.

The only idea that I have is in the last patch. It's just an example, if
people agrees on it, I will update all users and submit it formally.
Any better idea is welcomed.

Regards,
Nicolas

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net 1/3] bridge/mdb: remove wrong use of NLM_F_MULTI
  2015-04-28 16:33 [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI Nicolas Dichtel
@ 2015-04-28 16:33 ` Nicolas Dichtel
  2015-04-28 16:33 ` [PATCH net 2/3] bridge/nl: " Nicolas Dichtel
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Nicolas Dichtel @ 2015-04-28 16:33 UTC (permalink / raw)
  To: netdev
  Cc: davem, dmitry.tarnyaginuug.ch, Nicolas Dichtel, Cong Wang,
	Stephen Hemminger, bridge

NLM_F_MULTI must be used only when a NLMSG_DONE message is sent. In fact,
it is sent only at the end of a dump.

Libraries like libnl will wait forever for NLMSG_DONE.

Fixes: 37a393bc4932 ("bridge: notify mdb changes via netlink")
CC: Cong Wang <amwang@redhat.com>
CC: Stephen Hemminger <stephen@networkplumber.org>
CC: bridge@lists.linux-foundation.org
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/bridge/br_mdb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 409608960899..e29ad70b3000 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -170,7 +170,7 @@ static int nlmsg_populate_mdb_fill(struct sk_buff *skb,
 	struct br_port_msg *bpm;
 	struct nlattr *nest, *nest2;
 
-	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*bpm), NLM_F_MULTI);
+	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*bpm), 0);
 	if (!nlh)
 		return -EMSGSIZE;
 
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net 2/3] bridge/nl: remove wrong use of NLM_F_MULTI
  2015-04-28 16:33 [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI Nicolas Dichtel
  2015-04-28 16:33 ` [PATCH net 1/3] bridge/mdb: remove wrong use of NLM_F_MULTI Nicolas Dichtel
@ 2015-04-28 16:33 ` Nicolas Dichtel
  2015-04-30 13:01   ` Jeff Kirsher
  2015-04-28 16:33 ` [PATCH net 3/3] tipc: " Nicolas Dichtel
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Nicolas Dichtel @ 2015-04-28 16:33 UTC (permalink / raw)
  To: netdev
  Cc: davem, dmitry.tarnyaginuug.ch, Nicolas Dichtel, John Fastabend,
	Sathya Perla, Subbu Seetharaman, Ajit Khaparde, Jeff Kirsher,
	intel-wired-lan, Jiri Pirko, Scott Feldman, Stephen Hemminger,
	bridge

NLM_F_MULTI must be used only when a NLMSG_DONE message is sent. In fact,
it is sent only at the end of a dump.

Libraries like libnl will wait forever for NLMSG_DONE.

Fixes: e5a55a898720 ("net: create generic bridge ops")
Fixes: 815cccbf10b2 ("ixgbe: add setlink, getlink support to ixgbe and ixgbevf")
CC: John Fastabend <john.r.fastabend@intel.com>
CC: Sathya Perla <sathya.perla@emulex.com>
CC: Subbu Seetharaman <subbu.seetharaman@emulex.com>
CC: Ajit Khaparde <ajit.khaparde@emulex.com>
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: intel-wired-lan@lists.osuosl.org
CC: Jiri Pirko <jiri@resnulli.us>
CC: Scott Feldman <sfeldma@gmail.com>
CC: Stephen Hemminger <stephen@networkplumber.org>
CC: bridge@lists.linux-foundation.org
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c   |  5 +++--
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  7 ++++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  4 ++--
 drivers/net/ethernet/rocker/rocker.c          |  5 +++--
 include/linux/netdevice.h                     |  6 ++++--
 include/linux/rtnetlink.h                     |  2 +-
 net/bridge/br_netlink.c                       |  4 ++--
 net/bridge/br_private.h                       |  2 +-
 net/core/rtnetlink.c                          | 12 +++++++-----
 9 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index fb0bc3c3620e..a6dcbf850c1f 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4846,7 +4846,8 @@ err:
 }
 
 static int be_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
-				 struct net_device *dev, u32 filter_mask)
+				 struct net_device *dev, u32 filter_mask,
+				 int nlflags)
 {
 	struct be_adapter *adapter = netdev_priv(dev);
 	int status = 0;
@@ -4868,7 +4869,7 @@ static int be_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 	return ndo_dflt_bridge_getlink(skb, pid, seq, dev,
 				       hsw_mode == PORT_FWD_TYPE_VEPA ?
 				       BRIDGE_MODE_VEPA : BRIDGE_MODE_VEB,
-				       0, 0);
+				       0, 0, nlflags);
 }
 
 #ifdef CONFIG_BE2NET_VXLAN
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 24481cd7e59a..a54c14491e3b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8053,10 +8053,10 @@ static int i40e_ndo_bridge_setlink(struct net_device *dev,
 #ifdef HAVE_BRIDGE_FILTER
 static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				   struct net_device *dev,
-				   u32 __always_unused filter_mask)
+				   u32 __always_unused filter_mask, int nlflags)
 #else
 static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
-				   struct net_device *dev)
+				   struct net_device *dev, int nlflags)
 #endif /* HAVE_BRIDGE_FILTER */
 {
 	struct i40e_netdev_priv *np = netdev_priv(dev);
@@ -8078,7 +8078,8 @@ static int i40e_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 	if (!veb)
 		return 0;
 
-	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, veb->bridge_mode);
+	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, veb->bridge_mode,
+				       nlflags);
 }
 #endif /* HAVE_BRIDGE_ATTRIBS */
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d3f4b0ceb3f7..5be12a00e1f4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8044,7 +8044,7 @@ static int ixgbe_ndo_bridge_setlink(struct net_device *dev,
 
 static int ixgbe_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				    struct net_device *dev,
-				    u32 filter_mask)
+				    u32 filter_mask, int nlflags)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
 
@@ -8052,7 +8052,7 @@ static int ixgbe_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 		return 0;
 
 	return ndo_dflt_bridge_getlink(skb, pid, seq, dev,
-				       adapter->bridge_mode, 0, 0);
+				       adapter->bridge_mode, 0, 0, nlflags);
 }
 
 static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index a570a60533be..ec251531bd9f 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4176,14 +4176,15 @@ static int rocker_port_bridge_setlink(struct net_device *dev,
 
 static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				      struct net_device *dev,
-				      u32 filter_mask)
+				      u32 filter_mask, int nlflags)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
 	u16 mode = BRIDGE_MODE_UNDEF;
 	u32 mask = BR_LEARNING | BR_LEARNING_SYNC;
 
 	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode,
-				       rocker_port->brport_flags, mask);
+				       rocker_port->brport_flags, mask,
+				       nlflags);
 }
 
 static int rocker_port_get_phys_port_name(struct net_device *dev,
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dbad4d728b4b..1899c74a7127 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -977,7 +977,8 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  * int (*ndo_bridge_setlink)(struct net_device *dev, struct nlmsghdr *nlh,
  *			     u16 flags)
  * int (*ndo_bridge_getlink)(struct sk_buff *skb, u32 pid, u32 seq,
- *			     struct net_device *dev, u32 filter_mask)
+ *			     struct net_device *dev, u32 filter_mask,
+ *			     int nlflags)
  * int (*ndo_bridge_dellink)(struct net_device *dev, struct nlmsghdr *nlh,
  *			     u16 flags);
  *
@@ -1173,7 +1174,8 @@ struct net_device_ops {
 	int			(*ndo_bridge_getlink)(struct sk_buff *skb,
 						      u32 pid, u32 seq,
 						      struct net_device *dev,
-						      u32 filter_mask);
+						      u32 filter_mask,
+						      int nlflags);
 	int			(*ndo_bridge_dellink)(struct net_device *dev,
 						      struct nlmsghdr *nlh,
 						      u16 flags);
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 2da5d1081ad9..7b8e260c4a27 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -122,5 +122,5 @@ extern int ndo_dflt_fdb_del(struct ndmsg *ndm,
 
 extern int ndo_dflt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				   struct net_device *dev, u16 mode,
-				   u32 flags, u32 mask);
+				   u32 flags, u32 mask, int nlflags);
 #endif	/* __LINUX_RTNETLINK_H */
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 0e4ddb81610d..4b5c236998ff 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -394,7 +394,7 @@ errout:
  * Dump information about all ports, in response to GETLINK
  */
 int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
-	       struct net_device *dev, u32 filter_mask)
+	       struct net_device *dev, u32 filter_mask, int nlflags)
 {
 	struct net_bridge_port *port = br_port_get_rtnl(dev);
 
@@ -402,7 +402,7 @@ int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 	    !(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED))
 		return 0;
 
-	return br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, NLM_F_MULTI,
+	return br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, nlflags,
 			      filter_mask, dev);
 }
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6ca0251cb478..3362c29400f1 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -828,7 +828,7 @@ void br_ifinfo_notify(int event, struct net_bridge_port *port);
 int br_setlink(struct net_device *dev, struct nlmsghdr *nlmsg, u16 flags);
 int br_dellink(struct net_device *dev, struct nlmsghdr *nlmsg, u16 flags);
 int br_getlink(struct sk_buff *skb, u32 pid, u32 seq, struct net_device *dev,
-	       u32 filter_mask);
+	       u32 filter_mask, int nlflags);
 
 #ifdef CONFIG_SYSFS
 /* br_sysfs_if.c */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 358d52a38533..666e0928ba40 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2854,7 +2854,7 @@ static int brport_nla_put_flag(struct sk_buff *skb, u32 flags, u32 mask,
 
 int ndo_dflt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 			    struct net_device *dev, u16 mode,
-			    u32 flags, u32 mask)
+			    u32 flags, u32 mask, int nlflags)
 {
 	struct nlmsghdr *nlh;
 	struct ifinfomsg *ifm;
@@ -2863,7 +2863,7 @@ int ndo_dflt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 	u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN;
 	struct net_device *br_dev = netdev_master_upper_dev_get(dev);
 
-	nlh = nlmsg_put(skb, pid, seq, RTM_NEWLINK, sizeof(*ifm), NLM_F_MULTI);
+	nlh = nlmsg_put(skb, pid, seq, RTM_NEWLINK, sizeof(*ifm), nlflags);
 	if (nlh == NULL)
 		return -EMSGSIZE;
 
@@ -2969,7 +2969,8 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
 		if (br_dev && br_dev->netdev_ops->ndo_bridge_getlink) {
 			if (idx >= cb->args[0] &&
 			    br_dev->netdev_ops->ndo_bridge_getlink(
-				    skb, portid, seq, dev, filter_mask) < 0)
+				    skb, portid, seq, dev, filter_mask,
+				    NLM_F_MULTI) < 0)
 				break;
 			idx++;
 		}
@@ -2977,7 +2978,8 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
 		if (ops->ndo_bridge_getlink) {
 			if (idx >= cb->args[0] &&
 			    ops->ndo_bridge_getlink(skb, portid, seq, dev,
-						    filter_mask) < 0)
+						    filter_mask,
+						    NLM_F_MULTI) < 0)
 				break;
 			idx++;
 		}
@@ -3018,7 +3020,7 @@ static int rtnl_bridge_notify(struct net_device *dev)
 		goto errout;
 	}
 
-	err = dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
+	err = dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0, 0);
 	if (err < 0)
 		goto errout;
 
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net 3/3] tipc: remove wrong use of NLM_F_MULTI
  2015-04-28 16:33 [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI Nicolas Dichtel
  2015-04-28 16:33 ` [PATCH net 1/3] bridge/mdb: remove wrong use of NLM_F_MULTI Nicolas Dichtel
  2015-04-28 16:33 ` [PATCH net 2/3] bridge/nl: " Nicolas Dichtel
@ 2015-04-28 16:33 ` Nicolas Dichtel
  2015-04-28 16:33 ` [RFC PATCH net 4/4] netlink: add a flags field in the cb structure Nicolas Dichtel
  2015-04-29 19:00 ` [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI David Miller
  4 siblings, 0 replies; 7+ messages in thread
From: Nicolas Dichtel @ 2015-04-28 16:33 UTC (permalink / raw)
  To: netdev
  Cc: davem, dmitry.tarnyaginuug.ch, Nicolas Dichtel, Richard Alpe,
	Jon Maloy, Ying Xue, tipc-discussion

NLM_F_MULTI must be used only when a NLMSG_DONE message is sent. In fact,
it is sent only at the end of a dump.

Libraries like libnl will wait forever for NLMSG_DONE.

Fixes: 35b9dd7607f0 ("tipc: add bearer get/dump to new netlink api")
Fixes: 7be57fc69184 ("tipc: add link get/dump to new netlink api")
Fixes: 46f15c6794fb ("tipc: add media get/dump to new netlink api")
CC: Richard Alpe <richard.alpe@ericsson.com>
CC: Jon Maloy <jon.maloy@ericsson.com>
CC: Ying Xue <ying.xue@windriver.com>
CC: tipc-discussion@lists.sourceforge.net
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/tipc/bearer.c | 17 +++++++++--------
 net/tipc/link.c   |  8 ++++----
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 3613e72e858e..70e3dacbf84a 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -591,14 +591,14 @@ void tipc_bearer_stop(struct net *net)
 
 /* Caller should hold rtnl_lock to protect the bearer */
 static int __tipc_nl_add_bearer(struct tipc_nl_msg *msg,
-				struct tipc_bearer *bearer)
+				struct tipc_bearer *bearer, int nlflags)
 {
 	void *hdr;
 	struct nlattr *attrs;
 	struct nlattr *prop;
 
 	hdr = genlmsg_put(msg->skb, msg->portid, msg->seq, &tipc_genl_family,
-			  NLM_F_MULTI, TIPC_NL_BEARER_GET);
+			  nlflags, TIPC_NL_BEARER_GET);
 	if (!hdr)
 		return -EMSGSIZE;
 
@@ -657,7 +657,7 @@ int tipc_nl_bearer_dump(struct sk_buff *skb, struct netlink_callback *cb)
 		if (!bearer)
 			continue;
 
-		err = __tipc_nl_add_bearer(&msg, bearer);
+		err = __tipc_nl_add_bearer(&msg, bearer, NLM_F_MULTI);
 		if (err)
 			break;
 	}
@@ -705,7 +705,7 @@ int tipc_nl_bearer_get(struct sk_buff *skb, struct genl_info *info)
 		goto err_out;
 	}
 
-	err = __tipc_nl_add_bearer(&msg, bearer);
+	err = __tipc_nl_add_bearer(&msg, bearer, 0);
 	if (err)
 		goto err_out;
 	rtnl_unlock();
@@ -857,14 +857,14 @@ int tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
 }
 
 static int __tipc_nl_add_media(struct tipc_nl_msg *msg,
-			       struct tipc_media *media)
+			       struct tipc_media *media, int nlflags)
 {
 	void *hdr;
 	struct nlattr *attrs;
 	struct nlattr *prop;
 
 	hdr = genlmsg_put(msg->skb, msg->portid, msg->seq, &tipc_genl_family,
-			  NLM_F_MULTI, TIPC_NL_MEDIA_GET);
+			  nlflags, TIPC_NL_MEDIA_GET);
 	if (!hdr)
 		return -EMSGSIZE;
 
@@ -916,7 +916,8 @@ int tipc_nl_media_dump(struct sk_buff *skb, struct netlink_callback *cb)
 
 	rtnl_lock();
 	for (; media_info_array[i] != NULL; i++) {
-		err = __tipc_nl_add_media(&msg, media_info_array[i]);
+		err = __tipc_nl_add_media(&msg, media_info_array[i],
+					  NLM_F_MULTI);
 		if (err)
 			break;
 	}
@@ -963,7 +964,7 @@ int tipc_nl_media_get(struct sk_buff *skb, struct genl_info *info)
 		goto err_out;
 	}
 
-	err = __tipc_nl_add_media(&msg, media);
+	err = __tipc_nl_add_media(&msg, media, 0);
 	if (err)
 		goto err_out;
 	rtnl_unlock();
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 57be6e6aff99..a11cc5e5e0ae 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -2013,7 +2013,7 @@ msg_full:
 
 /* Caller should hold appropriate locks to protect the link */
 static int __tipc_nl_add_link(struct net *net, struct tipc_nl_msg *msg,
-			      struct tipc_link *link)
+			      struct tipc_link *link, int nlflags)
 {
 	int err;
 	void *hdr;
@@ -2022,7 +2022,7 @@ static int __tipc_nl_add_link(struct net *net, struct tipc_nl_msg *msg,
 	struct tipc_net *tn = net_generic(net, tipc_net_id);
 
 	hdr = genlmsg_put(msg->skb, msg->portid, msg->seq, &tipc_genl_family,
-			  NLM_F_MULTI, TIPC_NL_LINK_GET);
+			  nlflags, TIPC_NL_LINK_GET);
 	if (!hdr)
 		return -EMSGSIZE;
 
@@ -2095,7 +2095,7 @@ static int __tipc_nl_add_node_links(struct net *net, struct tipc_nl_msg *msg,
 		if (!node->links[i])
 			continue;
 
-		err = __tipc_nl_add_link(net, msg, node->links[i]);
+		err = __tipc_nl_add_link(net, msg, node->links[i], NLM_F_MULTI);
 		if (err)
 			return err;
 	}
@@ -2209,7 +2209,7 @@ int tipc_nl_link_get(struct sk_buff *skb, struct genl_info *info)
 		goto err_out;
 	}
 
-	err = __tipc_nl_add_link(net, &msg, link);
+	err = __tipc_nl_add_link(net, &msg, link, 0);
 	if (err)
 		goto err_out;
 
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH net 4/4] netlink: add a flags field in the cb structure
  2015-04-28 16:33 [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI Nicolas Dichtel
                   ` (2 preceding siblings ...)
  2015-04-28 16:33 ` [PATCH net 3/3] tipc: " Nicolas Dichtel
@ 2015-04-28 16:33 ` Nicolas Dichtel
  2015-04-29 19:00 ` [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI David Miller
  4 siblings, 0 replies; 7+ messages in thread
From: Nicolas Dichtel @ 2015-04-28 16:33 UTC (permalink / raw)
  To: netdev; +Cc: davem, dmitry.tarnyaginuug.ch, Nicolas Dichtel

The goal of this patch is to minimize copy and paste error. The flag
NLM_F_MULTI is often set in wrong cases because people copy and paste code
that uses this flag. When this flag is set, it means that it is a
multi-part netlink message which ends with a NLMSG_DONE.

In most of the cases, only a dump sends this NLMSG_DONE message. Let's set
this flag directly into the dump, hence people won't copy and paste
NLM_F_MULTI.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/linux/netlink.h  | 1 +
 net/core/rtnetlink.c     | 9 ++++-----
 net/netlink/af_netlink.c | 1 +
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 6835c1279df7..0a29bd255639 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -129,6 +129,7 @@ struct netlink_callback {
 	u16			family;
 	u16			min_dump_alloc;
 	unsigned int		prev_seq, seq;
+	int			flags;
 	long			args[6];
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 666e0928ba40..837d30b5ffed 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1357,8 +1357,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 			err = rtnl_fill_ifinfo(skb, dev, RTM_NEWLINK,
 					       NETLINK_CB(cb->skb).portid,
 					       cb->nlh->nlmsg_seq, 0,
-					       NLM_F_MULTI,
-					       ext_filter_mask);
+					       cb->flags, ext_filter_mask);
 			/* If we ran out of room on the first message,
 			 * we're in trouble
 			 */
@@ -2738,7 +2737,7 @@ static int nlmsg_populate_fdb(struct sk_buff *skb,
 		err = nlmsg_populate_fdb_fill(skb, dev, ha->addr, 0,
 					      portid, seq,
 					      RTM_NEWNEIGH, NTF_SELF,
-					      NLM_F_MULTI);
+					      cb->flags);
 		if (err < 0)
 			return err;
 skip:
@@ -2970,7 +2969,7 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
 			if (idx >= cb->args[0] &&
 			    br_dev->netdev_ops->ndo_bridge_getlink(
 				    skb, portid, seq, dev, filter_mask,
-				    NLM_F_MULTI) < 0)
+				    cb->flags) < 0)
 				break;
 			idx++;
 		}
@@ -2979,7 +2978,7 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
 			if (idx >= cb->args[0] &&
 			    ops->ndo_bridge_getlink(skb, portid, seq, dev,
 						    filter_mask,
-						    NLM_F_MULTI) < 0)
+						    cb->flags) < 0)
 				break;
 			idx++;
 		}
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index ec4adbdcb9b4..3e2d29703499 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2755,6 +2755,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 	cb->module = control->module;
 	cb->min_dump_alloc = control->min_dump_alloc;
 	cb->skb = skb;
+	cb->flags = NLM_F_MULTI;
 
 	nlk->cb_running = true;
 
-- 
2.2.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI
  2015-04-28 16:33 [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI Nicolas Dichtel
                   ` (3 preceding siblings ...)
  2015-04-28 16:33 ` [RFC PATCH net 4/4] netlink: add a flags field in the cb structure Nicolas Dichtel
@ 2015-04-29 19:00 ` David Miller
  4 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2015-04-29 19:00 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: netdev, dmitry.tarnyaginuug.ch

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 28 Apr 2015 18:33:47 +0200

> The first three patches fix a wrong use of this flag.
> 
> Because it's not the first time that this kind of bug happens, it could be
> good to find something to help people.
> 
> The only idea that I have is in the last patch. It's just an example, if
> people agrees on it, I will update all users and submit it formally.
> Any better idea is welcomed.

I've applied this series.

The only thing with patch #4 is that we have to make sure everyone
initializes all fields.  It would be nice if there were a way to
sort-of enforce that.

Even just having a static inline initializer helper func might be
enough, like I added for nf_hook_info.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 2/3] bridge/nl: remove wrong use of NLM_F_MULTI
  2015-04-28 16:33 ` [PATCH net 2/3] bridge/nl: " Nicolas Dichtel
@ 2015-04-30 13:01   ` Jeff Kirsher
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Kirsher @ 2015-04-30 13:01 UTC (permalink / raw)
  To: Nicolas Dichtel
  Cc: netdev, davem, dmitry.tarnyaginuug.ch, John Fastabend,
	Sathya Perla, Subbu Seetharaman, Ajit Khaparde, intel-wired-lan,
	Jiri Pirko, Scott Feldman, Stephen Hemminger, bridge

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]

On Tue, 2015-04-28 at 18:33 +0200, Nicolas Dichtel wrote:
> NLM_F_MULTI must be used only when a NLMSG_DONE message is sent. In
> fact,
> it is sent only at the end of a dump.
> 
> Libraries like libnl will wait forever for NLMSG_DONE.
> 
> Fixes: e5a55a898720 ("net: create generic bridge ops")
> Fixes: 815cccbf10b2 ("ixgbe: add setlink, getlink support to ixgbe and
> ixgbevf")
> CC: John Fastabend <john.r.fastabend@intel.com>
> CC: Sathya Perla <sathya.perla@emulex.com>
> CC: Subbu Seetharaman <subbu.seetharaman@emulex.com>
> CC: Ajit Khaparde <ajit.khaparde@emulex.com>
> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> CC: intel-wired-lan@lists.osuosl.org
> CC: Jiri Pirko <jiri@resnulli.us>
> CC: Scott Feldman <sfeldma@gmail.com>
> CC: Stephen Hemminger <stephen@networkplumber.org>
> CC: bridge@lists.linux-foundation.org
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

For the i40e and ixgbe driver changes

> ---
>  drivers/net/ethernet/emulex/benet/be_main.c   |  5 +++--
>  drivers/net/ethernet/intel/i40e/i40e_main.c   |  7 ++++---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  4 ++--
>  drivers/net/ethernet/rocker/rocker.c          |  5 +++--
>  include/linux/netdevice.h                     |  6 ++++--
>  include/linux/rtnetlink.h                     |  2 +-
>  net/bridge/br_netlink.c                       |  4 ++--
>  net/bridge/br_private.h                       |  2 +-
>  net/core/rtnetlink.c                          | 12 +++++++-----
>  9 files changed, 27 insertions(+), 20 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-04-30 13:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-28 16:33 [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI Nicolas Dichtel
2015-04-28 16:33 ` [PATCH net 1/3] bridge/mdb: remove wrong use of NLM_F_MULTI Nicolas Dichtel
2015-04-28 16:33 ` [PATCH net 2/3] bridge/nl: " Nicolas Dichtel
2015-04-30 13:01   ` Jeff Kirsher
2015-04-28 16:33 ` [PATCH net 3/3] tipc: " Nicolas Dichtel
2015-04-28 16:33 ` [RFC PATCH net 4/4] netlink: add a flags field in the cb structure Nicolas Dichtel
2015-04-29 19:00 ` [PATCH net 0/3] netlink: fix wrong use of the flag NLM_F_MULTI David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).