* [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures
@ 2015-09-08 20:47 Vivien Didelot
2015-09-08 20:47 ` [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan Vivien Didelot
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Vivien Didelot @ 2015-09-08 20:47 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, Scott Feldman, Florian Fainelli, Andrew Lunn, kernel,
Vivien Didelot
Hi!
Current implementations of .switchdev_port_obj_add and .switchdev_port_obj_dump
must pass the generic switchdev_obj structure instead of a specific one (e.g.
switchdev_obj_fdb) to the related driver accessors, because it contains the
object transaction and callback. This is not very convenient for drivers.
Instead of having all specific structures included into a switchdev_obj union,
it would be simpler to embed a switchdev_obj structure as the first member of
the more specific ones. That way, a driver can cast the switchdev_obj pointer
to the specific structure and have access to all the information from it.
As an example, this allows Rocker to change its two FDB accessors:
rocker_port_fdb_add(rocker_port, obj->trans, &obj->u.fdb);
rocker_port_fdb_dump(rocker_port, obj);
for these most consistent ones:
fdb = (struct switchdev_obj_fdb *) obj;
rocker_port_fdb_add(rocker_port, fdb);
rocker_port_fdb_dump(rocker_port, fdb);
This is what struct netdev_notifier_info and its specific supersets (e.g.
struct netdev_notifier_changeupper_info) do in include/linux/netdevice.h.
This patchset does that and updates bridge, Rocker and DSA accordingly.
Note that this patchset was sent as an RFC not to bother David with new
net-next stuffs, but if the changes look good, it is ready to merge.
Also, please take note that the change sits on top of:
http://patchwork.ozlabs.org/patch/514894/
Vivien Didelot (3):
net: switchdev: extract switchdev_obj_vlan
net: switchdev: extract switchdev_obj_ipv4_fib
net: switchdev: extract switchdev_obj_fdb
drivers/net/ethernet/rocker/rocker.c | 50 ++++++++------
include/net/switchdev.h | 49 ++++++++------
net/bridge/br_fdb.c | 12 ++--
net/bridge/br_vlan.c | 26 +++----
net/dsa/slave.c | 59 +++++++++-------
net/switchdev/switchdev.c | 127 ++++++++++++++++-------------------
6 files changed, 165 insertions(+), 158 deletions(-)
--
2.5.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan
2015-09-08 20:47 [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Vivien Didelot
@ 2015-09-08 20:47 ` Vivien Didelot
2015-09-08 20:58 ` Andrew Lunn
2015-09-09 16:12 ` Scott Feldman
2015-09-08 20:47 ` [RFC PATCH net-next 2/3] net: switchdev: extract switchdev_obj_ipv4_fib Vivien Didelot
` (3 subsequent siblings)
4 siblings, 2 replies; 8+ messages in thread
From: Vivien Didelot @ 2015-09-08 20:47 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, Scott Feldman, Florian Fainelli, Andrew Lunn, kernel,
Vivien Didelot
Move the switchdev_obj_vlan structure out of the switchdev_obj union.
This lightens the switchdev_obj structure and allows drivers to access
the object transaction and callback directly from a switchdev_obj_vlan.
This is more consistent and convenient for add and dump operations.
The patch updates bridge, the Rocker driver and DSA accordingly.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
drivers/net/ethernet/rocker/rocker.c | 21 +++++++++--------
include/net/switchdev.h | 13 +++++++----
net/bridge/br_vlan.c | 26 +++++++++------------
net/dsa/slave.c | 27 ++++++++++++----------
net/switchdev/switchdev.c | 44 ++++++++++++++++++------------------
5 files changed, 68 insertions(+), 63 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 34ac41a..e72d49a 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4394,14 +4394,13 @@ static int rocker_port_vlan_add(struct rocker_port *rocker_port,
}
static int rocker_port_vlans_add(struct rocker_port *rocker_port,
- enum switchdev_trans trans,
const struct switchdev_obj_vlan *vlan)
{
u16 vid;
int err;
for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
- err = rocker_port_vlan_add(rocker_port, trans,
+ err = rocker_port_vlan_add(rocker_port, vlan->obj.trans,
vid, vlan->flags);
if (err)
return err;
@@ -4427,6 +4426,7 @@ static int rocker_port_obj_add(struct net_device *dev,
struct switchdev_obj *obj)
{
struct rocker_port *rocker_port = netdev_priv(dev);
+ const struct switchdev_obj_vlan *vlan;
const struct switchdev_obj_ipv4_fib *fib4;
int err = 0;
@@ -4443,8 +4443,8 @@ static int rocker_port_obj_add(struct net_device *dev,
switch (obj->id) {
case SWITCHDEV_OBJ_PORT_VLAN:
- err = rocker_port_vlans_add(rocker_port, obj->trans,
- &obj->u.vlan);
+ vlan = (struct switchdev_obj_vlan *) obj;
+ err = rocker_port_vlans_add(rocker_port, vlan);
break;
case SWITCHDEV_OBJ_IPV4_FIB:
fib4 = &obj->u.ipv4_fib;
@@ -4509,12 +4509,14 @@ static int rocker_port_obj_del(struct net_device *dev,
struct switchdev_obj *obj)
{
struct rocker_port *rocker_port = netdev_priv(dev);
+ const struct switchdev_obj_vlan *vlan;
const struct switchdev_obj_ipv4_fib *fib4;
int err = 0;
switch (obj->id) {
case SWITCHDEV_OBJ_PORT_VLAN:
- err = rocker_port_vlans_del(rocker_port, &obj->u.vlan);
+ vlan = (struct switchdev_obj_vlan *) obj;
+ err = rocker_port_vlans_del(rocker_port, vlan);
break;
case SWITCHDEV_OBJ_IPV4_FIB:
fib4 = &obj->u.ipv4_fib;
@@ -4563,9 +4565,8 @@ static int rocker_port_fdb_dump(const struct rocker_port *rocker_port,
}
static int rocker_port_vlan_dump(const struct rocker_port *rocker_port,
- struct switchdev_obj *obj)
+ struct switchdev_obj_vlan *vlan)
{
- struct switchdev_obj_vlan *vlan = &obj->u.vlan;
u16 vid;
int err = 0;
@@ -4576,7 +4577,7 @@ static int rocker_port_vlan_dump(const struct rocker_port *rocker_port,
if (rocker_vlan_id_is_internal(htons(vid)))
vlan->flags |= BRIDGE_VLAN_INFO_PVID;
vlan->vid_begin = vlan->vid_end = vid;
- err = obj->cb(rocker_port->dev, obj);
+ err = vlan->obj.cb(rocker_port->dev, &vlan->obj);
if (err)
break;
}
@@ -4588,6 +4589,7 @@ static int rocker_port_obj_dump(struct net_device *dev,
struct switchdev_obj *obj)
{
const struct rocker_port *rocker_port = netdev_priv(dev);
+ struct switchdev_obj_vlan *vlan;
int err = 0;
switch (obj->id) {
@@ -4595,7 +4597,8 @@ static int rocker_port_obj_dump(struct net_device *dev,
err = rocker_port_fdb_dump(rocker_port, obj);
break;
case SWITCHDEV_OBJ_PORT_VLAN:
- err = rocker_port_vlan_dump(rocker_port, obj);
+ vlan = (struct switchdev_obj_vlan *) obj;
+ err = rocker_port_vlan_dump(rocker_port, vlan);
break;
default:
err = -EOPNOTSUPP;
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 319baab..55fa106 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -55,11 +55,6 @@ struct switchdev_obj {
enum switchdev_trans trans;
int (*cb)(struct net_device *dev, struct switchdev_obj *obj);
union {
- struct switchdev_obj_vlan { /* PORT_VLAN */
- u16 flags;
- u16 vid_begin;
- u16 vid_end;
- } vlan;
struct switchdev_obj_ipv4_fib { /* IPV4_FIB */
u32 dst;
int dst_len;
@@ -77,6 +72,14 @@ struct switchdev_obj {
} u;
};
+/* SWITCHDEV_OBJ_PORT_VLAN */
+struct switchdev_obj_vlan {
+ struct switchdev_obj obj; /* must be first */
+ u16 flags;
+ u16 vid_begin;
+ u16 vid_end;
+};
+
/**
* struct switchdev_ops - switchdev operations
*
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 5f5a02b..44e8bc1 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -50,16 +50,14 @@ static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
if (ops->ndo_vlan_rx_add_vid) {
err = vlan_vid_add(dev, br->vlan_proto, vid);
} else {
- struct switchdev_obj vlan_obj = {
- .id = SWITCHDEV_OBJ_PORT_VLAN,
- .u.vlan = {
- .flags = flags,
- .vid_begin = vid,
- .vid_end = vid,
- },
+ struct switchdev_obj_vlan vlan = {
+ .obj.id = SWITCHDEV_OBJ_PORT_VLAN,
+ .flags = flags,
+ .vid_begin = vid,
+ .vid_end = vid,
};
- err = switchdev_port_obj_add(dev, &vlan_obj);
+ err = switchdev_port_obj_add(dev, &vlan.obj);
if (err == -EOPNOTSUPP)
err = 0;
}
@@ -130,15 +128,13 @@ static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br,
if (ops->ndo_vlan_rx_kill_vid) {
vlan_vid_del(dev, br->vlan_proto, vid);
} else {
- struct switchdev_obj vlan_obj = {
- .id = SWITCHDEV_OBJ_PORT_VLAN,
- .u.vlan = {
- .vid_begin = vid,
- .vid_end = vid,
- },
+ struct switchdev_obj_vlan vlan = {
+ .obj.id = SWITCHDEV_OBJ_PORT_VLAN,
+ .vid_begin = vid,
+ .vid_end = vid,
};
- err = switchdev_port_obj_del(dev, &vlan_obj);
+ err = switchdev_port_obj_del(dev, &vlan.obj);
if (err == -EOPNOTSUPP)
err = 0;
}
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index cce9738..654afc8 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -242,15 +242,14 @@ static int dsa_bridge_check_vlan_range(struct dsa_switch *ds,
}
static int dsa_slave_port_vlan_add(struct net_device *dev,
- struct switchdev_obj *obj)
+ struct switchdev_obj_vlan *vlan)
{
- struct switchdev_obj_vlan *vlan = &obj->u.vlan;
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->parent;
u16 vid;
int err;
- switch (obj->trans) {
+ switch (vlan->obj.trans) {
case SWITCHDEV_TRANS_PREPARE:
if (!ds->drv->port_vlan_add || !ds->drv->port_pvid_set)
return -EOPNOTSUPP;
@@ -283,9 +282,8 @@ static int dsa_slave_port_vlan_add(struct net_device *dev,
}
static int dsa_slave_port_vlan_del(struct net_device *dev,
- struct switchdev_obj *obj)
+ struct switchdev_obj_vlan *vlan)
{
- struct switchdev_obj_vlan *vlan = &obj->u.vlan;
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->parent;
u16 vid;
@@ -304,9 +302,8 @@ static int dsa_slave_port_vlan_del(struct net_device *dev,
}
static int dsa_slave_port_vlan_dump(struct net_device *dev,
- struct switchdev_obj *obj)
+ struct switchdev_obj_vlan *vlan)
{
- struct switchdev_obj_vlan *vlan = &obj->u.vlan;
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->parent;
DECLARE_BITMAP(members, DSA_MAX_PORTS);
@@ -329,7 +326,7 @@ static int dsa_slave_port_vlan_dump(struct net_device *dev,
if (!test_bit(p->port, members))
continue;
- memset(vlan, 0, sizeof(*vlan));
+ vlan->flags = 0;
vlan->vid_begin = vlan->vid_end = vid;
if (vid == pvid)
@@ -338,7 +335,7 @@ static int dsa_slave_port_vlan_dump(struct net_device *dev,
if (test_bit(p->port, untagged))
vlan->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
- err = obj->cb(dev, obj);
+ err = vlan->obj.cb(dev, &vlan->obj);
if (err)
break;
}
@@ -476,6 +473,7 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
static int dsa_slave_port_obj_add(struct net_device *dev,
struct switchdev_obj *obj)
{
+ struct switchdev_obj_vlan *vlan;
int err;
/* For the prepare phase, ensure the full set of changes is feasable in
@@ -488,7 +486,8 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
err = dsa_slave_port_fdb_add(dev, obj);
break;
case SWITCHDEV_OBJ_PORT_VLAN:
- err = dsa_slave_port_vlan_add(dev, obj);
+ vlan = (struct switchdev_obj_vlan *) obj;
+ err = dsa_slave_port_vlan_add(dev, vlan);
break;
default:
err = -EOPNOTSUPP;
@@ -501,6 +500,7 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
static int dsa_slave_port_obj_del(struct net_device *dev,
struct switchdev_obj *obj)
{
+ struct switchdev_obj_vlan *vlan;
int err;
switch (obj->id) {
@@ -508,7 +508,8 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
err = dsa_slave_port_fdb_del(dev, obj);
break;
case SWITCHDEV_OBJ_PORT_VLAN:
- err = dsa_slave_port_vlan_del(dev, obj);
+ vlan = (struct switchdev_obj_vlan *) obj;
+ err = dsa_slave_port_vlan_del(dev, vlan);
break;
default:
err = -EOPNOTSUPP;
@@ -521,6 +522,7 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
static int dsa_slave_port_obj_dump(struct net_device *dev,
struct switchdev_obj *obj)
{
+ struct switchdev_obj_vlan *vlan;
int err;
switch (obj->id) {
@@ -528,7 +530,8 @@ static int dsa_slave_port_obj_dump(struct net_device *dev,
err = dsa_slave_port_fdb_dump(dev, obj);
break;
case SWITCHDEV_OBJ_PORT_VLAN:
- err = dsa_slave_port_vlan_dump(dev, obj);
+ vlan = (struct switchdev_obj_vlan *) obj;
+ err = dsa_slave_port_vlan_dump(dev, vlan);
break;
default:
err = -EOPNOTSUPP;
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 16c1c43..9923a97 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -397,7 +397,7 @@ int call_switchdev_notifiers(unsigned long val, struct net_device *dev,
EXPORT_SYMBOL_GPL(call_switchdev_notifiers);
struct switchdev_vlan_dump {
- struct switchdev_obj obj;
+ struct switchdev_obj_vlan vlan; /* must be first */
struct sk_buff *skb;
u32 filter_mask;
u16 flags;
@@ -439,9 +439,8 @@ static int switchdev_port_vlan_dump_put(struct net_device *dev,
static int switchdev_port_vlan_dump_cb(struct net_device *dev,
struct switchdev_obj *obj)
{
- struct switchdev_vlan_dump *dump =
- container_of(obj, struct switchdev_vlan_dump, obj);
- struct switchdev_obj_vlan *vlan = &dump->obj.u.vlan;
+ struct switchdev_vlan_dump *dump = (struct switchdev_vlan_dump *) obj;
+ struct switchdev_obj_vlan *vlan = (struct switchdev_obj_vlan *) obj;
int err = 0;
if (vlan->vid_begin > vlan->vid_end)
@@ -493,7 +492,7 @@ static int switchdev_port_vlan_fill(struct sk_buff *skb, struct net_device *dev,
u32 filter_mask)
{
struct switchdev_vlan_dump dump = {
- .obj = {
+ .vlan.obj = {
.id = SWITCHDEV_OBJ_PORT_VLAN,
.cb = switchdev_port_vlan_dump_cb,
},
@@ -504,7 +503,7 @@ static int switchdev_port_vlan_fill(struct sk_buff *skb, struct net_device *dev,
if ((filter_mask & RTEXT_FILTER_BRVLAN) ||
(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED)) {
- err = switchdev_port_obj_dump(dev, &dump.obj);
+ err = switchdev_port_obj_dump(dev, &dump.vlan.obj);
if (err)
goto err_out;
if (filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED)
@@ -621,10 +620,9 @@ static int switchdev_port_br_afspec(struct net_device *dev,
{
struct nlattr *attr;
struct bridge_vlan_info *vinfo;
- struct switchdev_obj obj = {
- .id = SWITCHDEV_OBJ_PORT_VLAN,
+ struct switchdev_obj_vlan vlan = {
+ .obj.id = SWITCHDEV_OBJ_PORT_VLAN,
};
- struct switchdev_obj_vlan *vlan = &obj.u.vlan;
int rem;
int err;
@@ -634,30 +632,32 @@ static int switchdev_port_br_afspec(struct net_device *dev,
if (nla_len(attr) != sizeof(struct bridge_vlan_info))
return -EINVAL;
vinfo = nla_data(attr);
- vlan->flags = vinfo->flags;
+ vlan.flags = vinfo->flags;
if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
- if (vlan->vid_begin)
+ if (vlan.vid_begin)
return -EINVAL;
- vlan->vid_begin = vinfo->vid;
+ vlan.vid_begin = vinfo->vid;
} else if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END) {
- if (!vlan->vid_begin)
+ if (!vlan.vid_begin)
return -EINVAL;
- vlan->vid_end = vinfo->vid;
- if (vlan->vid_end <= vlan->vid_begin)
+ vlan.vid_end = vinfo->vid;
+ if (vlan.vid_end <= vlan.vid_begin)
return -EINVAL;
- err = f(dev, &obj);
+ err = f(dev, &vlan.obj);
if (err)
return err;
- memset(vlan, 0, sizeof(*vlan));
+ vlan.flags = 0;
+ vlan.vid_begin = vlan.vid_end = 0;
} else {
- if (vlan->vid_begin)
+ if (vlan.vid_begin)
return -EINVAL;
- vlan->vid_begin = vinfo->vid;
- vlan->vid_end = vinfo->vid;
- err = f(dev, &obj);
+ vlan.vid_begin = vinfo->vid;
+ vlan.vid_end = vinfo->vid;
+ err = f(dev, &vlan.obj);
if (err)
return err;
- memset(vlan, 0, sizeof(*vlan));
+ vlan.flags = 0;
+ vlan.vid_begin = vlan.vid_end = 0;
}
}
--
2.5.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH net-next 2/3] net: switchdev: extract switchdev_obj_ipv4_fib
2015-09-08 20:47 [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Vivien Didelot
2015-09-08 20:47 ` [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan Vivien Didelot
@ 2015-09-08 20:47 ` Vivien Didelot
2015-09-08 20:47 ` [RFC PATCH net-next 3/3] net: switchdev: extract switchdev_obj_fdb Vivien Didelot
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Vivien Didelot @ 2015-09-08 20:47 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, Scott Feldman, Florian Fainelli, Andrew Lunn, kernel,
Vivien Didelot
Move the switchdev_obj_ipv4_fib structure out of the switchdev_obj
union.
This lightens the switchdev_obj structure and allows drivers to access
the object transaction and callback directly from a
switchdev_obj_ipv4_fib. This is more consistent and convenient for add
and dump operations.
The patch updates the Rocker driver accordingly.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
drivers/net/ethernet/rocker/rocker.c | 4 ++--
include/net/switchdev.h | 21 +++++++++--------
net/switchdev/switchdev.c | 44 ++++++++++++++++--------------------
3 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index e72d49a..41aabbc 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4447,7 +4447,7 @@ static int rocker_port_obj_add(struct net_device *dev,
err = rocker_port_vlans_add(rocker_port, vlan);
break;
case SWITCHDEV_OBJ_IPV4_FIB:
- fib4 = &obj->u.ipv4_fib;
+ fib4 = (struct switchdev_obj_ipv4_fib *) obj;
err = rocker_port_fib_ipv4(rocker_port, obj->trans,
htonl(fib4->dst), fib4->dst_len,
fib4->fi, fib4->tb_id, 0);
@@ -4519,7 +4519,7 @@ static int rocker_port_obj_del(struct net_device *dev,
err = rocker_port_vlans_del(rocker_port, vlan);
break;
case SWITCHDEV_OBJ_IPV4_FIB:
- fib4 = &obj->u.ipv4_fib;
+ fib4 = (struct switchdev_obj_ipv4_fib *) obj;
err = rocker_port_fib_ipv4(rocker_port, SWITCHDEV_TRANS_NONE,
htonl(fib4->dst), fib4->dst_len,
fib4->fi, fib4->tb_id,
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 55fa106..0b76aa8 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -55,15 +55,6 @@ struct switchdev_obj {
enum switchdev_trans trans;
int (*cb)(struct net_device *dev, struct switchdev_obj *obj);
union {
- struct switchdev_obj_ipv4_fib { /* IPV4_FIB */
- u32 dst;
- int dst_len;
- struct fib_info *fi;
- u8 tos;
- u8 type;
- u32 nlflags;
- u32 tb_id;
- } ipv4_fib;
struct switchdev_obj_fdb { /* PORT_FDB */
const unsigned char *addr;
u16 vid;
@@ -80,6 +71,18 @@ struct switchdev_obj_vlan {
u16 vid_end;
};
+/* SWITCHDEV_OBJ_IPV4_FIB */
+struct switchdev_obj_ipv4_fib {
+ struct switchdev_obj obj; /* must be first */
+ u32 dst;
+ int dst_len;
+ struct fib_info *fi;
+ u8 tos;
+ u8 type;
+ u32 nlflags;
+ u32 tb_id;
+};
+
/**
* struct switchdev_ops - switchdev operations
*
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 9923a97..10fde6f 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -936,17 +936,15 @@ static struct net_device *switchdev_get_dev_by_nhs(struct fib_info *fi)
int switchdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
u8 tos, u8 type, u32 nlflags, u32 tb_id)
{
- struct switchdev_obj fib_obj = {
- .id = SWITCHDEV_OBJ_IPV4_FIB,
- .u.ipv4_fib = {
- .dst = dst,
- .dst_len = dst_len,
- .fi = fi,
- .tos = tos,
- .type = type,
- .nlflags = nlflags,
- .tb_id = tb_id,
- },
+ struct switchdev_obj_ipv4_fib fib_obj = {
+ .obj.id = SWITCHDEV_OBJ_IPV4_FIB,
+ .dst = dst,
+ .dst_len = dst_len,
+ .fi = fi,
+ .tos = tos,
+ .type = type,
+ .nlflags = nlflags,
+ .tb_id = tb_id,
};
struct net_device *dev;
int err = 0;
@@ -967,7 +965,7 @@ int switchdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
if (!dev)
return 0;
- err = switchdev_port_obj_add(dev, &fib_obj);
+ err = switchdev_port_obj_add(dev, &fib_obj.obj);
if (!err)
fi->fib_flags |= RTNH_F_OFFLOAD;
@@ -990,17 +988,15 @@ EXPORT_SYMBOL_GPL(switchdev_fib_ipv4_add);
int switchdev_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
u8 tos, u8 type, u32 tb_id)
{
- struct switchdev_obj fib_obj = {
- .id = SWITCHDEV_OBJ_IPV4_FIB,
- .u.ipv4_fib = {
- .dst = dst,
- .dst_len = dst_len,
- .fi = fi,
- .tos = tos,
- .type = type,
- .nlflags = 0,
- .tb_id = tb_id,
- },
+ struct switchdev_obj_ipv4_fib fib_obj = {
+ .obj.id = SWITCHDEV_OBJ_IPV4_FIB,
+ .dst = dst,
+ .dst_len = dst_len,
+ .fi = fi,
+ .tos = tos,
+ .type = type,
+ .nlflags = 0,
+ .tb_id = tb_id,
};
struct net_device *dev;
int err = 0;
@@ -1012,7 +1008,7 @@ int switchdev_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
if (!dev)
return 0;
- err = switchdev_port_obj_del(dev, &fib_obj);
+ err = switchdev_port_obj_del(dev, &fib_obj.obj);
if (!err)
fi->fib_flags &= ~RTNH_F_OFFLOAD;
--
2.5.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH net-next 3/3] net: switchdev: extract switchdev_obj_fdb
2015-09-08 20:47 [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Vivien Didelot
2015-09-08 20:47 ` [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan Vivien Didelot
2015-09-08 20:47 ` [RFC PATCH net-next 2/3] net: switchdev: extract switchdev_obj_ipv4_fib Vivien Didelot
@ 2015-09-08 20:47 ` Vivien Didelot
2015-09-09 6:45 ` [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Jiri Pirko
2015-09-09 16:25 ` Scott Feldman
4 siblings, 0 replies; 8+ messages in thread
From: Vivien Didelot @ 2015-09-08 20:47 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, Scott Feldman, Florian Fainelli, Andrew Lunn, kernel,
Vivien Didelot
Move the switchdev_obj_fdb structure out of the switchdev_obj union.
This lightens the switchdev_obj structure and allows drivers to access
the object transaction and callback directly from a switchdev_obj_fdb.
This is more consistent and convenient for add and dump operations.
The patch updates bridge, the Rocker driver and DSA accordingly.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
drivers/net/ethernet/rocker/rocker.c | 25 ++++++++++++++---------
include/net/switchdev.h | 15 +++++++-------
net/bridge/br_fdb.c | 12 +++++------
net/dsa/slave.c | 32 ++++++++++++++++-------------
net/switchdev/switchdev.c | 39 ++++++++++++++++--------------------
5 files changed, 63 insertions(+), 60 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 41aabbc..c3f25a9 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4410,7 +4410,6 @@ static int rocker_port_vlans_add(struct rocker_port *rocker_port,
}
static int rocker_port_fdb_add(struct rocker_port *rocker_port,
- enum switchdev_trans trans,
const struct switchdev_obj_fdb *fdb)
{
__be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, fdb->vid, NULL);
@@ -4419,7 +4418,8 @@ static int rocker_port_fdb_add(struct rocker_port *rocker_port,
if (!rocker_port_is_bridged(rocker_port))
return -EINVAL;
- return rocker_port_fdb(rocker_port, trans, fdb->addr, vlan_id, flags);
+ return rocker_port_fdb(rocker_port, fdb->obj.trans, fdb->addr, vlan_id,
+ flags);
}
static int rocker_port_obj_add(struct net_device *dev,
@@ -4428,6 +4428,7 @@ static int rocker_port_obj_add(struct net_device *dev,
struct rocker_port *rocker_port = netdev_priv(dev);
const struct switchdev_obj_vlan *vlan;
const struct switchdev_obj_ipv4_fib *fib4;
+ const struct switchdev_obj_fdb *fdb;
int err = 0;
switch (obj->trans) {
@@ -4453,7 +4454,8 @@ static int rocker_port_obj_add(struct net_device *dev,
fib4->fi, fib4->tb_id, 0);
break;
case SWITCHDEV_OBJ_PORT_FDB:
- err = rocker_port_fdb_add(rocker_port, obj->trans, &obj->u.fdb);
+ fdb = (struct switchdev_obj_fdb *) obj;
+ err = rocker_port_fdb_add(rocker_port, fdb);
break;
default:
err = -EOPNOTSUPP;
@@ -4493,7 +4495,6 @@ static int rocker_port_vlans_del(struct rocker_port *rocker_port,
}
static int rocker_port_fdb_del(struct rocker_port *rocker_port,
- enum switchdev_trans trans,
const struct switchdev_obj_fdb *fdb)
{
__be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, fdb->vid, NULL);
@@ -4502,7 +4503,8 @@ static int rocker_port_fdb_del(struct rocker_port *rocker_port,
if (!rocker_port_is_bridged(rocker_port))
return -EINVAL;
- return rocker_port_fdb(rocker_port, trans, fdb->addr, vlan_id, flags);
+ return rocker_port_fdb(rocker_port, fdb->obj.trans, fdb->addr, vlan_id,
+ flags);
}
static int rocker_port_obj_del(struct net_device *dev,
@@ -4511,6 +4513,7 @@ static int rocker_port_obj_del(struct net_device *dev,
struct rocker_port *rocker_port = netdev_priv(dev);
const struct switchdev_obj_vlan *vlan;
const struct switchdev_obj_ipv4_fib *fib4;
+ const struct switchdev_obj_fdb *fdb;
int err = 0;
switch (obj->id) {
@@ -4526,7 +4529,8 @@ static int rocker_port_obj_del(struct net_device *dev,
ROCKER_OP_FLAG_REMOVE);
break;
case SWITCHDEV_OBJ_PORT_FDB:
- err = rocker_port_fdb_del(rocker_port, obj->trans, &obj->u.fdb);
+ fdb = (struct switchdev_obj_fdb *) obj;
+ err = rocker_port_fdb_del(rocker_port, fdb);
break;
default:
err = -EOPNOTSUPP;
@@ -4537,10 +4541,9 @@ static int rocker_port_obj_del(struct net_device *dev,
}
static int rocker_port_fdb_dump(const struct rocker_port *rocker_port,
- struct switchdev_obj *obj)
+ struct switchdev_obj_fdb *fdb)
{
struct rocker *rocker = rocker_port->rocker;
- struct switchdev_obj_fdb *fdb = &obj->u.fdb;
struct rocker_fdb_tbl_entry *found;
struct hlist_node *tmp;
unsigned long lock_flags;
@@ -4555,7 +4558,7 @@ static int rocker_port_fdb_dump(const struct rocker_port *rocker_port,
fdb->ndm_state = NUD_REACHABLE;
fdb->vid = rocker_port_vlan_to_vid(rocker_port,
found->key.vlan_id);
- err = obj->cb(rocker_port->dev, obj);
+ err = fdb->obj.cb(rocker_port->dev, &fdb->obj);
if (err)
break;
}
@@ -4590,11 +4593,13 @@ static int rocker_port_obj_dump(struct net_device *dev,
{
const struct rocker_port *rocker_port = netdev_priv(dev);
struct switchdev_obj_vlan *vlan;
+ struct switchdev_obj_fdb *fdb;
int err = 0;
switch (obj->id) {
case SWITCHDEV_OBJ_PORT_FDB:
- err = rocker_port_fdb_dump(rocker_port, obj);
+ fdb = (struct switchdev_obj_fdb *) obj;
+ err = rocker_port_fdb_dump(rocker_port, fdb);
break;
case SWITCHDEV_OBJ_PORT_VLAN:
vlan = (struct switchdev_obj_vlan *) obj;
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 0b76aa8..302f027 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -54,13 +54,6 @@ struct switchdev_obj {
enum switchdev_obj_id id;
enum switchdev_trans trans;
int (*cb)(struct net_device *dev, struct switchdev_obj *obj);
- union {
- struct switchdev_obj_fdb { /* PORT_FDB */
- const unsigned char *addr;
- u16 vid;
- u16 ndm_state;
- } fdb;
- } u;
};
/* SWITCHDEV_OBJ_PORT_VLAN */
@@ -83,6 +76,14 @@ struct switchdev_obj_ipv4_fib {
u32 tb_id;
};
+/* SWITCHDEV_OBJ_PORT_FDB */
+struct switchdev_obj_fdb {
+ struct switchdev_obj obj; /* must be first */
+ const unsigned char *addr;
+ u16 vid;
+ u16 ndm_state;
+};
+
/**
* struct switchdev_ops - switchdev operations
*
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 9e9875d..d20d9b9 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -133,15 +133,13 @@ static void fdb_del_hw_addr(struct net_bridge *br, const unsigned char *addr)
static void fdb_del_external_learn(struct net_bridge_fdb_entry *f)
{
- struct switchdev_obj obj = {
- .id = SWITCHDEV_OBJ_PORT_FDB,
- .u.fdb = {
- .addr = f->addr.addr,
- .vid = f->vlan_id,
- },
+ struct switchdev_obj_fdb fdb = {
+ .obj.id = SWITCHDEV_OBJ_PORT_FDB,
+ .addr = f->addr.addr,
+ .vid = f->vlan_id,
};
- switchdev_port_obj_del(f->dst->dev, &obj);
+ switchdev_port_obj_del(f->dst->dev, &fdb.obj);
}
static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 654afc8..d1de9df 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -344,25 +344,23 @@ static int dsa_slave_port_vlan_dump(struct net_device *dev,
}
static int dsa_slave_port_fdb_add(struct net_device *dev,
- struct switchdev_obj *obj)
+ struct switchdev_obj_fdb *fdb)
{
- struct switchdev_obj_fdb *fdb = &obj->u.fdb;
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->parent;
int ret = -EOPNOTSUPP;
- if (obj->trans == SWITCHDEV_TRANS_PREPARE)
+ if (fdb->obj.trans == SWITCHDEV_TRANS_PREPARE)
ret = ds->drv->port_fdb_add ? 0 : -EOPNOTSUPP;
- else if (obj->trans == SWITCHDEV_TRANS_COMMIT)
+ else if (fdb->obj.trans == SWITCHDEV_TRANS_COMMIT)
ret = ds->drv->port_fdb_add(ds, p->port, fdb->addr, fdb->vid);
return ret;
}
static int dsa_slave_port_fdb_del(struct net_device *dev,
- struct switchdev_obj *obj)
+ struct switchdev_obj_fdb *fdb)
{
- struct switchdev_obj_fdb *fdb = &obj->u.fdb;
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->parent;
int ret = -EOPNOTSUPP;
@@ -374,7 +372,7 @@ static int dsa_slave_port_fdb_del(struct net_device *dev,
}
static int dsa_slave_port_fdb_dump(struct net_device *dev,
- struct switchdev_obj *obj)
+ struct switchdev_obj_fdb *fdb)
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->parent;
@@ -393,11 +391,11 @@ static int dsa_slave_port_fdb_dump(struct net_device *dev,
if (ret < 0)
break;
- obj->u.fdb.addr = addr;
- obj->u.fdb.vid = vid;
- obj->u.fdb.ndm_state = is_static ? NUD_NOARP : NUD_REACHABLE;
+ fdb->addr = addr;
+ fdb->vid = vid;
+ fdb->ndm_state = is_static ? NUD_NOARP : NUD_REACHABLE;
- ret = obj->cb(dev, obj);
+ ret = fdb->obj.cb(dev, &fdb->obj);
if (ret < 0)
break;
}
@@ -474,6 +472,7 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
struct switchdev_obj *obj)
{
struct switchdev_obj_vlan *vlan;
+ struct switchdev_obj_fdb *fdb;
int err;
/* For the prepare phase, ensure the full set of changes is feasable in
@@ -483,7 +482,8 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
switch (obj->id) {
case SWITCHDEV_OBJ_PORT_FDB:
- err = dsa_slave_port_fdb_add(dev, obj);
+ fdb = (struct switchdev_obj_fdb *) obj;
+ err = dsa_slave_port_fdb_add(dev, fdb);
break;
case SWITCHDEV_OBJ_PORT_VLAN:
vlan = (struct switchdev_obj_vlan *) obj;
@@ -501,11 +501,13 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
struct switchdev_obj *obj)
{
struct switchdev_obj_vlan *vlan;
+ struct switchdev_obj_fdb *fdb;
int err;
switch (obj->id) {
case SWITCHDEV_OBJ_PORT_FDB:
- err = dsa_slave_port_fdb_del(dev, obj);
+ fdb = (struct switchdev_obj_fdb *) obj;
+ err = dsa_slave_port_fdb_del(dev, fdb);
break;
case SWITCHDEV_OBJ_PORT_VLAN:
vlan = (struct switchdev_obj_vlan *) obj;
@@ -523,11 +525,13 @@ static int dsa_slave_port_obj_dump(struct net_device *dev,
struct switchdev_obj *obj)
{
struct switchdev_obj_vlan *vlan;
+ struct switchdev_obj_fdb *fdb;
int err;
switch (obj->id) {
case SWITCHDEV_OBJ_PORT_FDB:
- err = dsa_slave_port_fdb_dump(dev, obj);
+ fdb = (struct switchdev_obj_fdb *) obj;
+ err = dsa_slave_port_fdb_dump(dev, fdb);
break;
case SWITCHDEV_OBJ_PORT_VLAN:
vlan = (struct switchdev_obj_vlan *) obj;
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 10fde6f..5349752 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -739,15 +739,13 @@ int switchdev_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev, const unsigned char *addr,
u16 vid, u16 nlm_flags)
{
- struct switchdev_obj obj = {
- .id = SWITCHDEV_OBJ_PORT_FDB,
- .u.fdb = {
- .addr = addr,
- .vid = vid,
- },
+ struct switchdev_obj_fdb fdb = {
+ .obj.id = SWITCHDEV_OBJ_PORT_FDB,
+ .addr = addr,
+ .vid = vid,
};
- return switchdev_port_obj_add(dev, &obj);
+ return switchdev_port_obj_add(dev, &fdb.obj);
}
EXPORT_SYMBOL_GPL(switchdev_port_fdb_add);
@@ -766,20 +764,18 @@ int switchdev_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev, const unsigned char *addr,
u16 vid)
{
- struct switchdev_obj obj = {
- .id = SWITCHDEV_OBJ_PORT_FDB,
- .u.fdb = {
- .addr = addr,
- .vid = vid,
- },
+ struct switchdev_obj_fdb fdb = {
+ .obj.id = SWITCHDEV_OBJ_PORT_FDB,
+ .addr = addr,
+ .vid = vid,
};
- return switchdev_port_obj_del(dev, &obj);
+ return switchdev_port_obj_del(dev, &fdb.obj);
}
EXPORT_SYMBOL_GPL(switchdev_port_fdb_del);
struct switchdev_fdb_dump {
- struct switchdev_obj obj;
+ struct switchdev_obj_fdb fdb; /* must be first */
struct sk_buff *skb;
struct netlink_callback *cb;
int idx;
@@ -788,8 +784,7 @@ struct switchdev_fdb_dump {
static int switchdev_port_fdb_dump_cb(struct net_device *dev,
struct switchdev_obj *obj)
{
- struct switchdev_fdb_dump *dump =
- container_of(obj, struct switchdev_fdb_dump, obj);
+ struct switchdev_fdb_dump *dump = (struct switchdev_fdb_dump *) obj;
u32 portid = NETLINK_CB(dump->cb->skb).portid;
u32 seq = dump->cb->nlh->nlmsg_seq;
struct nlmsghdr *nlh;
@@ -810,12 +805,12 @@ static int switchdev_port_fdb_dump_cb(struct net_device *dev,
ndm->ndm_flags = NTF_SELF;
ndm->ndm_type = 0;
ndm->ndm_ifindex = dev->ifindex;
- ndm->ndm_state = obj->u.fdb.ndm_state;
+ ndm->ndm_state = dump->fdb.ndm_state;
- if (nla_put(dump->skb, NDA_LLADDR, ETH_ALEN, obj->u.fdb.addr))
+ if (nla_put(dump->skb, NDA_LLADDR, ETH_ALEN, dump->fdb.addr))
goto nla_put_failure;
- if (obj->u.fdb.vid && nla_put_u16(dump->skb, NDA_VLAN, obj->u.fdb.vid))
+ if (dump->fdb.vid && nla_put_u16(dump->skb, NDA_VLAN, dump->fdb.vid))
goto nla_put_failure;
nlmsg_end(dump->skb, nlh);
@@ -845,7 +840,7 @@ int switchdev_port_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
struct net_device *filter_dev, int idx)
{
struct switchdev_fdb_dump dump = {
- .obj = {
+ .fdb.obj = {
.id = SWITCHDEV_OBJ_PORT_FDB,
.cb = switchdev_port_fdb_dump_cb,
},
@@ -855,7 +850,7 @@ int switchdev_port_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
};
int err;
- err = switchdev_port_obj_dump(dev, &dump.obj);
+ err = switchdev_port_obj_dump(dev, &dump.fdb.obj);
if (err)
return err;
--
2.5.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan
2015-09-08 20:47 ` [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan Vivien Didelot
@ 2015-09-08 20:58 ` Andrew Lunn
2015-09-09 16:12 ` Scott Feldman
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2015-09-08 20:58 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, Jiri Pirko, Scott Feldman, Florian Fainelli, kernel
> +/* SWITCHDEV_OBJ_PORT_VLAN */
> +struct switchdev_obj_vlan {
> + struct switchdev_obj obj; /* must be first */
> + u16 flags;
> + u16 vid_begin;
> + u16 vid_end;
> +};
I know you give a few examples for where this is done in network code,
but i think container_of() is used much more. You can then place the
struct switchdev_obj anywhere in the structure, and it will work.
#define to_switchdev_obj_vlan(o) container_of(o, struct switchdev_obj_vlan, obj)
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures
2015-09-08 20:47 [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Vivien Didelot
` (2 preceding siblings ...)
2015-09-08 20:47 ` [RFC PATCH net-next 3/3] net: switchdev: extract switchdev_obj_fdb Vivien Didelot
@ 2015-09-09 6:45 ` Jiri Pirko
2015-09-09 16:25 ` Scott Feldman
4 siblings, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2015-09-09 6:45 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, Scott Feldman, Florian Fainelli, Andrew Lunn, kernel
Tue, Sep 08, 2015 at 10:47:52PM CEST, vivien.didelot@savoirfairelinux.com wrote:
>Hi!
>
>Current implementations of .switchdev_port_obj_add and .switchdev_port_obj_dump
>must pass the generic switchdev_obj structure instead of a specific one (e.g.
>switchdev_obj_fdb) to the related driver accessors, because it contains the
>object transaction and callback. This is not very convenient for drivers.
>
>Instead of having all specific structures included into a switchdev_obj union,
>it would be simpler to embed a switchdev_obj structure as the first member of
>the more specific ones. That way, a driver can cast the switchdev_obj pointer
>to the specific structure and have access to all the information from it.
>
>As an example, this allows Rocker to change its two FDB accessors:
>
> rocker_port_fdb_add(rocker_port, obj->trans, &obj->u.fdb);
> rocker_port_fdb_dump(rocker_port, obj);
>
>for these most consistent ones:
>
> fdb = (struct switchdev_obj_fdb *) obj;
> rocker_port_fdb_add(rocker_port, fdb);
> rocker_port_fdb_dump(rocker_port, fdb);
>
>This is what struct netdev_notifier_info and its specific supersets (e.g.
>struct netdev_notifier_changeupper_info) do in include/linux/netdevice.h.
>
>This patchset does that and updates bridge, Rocker and DSA accordingly.
>
>Note that this patchset was sent as an RFC not to bother David with new
>net-next stuffs, but if the changes look good, it is ready to merge.
>
>Also, please take note that the change sits on top of:
>http://patchwork.ozlabs.org/patch/514894/
>
>Vivien Didelot (3):
> net: switchdev: extract switchdev_obj_vlan
> net: switchdev: extract switchdev_obj_ipv4_fib
> net: switchdev: extract switchdev_obj_fdb
I like this patchset a lot! I think it is the right way. Thanks for doing this.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan
2015-09-08 20:47 ` [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan Vivien Didelot
2015-09-08 20:58 ` Andrew Lunn
@ 2015-09-09 16:12 ` Scott Feldman
1 sibling, 0 replies; 8+ messages in thread
From: Scott Feldman @ 2015-09-09 16:12 UTC (permalink / raw)
To: Vivien Didelot; +Cc: Netdev, Jiri Pirko, Florian Fainelli, Andrew Lunn, kernel
On Tue, Sep 8, 2015 at 1:47 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Move the switchdev_obj_vlan structure out of the switchdev_obj union.
>
> This lightens the switchdev_obj structure and allows drivers to access
> the object transaction and callback directly from a switchdev_obj_vlan.
> This is more consistent and convenient for add and dump operations.
>
> The patch updates bridge, the Rocker driver and DSA accordingly.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
[cut]
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index 16c1c43..9923a97 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -397,7 +397,7 @@ int call_switchdev_notifiers(unsigned long val, struct net_device *dev,
> EXPORT_SYMBOL_GPL(call_switchdev_notifiers);
>
> struct switchdev_vlan_dump {
> - struct switchdev_obj obj;
> + struct switchdev_obj_vlan vlan; /* must be first */
> struct sk_buff *skb;
> u32 filter_mask;
> u16 flags;
> @@ -439,9 +439,8 @@ static int switchdev_port_vlan_dump_put(struct net_device *dev,
> static int switchdev_port_vlan_dump_cb(struct net_device *dev,
> struct switchdev_obj *obj)
> {
> - struct switchdev_vlan_dump *dump =
> - container_of(obj, struct switchdev_vlan_dump, obj);
> - struct switchdev_obj_vlan *vlan = &dump->obj.u.vlan;
> + struct switchdev_vlan_dump *dump = (struct switchdev_vlan_dump *) obj;
> + struct switchdev_obj_vlan *vlan = (struct switchdev_obj_vlan *) obj;
Same comment as Andrew's: use container_of() rather than casting for
both of the use-cases above. (And don't require inner-structure to be
fist in outer structure).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures
2015-09-08 20:47 [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Vivien Didelot
` (3 preceding siblings ...)
2015-09-09 6:45 ` [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Jiri Pirko
@ 2015-09-09 16:25 ` Scott Feldman
4 siblings, 0 replies; 8+ messages in thread
From: Scott Feldman @ 2015-09-09 16:25 UTC (permalink / raw)
To: Vivien Didelot; +Cc: Netdev, Jiri Pirko, Florian Fainelli, Andrew Lunn, kernel
On Tue, Sep 8, 2015 at 1:47 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Hi!
>
> Current implementations of .switchdev_port_obj_add and .switchdev_port_obj_dump
> must pass the generic switchdev_obj structure instead of a specific one (e.g.
> switchdev_obj_fdb) to the related driver accessors, because it contains the
> object transaction and callback. This is not very convenient for drivers.
>
> Instead of having all specific structures included into a switchdev_obj union,
> it would be simpler to embed a switchdev_obj structure as the first member of
> the more specific ones. That way, a driver can cast the switchdev_obj pointer
> to the specific structure and have access to all the information from it.
>
> As an example, this allows Rocker to change its two FDB accessors:
>
> rocker_port_fdb_add(rocker_port, obj->trans, &obj->u.fdb);
> rocker_port_fdb_dump(rocker_port, obj);
>
> for these most consistent ones:
>
> fdb = (struct switchdev_obj_fdb *) obj;
> rocker_port_fdb_add(rocker_port, fdb);
> rocker_port_fdb_dump(rocker_port, fdb);
>
> This is what struct netdev_notifier_info and its specific supersets (e.g.
> struct netdev_notifier_changeupper_info) do in include/linux/netdevice.h.
>
> This patchset does that and updates bridge, Rocker and DSA accordingly.
>
> Note that this patchset was sent as an RFC not to bother David with new
> net-next stuffs, but if the changes look good, it is ready to merge.
>
> Also, please take note that the change sits on top of:
> http://patchwork.ozlabs.org/patch/514894/
>
HI Vivien, looks good. Some requests for the next version:
1) Move the cb() member out of struct switchdev_obj and into the
containing struct switchdev_obj_xxx structure. This way the second
arg to cb() can be struct switchdev_obj_xxx *. (And, I have pending
patches where the first arg to cb() is different, so making the dump
cb() func specific to obj type will help).
2) As already mentioned, use container_of where possible to avoid
locking struct member at a position within struct.
Should the same treatment be done for switchdev_attr? Maybe that's a
second patchset.
-scott
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-09-09 16:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-08 20:47 [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Vivien Didelot
2015-09-08 20:47 ` [RFC PATCH net-next 1/3] net: switchdev: extract switchdev_obj_vlan Vivien Didelot
2015-09-08 20:58 ` Andrew Lunn
2015-09-09 16:12 ` Scott Feldman
2015-09-08 20:47 ` [RFC PATCH net-next 2/3] net: switchdev: extract switchdev_obj_ipv4_fib Vivien Didelot
2015-09-08 20:47 ` [RFC PATCH net-next 3/3] net: switchdev: extract switchdev_obj_fdb Vivien Didelot
2015-09-09 6:45 ` [RFC PATCH net-next 0/3] net: switchdev: extract specific object structures Jiri Pirko
2015-09-09 16:25 ` Scott Feldman
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).