[ovs-dev] [PreQoS 07/13] netlink: Drop sock parameter from nl_msg_put_(ge)nlmsghdr().

Ben Pfaff blp at nicira.com
Thu May 27 20:33:06 UTC 2010


These two functions use their "sock" parameter only to figure out the
nlmsg_pid to put in the nlmsghdr.  But that field can be filled in just
as well right before sending the message.  Since our functions for sending
Netlink messages always modify the nlmsghdr anyhow (to fill in the length),
there is little benefit to filling in the nlmsg_pid in advance.  The cost,
on the other hand, is having to pass another argument to functions that
already have too many.  So this commit removes the argument.
---
 lib/netdev-linux.c         |    6 ++----
 lib/netlink.c              |   30 +++++++++++++++++-------------
 lib/netlink.h              |    9 ++++-----
 vswitchd/ovs-brcompatd.c   |    4 ++--
 vswitchd/proc-net-compat.c |    2 +-
 5 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 9a6d70a..554c5b0 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1151,8 +1151,7 @@ netdev_linux_remove_policing(struct netdev *netdev)
     }
 
     ofpbuf_init(&request, 0);
-    nl_msg_put_nlmsghdr(&request, rtnl_sock, sizeof *tcmsg,
-                        RTM_DELQDISC, NLM_F_REQUEST);
+    nl_msg_put_nlmsghdr(&request, sizeof *tcmsg, RTM_DELQDISC, NLM_F_REQUEST);
     tcmsg = ofpbuf_put_zeros(&request, sizeof *tcmsg);
     tcmsg->tcm_family = AF_UNSPEC;
     tcmsg->tcm_ifindex = ifindex;
@@ -1713,8 +1712,7 @@ get_stats_via_netlink(int ifindex, struct netdev_stats *stats)
     }
 
     ofpbuf_init(&request, 0);
-    nl_msg_put_nlmsghdr(&request, rtnl_sock, sizeof *ifi,
-                        RTM_GETLINK, NLM_F_REQUEST);
+    nl_msg_put_nlmsghdr(&request, sizeof *ifi, RTM_GETLINK, NLM_F_REQUEST);
     ifi = ofpbuf_put_zeros(&request, sizeof *ifi);
     ifi->ifi_family = PF_UNSPEC;
     ifi->ifi_index = ifindex;
diff --git a/lib/netlink.c b/lib/netlink.c
index e652a18..2ff20c8 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -195,8 +195,8 @@ nl_sock_destroy(struct nl_sock *sock)
 }
 
 /* Tries to send 'msg', which must contain a Netlink message, to the kernel on
- * 'sock'.  nlmsg_len in 'msg' will be finalized to match msg->size before the
- * message is sent.
+ * 'sock'.  nlmsg_len in 'msg' will be finalized to match msg->size, and
+ * nlmsg_pid will be set to 'sock''s pid, before the message is sent.
  *
  * Returns 0 if successful, otherwise a positive errno value.  If
  * 'wait' is true, then the send will wait until buffer space is ready;
@@ -204,9 +204,11 @@ nl_sock_destroy(struct nl_sock *sock)
 int
 nl_sock_send(struct nl_sock *sock, const struct ofpbuf *msg, bool wait) 
 {
+    struct nlmsghdr *nlmsg = nl_msg_nlmsghdr(msg);
     int error;
 
-    nl_msg_nlmsghdr(msg)->nlmsg_len = msg->size;
+    nlmsg->nlmsg_len = msg->size;
+    nlmsg->nlmsg_pid = sock->pid;
     do {
         int retval;
         retval = send(sock->fd, msg->data, msg->size, wait ? 0 : MSG_DONTWAIT);
@@ -221,7 +223,7 @@ nl_sock_send(struct nl_sock *sock, const struct ofpbuf *msg, bool wait)
 
 /* Tries to send the 'n_iov' chunks of data in 'iov' to the kernel on 'sock' as
  * a single Netlink message.  (The message must be fully formed and not require
- * finalization of its nlmsg_len field.)
+ * finalization of its nlmsg_len or nlmsg_pid fields.)
  *
  * Returns 0 if successful, otherwise a positive errno value.  If 'wait' is
  * true, then the send will wait until buffer space is ready; otherwise,
@@ -343,6 +345,10 @@ try_again:
  * responsible for destroying the reply with ofpbuf_delete().  On failure,
  * returns a positive errno value and stores a null pointer into '*replyp'.
  *
+ * nlmsg_len in 'msg' will be finalized to match msg->size, and nlmsg_pid will
+ * be set to 'sock''s pid, before the message is sent.  NLM_F_ACK will be set
+ * in nlmsg_flags.
+ *
  * The caller is responsible for destroying 'request'.
  *
  * Bare Netlink is an unreliable transport protocol.  This function layers
@@ -503,7 +509,7 @@ nl_msg_reserve(struct ofpbuf *msg, size_t size)
  * nl_msg_put_genlmsghdr is more convenient for composing a Generic Netlink
  * message. */
 void
