[ovs-dev] brport_sysfs_ops

Simon Horman horms at verge.net.au
Thu Sep 2 01:56:32 UTC 2010


On Thu, Sep 02, 2010 at 10:07:00AM +0900, Simon Horman wrote:
> On Wed, Sep 01, 2010 at 11:48:54AM -0700, Jesse Gross wrote:
> > On Tue, Aug 31, 2010 at 7:55 PM, Simon Horman <horms at verge.net.au> wrote:
> > > On Tue, Aug 31, 2010 at 12:53:38PM -0700, Jesse Gross wrote:
> > >> On Tue, Aug 31, 2010 at 12:53 AM, Simon Horman <horms at verge.net.au> wrote:
> > >> > Hi Jesse,
> > >> >
> > >> > I just came across the following error:
> > >> >
> > >> > net/openvswitch/built-in.o:(.data+0x610): multiple definition of `brport_sysfs_ops'
> > >> > net/bridge/built-in.o:(.rodata+0x1140): first defined here
> > >> >
> > >> > I seem to recall some discussion of these sysfs entries at the meeting in
> > >> > Boston. Was the decision to remove them?
> > >>
> > >> I'm not sure that we actually reached a decision regarding sysfs
> > >> entries.  However, I would be inclined to remove them now and then
> > >> perhaps add a few back in later if needed.  A lot of them are purely
> > >> for bridge compatibility and will most likely never make sense for
> > >> Open vSwitch.  For the ones that do, we can figure it out once we know
> > >> exactly how they will be used in their own right.
> > >
> > > I think that is a good strategy.
> > >
> > >> Unfortunately, this can probably only be done for the upstreamed
> > >> version.  Since a number of people are using bridge compatibility from
> > >> the Open vSwitch tree, we'll need to leave the sysfs code there.  I'm
> > >> hoping that this won't be too difficult as the sysfs code is
> > >> relatively self contained.
> > >
> > > Understood.
> > >
> > > The following patch seems to do the trick.
> > > I'll include it in the merge.
> > 
> > This patch looks good but I think we can go a step farther.  There's a
> > bunch of kobj stuff that is sprinkled around that was only used by
> > sysfs.  I would be tempted to remove it now that we no longer need it.
> 
> I was wondering about that, I'll remove it.

The following patch seems to do the trick.
It applies on top of the previous sysfs removal patch.

Index: net-next-2.6/net/openvswitch/datapath.c
===================================================================
--- net-next-2.6.orig/net/openvswitch/datapath.c	2010-09-02 10:37:43.000000000 +0900
+++ net-next-2.6/net/openvswitch/datapath.c	2010-09-02 10:44:31.000000000 +0900
@@ -199,16 +199,6 @@ errout:
 		rtnl_set_sk_err(&init_net, RTNLGRP_LINK, err);
 }
 
-static void release_dp(struct kobject *kobj)
-{
-	struct datapath *dp = container_of(kobj, struct datapath, ifobj);
-	kfree(dp);
-}
-
-static struct kobj_type dp_ktype = {
-	.release = release_dp
-};
-
 static int create_dp(int dp_idx, const char __user *devnamep)
 {
 	struct odp_port internal_dev_port;
@@ -254,11 +244,6 @@ static int create_dp(int dp_idx, const c
 		skb_queue_head_init(&dp->queues[i]);
 	init_waitqueue_head(&dp->waitqueue);
 
-	/* Initialize kobject for bridge.  This will be added as
-	 * /sys/class/net/<devname>/brif later, if sysfs is enabled. */
-	dp->ifobj.kset = NULL;
-	kobject_init(&dp->ifobj, &dp_ktype);
-
 	/* Allocate table. */
 	err = -ENOMEM;
 	rcu_assign_pointer(dp->table, tbl_create(0));
@@ -323,7 +308,6 @@ static void do_destroy_dp(struct datapat
 	for (i = 0; i < DP_MAX_GROUPS; i++)
 		kfree(dp->groups[i]);
 	free_percpu(dp->stats_percpu);
-	kobject_put(&dp->ifobj);
 	module_put(THIS_MODULE);
 }
 
@@ -348,16 +332,6 @@ err_unlock:
 	return err;
 }
 
-static void release_dp_port(struct kobject *kobj)
-{
-	struct dp_port *p = container_of(kobj, struct dp_port, kobj);
-	kfree(p);
-}
-
-static struct kobj_type brport_ktype = {
-	.release = release_dp_port
-};
-
 /* Called with RTNL lock and dp_mutex. */
 static int new_dp_port(struct datapath *dp, struct odp_port *odp_port, int port_no)
 {
@@ -399,11 +373,6 @@ static int new_dp_port(struct datapath *
 	list_add_rcu(&p->node, &dp->port_list);
 	dp->n_ports++;
 
-	/* Initialize kobject for bridge.  This will be added as
-	 * /sys/class/net/<devname>/brport later, if sysfs is enabled. */
-	p->kobj.kset = NULL;
-	kobject_init(&p->kobj, &brport_ktype);
-
 	dp_ifinfo_notify(RTM_NEWLINK, p);
 
 	return 0;
@@ -481,8 +450,6 @@ int dp_detach_port(struct dp_port *p, in
 		}
 	}
 
-	kobject_put(&p->kobj);
-
 	return 0;
 }
 
Index: net-next-2.6/net/openvswitch/datapath.h
===================================================================
--- net-next-2.6.orig/net/openvswitch/datapath.h	2010-09-02 10:37:43.000000000 +0900
+++ net-next-2.6/net/openvswitch/datapath.h	2010-09-02 10:44:51.000000000 +0900
@@ -85,7 +85,6 @@ struct dp_port_group {
 struct datapath {
 	struct mutex mutex;
 	int dp_idx;
-	struct kobject ifobj;
 
 	int drop_frags;
 
@@ -117,7 +116,6 @@ struct datapath {
  * @dp: Datapath to which this port belongs.
  * @vport: The network device attached to this port.  The contents depends on
  * the device and should be accessed only through the vport_* functions.
- * @kobj: Represents /sys/class/net/<devname>/brport.
  * @linkname: The name of the link from /sys/class/net/<datapath>/brif to this
  * &struct dp_port.  (We keep this around so that we can delete it if the
  * device gets renamed.)  Set to the null string when no link exists.
@@ -129,7 +127,6 @@ struct dp_port {
 	u16 port_no;
 	struct datapath	*dp;
 	struct vport *vport;
-	struct kobject kobj;
 	char linkname[IFNAMSIZ];
 	struct list_head node;
 	atomic_t sflow_pool;
Index: net-next-2.6/net/openvswitch/vport-internal_dev.c
===================================================================
--- net-next-2.6.orig/net/openvswitch/vport-internal_dev.c	2010-09-02 10:37:45.000000000 +0900
+++ net-next-2.6/net/openvswitch/vport-internal_dev.c	2010-09-02 10:44:58.000000000 +0900
@@ -302,7 +302,6 @@ struct vport_ops internal_vport_ops = {
 	.set_addr	= netdev_set_addr,
 	.get_name	= netdev_get_name,
 	.get_addr	= netdev_get_addr,
-	.get_kobj	= netdev_get_kobj,
 	.get_dev_flags	= netdev_get_dev_flags,
 	.is_running	= netdev_is_running,
 	.get_operstate	= netdev_get_operstate,
Index: net-next-2.6/net/openvswitch/vport-netdev.c
===================================================================
--- net-next-2.6.orig/net/openvswitch/vport-netdev.c	2010-09-02 10:37:45.000000000 +0900
+++ net-next-2.6/net/openvswitch/vport-netdev.c	2010-09-02 10:45:09.000000000 +0900
@@ -200,12 +200,6 @@ const unsigned char *netdev_get_addr(con
 	return netdev_vport->dev->dev_addr;
 }
 
-struct kobject *netdev_get_kobj(const struct vport *vport)
-{
-	const struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
-	return &netdev_vport->dev->dev.kobj;
-}
-
 int netdev_get_stats(const struct vport *vport, struct odp_vport_stats *stats)
 {
 	const struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
@@ -335,7 +329,6 @@ struct vport_ops netdev_vport_ops = {
 	.set_addr	= netdev_set_addr,
 	.get_name	= netdev_get_name,
 	.get_addr	= netdev_get_addr,
-	.get_kobj	= netdev_get_kobj,
 	.get_stats	= netdev_get_stats,
 	.get_dev_flags	= netdev_get_dev_flags,
 	.is_running	= netdev_is_running,
Index: net-next-2.6/net/openvswitch/vport-netdev.h
===================================================================
--- net-next-2.6.orig/net/openvswitch/vport-netdev.h	2010-09-02 10:37:45.000000000 +0900
+++ net-next-2.6/net/openvswitch/vport-netdev.h	2010-09-02 10:45:16.000000000 +0900
@@ -29,7 +29,6 @@ int netdev_set_mtu(struct vport *, int m
 int netdev_set_addr(struct vport *, const unsigned char *addr);
 const char *netdev_get_name(const struct vport *);
 const unsigned char *netdev_get_addr(const struct vport *);
-struct kobject *netdev_get_kobj(const struct vport *);
 int netdev_get_stats(const struct vport *, struct odp_vport_stats *);
 unsigned netdev_get_dev_flags(const struct vport *);
 int netdev_is_running(const struct vport *);
Index: net-next-2.6/net/openvswitch/vport.c
===================================================================
--- net-next-2.6.orig/net/openvswitch/vport.c	2010-09-02 10:37:45.000000000 +0900
+++ net-next-2.6/net/openvswitch/vport.c	2010-09-02 10:45:30.000000000 +0900
@@ -975,22 +975,6 @@ struct dp_port *vport_get_dp_port(const
 }
 
 /**
- *	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 (for kernel callers)
  *
  * @vport: vport from which to retrieve the stats
Index: net-next-2.6/net/openvswitch/vport.h
===================================================================
--- net-next-2.6.orig/net/openvswitch/vport.h	2010-09-02 10:37:46.000000000 +0900
+++ net-next-2.6/net/openvswitch/vport.h	2010-09-02 10:45:48.000000000 +0900
@@ -63,7 +63,6 @@ const char *vport_get_type(const struct
 const unsigned char *vport_get_addr(const struct vport *);
 
 struct dp_port *vport_get_dp_port(const struct vport *);
-struct kobject *vport_get_kobj(const struct vport *);
 int vport_get_stats(struct vport *, struct odp_vport_stats *);
 
 unsigned vport_get_flags(const struct vport *);
@@ -142,7 +141,6 @@ struct vport {
  * May be null if not supported.
  * @get_name: Get the device's name.
  * @get_addr: Get the device's MAC address.
- * @get_kobj: Get the kobj associated with the device (may return null).
  * @get_stats: Fill in the transmit/receive stats.  May be null if stats are
  * not supported or if generic stats are in use.  If defined and
  * VPORT_F_GEN_STATS is also set, the error stats are added to those already
@@ -181,7 +179,6 @@ struct vport_ops {
 	/* Called with rcu_read_lock or RTNL lock. */
 	const char *(*get_name)(const struct vport *);
 	const unsigned char *(*get_addr)(const struct vport *);
-	struct kobject *(*get_kobj)(const struct vport *);
 	int (*get_stats)(const struct vport *, struct odp_vport_stats *);
 
 	unsigned (*get_dev_flags)(const struct vport *);




More information about the dev mailing list