[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20140625084002.GA2972@minipsycho.orion>
Date: Wed, 25 Jun 2014 10:40:02 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, pshelar@...ira.com,
cwang@...pensource.com, nicolas.dichtel@...nd.com,
david@...son.dropbear.id.au, sfeldma@...ulusnetworks.com,
sucheta.chakraborty@...gic.com, stephen@...workplumber.org
Subject: Re: [patch net-next] openvswitch: introduce rtnl ops stub
Wed, Jun 25, 2014 at 10:34:27AM CEST, ebiederm@...ssion.com wrote:
>Jiri Pirko <jiri@...nulli.us> writes:
>
>> This stub now allows userspace to see IFLA_INFO_KIND for ovs master and
>> IFLA_INFO_SLAVE_KIND for slave.
>>
>> Note that I added ops->setup check into newlink and dellink in order to
>> prevent creating and deleting openvswitch instances using rtnl for
>> now.
>
>Nacked-by: "Eric W. Biederman" <ebiederm@...ssion.com>
>
>Don't add hacks to the core code to only satisfy your openvswitch
>device.
>
>If the change is justified make it a separate patch and describe why
>it is ok for everyone. As this sits I think you have broken some
>other users of rtnl_link_ops.
Fair enough. Thanks Eric!
>
>Eric
>
>
>> Signed-off-by: Jiri Pirko <jiri@...nulli.us>
>> ---
>> net/core/rtnetlink.c | 5 ++++-
>> net/openvswitch/datapath.c | 9 ++++++++-
>> net/openvswitch/vport-internal_dev.c | 16 ++++++++++++++++
>> net/openvswitch/vport-internal_dev.h | 2 ++
>> 4 files changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 1063996..84affd7 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -1777,7 +1777,7 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh)
>> return -ENODEV;
>>
>> ops = dev->rtnl_link_ops;
>> - if (!ops)
>> + if (!ops || !ops->setup)
>> return -EOPNOTSUPP;
>>
>> ops->dellink(dev, &list_kill);
>> @@ -2038,6 +2038,9 @@ replay:
>> return -EOPNOTSUPP;
>> }
>>
>> + if (!ops->setup)
>> + return -EOPNOTSUPP;
>> +
>> if (!ifname[0])
>> snprintf(ifname, IFNAMSIZ, "%s%%d", ops->kind);
>>
>> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
>> index 0d407bc..fe95b6c 100644
>> --- a/net/openvswitch/datapath.c
>> +++ b/net/openvswitch/datapath.c
>> @@ -2054,10 +2054,14 @@ static int __init dp_init(void)
>>
>> pr_info("Open vSwitch switching datapath\n");
>>
>> - err = ovs_flow_init();
>> + err = ovs_internal_dev_rtnl_link_register();
>> if (err)
>> goto error;
>>
>> + err = ovs_flow_init();
>> + if (err)
>> + goto error_unreg_rtnl_link;
>> +
>> err = ovs_vport_init();
>> if (err)
>> goto error_flow_exit;
>> @@ -2084,6 +2088,8 @@ error_vport_exit:
>> ovs_vport_exit();
>> error_flow_exit:
>> ovs_flow_exit();
>> +error_unreg_rtnl_link:
>> + ovs_internal_dev_rtnl_link_unregister();
>> error:
>> return err;
>> }
>> @@ -2096,6 +2102,7 @@ static void dp_cleanup(void)
>> rcu_barrier();
>> ovs_vport_exit();
>> ovs_flow_exit();
>> + ovs_internal_dev_rtnl_link_unregister();
>> }
>>
>> module_init(dp_init);
>> diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
>> index 789af92..295471a 100644
>> --- a/net/openvswitch/vport-internal_dev.c
>> +++ b/net/openvswitch/vport-internal_dev.c
>> @@ -26,6 +26,7 @@
>>
>> #include <net/dst.h>
>> #include <net/xfrm.h>
>> +#include <net/rtnetlink.h>
>>
>> #include "datapath.h"
>> #include "vport-internal_dev.h"
>> @@ -121,6 +122,10 @@ static const struct net_device_ops internal_dev_netdev_ops = {
>> .ndo_get_stats64 = internal_dev_get_stats,
>> };
>>
>> +static struct rtnl_link_ops internal_dev_link_ops __read_mostly = {
>> + .kind = "openvswitch",
>> +};
>> +
>> static void do_setup(struct net_device *netdev)
>> {
>> ether_setup(netdev);
>> @@ -131,6 +136,7 @@ static void do_setup(struct net_device *netdev)
>> netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>> netdev->destructor = internal_dev_destructor;
>> netdev->ethtool_ops = &internal_dev_ethtool_ops;
>> + netdev->rtnl_link_ops = &internal_dev_link_ops;
>> netdev->tx_queue_len = 0;
>>
>> netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST |
>> @@ -248,3 +254,13 @@ struct vport *ovs_internal_dev_get_vport(struct net_device *netdev)
>>
>> return internal_dev_priv(netdev)->vport;
>> }
>> +
>> +int ovs_internal_dev_rtnl_link_register(void)
>> +{
>> + return rtnl_link_register(&internal_dev_link_ops);
>> +}
>> +
>> +void ovs_internal_dev_rtnl_link_unregister(void)
>> +{
>> + rtnl_link_unregister(&internal_dev_link_ops);
>> +}
>> diff --git a/net/openvswitch/vport-internal_dev.h b/net/openvswitch/vport-internal_dev.h
>> index 9a7d30e..1b179a1 100644
>> --- a/net/openvswitch/vport-internal_dev.h
>> +++ b/net/openvswitch/vport-internal_dev.h
>> @@ -24,5 +24,7 @@
>>
>> int ovs_is_internal_dev(const struct net_device *);
>> struct vport *ovs_internal_dev_get_vport(struct net_device *);
>> +int ovs_internal_dev_rtnl_link_register(void);
>> +void ovs_internal_dev_rtnl_link_unregister(void);
>>
>> #endif /* vport-internal_dev.h */
--
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