-nl_msg_put_nlmsghdr(struct ofpbuf *msg, struct nl_sock *sock,
+nl_msg_put_nlmsghdr(struct ofpbuf *msg,
                     size_t expected_payload, uint32_t type, uint32_t flags) 
 {
     struct nlmsghdr *nlmsghdr;
@@ -516,7 +522,7 @@ nl_msg_put_nlmsghdr(struct ofpbuf *msg, struct nl_sock *sock,
     nlmsghdr->nlmsg_type = type;
     nlmsghdr->nlmsg_flags = flags;
     nlmsghdr->nlmsg_seq = ++next_seq;
-    nlmsghdr->nlmsg_pid = sock->pid;
+    nlmsghdr->nlmsg_pid = 0;
 }
 
 /* Puts a nlmsghdr and genlmsghdr at the beginning of 'msg', which must be
@@ -539,14 +545,12 @@ nl_msg_put_nlmsghdr(struct ofpbuf *msg, struct nl_sock *sock,
  * nl_msg_put_nlmsghdr should be used to compose Netlink messages that are not
  * Generic Netlink messages. */
 void
-nl_msg_put_genlmsghdr(struct ofpbuf *msg, struct nl_sock *sock,
-                      size_t expected_payload, int family, uint32_t flags,
-                      uint8_t cmd, uint8_t version)
+nl_msg_put_genlmsghdr(struct ofpbuf *msg, size_t expected_payload,
+                      int family, uint32_t flags, uint8_t cmd, uint8_t version)
 {
     struct genlmsghdr *genlmsghdr;
 
-    nl_msg_put_nlmsghdr(msg, sock, GENL_HDRLEN + expected_payload,
-                        family, flags);
+    nl_msg_put_nlmsghdr(msg, GENL_HDRLEN + expected_payload, family, flags);
     assert(msg->size == NLMSG_HDRLEN);
     genlmsghdr = nl_msg_put_uninit(msg, GENL_HDRLEN);
     genlmsghdr->cmd = cmd;
@@ -877,7 +881,7 @@ static int do_lookup_genl_family(const char *name)
     }
 
     ofpbuf_init(&request, 0);
-    nl_msg_put_genlmsghdr(&request, sock, 0, GENL_ID_CTRL, NLM_F_REQUEST,
+    nl_msg_put_genlmsghdr(&request, 0, GENL_ID_CTRL, NLM_F_REQUEST,
                           CTRL_CMD_GETFAMILY, 1);
     nl_msg_put_string(&request, CTRL_ATTR_FAMILY_NAME, name);
     retval = nl_sock_transact(sock, &request, &reply);
diff --git a/lib/netlink.h b/lib/netlink.h
index fd918d1..dc68921 100644
--- a/lib/netlink.h
+++ b/lib/netlink.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -59,11 +59,10 @@ bool nl_msg_nlmsgerr(const struct ofpbuf *, int *error);
 void nl_msg_reserve(struct ofpbuf *, size_t);
 
 /* Appending headers and raw data. */
-void nl_msg_put_nlmsghdr(struct ofpbuf *, struct nl_sock *,
-                         size_t expected_payload,
+void nl_msg_put_nlmsghdr(struct ofpbuf *, size_t expected_payload,
                          uint32_t type, uint32_t flags);
-void nl_msg_put_genlmsghdr(struct ofpbuf *, struct nl_sock *,
-                           size_t expected_payload, int family, uint32_t flags,
+void nl_msg_put_genlmsghdr(struct ofpbuf *, size_t expected_payload,
+                           int family, uint32_t flags,
                            uint8_t cmd, uint8_t version);
 void nl_msg_put(struct ofpbuf *, const void *, size_t);
 void *nl_msg_put_uninit(struct ofpbuf *, size_t);
diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c
index 2950301..53da789 100644
--- a/vswitchd/ovs-brcompatd.c
+++ b/vswitchd/ovs-brcompatd.c
@@ -115,7 +115,7 @@ lookup_brc_multicast_group(int *multicast_group)
         return retval;
     }
     ofpbuf_init(&request, 0);
-    nl_msg_put_genlmsghdr(&request, sock, 0, brc_family,
+    nl_msg_put_genlmsghdr(&request, 0, brc_family,
             NLM_F_REQUEST, BRC_GENL_C_QUERY_MC, 1);
     retval = nl_sock_transact(sock, &request, &reply);
     ofpbuf_uninit(&request);
@@ -725,7 +725,7 @@ static struct ofpbuf *
 compose_reply(uint32_t seq, int error)
 {
     struct ofpbuf *reply = ofpbuf_new(4096);
-    nl_msg_put_genlmsghdr(reply, brc_sock, 32, brc_family, NLM_F_REQUEST,
+    nl_msg_put_genlmsghdr(reply, 32, brc_family, NLM_F_REQUEST,
                           BRC_GENL_C_DP_RESULT, 1);
     ((struct nlmsghdr *) reply->data)->nlmsg_seq = seq;
     nl_msg_put_u32(reply, BRC_GENL_A_ERR_CODE, error);
diff --git a/vswitchd/proc-net-compat.c b/vswitchd/proc-net-compat.c
index 3b5c0ca..72eb7be 100644
--- a/vswitchd/proc-net-compat.c
+++ b/vswitchd/proc-net-compat.c
@@ -76,7 +76,7 @@ set_proc_file(const char *dir, const char *file, const char *data)
     int retval;
 
     ofpbuf_init(&request, 0);
-    nl_msg_put_genlmsghdr(&request, brc_sock, 1024, brc_family, NLM_F_REQUEST,
+    nl_msg_put_genlmsghdr(&request, 1024, brc_family, NLM_F_REQUEST,
                           BRC_GENL_C_SET_PROC, 1);
     nl_msg_put_string(&request, BRC_GENL_A_PROC_DIR, dir);
     nl_msg_put_string(&request, BRC_GENL_A_PROC_NAME, file);
-- 
1.7.1





More information about the dev mailing list