| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150401160522.GE2025@nanopsycho.orion>
Date: Wed, 1 Apr 2015 18:05:22 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: roopa <roopa@...ulusnetworks.com>
Cc: sfeldma@...il.com, netdev@...r.kernel.org, linux@...ck-us.net,
f.fainelli@...il.com, sridhar.samudrala@...el.com,
ronen.arad@...el.com
Subject: Re: [PATCH net-next v2 25/26] switchdev: convert
swdev_fib_ipv4_add/del over to swdev_port_obj_add/del
Wed, Apr 01, 2015 at 05:05:49PM CEST, roopa@...ulusnetworks.com wrote:
>On 4/1/15, 3:08 AM, sfeldma@...il.com wrote:
>>From: Scott Feldman <sfeldma@...il.com>
>>
>>The IPv4 FIB ops convert nicely to the swdev objs and we're left with only
>>four swdev ops: port get/set and port add/del. Other objs will follow, such
>>as FDB. So go ahead and convert IPv4 FIB over to swdev obj for consistency,
>>anticipating more objs to come.
>>
>>Signed-off-by: Scott Feldman <sfeldma@...il.com>
>>---
>> drivers/net/ethernet/rocker/rocker.c | 42 +++++++++++------------------
>> include/net/switchdev.h | 21 +++++++--------
>> net/switchdev/switchdev.c | 49 +++++++++++++++++++++-------------
>> 3 files changed, 56 insertions(+), 56 deletions(-)
>>
>>diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>>index b7e54c3..b34efd8 100644
>>--- a/drivers/net/ethernet/rocker/rocker.c
>>+++ b/drivers/net/ethernet/rocker/rocker.c
>>@@ -4244,12 +4244,19 @@ static int rocker_port_vlans_add(struct rocker_port *rocker_port,
>> static int rocker_port_obj_add(struct net_device *dev, struct swdev_obj *obj)
>> {
>> struct rocker_port *rocker_port = netdev_priv(dev);
>>+ struct swdev_obj_ipv4_fib *fib4;
>> int err = 0;
>> switch (obj->id) {
>> case SWDEV_OBJ_PORT_VLAN:
>> err = rocker_port_vlans_add(rocker_port, &obj->vlan);
>> break;
>>+ case SWDEV_OBJ_IPV4_FIB:
>>+ fib4 = &obj->ipv4_fib;
>>+ err = rocker_port_fib_ipv4(rocker_port, fib4->dst,
>>+ fib4->dst_len, fib4->fi,
>>+ fib4->tb_id, 0);
>>+ break;
>> default:
>> err = -EOPNOTSUPP;
>> break;
>>@@ -4289,12 +4296,20 @@ static int rocker_port_vlans_del(struct rocker_port *rocker_port,
>> static int rocker_port_obj_del(struct net_device *dev, struct swdev_obj *obj)
>> {
>> struct rocker_port *rocker_port = netdev_priv(dev);
>>+ struct swdev_obj_ipv4_fib *fib4;
>> int err = 0;
>> switch (obj->id) {
>> case SWDEV_OBJ_PORT_VLAN:
>> err = rocker_port_vlans_del(rocker_port, &obj->vlan);
>> break;
>>+ case SWDEV_OBJ_IPV4_FIB:
>>+ fib4 = &obj->ipv4_fib;
>>+ err = rocker_port_fib_ipv4(rocker_port, fib4->dst,
>>+ fib4->dst_len, fib4->fi,
>>+ fib4->tb_id,
>>+ ROCKER_OP_FLAG_REMOVE);
>>+ break;
>> default:
>> err = -EOPNOTSUPP;
>> break;
>>@@ -4303,38 +4318,11 @@ static int rocker_port_obj_del(struct net_device *dev, struct swdev_obj *obj)
>> return err;
>> }
>>-static int rocker_port_swdev_fib_ipv4_add(struct net_device *dev,
>>- __be32 dst, int dst_len,
>>- struct fib_info *fi,
>>- u8 tos, u8 type,
>>- u32 nlflags, u32 tb_id)
>>-{
>>- struct rocker_port *rocker_port = netdev_priv(dev);
>>- int flags = 0;
>>-
>>- return rocker_port_fib_ipv4(rocker_port, dst, dst_len,
>>- fi, tb_id, flags);
>>-}
>>-
>>-static int rocker_port_swdev_fib_ipv4_del(struct net_device *dev,
>>- __be32 dst, int dst_len,
>>- struct fib_info *fi,
>>- u8 tos, u8 type, u32 tb_id)
>>-{
>>- struct rocker_port *rocker_port = netdev_priv(dev);
>>- int flags = ROCKER_OP_FLAG_REMOVE;
>>-
>>- return rocker_port_fib_ipv4(rocker_port, dst, dst_len,
>>- fi, tb_id, flags);
>>-}
>>-
>> static const struct swdev_ops rocker_port_swdev_ops = {
>> .swdev_port_attr_get = rocker_port_attr_get,
>> .swdev_port_attr_set = rocker_port_attr_set,
>> .swdev_port_obj_add = rocker_port_obj_add,
>> .swdev_port_obj_del = rocker_port_obj_del,
>>- .swdev_fib_ipv4_add = rocker_port_swdev_fib_ipv4_add,
>>- .swdev_fib_ipv4_del = rocker_port_swdev_fib_ipv4_del,
>> };
>> /********************
>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>index 37a2607..efaac55 100644
>>--- a/include/net/switchdev.h
>>+++ b/include/net/switchdev.h
>>@@ -39,6 +39,7 @@ struct fib_info;
>> enum swdev_obj_id {
>> SWDEV_OBJ_UNDEFINED,
>> SWDEV_OBJ_PORT_VLAN,
>>+ SWDEV_OBJ_IPV4_FIB,
>> };
>> struct swdev_obj {
>>@@ -50,6 +51,15 @@ struct swdev_obj {
>> u16 vid_start;
>> u16 vid_end;
>> } vlan;
>>+ struct swdev_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;
>> };
>> };
>>@@ -63,10 +73,6 @@ struct swdev_obj {
>> * @swdev_port_obj_add: Add an object to port (see swdev_obj).
>> *
>> * @swdev_port_obj_del: Delete an object from port (see swdev_obj).
>>- *
>>- * @swdev_fib_ipv4_add: Called to add/modify IPv4 route to switch device.
>>- *
>>- * @swdev_fib_ipv4_del: Called to delete IPv4 route from switch device.
>> */
>> struct swdev_ops {
>> int (*swdev_port_attr_get)(struct net_device *dev,
>>@@ -77,13 +83,6 @@ struct swdev_ops {
>> struct swdev_obj *obj);
>> int (*swdev_port_obj_del)(struct net_device *dev,
>> struct swdev_obj *obj);
>>- int (*swdev_fib_ipv4_add)(struct net_device *dev, __be32 dst,
>>- int dst_len, struct fib_info *fi,
>>- u8 tos, u8 type, u32 nlflags,
>>- u32 tb_id);
>>- int (*swdev_fib_ipv4_del)(struct net_device *dev, __be32 dst,
>>- int dst_len, struct fib_info *fi,
>>- u8 tos, u8 type, u32 tb_id);
>> };
>> enum swdev_notifier_type {
>>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>index 98c631c..4dbd4c4 100644
>>--- a/net/switchdev/switchdev.c
>>+++ b/net/switchdev/switchdev.c
>>@@ -535,8 +535,19 @@ static struct net_device *swdev_get_dev_by_nhs(struct fib_info *fi)
>> int swdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
>> u8 tos, u8 type, u32 nlflags, u32 tb_id)
>> {
>>+ struct swdev_obj fib_obj = {
>>+ .id = SWDEV_OBJ_IPV4_FIB,
>>+ .ipv4_fib = {
>>+ .dst = htonl(dst),
>>+ .dst_len = dst_len,
>>+ .fi = fi,
>>+ .tos = tos,
>>+ .type = type,
>>+ .nlflags = nlflags,
>>+ .tb_id = tb_id,
>>+ },
>>+ };
>> struct net_device *dev;
>>- const struct swdev_ops *ops;
>> int err = 0;
>> /* Don't offload route if using custom ip rules or if
>>@@ -554,15 +565,10 @@ int swdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
>> dev = swdev_get_dev_by_nhs(fi);
>> if (!dev)
>> return 0;
>>- ops = dev->swdev_ops;
>>-
>>- if (ops->swdev_fib_ipv4_add) {
>>- err = ops->swdev_fib_ipv4_add(dev, htonl(dst), dst_len,
>>- fi, tos, type, nlflags,
>>- tb_id);
>>- if (!err)
>>- fi->fib_flags |= RTNH_F_EXTERNAL;
>>- }
>>+
>>+ err = swdev_port_obj_add(dev, &fib_obj);
>>+ if (!err)
>>+ fi->fib_flags |= RTNH_F_EXTERNAL;
>> return err;
>> }
>>@@ -583,8 +589,19 @@ EXPORT_SYMBOL_GPL(swdev_fib_ipv4_add);
>> int swdev_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
>> u8 tos, u8 type, u32 tb_id)
>> {
>>+ struct swdev_obj fib_obj = {
>>+ .id = SWDEV_OBJ_IPV4_FIB,
>>+ .ipv4_fib = {
>>+ .dst = htonl(dst),
>>+ .dst_len = dst_len,
>>+ .fi = fi,
>>+ .tos = tos,
>>+ .type = type,
>>+ .nlflags = 0,
>>+ .tb_id = tb_id,
>>+ },
>>+ };
>This looks nice. However, my only concern is we have now ended up adding a
>whole layer of abstraction to all objects. The api abstraction seemed fine.
>But, carrying it to objects and duplicating every object in the swdev layer
>seems too much duplication. Maybe its just me.
>we will end up adding this for bond attributes...vxlan...fdb and nftables
+1
>etc.
>
>The advantage of switchdev in the kernel was to have access the existing
>kernel api and objects directly from switchdev layer.
>In which case, to me, it would be better to skip this extra layer of objects.
>And keep the way you had it originally.
I agree.
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists