[ovs-dev] [PATCH 2/3] datapath: Slim down the vport interface.

Jesse Gross jesse at nicira.com
Sun Nov 6 20:34:24 UTC 2011


Many of the function in vport.c are simply pass throughs to their
underlying vport implementation and, of these, many are used only
for bridge compatibility code.  This allows users of these functions
to directly call through the ops structure, reducing boilerplate code
and keeping more of the compatibility code together.

Signed-off-by: Jesse Gross <jesse at nicira.com>
---
 datapath/datapath.c    |   62 ++++++++++++---------
 datapath/dp_sysfs_dp.c |   10 ++--
 datapath/dp_sysfs_if.c |   14 +++---
 datapath/vport.c       |  139 ++----------------------------------------------
 datapath/vport.h       |   14 -----
 5 files changed, 52 insertions(+), 187 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 3657fe0..929ea09 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -123,7 +123,8 @@ static struct vport *get_vport_protected(struct datapath *dp, u16 port_no)
 /* Must be called with rcu_read_lock or RTNL lock. */
 const char *dp_name(const struct datapath *dp)
 {
-	return vport_get_name(rcu_dereference_rtnl(dp->ports[OVSP_LOCAL]));
+	struct vport *vport = rcu_dereference_rtnl(dp->ports[OVSP_LOCAL]);
+	return vport->ops->get_name(vport);
 }
 
 static int get_dpifindex(struct datapath *dp)
@@ -135,7 +136,7 @@ static int get_dpifindex(struct datapath *dp)
 
 	local = get_vport_protected(dp, OVSP_LOCAL);
 	if (local)
-		ifindex = vport_get_ifindex(local);
+		ifindex = local->ops->get_ifindex(local);
 	else
 		ifindex = 0;
 
@@ -160,12 +161,11 @@ static int dp_fill_ifinfo(struct sk_buff *skb,
 			  int event, unsigned int flags)
 {
 	struct datapath *dp = port->dp;
-	int ifindex = vport_get_ifindex(port);
 	struct ifinfomsg *hdr;
 	struct nlmsghdr *nlh;
 
-	if (ifindex < 0)
-		return ifindex;
+	if (!port->ops->get_ifindex)
+		return -ENODEV;
 
 	nlh = nlmsg_put(skb, 0, 0, event, sizeof(*hdr), flags);
 	if (nlh == NULL)
@@ -175,21 +175,21 @@ static int dp_fill_ifinfo(struct sk_buff *skb,
 	hdr->ifi_family = AF_BRIDGE;
 	hdr->__ifi_pad = 0;
 	hdr->ifi_type = ARPHRD_ETHER;
-	hdr->ifi_index = ifindex;
-	hdr->ifi_flags = vport_get_flags(port);
+	hdr->ifi_index = port->ops->get_ifindex(port);
+	hdr->ifi_flags = port->ops->get_dev_flags(port);
 	hdr->ifi_change = 0;
 
-	NLA_PUT_STRING(skb, IFLA_IFNAME, vport_get_name(port));
+	NLA_PUT_STRING(skb, IFLA_IFNAME, port->ops->get_name(port));
 	NLA_PUT_U32(skb, IFLA_MASTER, get_dpifindex(dp));
-	NLA_PUT_U32(skb, IFLA_MTU, vport_get_mtu(port));
+	NLA_PUT_U32(skb, IFLA_MTU, port->ops->get_mtu(port));
 #ifdef IFLA_OPERSTATE
 	NLA_PUT_U8(skb, IFLA_OPERSTATE,
-		   vport_is_running(port)
-			? vport_get_operstate(port)
+		   port->ops->is_running(port)
+			? port->ops->get_operstate(port)
 			: IF_OPER_DOWN);
 #endif
 
-	NLA_PUT(skb, IFLA_ADDRESS, ETH_ALEN, vport_get_addr(port));
+	NLA_PUT(skb, IFLA_ADDRESS, ETH_ALEN, port->ops->get_addr(port));
 
 	return nlmsg_end(skb, nlh);
 
@@ -202,24 +202,32 @@ nla_put_failure:
 static void dp_ifinfo_notify(int event, struct vport *port)
 {
 	struct sk_buff *skb;
-	int err = -ENOBUFS;
+	int err;
 
 	skb = nlmsg_new(br_nlmsg_size(), GFP_KERNEL);
-	if (skb == NULL)
-		goto errout;
+	if (!skb) {
+		err = -ENOBUFS;
+		goto err;
+	}
 
 	err = dp_fill_ifinfo(skb, port, event, 0);
 	if (err < 0) {
-		/* -EMSGSIZE implies BUG in br_nlmsg_size() */
-		WARN_ON(err == -EMSGSIZE);
-		kfree_skb(skb);
-		goto errout;
+		if (err == -ENODEV) {
+			goto out;
+		} else {
+			/* -EMSGSIZE implies BUG in br_nlmsg_size() */
+			WARN_ON(err == -EMSGSIZE);
+			goto err;
+		}
 	}
+
 	rtnl_notify(skb, &init_net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL);
+
 	return;
-errout:
-	if (err < 0)
-		rtnl_set_sk_err(&init_net, RTNLGRP_LINK, err);
+err:
+	rtnl_set_sk_err(&init_net, RTNLGRP_LINK, err);
+out:
+	kfree_skb(skb);
 }
 
 static void release_dp(struct kobject *kobj)
@@ -1609,8 +1617,8 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
 	ovs_header->dp_ifindex = get_dpifindex(vport->dp);
 
 	NLA_PUT_U32(skb, OVS_VPORT_ATTR_PORT_NO, vport->port_no);
-	NLA_PUT_U32(skb, OVS_VPORT_ATTR_TYPE, vport_get_type(vport));
-	NLA_PUT_STRING(skb, OVS_VPORT_ATTR_NAME, vport_get_name(vport));
+	NLA_PUT_U32(skb, OVS_VPORT_ATTR_TYPE, vport->ops->type);
+	NLA_PUT_STRING(skb, OVS_VPORT_ATTR_NAME, vport->ops->get_name(vport));
 	NLA_PUT_U32(skb, OVS_VPORT_ATTR_UPCALL_PID, vport->upcall_pid);
 
 	nla = nla_reserve(skb, OVS_VPORT_ATTR_STATS, sizeof(struct ovs_vport_stats));
@@ -1619,7 +1627,8 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
 
 	vport_get_stats(vport, nla_data(nla));
 
-	NLA_PUT(skb, OVS_VPORT_ATTR_ADDRESS, ETH_ALEN, vport_get_addr(vport));
+	NLA_PUT(skb, OVS_VPORT_ATTR_ADDRESS, ETH_ALEN,
+		vport->ops->get_addr(vport));
 
 	err = vport_get_options(vport, skb);
 	if (err == -EMSGSIZE)
@@ -1804,7 +1813,8 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
 		goto exit_unlock;
 
 	err = 0;
-	if (a[OVS_VPORT_ATTR_TYPE] && nla_get_u32(a[OVS_VPORT_ATTR_TYPE]) != vport_get_type(vport))
+	if (a[OVS_VPORT_ATTR_TYPE] &&
+	    nla_get_u32(a[OVS_VPORT_ATTR_TYPE]) != vport->ops->type)
 		err = -EINVAL;
 	if (!err && a[OVS_VPORT_ATTR_OPTIONS])
 		err = vport_set_options(vport, a[OVS_VPORT_ATTR_OPTIONS]);
diff --git a/datapath/dp_sysfs_dp.c b/datapath/dp_sysfs_dp.c
index 16aa787..9783838 100644
--- a/datapath/dp_sysfs_dp.c
+++ b/datapath/dp_sysfs_dp.c
@@ -223,7 +223,7 @@ static ssize_t show_bridge_id(DEVICE_PARAMS, char *buf)
 	if (vport) {
 		const unsigned char *addr;
 
-		addr = vport_get_addr(vport);
+		addr = vport->ops->get_addr(vport);
 		result = sprintf(buf, "%.2x%.2x.%.2x%.2x%.2x%.2x%.2x%.2x\n",
 				 0, 0, addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
 	} else
@@ -350,8 +350,8 @@ static struct attribute_group bridge_group = {
  */
 int dp_sysfs_add_dp(struct datapath *dp)
 {
-	struct kobject *kobj =
-		vport_get_kobj(rtnl_dereference(dp->ports[OVSP_LOCAL]));
+	struct vport *vport = rtnl_dereference(dp->ports[OVSP_LOCAL]);
+	struct kobject *kobj = vport->ops->get_kobj(vport);
 	int err;
 
 	/* Create /sys/class/net/<devname>/bridge directory. */
@@ -380,8 +380,8 @@ int dp_sysfs_add_dp(struct datapath *dp)
 
 int dp_sysfs_del_dp(struct datapath *dp)
 {
-	struct kobject *kobj =
-		vport_get_kobj(rtnl_dereference(dp->ports[OVSP_LOCAL]));
+	struct vport *vport = rtnl_dereference(dp->ports[OVSP_LOCAL]);
+	struct kobject *kobj = vport->ops->get_kobj(vport);
 
 	kobject_del(&dp->ifobj);
 	sysfs_remove_group(kobj, &bridge_group);
diff --git a/datapath/dp_sysfs_if.c b/datapath/dp_sysfs_if.c
index c37e11f..7843f56 100644
--- a/datapath/dp_sysfs_if.c
+++ b/datapath/dp_sysfs_if.c
@@ -205,23 +205,23 @@ struct sysfs_ops brport_sysfs_ops = {
  */
 int dp_sysfs_add_if(struct vport *p)
 {
-	struct kobject *kobj = vport_get_kobj(p);
 	struct datapath *dp = p->dp;
+	struct vport *local_port = rtnl_dereference(dp->ports[OVSP_LOCAL]);
 	struct brport_attribute **a;
 	int err;
 
 	/* Create /sys/class/net/<devname>/brport directory. */
-	if (!kobj)
+	if (!p->ops->get_kobj)
 		return -ENOENT;
 
-	err = kobject_add(&p->kobj, kobj, SYSFS_BRIDGE_PORT_ATTR);
+	err = kobject_add(&p->kobj, p->ops->get_kobj(p),
+			  SYSFS_BRIDGE_PORT_ATTR);
 	if (err)
 		goto err;
 
 	/* Create symlink from /sys/class/net/<devname>/brport/bridge to
 	 * /sys/class/net/<bridgename>. */
-	err = sysfs_create_link(&p->kobj,
-		vport_get_kobj(rtnl_dereference(dp->ports[OVSP_LOCAL])),
+	err = sysfs_create_link(&p->kobj, local_port->ops->get_kobj(local_port),
 		SYSFS_BRIDGE_PORT_LINK); /* "bridge" */
 	if (err)
 		goto err_del;
@@ -235,10 +235,10 @@ int dp_sysfs_add_if(struct vport *p)
 
 	/* Create symlink from /sys/class/net/<bridgename>/brif/<devname> to
 	 * /sys/class/net/<devname>/brport.  */
-	err = sysfs_create_link(&dp->ifobj, &p->kobj, vport_get_name(p));
+	err = sysfs_create_link(&dp->ifobj, &p->kobj, p->ops->get_name(p));
 	if (err)
 		goto err_del;
-	strcpy(p->linkname, vport_get_name(p));
+	strcpy(p->linkname, p->ops->get_name(p));
 
 	kobject_uevent(&p->kobj, KOBJ_ADD);
 
diff --git a/datapath/vport.c b/datapath/vport.c
index ad5a10e..6fe6042 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -129,7 +129,7 @@ struct vport *vport_locate(const char *name)
 	struct hlist_node *node;
 
 	hlist_for_each_entry_rcu(vport, node, bucket, hash_node)
-		if (!strcmp(name, vport_get_name(vport)))
+		if (!strcmp(name, vport->ops->get_name(vport)))
 			return vport;
 
 	return NULL;
@@ -159,7 +159,8 @@ static struct kobj_type brport_ktype = {
  * vport_priv().  vports that are no longer needed should be released with
  * vport_free().
  */
-struct vport *vport_alloc(int priv_size, const struct vport_ops *ops, const struct vport_parms *parms)
+struct vport *vport_alloc(int priv_size, const struct vport_ops *ops,
+			  const struct vport_parms *parms)
 {
 	struct vport *vport;
 	size_t alloc_size;
@@ -235,7 +236,7 @@ struct vport *vport_add(const struct vport_parms *parms)
 			}
 
 			hlist_add_head_rcu(&vport->hash_node,
-					   hash_bucket(vport_get_name(vport)));
+					   hash_bucket(vport->ops->get_name(vport)));
 			return vport;
 		}
 	}
@@ -327,61 +328,6 @@ void vport_set_stats(struct vport *vport, struct ovs_vport_stats *stats)
 }
 
 /**
- *	vport_get_name - retrieve device name
- *
- * @vport: vport from which to retrieve the name.
- *
- * Retrieves the name of the given device.  Either RTNL lock or rcu_read_lock
- * must be held for the entire duration that the name is in use.
- */
-const char *vport_get_name(const struct vport *vport)
-{
-	return vport->ops->get_name(vport);
-}
-
-/**
- *	vport_get_type - retrieve device type
- *
- * @vport: vport from which to retrieve the type.
- *
- * Retrieves the type of the given device.
- */
-enum ovs_vport_type vport_get_type(const struct vport *vport)
-{
-	return vport->ops->type;
-}
-
-/**
- *	vport_get_addr - retrieve device Ethernet address (for kernel callers)
- *
- * @vport: vport from which to retrieve the Ethernet address.
- *
- * Retrieves the Ethernet address of the given device.  Either RTNL lock or
- * rcu_read_lock must be held for the entire duration that the Ethernet address
- * is in use.
- */
-const unsigned char *vport_get_addr(const struct vport *vport)
-{
-	return vport->ops->get_addr(vport);
-}
-
-/**
- *	vport_get_kobj - retrieve associated kobj
- *
- * @vport: vport from which to retrieve the associated kobj
- *
- * Retrieves the associated kobj or null if no kobj.  The returned kobj is
- * valid for as long as the vport exists.
- */
-struct kobject *vport_get_kobj(const struct vport *vport)
-{
-	if (vport->ops->get_kobj)
-		return vport->ops->get_kobj(vport);
-	else
-		return NULL;
-}
-
-/**
  *	vport_get_stats - retrieve device stats
  *
  * @vport: vport from which to retrieve the stats
@@ -437,83 +383,6 @@ void vport_get_stats(struct vport *vport, struct ovs_vport_stats *stats)
 }
 
 /**
- *	vport_get_flags - retrieve device flags
- *
- * @vport: vport from which to retrieve the flags
- *
- * Retrieves the flags of the given device.
- *
- * Must be called with RTNL lock or rcu_read_lock.
- */
-unsigned vport_get_flags(const struct vport *vport)
-{
-	return vport->ops->get_dev_flags(vport);
-}
-
-/**
- *	vport_get_flags - check whether device is running
- *
- * @vport: vport on which to check status.
- *
- * Checks whether the given device is running.
- *
- * Must be called with RTNL lock or rcu_read_lock.
- */
-int vport_is_running(const struct vport *vport)
-{
-	return vport->ops->is_running(vport);
-}
-
-/**
- *	vport_get_flags - retrieve device operating state
- *
- * @vport: vport from which to check status
- *
- * Retrieves the RFC2863 operstate of the given device.
- *
- * Must be called with RTNL lock or rcu_read_lock.
- */
-unsigned char vport_get_operstate(const struct vport *vport)
-{
-	return vport->ops->get_operstate(vport);
-}
-
-/**
- *	vport_get_ifindex - retrieve device system interface index
- *
- * @vport: vport from which to retrieve index
- *
- * Retrieves the system interface index of the given device or 0 if
- * the device does not have one (in the case of virtual ports).
- * Returns a negative index on error.
- *
- * Must be called with RTNL lock or rcu_read_lock.
- */
-int vport_get_ifindex(const struct vport *vport)
-{
-	if (vport->ops->get_ifindex)
-		return vport->ops->get_ifindex(vport);
-	else
-		return 0;
-}
-
-/**
- *	vport_get_mtu - retrieve device MTU
- *
- * @vport: vport from which to retrieve MTU
- *
- * Retrieves the MTU of the given device.  Returns 0 if @vport does not have an
- * MTU (as e.g. some tunnels do not).  Either RTNL lock or rcu_read_lock must
- * be held.
- */
-int vport_get_mtu(const struct vport *vport)
-{
-	if (!vport->ops->get_mtu)
-		return 0;
-	return vport->ops->get_mtu(vport);
-}
-
-/**
  *	vport_get_options - retrieve device options
  *
  * @vport: vport from which to retrieve the options.
diff --git a/datapath/vport.h b/datapath/vport.h
index 2c9c4aa..1ce39ce 100644
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -32,22 +32,8 @@ struct vport *vport_locate(const char *name);
 
 int vport_set_addr(struct vport *, const unsigned char *);
 void vport_set_stats(struct vport *, struct ovs_vport_stats *);
-
-const char *vport_get_name(const struct vport *);
-enum ovs_vport_type vport_get_type(const struct vport *);
-const unsigned char *vport_get_addr(const struct vport *);
-
-struct kobject *vport_get_kobj(const struct vport *);
 void vport_get_stats(struct vport *, struct ovs_vport_stats *);
 
-unsigned vport_get_flags(const struct vport *);
-int vport_is_running(const struct vport *);
-unsigned char vport_get_operstate(const struct vport *);
-
-int vport_get_ifindex(const struct vport *);
-
-int vport_get_mtu(const struct vport *);
-
 int vport_set_options(struct vport *, struct nlattr *options);
 int vport_get_options(const struct vport *, struct sk_buff *);
 
-- 
1.7.5.4




More information about the dev mailing list