* [patch net-next rfc 1/7] net: procfs: use index hashlist instead of name hashlist
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
@ 2019-07-19 11:00 ` Jiri Pirko
2019-07-19 11:00 ` [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist Jiri Pirko
` (8 subsequent siblings)
9 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:00 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Name hashlist is going to be used for more than just dev->name, so use
rather index hashlist for iteration over net_device instances.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/core/net-procfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/net-procfs.c b/net/core/net-procfs.c
index 36347933ec3a..6bbd06f7dc7d 100644
--- a/net/core/net-procfs.c
+++ b/net/core/net-procfs.c
@@ -20,8 +20,8 @@ static inline struct net_device *dev_from_same_bucket(struct seq_file *seq, loff
struct hlist_head *h;
unsigned int count = 0, offset = get_offset(*pos);
- h = &net->dev_name_head[get_bucket(*pos)];
- hlist_for_each_entry_rcu(dev, h, name_hlist) {
+ h = &net->dev_index_head[get_bucket(*pos)];
+ hlist_for_each_entry_rcu(dev, h, index_hlist) {
if (++count == offset)
return dev;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
2019-07-19 11:00 ` [patch net-next rfc 1/7] net: procfs: use index hashlist instead of name hashlist Jiri Pirko
@ 2019-07-19 11:00 ` Jiri Pirko
2019-07-19 16:29 ` Stephen Hemminger
2019-08-08 4:34 ` kbuild test robot
2019-07-19 11:00 ` [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames Jiri Pirko
` (7 subsequent siblings)
9 siblings, 2 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:00 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/linux/netdevice.h | 10 +++-
net/core/dev.c | 96 +++++++++++++++++++++++++++++++--------
2 files changed, 86 insertions(+), 20 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 88292953aa6f..74f99f127b0e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -918,6 +918,12 @@ struct dev_ifalias {
struct devlink;
struct tlsdev_ops;
+struct netdev_name_node {
+ struct hlist_node hlist;
+ struct net_device *dev;
+ char *name;
+};
+
/*
* This structure defines the management hooks for network devices.
* The following hooks can be defined; unless noted otherwise, they are
@@ -1551,7 +1557,7 @@ enum netdev_priv_flags {
* (i.e. as seen by users in the "Space.c" file). It is the name
* of the interface.
*
- * @name_hlist: Device name hash chain, please keep it close to name[]
+ * @name_node: Name hashlist node
* @ifalias: SNMP alias
* @mem_end: Shared memory end
* @mem_start: Shared memory start
@@ -1761,7 +1767,7 @@ enum netdev_priv_flags {
struct net_device {
char name[IFNAMSIZ];
- struct hlist_node name_hlist;
+ struct netdev_name_node *name_node;
struct dev_ifalias __rcu *ifalias;
/*
* I/O specific fields
diff --git a/net/core/dev.c b/net/core/dev.c
index fc676b2610e3..ad0d42fbdeee 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -228,6 +228,66 @@ static inline void rps_unlock(struct softnet_data *sd)
#endif
}
+static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
+ char *name)
+{
+ struct netdev_name_node *name_node;
+
+ name_node = kzalloc(sizeof(*name_node), GFP_KERNEL);
+ if (!name_node)
+ return NULL;
+ name_node->dev = dev;
+ name_node->name = name;
+ return name_node;
+}
+
+static struct netdev_name_node *
+netdev_name_node_head_alloc(struct net_device *dev)
+{
+ return netdev_name_node_alloc(dev, dev->name);
+}
+
+static void netdev_name_node_free(struct netdev_name_node *name_node)
+{
+ kfree(name_node);
+}
+
+static void netdev_name_node_add(struct net *net,
+ struct netdev_name_node *name_node)
+{
+ hlist_add_head_rcu(&name_node->hlist,
+ dev_name_hash(net, name_node->name));
+}
+
+static void netdev_name_node_del(struct netdev_name_node *name_node)
+{
+ hlist_del_rcu(&name_node->hlist);
+}
+
+static struct netdev_name_node *netdev_name_node_lookup(struct net *net,
+ const char *name)
+{
+ struct hlist_head *head = dev_name_hash(net, name);
+ struct netdev_name_node *name_node;
+
+ hlist_for_each_entry(name_node, head, hlist)
+ if (!strcmp(name_node->name, name))
+ return name_node;
+ return NULL;
+}
+
+static struct netdev_name_node *netdev_name_node_lookup_rcu(struct net *net,
+ const char *name)
+{
+ struct hlist_head *head = dev_name_hash(net, name);
+ struct netdev_name_node *name_node;
+
+ hlist_for_each_entry_rcu(name_node, head, hlist)
+ if (!strcmp(name_node->name, name))
+ return name_node;
+ return NULL;
+}
+
/* Device list insertion */
static void list_netdevice(struct net_device *dev)
{
@@ -237,7 +297,7 @@ static void list_netdevice(struct net_device *dev)
write_lock_bh(&dev_base_lock);
list_add_tail_rcu(&dev->dev_list, &net->dev_base_head);
- hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
+ netdev_name_node_add(net, dev->name_node);
hlist_add_head_rcu(&dev->index_hlist,
dev_index_hash(net, dev->ifindex));
write_unlock_bh(&dev_base_lock);
@@ -255,7 +315,7 @@ static void unlist_netdevice(struct net_device *dev)
/* Unlink dev from the device chain */
write_lock_bh(&dev_base_lock);
list_del_rcu(&dev->dev_list);
- hlist_del_rcu(&dev->name_hlist);
+ netdev_name_node_del(dev->name_node);
hlist_del_rcu(&dev->index_hlist);
write_unlock_bh(&dev_base_lock);
@@ -733,14 +793,10 @@ EXPORT_SYMBOL_GPL(dev_fill_metadata_dst);
struct net_device *__dev_get_by_name(struct net *net, const char *name)
{
- struct net_device *dev;
- struct hlist_head *head = dev_name_hash(net, name);
+ struct netdev_name_node *node_name;
- hlist_for_each_entry(dev, head, name_hlist)
- if (!strncmp(dev->name, name, IFNAMSIZ))
- return dev;
-
- return NULL;
+ node_name = netdev_name_node_lookup(net, name);
+ return node_name ? node_name->dev : NULL;
}
EXPORT_SYMBOL(__dev_get_by_name);
@@ -758,14 +814,10 @@ EXPORT_SYMBOL(__dev_get_by_name);
struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
{
- struct net_device *dev;
- struct hlist_head *head = dev_name_hash(net, name);
-
- hlist_for_each_entry_rcu(dev, head, name_hlist)
- if (!strncmp(dev->name, name, IFNAMSIZ))
- return dev;
+ struct netdev_name_node *node_name;
- return NULL;
+ node_name = netdev_name_node_lookup_rcu(net, name);
+ return node_name ? node_name->dev : NULL;
}
EXPORT_SYMBOL(dev_get_by_name_rcu);
@@ -1232,13 +1284,13 @@ int dev_change_name(struct net_device *dev, const char *newname)
netdev_adjacent_rename_links(dev, oldname);
write_lock_bh(&dev_base_lock);
- hlist_del_rcu(&dev->name_hlist);
+ netdev_name_node_del(dev->name_node);
write_unlock_bh(&dev_base_lock);
synchronize_rcu();
write_lock_bh(&dev_base_lock);
- hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
+ netdev_name_node_add(net, dev->name_node);
write_unlock_bh(&dev_base_lock);
ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
@@ -8206,6 +8258,8 @@ static void rollback_registered_many(struct list_head *head)
dev_uc_flush(dev);
dev_mc_flush(dev);
+ netdev_name_node_free(dev->name_node);
+
if (dev->netdev_ops->ndo_uninit)
dev->netdev_ops->ndo_uninit(dev);
@@ -8648,6 +8702,10 @@ int register_netdevice(struct net_device *dev)
if (ret < 0)
goto out;
+ dev->name_node = netdev_name_node_head_alloc(dev);
+ if (!dev->name_node)
+ goto out;
+
/* Init, if this function is available */
if (dev->netdev_ops->ndo_init) {
ret = dev->netdev_ops->ndo_init(dev);
@@ -8767,6 +8825,8 @@ int register_netdevice(struct net_device *dev)
return ret;
err_uninit:
+ if (dev->name_node)
+ netdev_name_node_free(dev->name_node);
if (dev->netdev_ops->ndo_uninit)
dev->netdev_ops->ndo_uninit(dev);
if (dev->priv_destructor)
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist
2019-07-19 11:00 ` [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist Jiri Pirko
@ 2019-07-19 16:29 ` Stephen Hemminger
2019-07-19 19:17 ` Jiri Pirko
2019-08-08 4:34 ` kbuild test robot
1 sibling, 1 reply; 76+ messages in thread
From: Stephen Hemminger @ 2019-07-19 16:29 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
On Fri, 19 Jul 2019 13:00:24 +0200
Jiri Pirko <jiri@resnulli.us> wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/linux/netdevice.h | 10 +++-
> net/core/dev.c | 96 +++++++++++++++++++++++++++++++--------
> 2 files changed, 86 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 88292953aa6f..74f99f127b0e 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -918,6 +918,12 @@ struct dev_ifalias {
> struct devlink;
> struct tlsdev_ops;
>
> +struct netdev_name_node {
> + struct hlist_node hlist;
> + struct net_device *dev;
> + char *name
You probably can make this const char *
Do you want to add __rcu to this list?
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist
2019-07-19 16:29 ` Stephen Hemminger
@ 2019-07-19 19:17 ` Jiri Pirko
2019-07-19 20:26 ` Stephen Hemminger
0 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 19:17 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
Fri, Jul 19, 2019 at 06:29:36PM CEST, stephen@networkplumber.org wrote:
>On Fri, 19 Jul 2019 13:00:24 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> include/linux/netdevice.h | 10 +++-
>> net/core/dev.c | 96 +++++++++++++++++++++++++++++++--------
>> 2 files changed, 86 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 88292953aa6f..74f99f127b0e 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -918,6 +918,12 @@ struct dev_ifalias {
>> struct devlink;
>> struct tlsdev_ops;
>>
>> +struct netdev_name_node {
>> + struct hlist_node hlist;
>> + struct net_device *dev;
>> + char *name
>
>You probably can make this const char *
I'll try.
>
>Do you want to add __rcu to this list?
Which list?
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist
2019-07-19 19:17 ` Jiri Pirko
@ 2019-07-19 20:26 ` Stephen Hemminger
2019-07-20 7:15 ` Jiri Pirko
2019-09-13 9:52 ` Jiri Pirko
0 siblings, 2 replies; 76+ messages in thread
From: Stephen Hemminger @ 2019-07-19 20:26 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
On Fri, 19 Jul 2019 21:17:40 +0200
Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Jul 19, 2019 at 06:29:36PM CEST, stephen@networkplumber.org wrote:
> >On Fri, 19 Jul 2019 13:00:24 +0200
> >Jiri Pirko <jiri@resnulli.us> wrote:
> >
> >> From: Jiri Pirko <jiri@mellanox.com>
> >>
> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> >> ---
> >> include/linux/netdevice.h | 10 +++-
> >> net/core/dev.c | 96 +++++++++++++++++++++++++++++++--------
> >> 2 files changed, 86 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index 88292953aa6f..74f99f127b0e 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -918,6 +918,12 @@ struct dev_ifalias {
> >> struct devlink;
> >> struct tlsdev_ops;
> >>
> >> +struct netdev_name_node {
> >> + struct hlist_node hlist;
> >> + struct net_device *dev;
> >> + char *name
> >
> >You probably can make this const char *
Don't bother, it looks ok as is. the problem is you would have
to cast it when calling free.
> >Do you want to add __rcu to this list?
>
> Which list?
>
struct netdev_name_node __rcu *name_node;
You might also want to explictly init the hlist node rather
than relying on the fact that zero is an empty node ptr.
static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
- char *name)
+ const char *name)
{
struct netdev_name_node *name_node;
- name_node = kzalloc(sizeof(*name_node), GFP_KERNEL);
+ name_node = kmalloc(sizeof(*name_node));
if (!name_node)
return NULL;
+
+ INIT_HLIST_NODE(&name_node->hlist);
name_node->dev = dev;
name_node->name = name;
return name_node;
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist
2019-07-19 20:26 ` Stephen Hemminger
@ 2019-07-20 7:15 ` Jiri Pirko
2019-09-13 9:52 ` Jiri Pirko
1 sibling, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-20 7:15 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
Fri, Jul 19, 2019 at 10:26:49PM CEST, stephen@networkplumber.org wrote:
>On Fri, 19 Jul 2019 21:17:40 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Fri, Jul 19, 2019 at 06:29:36PM CEST, stephen@networkplumber.org wrote:
>> >On Fri, 19 Jul 2019 13:00:24 +0200
>> >Jiri Pirko <jiri@resnulli.us> wrote:
>> >
>> >> From: Jiri Pirko <jiri@mellanox.com>
>> >>
>> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> >> ---
>> >> include/linux/netdevice.h | 10 +++-
>> >> net/core/dev.c | 96 +++++++++++++++++++++++++++++++--------
>> >> 2 files changed, 86 insertions(+), 20 deletions(-)
>> >>
>> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> >> index 88292953aa6f..74f99f127b0e 100644
>> >> --- a/include/linux/netdevice.h
>> >> +++ b/include/linux/netdevice.h
>> >> @@ -918,6 +918,12 @@ struct dev_ifalias {
>> >> struct devlink;
>> >> struct tlsdev_ops;
>> >>
>> >> +struct netdev_name_node {
>> >> + struct hlist_node hlist;
>> >> + struct net_device *dev;
>> >> + char *name
>> >
>> >You probably can make this const char *
>
>Don't bother, it looks ok as is. the problem is you would have
>to cast it when calling free.
>
>> >Do you want to add __rcu to this list?
>>
>> Which list?
>>
>
> struct netdev_name_node __rcu *name_node;
>
>You might also want to explictly init the hlist node rather
>than relying on the fact that zero is an empty node ptr.
Okay. Will process this in. Thanks!
>
>
> static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
>- char *name)
>+ const char *name)
> {
> struct netdev_name_node *name_node;
>
>- name_node = kzalloc(sizeof(*name_node), GFP_KERNEL);
>+ name_node = kmalloc(sizeof(*name_node));
> if (!name_node)
> return NULL;
>+
>+ INIT_HLIST_NODE(&name_node->hlist);
> name_node->dev = dev;
> name_node->name = name;
> return name_node;
>
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist
2019-07-19 20:26 ` Stephen Hemminger
2019-07-20 7:15 ` Jiri Pirko
@ 2019-09-13 9:52 ` Jiri Pirko
1 sibling, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-09-13 9:52 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
Fri, Jul 19, 2019 at 10:26:49PM CEST, stephen@networkplumber.org wrote:
>On Fri, 19 Jul 2019 21:17:40 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Fri, Jul 19, 2019 at 06:29:36PM CEST, stephen@networkplumber.org wrote:
>> >On Fri, 19 Jul 2019 13:00:24 +0200
>> >Jiri Pirko <jiri@resnulli.us> wrote:
>> >
>> >> From: Jiri Pirko <jiri@mellanox.com>
>> >>
>> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> >> ---
>> >> include/linux/netdevice.h | 10 +++-
>> >> net/core/dev.c | 96 +++++++++++++++++++++++++++++++--------
>> >> 2 files changed, 86 insertions(+), 20 deletions(-)
>> >>
>> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> >> index 88292953aa6f..74f99f127b0e 100644
>> >> --- a/include/linux/netdevice.h
>> >> +++ b/include/linux/netdevice.h
>> >> @@ -918,6 +918,12 @@ struct dev_ifalias {
>> >> struct devlink;
>> >> struct tlsdev_ops;
>> >>
>> >> +struct netdev_name_node {
>> >> + struct hlist_node hlist;
>> >> + struct net_device *dev;
>> >> + char *name
>> >
>> >You probably can make this const char *
>
>Don't bother, it looks ok as is. the problem is you would have
>to cast it when calling free.
Actually, it does not. So I'll make it const.
>
>> >Do you want to add __rcu to this list?
>>
>> Which list?
>>
>
> struct netdev_name_node __rcu *name_node;
This is stored in struct net_device for ifname. The pointer is never
accessed from rcu read.
>
>You might also want to explictly init the hlist node rather
>than relying on the fact that zero is an empty node ptr.
>
>
> static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
>- char *name)
>+ const char *name)
> {
> struct netdev_name_node *name_node;
>
>- name_node = kzalloc(sizeof(*name_node), GFP_KERNEL);
>+ name_node = kmalloc(sizeof(*name_node));
> if (!name_node)
> return NULL;
>+
>+ INIT_HLIST_NODE(&name_node->hlist);
Will do. Thanks!
> name_node->dev = dev;
> name_node->name = name;
> return name_node;
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist
2019-07-19 11:00 ` [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist Jiri Pirko
2019-07-19 16:29 ` Stephen Hemminger
@ 2019-08-08 4:34 ` kbuild test robot
1 sibling, 0 replies; 76+ messages in thread
From: kbuild test robot @ 2019-08-08 4:34 UTC (permalink / raw)
To: Jiri Pirko
Cc: kbuild-all, netdev, davem, jakub.kicinski, sthemmin, dsahern,
dcbw, mkubecek, andrew, parav, saeedm, mlxsw
[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]
Hi Jiri,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jiri-Pirko/net-introduce-alternative-names-for-network-interfaces/20190720-094639
config: arc-defconfig (attached as .config)
compiler: arc-elf-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
{standard input}: Assembler messages:
{standard input}:147: Error: inappropriate arguments for opcode 'st'
>> {standard input}:150: Error: Insn j_s has a jump/branch instruction j_s in its delay slot.
{standard input}:175: Error: inappropriate arguments for opcode 'st'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8655 bytes --]
^ permalink raw reply [flat|nested] 76+ messages in thread
* [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
2019-07-19 11:00 ` [patch net-next rfc 1/7] net: procfs: use index hashlist instead of name hashlist Jiri Pirko
2019-07-19 11:00 ` [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist Jiri Pirko
@ 2019-07-19 11:00 ` Jiri Pirko
2019-07-20 3:58 ` Jakub Kicinski
2019-08-09 4:11 ` Roopa Prabhu
2019-07-19 11:00 ` [patch net-next rfc 4/7] net: rtnetlink: put alternative names to getlink message Jiri Pirko
` (6 subsequent siblings)
9 siblings, 2 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:00 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Add two commands to add and delete alternative ifnames for net device.
Each net device can have multiple alternative names.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/linux/netdevice.h | 4 ++
include/uapi/linux/if.h | 1 +
include/uapi/linux/if_link.h | 1 +
include/uapi/linux/rtnetlink.h | 7 +++
net/core/dev.c | 58 ++++++++++++++++++-
net/core/rtnetlink.c | 102 +++++++++++++++++++++++++++++++++
security/selinux/nlmsgtab.c | 4 +-
7 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 74f99f127b0e..6922fdb483ca 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -920,10 +920,14 @@ struct tlsdev_ops;
struct netdev_name_node {
struct hlist_node hlist;
+ struct list_head list;
struct net_device *dev;
char *name;
};
+int netdev_name_node_alt_create(struct net_device *dev, char *name);
+int netdev_name_node_alt_destroy(struct net_device *dev, char *name);
+
/*
* This structure defines the management hooks for network devices.
* The following hooks can be defined; unless noted otherwise, they are
diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 7fea0fd7d6f5..4bf33344aab1 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -33,6 +33,7 @@
#define IFNAMSIZ 16
#endif /* __UAPI_DEF_IF_IFNAMSIZ */
#define IFALIASZ 256
+#define ALTIFNAMSIZ 128
#include <linux/hdlc/ioctl.h>
/* For glibc compatibility. An empty enum does not compile. */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 4a8c02cafa9a..92268946e04a 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -167,6 +167,7 @@ enum {
IFLA_NEW_IFINDEX,
IFLA_MIN_MTU,
IFLA_MAX_MTU,
+ IFLA_ALT_IFNAME_MOD, /* Alternative ifname to add/delete */
__IFLA_MAX
};
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index ce2a623abb75..b36cfd83eb76 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -164,6 +164,13 @@ enum {
RTM_GETNEXTHOP,
#define RTM_GETNEXTHOP RTM_GETNEXTHOP
+ RTM_NEWALTIFNAME = 108,
+#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
+ RTM_DELALTIFNAME,
+#define RTM_DELALTIFNAME RTM_DELALTIFNAME
+ RTM_GETALTIFNAME,
+#define RTM_GETALTIFNAME RTM_GETALTIFNAME
+
__RTM_MAX,
#define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
};
diff --git a/net/core/dev.c b/net/core/dev.c
index ad0d42fbdeee..2a3be2b279d3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -244,7 +244,13 @@ static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
static struct netdev_name_node *
netdev_name_node_head_alloc(struct net_device *dev)
{
- return netdev_name_node_alloc(dev, dev->name);
+ struct netdev_name_node *name_node;
+
+ name_node = netdev_name_node_alloc(dev, dev->name);
+ if (!name_node)
+ return NULL;
+ INIT_LIST_HEAD(&name_node->list);
+ return name_node;
}
static void netdev_name_node_free(struct netdev_name_node *name_node)
@@ -288,6 +294,55 @@ static struct netdev_name_node *netdev_name_node_lookup_rcu(struct net *net,
return NULL;
}
+int netdev_name_node_alt_create(struct net_device *dev, char *name)
+{
+ struct netdev_name_node *name_node;
+ struct net *net = dev_net(dev);
+
+ name_node = netdev_name_node_lookup(net, name);
+ if (name_node)
+ return -EEXIST;
+ name_node = netdev_name_node_alloc(dev, name);
+ if (!name_node)
+ return -ENOMEM;
+ netdev_name_node_add(net, name_node);
+ /* The node that holds dev->name acts as a head of per-device list. */
+ list_add_tail(&name_node->list, &dev->name_node->list);
+
+ return 0;
+}
+EXPORT_SYMBOL(netdev_name_node_alt_create);
+
+static void __netdev_name_node_alt_destroy(struct netdev_name_node *name_node)
+{
+ list_del(&name_node->list);
+ netdev_name_node_del(name_node);
+ kfree(name_node->name);
+ netdev_name_node_free(name_node);
+}
+
+int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
+{
+ struct netdev_name_node *name_node;
+ struct net *net = dev_net(dev);
+
+ name_node = netdev_name_node_lookup(net, name);
+ if (!name_node)
+ return -ENOENT;
+ __netdev_name_node_alt_destroy(name_node);
+
+ return 0;
+}
+EXPORT_SYMBOL(netdev_name_node_alt_destroy);
+
+static void netdev_name_node_alt_flush(struct net_device *dev)
+{
+ struct netdev_name_node *name_node, *tmp;
+
+ list_for_each_entry_safe(name_node, tmp, &dev->name_node->list, list)
+ __netdev_name_node_alt_destroy(name_node);
+}
+
/* Device list insertion */
static void list_netdevice(struct net_device *dev)
{
@@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
dev_uc_flush(dev);
dev_mc_flush(dev);
+ netdev_name_node_alt_flush(dev);
netdev_name_node_free(dev->name_node);
if (dev->netdev_ops->ndo_uninit)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1ee6460f8275..7a2010b16e10 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
[IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
[IFLA_MIN_MTU] = { .type = NLA_U32 },
[IFLA_MAX_MTU] = { .type = NLA_U32 },
+ [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
+ .len = ALTIFNAMSIZ - 1 },
};
static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -3373,6 +3375,103 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
return err;
}
+static int rtnl_newaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
+ struct netlink_ext_ack *extack)
+{
+ struct net *net = sock_net(skb->sk);
+ struct nlattr *tb[IFLA_MAX + 1];
+ struct net_device *dev;
+ struct ifinfomsg *ifm;
+ char *new_alt_ifname;
+ int err;
+
+ err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
+ if (err)
+ return err;
+
+ err = rtnl_ensure_unique_netns(tb, extack, true);
+ if (err)
+ return err;
+
+ ifm = nlmsg_data(nlh);
+ if (ifm->ifi_index > 0) {
+ dev = __dev_get_by_index(net, ifm->ifi_index);
+ } else if (tb[IFLA_IFNAME]) {
+ char ifname[IFNAMSIZ];
+
+ nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+ dev = __dev_get_by_name(net, ifname);
+ } else {
+ return -EINVAL;
+ }
+
+ if (!dev)
+ return -ENODEV;
+
+ if (!tb[IFLA_ALT_IFNAME_MOD])
+ return -EINVAL;
+
+ new_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
+ if (!new_alt_ifname)
+ return -ENOMEM;
+
+ err = netdev_name_node_alt_create(dev, new_alt_ifname);
+ if (err)
+ goto out_free_new_alt_ifname;
+
+ return 0;
+
+out_free_new_alt_ifname:
+ kfree(new_alt_ifname);
+ return err;
+}
+
+static int rtnl_delaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
+ struct netlink_ext_ack *extack)
+{
+ struct net *net = sock_net(skb->sk);
+ struct nlattr *tb[IFLA_MAX + 1];
+ struct net_device *dev;
+ struct ifinfomsg *ifm;
+ char *del_alt_ifname;
+ int err;
+
+ err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
+ if (err)
+ return err;
+
+ err = rtnl_ensure_unique_netns(tb, extack, true);
+ if (err)
+ return err;
+
+ ifm = nlmsg_data(nlh);
+ if (ifm->ifi_index > 0) {
+ dev = __dev_get_by_index(net, ifm->ifi_index);
+ } else if (tb[IFLA_IFNAME]) {
+ char ifname[IFNAMSIZ];
+
+ nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+ dev = __dev_get_by_name(net, ifname);
+ } else {
+ return -EINVAL;
+ }
+
+ if (!dev)
+ return -ENODEV;
+
+ if (!tb[IFLA_ALT_IFNAME_MOD])
+ return -EINVAL;
+
+ del_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
+ if (!del_alt_ifname)
+ return -ENOMEM;
+
+ err = netdev_name_node_alt_destroy(dev, del_alt_ifname);
+ kfree(del_alt_ifname);
+
+ return err;
+}
+
static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct net *net = sock_net(skb->sk);
@@ -5331,6 +5430,9 @@ void __init rtnetlink_init(void)
rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, 0);
rtnl_register(PF_UNSPEC, RTM_GETNETCONF, NULL, rtnl_dump_all, 0);
+ rtnl_register(PF_UNSPEC, RTM_NEWALTIFNAME, rtnl_newaltifname, NULL, 0);
+ rtnl_register(PF_UNSPEC, RTM_DELALTIFNAME, rtnl_delaltifname, NULL, 0);
+
rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, 0);
rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, 0);
rtnl_register(PF_BRIDGE, RTM_GETNEIGH, rtnl_fdb_get, rtnl_fdb_dump, 0);
diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
index 58345ba0528e..a712b54c666c 100644
--- a/security/selinux/nlmsgtab.c
+++ b/security/selinux/nlmsgtab.c
@@ -83,6 +83,8 @@ static const struct nlmsg_perm nlmsg_route_perms[] =
{ RTM_NEWNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
{ RTM_DELNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
{ RTM_GETNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_READ },
+ { RTM_NEWALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
+ { RTM_DELALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
};
static const struct nlmsg_perm nlmsg_tcpdiag_perms[] =
@@ -166,7 +168,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
* structures at the top of this file with the new mappings
* before updating the BUILD_BUG_ON() macro!
*/
- BUILD_BUG_ON(RTM_MAX != (RTM_NEWNEXTHOP + 3));
+ BUILD_BUG_ON(RTM_MAX != (RTM_NEWALTIFNAME + 3));
err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
sizeof(nlmsg_route_perms));
break;
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-07-19 11:00 ` [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames Jiri Pirko
@ 2019-07-20 3:58 ` Jakub Kicinski
2019-07-20 7:20 ` Jiri Pirko
2019-08-09 4:11 ` Roopa Prabhu
1 sibling, 1 reply; 76+ messages in thread
From: Jakub Kicinski @ 2019-07-20 3:58 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
On Fri, 19 Jul 2019 13:00:25 +0200, Jiri Pirko wrote:
> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
> +{
> + struct netdev_name_node *name_node;
> + struct net *net = dev_net(dev);
> +
> + name_node = netdev_name_node_lookup(net, name);
> + if (!name_node)
> + return -ENOENT;
> + __netdev_name_node_alt_destroy(name_node);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
I was surprised to see the exports are they strictly necessary?
Just wondering..
> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
> dev_uc_flush(dev);
> dev_mc_flush(dev);
>
> + netdev_name_node_alt_flush(dev);
> netdev_name_node_free(dev->name_node);
>
> if (dev->netdev_ops->ndo_uninit)
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 1ee6460f8275..7a2010b16e10 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
> [IFLA_MIN_MTU] = { .type = NLA_U32 },
> [IFLA_MAX_MTU] = { .type = NLA_U32 },
> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
> + .len = ALTIFNAMSIZ - 1 },
Should we set:
.strict_start_type = IFLA_ALT_IFNAME_MOD
?
> };
>
> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-07-20 3:58 ` Jakub Kicinski
@ 2019-07-20 7:20 ` Jiri Pirko
0 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-20 7:20 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
Sat, Jul 20, 2019 at 05:58:49AM CEST, jakub.kicinski@netronome.com wrote:
>On Fri, 19 Jul 2019 13:00:25 +0200, Jiri Pirko wrote:
>> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
>> +{
>> + struct netdev_name_node *name_node;
>> + struct net *net = dev_net(dev);
>> +
>> + name_node = netdev_name_node_lookup(net, name);
>> + if (!name_node)
>> + return -ENOENT;
>> + __netdev_name_node_alt_destroy(name_node);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
>
>I was surprised to see the exports are they strictly necessary?
Well I call them from net/core/rtnetlink.c, so yes. Maybe I'm missing
your point...
>Just wondering..
>
>> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
>> dev_uc_flush(dev);
>> dev_mc_flush(dev);
>>
>> + netdev_name_node_alt_flush(dev);
>> netdev_name_node_free(dev->name_node);
>>
>> if (dev->netdev_ops->ndo_uninit)
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 1ee6460f8275..7a2010b16e10 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
>> [IFLA_MIN_MTU] = { .type = NLA_U32 },
>> [IFLA_MAX_MTU] = { .type = NLA_U32 },
>> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
>> + .len = ALTIFNAMSIZ - 1 },
>
>Should we set:
>
> .strict_start_type = IFLA_ALT_IFNAME_MOD
Probably yes. Will add it.
>
>?
>
>> };
>>
>> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-07-19 11:00 ` [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames Jiri Pirko
2019-07-20 3:58 ` Jakub Kicinski
@ 2019-08-09 4:11 ` Roopa Prabhu
2019-08-09 6:25 ` Jiri Pirko
1 sibling, 1 reply; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-09 4:11 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jakub Kicinski, Stephen Hemminger,
David Ahern, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Fri, Jul 19, 2019 at 4:00 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@mellanox.com>
>
> Add two commands to add and delete alternative ifnames for net device.
> Each net device can have multiple alternative names.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/linux/netdevice.h | 4 ++
> include/uapi/linux/if.h | 1 +
> include/uapi/linux/if_link.h | 1 +
> include/uapi/linux/rtnetlink.h | 7 +++
> net/core/dev.c | 58 ++++++++++++++++++-
> net/core/rtnetlink.c | 102 +++++++++++++++++++++++++++++++++
> security/selinux/nlmsgtab.c | 4 +-
> 7 files changed, 175 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 74f99f127b0e..6922fdb483ca 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -920,10 +920,14 @@ struct tlsdev_ops;
>
> struct netdev_name_node {
> struct hlist_node hlist;
> + struct list_head list;
> struct net_device *dev;
> char *name;
> };
>
> +int netdev_name_node_alt_create(struct net_device *dev, char *name);
> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name);
> +
> /*
> * This structure defines the management hooks for network devices.
> * The following hooks can be defined; unless noted otherwise, they are
> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> index 7fea0fd7d6f5..4bf33344aab1 100644
> --- a/include/uapi/linux/if.h
> +++ b/include/uapi/linux/if.h
> @@ -33,6 +33,7 @@
> #define IFNAMSIZ 16
> #endif /* __UAPI_DEF_IF_IFNAMSIZ */
> #define IFALIASZ 256
> +#define ALTIFNAMSIZ 128
> #include <linux/hdlc/ioctl.h>
>
> /* For glibc compatibility. An empty enum does not compile. */
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 4a8c02cafa9a..92268946e04a 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -167,6 +167,7 @@ enum {
> IFLA_NEW_IFINDEX,
> IFLA_MIN_MTU,
> IFLA_MAX_MTU,
> + IFLA_ALT_IFNAME_MOD, /* Alternative ifname to add/delete */
> __IFLA_MAX
> };
>
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index ce2a623abb75..b36cfd83eb76 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -164,6 +164,13 @@ enum {
> RTM_GETNEXTHOP,
> #define RTM_GETNEXTHOP RTM_GETNEXTHOP
>
> + RTM_NEWALTIFNAME = 108,
> +#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
> + RTM_DELALTIFNAME,
> +#define RTM_DELALTIFNAME RTM_DELALTIFNAME
> + RTM_GETALTIFNAME,
> +#define RTM_GETALTIFNAME RTM_GETALTIFNAME
> +
I might have missed the prior discussion, why do we need new commands
?. can't this simply be part of RTM_*LINK and we use RTM_SETLINK to
set alternate names ?
> __RTM_MAX,
> #define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
> };
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ad0d42fbdeee..2a3be2b279d3 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -244,7 +244,13 @@ static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
> static struct netdev_name_node *
> netdev_name_node_head_alloc(struct net_device *dev)
> {
> - return netdev_name_node_alloc(dev, dev->name);
> + struct netdev_name_node *name_node;
> +
> + name_node = netdev_name_node_alloc(dev, dev->name);
> + if (!name_node)
> + return NULL;
> + INIT_LIST_HEAD(&name_node->list);
> + return name_node;
> }
>
> static void netdev_name_node_free(struct netdev_name_node *name_node)
> @@ -288,6 +294,55 @@ static struct netdev_name_node *netdev_name_node_lookup_rcu(struct net *net,
> return NULL;
> }
>
> +int netdev_name_node_alt_create(struct net_device *dev, char *name)
> +{
> + struct netdev_name_node *name_node;
> + struct net *net = dev_net(dev);
> +
> + name_node = netdev_name_node_lookup(net, name);
> + if (name_node)
> + return -EEXIST;
> + name_node = netdev_name_node_alloc(dev, name);
> + if (!name_node)
> + return -ENOMEM;
> + netdev_name_node_add(net, name_node);
> + /* The node that holds dev->name acts as a head of per-device list. */
> + list_add_tail(&name_node->list, &dev->name_node->list);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(netdev_name_node_alt_create);
> +
> +static void __netdev_name_node_alt_destroy(struct netdev_name_node *name_node)
> +{
> + list_del(&name_node->list);
> + netdev_name_node_del(name_node);
> + kfree(name_node->name);
> + netdev_name_node_free(name_node);
> +}
> +
> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
> +{
> + struct netdev_name_node *name_node;
> + struct net *net = dev_net(dev);
> +
> + name_node = netdev_name_node_lookup(net, name);
> + if (!name_node)
> + return -ENOENT;
> + __netdev_name_node_alt_destroy(name_node);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
> +
> +static void netdev_name_node_alt_flush(struct net_device *dev)
> +{
> + struct netdev_name_node *name_node, *tmp;
> +
> + list_for_each_entry_safe(name_node, tmp, &dev->name_node->list, list)
> + __netdev_name_node_alt_destroy(name_node);
> +}
> +
> /* Device list insertion */
> static void list_netdevice(struct net_device *dev)
> {
> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
> dev_uc_flush(dev);
> dev_mc_flush(dev);
>
> + netdev_name_node_alt_flush(dev);
> netdev_name_node_free(dev->name_node);
>
> if (dev->netdev_ops->ndo_uninit)
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 1ee6460f8275..7a2010b16e10 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
> [IFLA_MIN_MTU] = { .type = NLA_U32 },
> [IFLA_MAX_MTU] = { .type = NLA_U32 },
> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
> + .len = ALTIFNAMSIZ - 1 },
> };
>
> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
> @@ -3373,6 +3375,103 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> return err;
> }
>
> +static int rtnl_newaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
> + struct netlink_ext_ack *extack)
> +{
> + struct net *net = sock_net(skb->sk);
> + struct nlattr *tb[IFLA_MAX + 1];
> + struct net_device *dev;
> + struct ifinfomsg *ifm;
> + char *new_alt_ifname;
> + int err;
> +
> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
> + if (err)
> + return err;
> +
> + err = rtnl_ensure_unique_netns(tb, extack, true);
> + if (err)
> + return err;
> +
> + ifm = nlmsg_data(nlh);
> + if (ifm->ifi_index > 0) {
> + dev = __dev_get_by_index(net, ifm->ifi_index);
> + } else if (tb[IFLA_IFNAME]) {
> + char ifname[IFNAMSIZ];
> +
> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
> + dev = __dev_get_by_name(net, ifname);
> + } else {
> + return -EINVAL;
> + }
> +
> + if (!dev)
> + return -ENODEV;
> +
> + if (!tb[IFLA_ALT_IFNAME_MOD])
> + return -EINVAL;
> +
> + new_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
> + if (!new_alt_ifname)
> + return -ENOMEM;
> +
> + err = netdev_name_node_alt_create(dev, new_alt_ifname);
> + if (err)
> + goto out_free_new_alt_ifname;
> +
> + return 0;
> +
> +out_free_new_alt_ifname:
> + kfree(new_alt_ifname);
> + return err;
> +}
> +
> +static int rtnl_delaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
> + struct netlink_ext_ack *extack)
> +{
> + struct net *net = sock_net(skb->sk);
> + struct nlattr *tb[IFLA_MAX + 1];
> + struct net_device *dev;
> + struct ifinfomsg *ifm;
> + char *del_alt_ifname;
> + int err;
> +
> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
> + if (err)
> + return err;
> +
> + err = rtnl_ensure_unique_netns(tb, extack, true);
> + if (err)
> + return err;
> +
> + ifm = nlmsg_data(nlh);
> + if (ifm->ifi_index > 0) {
> + dev = __dev_get_by_index(net, ifm->ifi_index);
> + } else if (tb[IFLA_IFNAME]) {
> + char ifname[IFNAMSIZ];
> +
> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
> + dev = __dev_get_by_name(net, ifname);
> + } else {
> + return -EINVAL;
> + }
> +
> + if (!dev)
> + return -ENODEV;
> +
> + if (!tb[IFLA_ALT_IFNAME_MOD])
> + return -EINVAL;
> +
> + del_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
> + if (!del_alt_ifname)
> + return -ENOMEM;
> +
> + err = netdev_name_node_alt_destroy(dev, del_alt_ifname);
> + kfree(del_alt_ifname);
> +
> + return err;
> +}
> +
> static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
> {
> struct net *net = sock_net(skb->sk);
> @@ -5331,6 +5430,9 @@ void __init rtnetlink_init(void)
> rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, 0);
> rtnl_register(PF_UNSPEC, RTM_GETNETCONF, NULL, rtnl_dump_all, 0);
>
> + rtnl_register(PF_UNSPEC, RTM_NEWALTIFNAME, rtnl_newaltifname, NULL, 0);
> + rtnl_register(PF_UNSPEC, RTM_DELALTIFNAME, rtnl_delaltifname, NULL, 0);
> +
> rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, 0);
> rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, 0);
> rtnl_register(PF_BRIDGE, RTM_GETNEIGH, rtnl_fdb_get, rtnl_fdb_dump, 0);
> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
> index 58345ba0528e..a712b54c666c 100644
> --- a/security/selinux/nlmsgtab.c
> +++ b/security/selinux/nlmsgtab.c
> @@ -83,6 +83,8 @@ static const struct nlmsg_perm nlmsg_route_perms[] =
> { RTM_NEWNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> { RTM_DELNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> { RTM_GETNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_READ },
> + { RTM_NEWALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> + { RTM_DELALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> };
>
> static const struct nlmsg_perm nlmsg_tcpdiag_perms[] =
> @@ -166,7 +168,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
> * structures at the top of this file with the new mappings
> * before updating the BUILD_BUG_ON() macro!
> */
> - BUILD_BUG_ON(RTM_MAX != (RTM_NEWNEXTHOP + 3));
> + BUILD_BUG_ON(RTM_MAX != (RTM_NEWALTIFNAME + 3));
> err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
> sizeof(nlmsg_route_perms));
> break;
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-09 4:11 ` Roopa Prabhu
@ 2019-08-09 6:25 ` Jiri Pirko
2019-08-09 15:40 ` Roopa Prabhu
0 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-08-09 6:25 UTC (permalink / raw)
To: Roopa Prabhu
Cc: netdev, David Miller, Jakub Kicinski, Stephen Hemminger,
David Ahern, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Fri, Aug 09, 2019 at 06:11:30AM CEST, roopa@cumulusnetworks.com wrote:
>On Fri, Jul 19, 2019 at 4:00 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Add two commands to add and delete alternative ifnames for net device.
>> Each net device can have multiple alternative names.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> include/linux/netdevice.h | 4 ++
>> include/uapi/linux/if.h | 1 +
>> include/uapi/linux/if_link.h | 1 +
>> include/uapi/linux/rtnetlink.h | 7 +++
>> net/core/dev.c | 58 ++++++++++++++++++-
>> net/core/rtnetlink.c | 102 +++++++++++++++++++++++++++++++++
>> security/selinux/nlmsgtab.c | 4 +-
>> 7 files changed, 175 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 74f99f127b0e..6922fdb483ca 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -920,10 +920,14 @@ struct tlsdev_ops;
>>
>> struct netdev_name_node {
>> struct hlist_node hlist;
>> + struct list_head list;
>> struct net_device *dev;
>> char *name;
>> };
>>
>> +int netdev_name_node_alt_create(struct net_device *dev, char *name);
>> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name);
>> +
>> /*
>> * This structure defines the management hooks for network devices.
>> * The following hooks can be defined; unless noted otherwise, they are
>> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
>> index 7fea0fd7d6f5..4bf33344aab1 100644
>> --- a/include/uapi/linux/if.h
>> +++ b/include/uapi/linux/if.h
>> @@ -33,6 +33,7 @@
>> #define IFNAMSIZ 16
>> #endif /* __UAPI_DEF_IF_IFNAMSIZ */
>> #define IFALIASZ 256
>> +#define ALTIFNAMSIZ 128
>> #include <linux/hdlc/ioctl.h>
>>
>> /* For glibc compatibility. An empty enum does not compile. */
>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> index 4a8c02cafa9a..92268946e04a 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -167,6 +167,7 @@ enum {
>> IFLA_NEW_IFINDEX,
>> IFLA_MIN_MTU,
>> IFLA_MAX_MTU,
>> + IFLA_ALT_IFNAME_MOD, /* Alternative ifname to add/delete */
>> __IFLA_MAX
>> };
>>
>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>> index ce2a623abb75..b36cfd83eb76 100644
>> --- a/include/uapi/linux/rtnetlink.h
>> +++ b/include/uapi/linux/rtnetlink.h
>> @@ -164,6 +164,13 @@ enum {
>> RTM_GETNEXTHOP,
>> #define RTM_GETNEXTHOP RTM_GETNEXTHOP
>>
>> + RTM_NEWALTIFNAME = 108,
>> +#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
>> + RTM_DELALTIFNAME,
>> +#define RTM_DELALTIFNAME RTM_DELALTIFNAME
>> + RTM_GETALTIFNAME,
>> +#define RTM_GETALTIFNAME RTM_GETALTIFNAME
>> +
>
>I might have missed the prior discussion, why do we need new commands
>?. can't this simply be part of RTM_*LINK and we use RTM_SETLINK to
>set alternate names ?
How? This is to add/remove. How do you suggest to to add/remove by
setlink?
>
>
>
>> __RTM_MAX,
>> #define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
>> };
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index ad0d42fbdeee..2a3be2b279d3 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -244,7 +244,13 @@ static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
>> static struct netdev_name_node *
>> netdev_name_node_head_alloc(struct net_device *dev)
>> {
>> - return netdev_name_node_alloc(dev, dev->name);
>> + struct netdev_name_node *name_node;
>> +
>> + name_node = netdev_name_node_alloc(dev, dev->name);
>> + if (!name_node)
>> + return NULL;
>> + INIT_LIST_HEAD(&name_node->list);
>> + return name_node;
>> }
>>
>> static void netdev_name_node_free(struct netdev_name_node *name_node)
>> @@ -288,6 +294,55 @@ static struct netdev_name_node *netdev_name_node_lookup_rcu(struct net *net,
>> return NULL;
>> }
>>
>> +int netdev_name_node_alt_create(struct net_device *dev, char *name)
>> +{
>> + struct netdev_name_node *name_node;
>> + struct net *net = dev_net(dev);
>> +
>> + name_node = netdev_name_node_lookup(net, name);
>> + if (name_node)
>> + return -EEXIST;
>> + name_node = netdev_name_node_alloc(dev, name);
>> + if (!name_node)
>> + return -ENOMEM;
>> + netdev_name_node_add(net, name_node);
>> + /* The node that holds dev->name acts as a head of per-device list. */
>> + list_add_tail(&name_node->list, &dev->name_node->list);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(netdev_name_node_alt_create);
>> +
>> +static void __netdev_name_node_alt_destroy(struct netdev_name_node *name_node)
>> +{
>> + list_del(&name_node->list);
>> + netdev_name_node_del(name_node);
>> + kfree(name_node->name);
>> + netdev_name_node_free(name_node);
>> +}
>> +
>> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
>> +{
>> + struct netdev_name_node *name_node;
>> + struct net *net = dev_net(dev);
>> +
>> + name_node = netdev_name_node_lookup(net, name);
>> + if (!name_node)
>> + return -ENOENT;
>> + __netdev_name_node_alt_destroy(name_node);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
>> +
>> +static void netdev_name_node_alt_flush(struct net_device *dev)
>> +{
>> + struct netdev_name_node *name_node, *tmp;
>> +
>> + list_for_each_entry_safe(name_node, tmp, &dev->name_node->list, list)
>> + __netdev_name_node_alt_destroy(name_node);
>> +}
>> +
>> /* Device list insertion */
>> static void list_netdevice(struct net_device *dev)
>> {
>> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
>> dev_uc_flush(dev);
>> dev_mc_flush(dev);
>>
>> + netdev_name_node_alt_flush(dev);
>> netdev_name_node_free(dev->name_node);
>>
>> if (dev->netdev_ops->ndo_uninit)
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 1ee6460f8275..7a2010b16e10 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
>> [IFLA_MIN_MTU] = { .type = NLA_U32 },
>> [IFLA_MAX_MTU] = { .type = NLA_U32 },
>> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
>> + .len = ALTIFNAMSIZ - 1 },
>> };
>>
>> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
>> @@ -3373,6 +3375,103 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>> return err;
>> }
>>
>> +static int rtnl_newaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct net *net = sock_net(skb->sk);
>> + struct nlattr *tb[IFLA_MAX + 1];
>> + struct net_device *dev;
>> + struct ifinfomsg *ifm;
>> + char *new_alt_ifname;
>> + int err;
>> +
>> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
>> + if (err)
>> + return err;
>> +
>> + err = rtnl_ensure_unique_netns(tb, extack, true);
>> + if (err)
>> + return err;
>> +
>> + ifm = nlmsg_data(nlh);
>> + if (ifm->ifi_index > 0) {
>> + dev = __dev_get_by_index(net, ifm->ifi_index);
>> + } else if (tb[IFLA_IFNAME]) {
>> + char ifname[IFNAMSIZ];
>> +
>> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
>> + dev = __dev_get_by_name(net, ifname);
>> + } else {
>> + return -EINVAL;
>> + }
>> +
>> + if (!dev)
>> + return -ENODEV;
>> +
>> + if (!tb[IFLA_ALT_IFNAME_MOD])
>> + return -EINVAL;
>> +
>> + new_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
>> + if (!new_alt_ifname)
>> + return -ENOMEM;
>> +
>> + err = netdev_name_node_alt_create(dev, new_alt_ifname);
>> + if (err)
>> + goto out_free_new_alt_ifname;
>> +
>> + return 0;
>> +
>> +out_free_new_alt_ifname:
>> + kfree(new_alt_ifname);
>> + return err;
>> +}
>> +
>> +static int rtnl_delaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct net *net = sock_net(skb->sk);
>> + struct nlattr *tb[IFLA_MAX + 1];
>> + struct net_device *dev;
>> + struct ifinfomsg *ifm;
>> + char *del_alt_ifname;
>> + int err;
>> +
>> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
>> + if (err)
>> + return err;
>> +
>> + err = rtnl_ensure_unique_netns(tb, extack, true);
>> + if (err)
>> + return err;
>> +
>> + ifm = nlmsg_data(nlh);
>> + if (ifm->ifi_index > 0) {
>> + dev = __dev_get_by_index(net, ifm->ifi_index);
>> + } else if (tb[IFLA_IFNAME]) {
>> + char ifname[IFNAMSIZ];
>> +
>> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
>> + dev = __dev_get_by_name(net, ifname);
>> + } else {
>> + return -EINVAL;
>> + }
>> +
>> + if (!dev)
>> + return -ENODEV;
>> +
>> + if (!tb[IFLA_ALT_IFNAME_MOD])
>> + return -EINVAL;
>> +
>> + del_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
>> + if (!del_alt_ifname)
>> + return -ENOMEM;
>> +
>> + err = netdev_name_node_alt_destroy(dev, del_alt_ifname);
>> + kfree(del_alt_ifname);
>> +
>> + return err;
>> +}
>> +
>> static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
>> {
>> struct net *net = sock_net(skb->sk);
>> @@ -5331,6 +5430,9 @@ void __init rtnetlink_init(void)
>> rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, 0);
>> rtnl_register(PF_UNSPEC, RTM_GETNETCONF, NULL, rtnl_dump_all, 0);
>>
>> + rtnl_register(PF_UNSPEC, RTM_NEWALTIFNAME, rtnl_newaltifname, NULL, 0);
>> + rtnl_register(PF_UNSPEC, RTM_DELALTIFNAME, rtnl_delaltifname, NULL, 0);
>> +
>> rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, 0);
>> rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, 0);
>> rtnl_register(PF_BRIDGE, RTM_GETNEIGH, rtnl_fdb_get, rtnl_fdb_dump, 0);
>> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
>> index 58345ba0528e..a712b54c666c 100644
>> --- a/security/selinux/nlmsgtab.c
>> +++ b/security/selinux/nlmsgtab.c
>> @@ -83,6 +83,8 @@ static const struct nlmsg_perm nlmsg_route_perms[] =
>> { RTM_NEWNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> { RTM_DELNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> { RTM_GETNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_READ },
>> + { RTM_NEWALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> + { RTM_DELALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> };
>>
>> static const struct nlmsg_perm nlmsg_tcpdiag_perms[] =
>> @@ -166,7 +168,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
>> * structures at the top of this file with the new mappings
>> * before updating the BUILD_BUG_ON() macro!
>> */
>> - BUILD_BUG_ON(RTM_MAX != (RTM_NEWNEXTHOP + 3));
>> + BUILD_BUG_ON(RTM_MAX != (RTM_NEWALTIFNAME + 3));
>> err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
>> sizeof(nlmsg_route_perms));
>> break;
>> --
>> 2.21.0
>>
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-09 6:25 ` Jiri Pirko
@ 2019-08-09 15:40 ` Roopa Prabhu
2019-08-09 15:46 ` Michal Kubecek
` (2 more replies)
0 siblings, 3 replies; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-09 15:40 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jakub Kicinski, Stephen Hemminger,
David Ahern, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Thu, Aug 8, 2019 at 11:25 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Fri, Aug 09, 2019 at 06:11:30AM CEST, roopa@cumulusnetworks.com wrote:
> >On Fri, Jul 19, 2019 at 4:00 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> From: Jiri Pirko <jiri@mellanox.com>
> >>
> >> Add two commands to add and delete alternative ifnames for net device.
> >> Each net device can have multiple alternative names.
> >>
> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> >> ---
> >> include/linux/netdevice.h | 4 ++
> >> include/uapi/linux/if.h | 1 +
> >> include/uapi/linux/if_link.h | 1 +
> >> include/uapi/linux/rtnetlink.h | 7 +++
> >> net/core/dev.c | 58 ++++++++++++++++++-
> >> net/core/rtnetlink.c | 102 +++++++++++++++++++++++++++++++++
> >> security/selinux/nlmsgtab.c | 4 +-
> >> 7 files changed, 175 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index 74f99f127b0e..6922fdb483ca 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -920,10 +920,14 @@ struct tlsdev_ops;
> >>
> >> struct netdev_name_node {
> >> struct hlist_node hlist;
> >> + struct list_head list;
> >> struct net_device *dev;
> >> char *name;
> >> };
> >>
> >> +int netdev_name_node_alt_create(struct net_device *dev, char *name);
> >> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name);
> >> +
> >> /*
> >> * This structure defines the management hooks for network devices.
> >> * The following hooks can be defined; unless noted otherwise, they are
> >> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> >> index 7fea0fd7d6f5..4bf33344aab1 100644
> >> --- a/include/uapi/linux/if.h
> >> +++ b/include/uapi/linux/if.h
> >> @@ -33,6 +33,7 @@
> >> #define IFNAMSIZ 16
> >> #endif /* __UAPI_DEF_IF_IFNAMSIZ */
> >> #define IFALIASZ 256
> >> +#define ALTIFNAMSIZ 128
> >> #include <linux/hdlc/ioctl.h>
> >>
> >> /* For glibc compatibility. An empty enum does not compile. */
> >> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> >> index 4a8c02cafa9a..92268946e04a 100644
> >> --- a/include/uapi/linux/if_link.h
> >> +++ b/include/uapi/linux/if_link.h
> >> @@ -167,6 +167,7 @@ enum {
> >> IFLA_NEW_IFINDEX,
> >> IFLA_MIN_MTU,
> >> IFLA_MAX_MTU,
> >> + IFLA_ALT_IFNAME_MOD, /* Alternative ifname to add/delete */
> >> __IFLA_MAX
> >> };
> >>
> >> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> >> index ce2a623abb75..b36cfd83eb76 100644
> >> --- a/include/uapi/linux/rtnetlink.h
> >> +++ b/include/uapi/linux/rtnetlink.h
> >> @@ -164,6 +164,13 @@ enum {
> >> RTM_GETNEXTHOP,
> >> #define RTM_GETNEXTHOP RTM_GETNEXTHOP
> >>
> >> + RTM_NEWALTIFNAME = 108,
> >> +#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
> >> + RTM_DELALTIFNAME,
> >> +#define RTM_DELALTIFNAME RTM_DELALTIFNAME
> >> + RTM_GETALTIFNAME,
> >> +#define RTM_GETALTIFNAME RTM_GETALTIFNAME
> >> +
> >
> >I might have missed the prior discussion, why do we need new commands
> >?. can't this simply be part of RTM_*LINK and we use RTM_SETLINK to
> >set alternate names ?
>
> How? This is to add/remove. How do you suggest to to add/remove by
> setlink?
to that point, I am also not sure why we have a new API For multiple
names. I mean why support more than two names (existing old name and
a new name to remove the length limitation) ?
Your patch series addresses a very important problem (we run into this
limitation all the time and its hard to explain it to network
operators) and
its already unfortunate that we have to have more than one name
because we cannot resize the existing one.
The best we can do for simpler transition/management from user-space
is to keep the api simple..
ie keep it close to the management of existing link attributes. Hence
the question.
I assumed this would be like alias. A single new field that can be
referenced in lieu of the old one.
Your series is very useful to many of us...but when i think about
changing our network manager to accommodate this, I am worried about
how many apps will have to change.
I agree they have to change regardless but now they will have to
listen to yet another notification and msg format for names ?
(apologies for joining the thread late and if i missed prior discussion on this)
>
>
> >
> >
> >
> >> __RTM_MAX,
> >> #define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
> >> };
> >> diff --git a/net/core/dev.c b/net/core/dev.c
> >> index ad0d42fbdeee..2a3be2b279d3 100644
> >> --- a/net/core/dev.c
> >> +++ b/net/core/dev.c
> >> @@ -244,7 +244,13 @@ static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
> >> static struct netdev_name_node *
> >> netdev_name_node_head_alloc(struct net_device *dev)
> >> {
> >> - return netdev_name_node_alloc(dev, dev->name);
> >> + struct netdev_name_node *name_node;
> >> +
> >> + name_node = netdev_name_node_alloc(dev, dev->name);
> >> + if (!name_node)
> >> + return NULL;
> >> + INIT_LIST_HEAD(&name_node->list);
> >> + return name_node;
> >> }
> >>
> >> static void netdev_name_node_free(struct netdev_name_node *name_node)
> >> @@ -288,6 +294,55 @@ static struct netdev_name_node *netdev_name_node_lookup_rcu(struct net *net,
> >> return NULL;
> >> }
> >>
> >> +int netdev_name_node_alt_create(struct net_device *dev, char *name)
> >> +{
> >> + struct netdev_name_node *name_node;
> >> + struct net *net = dev_net(dev);
> >> +
> >> + name_node = netdev_name_node_lookup(net, name);
> >> + if (name_node)
> >> + return -EEXIST;
> >> + name_node = netdev_name_node_alloc(dev, name);
> >> + if (!name_node)
> >> + return -ENOMEM;
> >> + netdev_name_node_add(net, name_node);
> >> + /* The node that holds dev->name acts as a head of per-device list. */
> >> + list_add_tail(&name_node->list, &dev->name_node->list);
> >> +
> >> + return 0;
> >> +}
> >> +EXPORT_SYMBOL(netdev_name_node_alt_create);
> >> +
> >> +static void __netdev_name_node_alt_destroy(struct netdev_name_node *name_node)
> >> +{
> >> + list_del(&name_node->list);
> >> + netdev_name_node_del(name_node);
> >> + kfree(name_node->name);
> >> + netdev_name_node_free(name_node);
> >> +}
> >> +
> >> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
> >> +{
> >> + struct netdev_name_node *name_node;
> >> + struct net *net = dev_net(dev);
> >> +
> >> + name_node = netdev_name_node_lookup(net, name);
> >> + if (!name_node)
> >> + return -ENOENT;
> >> + __netdev_name_node_alt_destroy(name_node);
> >> +
> >> + return 0;
> >> +}
> >> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
> >> +
> >> +static void netdev_name_node_alt_flush(struct net_device *dev)
> >> +{
> >> + struct netdev_name_node *name_node, *tmp;
> >> +
> >> + list_for_each_entry_safe(name_node, tmp, &dev->name_node->list, list)
> >> + __netdev_name_node_alt_destroy(name_node);
> >> +}
> >> +
> >> /* Device list insertion */
> >> static void list_netdevice(struct net_device *dev)
> >> {
> >> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
> >> dev_uc_flush(dev);
> >> dev_mc_flush(dev);
> >>
> >> + netdev_name_node_alt_flush(dev);
> >> netdev_name_node_free(dev->name_node);
> >>
> >> if (dev->netdev_ops->ndo_uninit)
> >> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> >> index 1ee6460f8275..7a2010b16e10 100644
> >> --- a/net/core/rtnetlink.c
> >> +++ b/net/core/rtnetlink.c
> >> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
> >> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
> >> [IFLA_MIN_MTU] = { .type = NLA_U32 },
> >> [IFLA_MAX_MTU] = { .type = NLA_U32 },
> >> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
> >> + .len = ALTIFNAMSIZ - 1 },
> >> };
> >>
> >> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
> >> @@ -3373,6 +3375,103 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> >> return err;
> >> }
> >>
> >> +static int rtnl_newaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
> >> + struct netlink_ext_ack *extack)
> >> +{
> >> + struct net *net = sock_net(skb->sk);
> >> + struct nlattr *tb[IFLA_MAX + 1];
> >> + struct net_device *dev;
> >> + struct ifinfomsg *ifm;
> >> + char *new_alt_ifname;
> >> + int err;
> >> +
> >> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
> >> + if (err)
> >> + return err;
> >> +
> >> + err = rtnl_ensure_unique_netns(tb, extack, true);
> >> + if (err)
> >> + return err;
> >> +
> >> + ifm = nlmsg_data(nlh);
> >> + if (ifm->ifi_index > 0) {
> >> + dev = __dev_get_by_index(net, ifm->ifi_index);
> >> + } else if (tb[IFLA_IFNAME]) {
> >> + char ifname[IFNAMSIZ];
> >> +
> >> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
> >> + dev = __dev_get_by_name(net, ifname);
> >> + } else {
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (!dev)
> >> + return -ENODEV;
> >> +
> >> + if (!tb[IFLA_ALT_IFNAME_MOD])
> >> + return -EINVAL;
> >> +
> >> + new_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
> >> + if (!new_alt_ifname)
> >> + return -ENOMEM;
> >> +
> >> + err = netdev_name_node_alt_create(dev, new_alt_ifname);
> >> + if (err)
> >> + goto out_free_new_alt_ifname;
> >> +
> >> + return 0;
> >> +
> >> +out_free_new_alt_ifname:
> >> + kfree(new_alt_ifname);
> >> + return err;
> >> +}
> >> +
> >> +static int rtnl_delaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
> >> + struct netlink_ext_ack *extack)
> >> +{
> >> + struct net *net = sock_net(skb->sk);
> >> + struct nlattr *tb[IFLA_MAX + 1];
> >> + struct net_device *dev;
> >> + struct ifinfomsg *ifm;
> >> + char *del_alt_ifname;
> >> + int err;
> >> +
> >> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
> >> + if (err)
> >> + return err;
> >> +
> >> + err = rtnl_ensure_unique_netns(tb, extack, true);
> >> + if (err)
> >> + return err;
> >> +
> >> + ifm = nlmsg_data(nlh);
> >> + if (ifm->ifi_index > 0) {
> >> + dev = __dev_get_by_index(net, ifm->ifi_index);
> >> + } else if (tb[IFLA_IFNAME]) {
> >> + char ifname[IFNAMSIZ];
> >> +
> >> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
> >> + dev = __dev_get_by_name(net, ifname);
> >> + } else {
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (!dev)
> >> + return -ENODEV;
> >> +
> >> + if (!tb[IFLA_ALT_IFNAME_MOD])
> >> + return -EINVAL;
> >> +
> >> + del_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
> >> + if (!del_alt_ifname)
> >> + return -ENOMEM;
> >> +
> >> + err = netdev_name_node_alt_destroy(dev, del_alt_ifname);
> >> + kfree(del_alt_ifname);
> >> +
> >> + return err;
> >> +}
> >> +
> >> static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
> >> {
> >> struct net *net = sock_net(skb->sk);
> >> @@ -5331,6 +5430,9 @@ void __init rtnetlink_init(void)
> >> rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, 0);
> >> rtnl_register(PF_UNSPEC, RTM_GETNETCONF, NULL, rtnl_dump_all, 0);
> >>
> >> + rtnl_register(PF_UNSPEC, RTM_NEWALTIFNAME, rtnl_newaltifname, NULL, 0);
> >> + rtnl_register(PF_UNSPEC, RTM_DELALTIFNAME, rtnl_delaltifname, NULL, 0);
> >> +
> >> rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, 0);
> >> rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, 0);
> >> rtnl_register(PF_BRIDGE, RTM_GETNEIGH, rtnl_fdb_get, rtnl_fdb_dump, 0);
> >> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
> >> index 58345ba0528e..a712b54c666c 100644
> >> --- a/security/selinux/nlmsgtab.c
> >> +++ b/security/selinux/nlmsgtab.c
> >> @@ -83,6 +83,8 @@ static const struct nlmsg_perm nlmsg_route_perms[] =
> >> { RTM_NEWNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> >> { RTM_DELNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> >> { RTM_GETNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_READ },
> >> + { RTM_NEWALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> >> + { RTM_DELALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
> >> };
> >>
> >> static const struct nlmsg_perm nlmsg_tcpdiag_perms[] =
> >> @@ -166,7 +168,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
> >> * structures at the top of this file with the new mappings
> >> * before updating the BUILD_BUG_ON() macro!
> >> */
> >> - BUILD_BUG_ON(RTM_MAX != (RTM_NEWNEXTHOP + 3));
> >> + BUILD_BUG_ON(RTM_MAX != (RTM_NEWALTIFNAME + 3));
> >> err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
> >> sizeof(nlmsg_route_perms));
> >> break;
> >> --
> >> 2.21.0
> >>
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-09 15:40 ` Roopa Prabhu
@ 2019-08-09 15:46 ` Michal Kubecek
2019-08-10 13:46 ` Roopa Prabhu
2019-08-09 16:14 ` David Ahern
2019-08-10 6:32 ` Jiri Pirko
2 siblings, 1 reply; 76+ messages in thread
From: Michal Kubecek @ 2019-08-09 15:46 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Jiri Pirko, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Fri, Aug 09, 2019 at 08:40:25AM -0700, Roopa Prabhu wrote:
> to that point, I am also not sure why we have a new API For multiple
> names. I mean why support more than two names (existing old name and
> a new name to remove the length limitation) ?
One use case is to allow "predictable names" from udev/systemd to work
the way do for e.g. block devices, see
http://lkml.kernel.org/r/20190628162716.GF29149@unicorn.suse.cz
Michal
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-09 15:46 ` Michal Kubecek
@ 2019-08-10 13:46 ` Roopa Prabhu
2019-08-10 15:50 ` Michal Kubecek
0 siblings, 1 reply; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-10 13:46 UTC (permalink / raw)
To: Michal Kubecek
Cc: Jiri Pirko, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Fri, Aug 9, 2019 at 8:46 AM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Fri, Aug 09, 2019 at 08:40:25AM -0700, Roopa Prabhu wrote:
> > to that point, I am also not sure why we have a new API For multiple
> > names. I mean why support more than two names (existing old name and
> > a new name to remove the length limitation) ?
>
> One use case is to allow "predictable names" from udev/systemd to work
> the way do for e.g. block devices, see
>
> http://lkml.kernel.org/r/20190628162716.GF29149@unicorn.suse.cz
>
thanks for the link. don't know the details about alternate block
device names. Does user-space generate multiple and assign them to a
kernel object as proposed in this series ?. is there a limit to number
of names ?. my understanding of 'predictable names' was still a single
name but predictable structure to the name.
No strong objections around multiple alternate names as long as the
need for this does not make the base long name setting complex :).
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-10 13:46 ` Roopa Prabhu
@ 2019-08-10 15:50 ` Michal Kubecek
2019-08-10 19:39 ` Roopa Prabhu
0 siblings, 1 reply; 76+ messages in thread
From: Michal Kubecek @ 2019-08-10 15:50 UTC (permalink / raw)
To: netdev
Cc: Roopa Prabhu, Jiri Pirko, David Miller, Jakub Kicinski,
Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Sat, Aug 10, 2019 at 06:46:57AM -0700, Roopa Prabhu wrote:
> On Fri, Aug 9, 2019 at 8:46 AM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > On Fri, Aug 09, 2019 at 08:40:25AM -0700, Roopa Prabhu wrote:
> > > to that point, I am also not sure why we have a new API For multiple
> > > names. I mean why support more than two names (existing old name and
> > > a new name to remove the length limitation) ?
> >
> > One use case is to allow "predictable names" from udev/systemd to work
> > the way do for e.g. block devices, see
> >
> > http://lkml.kernel.org/r/20190628162716.GF29149@unicorn.suse.cz
> >
>
> thanks for the link. don't know the details about alternate block
> device names. Does user-space generate multiple and assign them to a
> kernel object as proposed in this series ?. is there a limit to number
> of names ?. my understanding of 'predictable names' was still a single
> name but predictable structure to the name.
It is a single name but IMHO mostly because we can only have one name.
For block devices, udev uses symlinks to create multiple aliases based
on different naming schemes, e.g.
mike@lion:~> find -L /dev/disk/ -samefile /dev/sda2 -exec ls -l {} +
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/ata-WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68_WD-WMC1T3114933-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-0ATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-1ATA_WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-350014ee6589cfea0-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/wwn-0x50014ee6589cfea0-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-partlabel/root2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-partuuid/71affb47-a93b-40fd-8986-d2e227e1b39d -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-path/pci-0000:00:11.0-ata-1-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-path/pci-0000:00:11.0-scsi-0:0:0:0-part2 -> ../../sda2
Few years ago, udev even dropped support for renaming block and
character devices (NAME="...") so that it now keeps kernel name and only
creates symlinks to it. Recent versions only allow NAME="..." for
network devices.
Michal
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-10 15:50 ` Michal Kubecek
@ 2019-08-10 19:39 ` Roopa Prabhu
2019-08-11 22:10 ` Michal Kubecek
0 siblings, 1 reply; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-10 19:39 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, Jiri Pirko, David Miller, Jakub Kicinski,
Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Sat, Aug 10, 2019 at 8:50 AM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Sat, Aug 10, 2019 at 06:46:57AM -0700, Roopa Prabhu wrote:
> > On Fri, Aug 9, 2019 at 8:46 AM Michal Kubecek <mkubecek@suse.cz> wrote:
> > >
> > > On Fri, Aug 09, 2019 at 08:40:25AM -0700, Roopa Prabhu wrote:
> > > > to that point, I am also not sure why we have a new API For multiple
> > > > names. I mean why support more than two names (existing old name and
> > > > a new name to remove the length limitation) ?
> > >
> > > One use case is to allow "predictable names" from udev/systemd to work
> > > the way do for e.g. block devices, see
> > >
> > > http://lkml.kernel.org/r/20190628162716.GF29149@unicorn.suse.cz
> > >
> >
> > thanks for the link. don't know the details about alternate block
> > device names. Does user-space generate multiple and assign them to a
> > kernel object as proposed in this series ?. is there a limit to number
> > of names ?. my understanding of 'predictable names' was still a single
> > name but predictable structure to the name.
>
> It is a single name but IMHO mostly because we can only have one name.
> For block devices, udev uses symlinks to create multiple aliases based
> on different naming schemes, e.g.
>
> mike@lion:~> find -L /dev/disk/ -samefile /dev/sda2 -exec ls -l {} +
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/ata-WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68_WD-WMC1T3114933-part2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-0ATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-1ATA_WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-350014ee6589cfea0-part2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/wwn-0x50014ee6589cfea0-part2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-partlabel/root2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-partuuid/71affb47-a93b-40fd-8986-d2e227e1b39d -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-path/pci-0000:00:11.0-ata-1-part2 -> ../../sda2
> lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-path/pci-0000:00:11.0-scsi-0:0:0:0-part2 -> ../../sda2
>
> Few years ago, udev even dropped support for renaming block and
> character devices (NAME="...") so that it now keeps kernel name and only
> creates symlinks to it. Recent versions only allow NAME="..." for
> network devices.
ok thanks for the details. This looks like names that are structured
on hardware info which could fall into devlinks scope and they point
to a single name.
We should think about keeping them under devlink (by-id, by-mac etc).
It already can recognize network interfaces by id.
The netdev IFLA_NAME falls more under user-defined name with no
structure (dummy1, dummy1-longname) which network interface managers,
protocols etc use.
Since the goal of the series is to relax the IFLA_NAME limit, I was
hoping it can be replaced by a single attribute that apps can use in
lieu of IFLA_NAME when available.
I would vote for keeping the structured and user defined unstructured
names separate and that would help with simplifying the API in this
series.
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-10 19:39 ` Roopa Prabhu
@ 2019-08-11 22:10 ` Michal Kubecek
2019-08-12 15:21 ` Roopa Prabhu
0 siblings, 1 reply; 76+ messages in thread
From: Michal Kubecek @ 2019-08-11 22:10 UTC (permalink / raw)
To: netdev
Cc: Roopa Prabhu, Jiri Pirko, David Miller, Jakub Kicinski,
Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Sat, Aug 10, 2019 at 12:39:31PM -0700, Roopa Prabhu wrote:
> On Sat, Aug 10, 2019 at 8:50 AM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > On Sat, Aug 10, 2019 at 06:46:57AM -0700, Roopa Prabhu wrote:
> > > On Fri, Aug 9, 2019 at 8:46 AM Michal Kubecek <mkubecek@suse.cz> wrote:
> > > >
> > > > On Fri, Aug 09, 2019 at 08:40:25AM -0700, Roopa Prabhu wrote:
> > > > > to that point, I am also not sure why we have a new API For multiple
> > > > > names. I mean why support more than two names (existing old name and
> > > > > a new name to remove the length limitation) ?
> > > >
> > > > One use case is to allow "predictable names" from udev/systemd to work
> > > > the way do for e.g. block devices, see
> > > >
> > > > http://lkml.kernel.org/r/20190628162716.GF29149@unicorn.suse.cz
> > > >
> > >
> > > thanks for the link. don't know the details about alternate block
> > > device names. Does user-space generate multiple and assign them to a
> > > kernel object as proposed in this series ?. is there a limit to number
> > > of names ?. my understanding of 'predictable names' was still a single
> > > name but predictable structure to the name.
> >
> > It is a single name but IMHO mostly because we can only have one name.
> > For block devices, udev uses symlinks to create multiple aliases based
> > on different naming schemes, e.g.
> >
> > mike@lion:~> find -L /dev/disk/ -samefile /dev/sda2 -exec ls -l {} +
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/ata-WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68_WD-WMC1T3114933-part2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-0ATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-1ATA_WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-350014ee6589cfea0-part2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/wwn-0x50014ee6589cfea0-part2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-partlabel/root2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-partuuid/71affb47-a93b-40fd-8986-d2e227e1b39d -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-path/pci-0000:00:11.0-ata-1-part2 -> ../../sda2
> > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-path/pci-0000:00:11.0-scsi-0:0:0:0-part2 -> ../../sda2
> >
> > Few years ago, udev even dropped support for renaming block and
> > character devices (NAME="...") so that it now keeps kernel name and only
> > creates symlinks to it. Recent versions only allow NAME="..." for
> > network devices.
>
> ok thanks for the details. This looks like names that are structured
> on hardware info which could fall into devlinks scope and they point
> to a single name.
> We should think about keeping them under devlink (by-id, by-mac etc).
> It already can recognize network interfaces by id.
Not all of them are hardware based, there are also links based on
filesystem label or UUID. But my point is rather that udev creates
multiple links so that any of them can be used in any place where
a block device is to be identified.
As network devices can have only one name, udev drops kernel provided
name completely and replaces it with name following one naming scheme.
Thus we have to know which naming scheme is going to be used and make
sure it does not change. With multiple alternative names, we could also
have all udev provided names at once (and also the original one from
kernel).
Michal
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-11 22:10 ` Michal Kubecek
@ 2019-08-12 15:21 ` Roopa Prabhu
2019-08-12 15:43 ` Michal Kubecek
0 siblings, 1 reply; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-12 15:21 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, Jiri Pirko, David Miller, Jakub Kicinski,
Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Sun, Aug 11, 2019 at 3:10 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Sat, Aug 10, 2019 at 12:39:31PM -0700, Roopa Prabhu wrote:
> > On Sat, Aug 10, 2019 at 8:50 AM Michal Kubecek <mkubecek@suse.cz> wrote:
> > >
> > > On Sat, Aug 10, 2019 at 06:46:57AM -0700, Roopa Prabhu wrote:
> > > > On Fri, Aug 9, 2019 at 8:46 AM Michal Kubecek <mkubecek@suse.cz> wrote:
> > > > >
> > > > > On Fri, Aug 09, 2019 at 08:40:25AM -0700, Roopa Prabhu wrote:
> > > > > > to that point, I am also not sure why we have a new API For multiple
> > > > > > names. I mean why support more than two names (existing old name and
> > > > > > a new name to remove the length limitation) ?
> > > > >
> > > > > One use case is to allow "predictable names" from udev/systemd to work
> > > > > the way do for e.g. block devices, see
> > > > >
> > > > > http://lkml.kernel.org/r/20190628162716.GF29149@unicorn.suse.cz
> > > > >
> > > >
> > > > thanks for the link. don't know the details about alternate block
> > > > device names. Does user-space generate multiple and assign them to a
> > > > kernel object as proposed in this series ?. is there a limit to number
> > > > of names ?. my understanding of 'predictable names' was still a single
> > > > name but predictable structure to the name.
> > >
> > > It is a single name but IMHO mostly because we can only have one name.
> > > For block devices, udev uses symlinks to create multiple aliases based
> > > on different naming schemes, e.g.
> > >
> > > mike@lion:~> find -L /dev/disk/ -samefile /dev/sda2 -exec ls -l {} +
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/ata-WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-0ATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-1ATA_WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/scsi-350014ee6589cfea0-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-id/wwn-0x50014ee6589cfea0-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-partlabel/root2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-partuuid/71affb47-a93b-40fd-8986-d2e227e1b39d -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-path/pci-0000:00:11.0-ata-1-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp 5 21:47 /dev/disk/by-path/pci-0000:00:11.0-scsi-0:0:0:0-part2 -> ../../sda2
> > >
> > > Few years ago, udev even dropped support for renaming block and
> > > character devices (NAME="...") so that it now keeps kernel name and only
> > > creates symlinks to it. Recent versions only allow NAME="..." for
> > > network devices.
> >
> > ok thanks for the details. This looks like names that are structured
> > on hardware info which could fall into devlinks scope and they point
> > to a single name.
> > We should think about keeping them under devlink (by-id, by-mac etc).
> > It already can recognize network interfaces by id.
>
> Not all of them are hardware based, there are also links based on
> filesystem label or UUID. But my point is rather that udev creates
> multiple links so that any of them can be used in any place where
> a block device is to be identified.
>
> As network devices can have only one name, udev drops kernel provided
> name completely and replaces it with name following one naming scheme.
> Thus we have to know which naming scheme is going to be used and make
> sure it does not change. With multiple alternative names, we could also
> have all udev provided names at once (and also the original one from
> kernel).
ok, understand the use-case.
But, Its hard for me to understand how udev is going to manage this
list of names without structure to them.
Plus how is udev going to distinguish its own names from user given name ?.
I thought this list was giving an opportunity to use the long name
everywhere else.
But if this is going to be managed by udev with a bunch of structured
names, I don't understand how the rest of the system is going to use
these names.
Maybe we should just call this a udev managed list of names.
(again, i think the best way to do this for udev is to provide the
symlink like facility via devlink or any other infra).
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 15:21 ` Roopa Prabhu
@ 2019-08-12 15:43 ` Michal Kubecek
0 siblings, 0 replies; 76+ messages in thread
From: Michal Kubecek @ 2019-08-12 15:43 UTC (permalink / raw)
To: netdev
Cc: Roopa Prabhu, Jiri Pirko, David Miller, Jakub Kicinski,
Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Mon, Aug 12, 2019 at 08:21:31AM -0700, Roopa Prabhu wrote:
> On Sun, Aug 11, 2019 at 3:10 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > Not all of them are hardware based, there are also links based on
> > filesystem label or UUID. But my point is rather that udev creates
> > multiple links so that any of them can be used in any place where
> > a block device is to be identified.
> >
> > As network devices can have only one name, udev drops kernel provided
> > name completely and replaces it with name following one naming scheme.
> > Thus we have to know which naming scheme is going to be used and make
> > sure it does not change. With multiple alternative names, we could also
> > have all udev provided names at once (and also the original one from
> > kernel).
>
> ok, understand the use-case.
> But, Its hard for me to understand how udev is going to manage this
> list of names without structure to them.
> Plus how is udev going to distinguish its own names from user given name ?.
>
> I thought this list was giving an opportunity to use the long name
> everywhere else.
> But if this is going to be managed by udev with a bunch of structured
> names, I don't understand how the rest of the system is going to use
> these names.
>
> Maybe we should just call this a udev managed list of names.
>
> (again, i think the best way to do this for udev is to provide the
> symlink like facility via devlink or any other infra).
I certainly didn't want to suggest for alternative names to be managed
by udev. What I meant was that supporting multiple alternative names
would allow udev to create its names based on e.g. device bus address,
BIOS/UEFI slot number, MAC address etc. But it would still be up to
admins if they want to create their own names.
Michal
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-09 15:40 ` Roopa Prabhu
2019-08-09 15:46 ` Michal Kubecek
@ 2019-08-09 16:14 ` David Ahern
2019-08-10 6:30 ` Jiri Pirko
2019-08-10 6:32 ` Jiri Pirko
2 siblings, 1 reply; 76+ messages in thread
From: David Ahern @ 2019-08-09 16:14 UTC (permalink / raw)
To: Roopa Prabhu, Jiri Pirko
Cc: netdev, David Miller, Jakub Kicinski, Stephen Hemminger, dcbw,
Michal Kubecek, Andrew Lunn, parav, Saeed Mahameed, mlxsw
On 8/9/19 9:40 AM, Roopa Prabhu wrote:
>>>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>>>> index ce2a623abb75..b36cfd83eb76 100644
>>>> --- a/include/uapi/linux/rtnetlink.h
>>>> +++ b/include/uapi/linux/rtnetlink.h
>>>> @@ -164,6 +164,13 @@ enum {
>>>> RTM_GETNEXTHOP,
>>>> #define RTM_GETNEXTHOP RTM_GETNEXTHOP
>>>>
>>>> + RTM_NEWALTIFNAME = 108,
>>>> +#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
>>>> + RTM_DELALTIFNAME,
>>>> +#define RTM_DELALTIFNAME RTM_DELALTIFNAME
>>>> + RTM_GETALTIFNAME,
>>>> +#define RTM_GETALTIFNAME RTM_GETALTIFNAME
>>>> +
>>>
>>> I might have missed the prior discussion, why do we need new commands
>>> ?. can't this simply be part of RTM_*LINK and we use RTM_SETLINK to
>>> set alternate names ?
>>
>> How? This is to add/remove. How do you suggest to to add/remove by
>> setlink?
>
> to that point, I am also not sure why we have a new API For multiple
> names. I mean why support more than two names (existing old name and
> a new name to remove the length limitation) ?
>
> Your patch series addresses a very important problem (we run into this
> limitation all the time and its hard to explain it to network
> operators) and
> its already unfortunate that we have to have more than one name
> because we cannot resize the existing one.
>
> The best we can do for simpler transition/management from user-space
> is to keep the api simple..
> ie keep it close to the management of existing link attributes. Hence
> the question.
>
> I assumed this would be like alias. A single new field that can be
> referenced in lieu of the old one.
>
> Your series is very useful to many of us...but when i think about
> changing our network manager to accommodate this, I am worried about
> how many apps will have to change.
> I agree they have to change regardless but now they will have to
> listen to yet another notification and msg format for names ?
>
> (apologies for joining the thread late and if i missed prior discussion on this)
I agree with Roopa. I do not understand why new RTM commands are needed.
The existing IFLA + ifinfomsg struct give more than enough ways to id
the device for adding / deleting an alternate name.
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-09 16:14 ` David Ahern
@ 2019-08-10 6:30 ` Jiri Pirko
2019-08-12 1:34 ` David Ahern
0 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-08-10 6:30 UTC (permalink / raw)
To: David Ahern
Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Fri, Aug 09, 2019 at 06:14:03PM CEST, dsahern@gmail.com wrote:
>On 8/9/19 9:40 AM, Roopa Prabhu wrote:
>>>>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>>>>> index ce2a623abb75..b36cfd83eb76 100644
>>>>> --- a/include/uapi/linux/rtnetlink.h
>>>>> +++ b/include/uapi/linux/rtnetlink.h
>>>>> @@ -164,6 +164,13 @@ enum {
>>>>> RTM_GETNEXTHOP,
>>>>> #define RTM_GETNEXTHOP RTM_GETNEXTHOP
>>>>>
>>>>> + RTM_NEWALTIFNAME = 108,
>>>>> +#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
>>>>> + RTM_DELALTIFNAME,
>>>>> +#define RTM_DELALTIFNAME RTM_DELALTIFNAME
>>>>> + RTM_GETALTIFNAME,
>>>>> +#define RTM_GETALTIFNAME RTM_GETALTIFNAME
>>>>> +
>>>>
>>>> I might have missed the prior discussion, why do we need new commands
>>>> ?. can't this simply be part of RTM_*LINK and we use RTM_SETLINK to
>>>> set alternate names ?
>>>
>>> How? This is to add/remove. How do you suggest to to add/remove by
>>> setlink?
>>
>> to that point, I am also not sure why we have a new API For multiple
>> names. I mean why support more than two names (existing old name and
>> a new name to remove the length limitation) ?
>>
>> Your patch series addresses a very important problem (we run into this
>> limitation all the time and its hard to explain it to network
>> operators) and
>> its already unfortunate that we have to have more than one name
>> because we cannot resize the existing one.
>>
>> The best we can do for simpler transition/management from user-space
>> is to keep the api simple..
>> ie keep it close to the management of existing link attributes. Hence
>> the question.
>>
>> I assumed this would be like alias. A single new field that can be
>> referenced in lieu of the old one.
>>
>> Your series is very useful to many of us...but when i think about
>> changing our network manager to accommodate this, I am worried about
>> how many apps will have to change.
>> I agree they have to change regardless but now they will have to
>> listen to yet another notification and msg format for names ?
>>
>> (apologies for joining the thread late and if i missed prior discussion on this)
>
>I agree with Roopa. I do not understand why new RTM commands are needed.
>The existing IFLA + ifinfomsg struct give more than enough ways to id
>the device for adding / deleting an alternate name.
>
Could you please write me an example message of add/remove?
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-10 6:30 ` Jiri Pirko
@ 2019-08-12 1:34 ` David Ahern
2019-08-12 1:37 ` David Ahern
0 siblings, 1 reply; 76+ messages in thread
From: David Ahern @ 2019-08-12 1:34 UTC (permalink / raw)
To: Jiri Pirko
Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On 8/10/19 12:30 AM, Jiri Pirko wrote:
> Could you please write me an example message of add/remove?
altnames are for existing netdevs, yes? existing netdevs have an id and
a name - 2 existing references for identifying the existing netdev for
which an altname will be added. Even using the altname as the main
'handle' for a setlink change, I see no reason why the GETLINK api can
not take an the IFLA_ALT_IFNAME and return the full details of the
device if the altname is unique.
So, what do the new RTM commands give you that you can not do with
RTM_*LINK?
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 1:34 ` David Ahern
@ 2019-08-12 1:37 ` David Ahern
2019-08-12 8:31 ` Jiri Pirko
0 siblings, 1 reply; 76+ messages in thread
From: David Ahern @ 2019-08-12 1:37 UTC (permalink / raw)
To: Jiri Pirko
Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On 8/11/19 7:34 PM, David Ahern wrote:
> On 8/10/19 12:30 AM, Jiri Pirko wrote:
>> Could you please write me an example message of add/remove?
>
> altnames are for existing netdevs, yes? existing netdevs have an id and
> a name - 2 existing references for identifying the existing netdev for
> which an altname will be added. Even using the altname as the main
> 'handle' for a setlink change, I see no reason why the GETLINK api can
> not take an the IFLA_ALT_IFNAME and return the full details of the
> device if the altname is unique.
>
> So, what do the new RTM commands give you that you can not do with
> RTM_*LINK?
>
To put this another way, the ALT_NAME is an attribute of an object - a
LINK. It is *not* a separate object which requires its own set of
commands for manipulating.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 1:37 ` David Ahern
@ 2019-08-12 8:31 ` Jiri Pirko
2019-08-12 15:13 ` Roopa Prabhu
` (2 more replies)
0 siblings, 3 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-08-12 8:31 UTC (permalink / raw)
To: David Ahern
Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
>On 8/11/19 7:34 PM, David Ahern wrote:
>> On 8/10/19 12:30 AM, Jiri Pirko wrote:
>>> Could you please write me an example message of add/remove?
>>
>> altnames are for existing netdevs, yes? existing netdevs have an id and
>> a name - 2 existing references for identifying the existing netdev for
>> which an altname will be added. Even using the altname as the main
>> 'handle' for a setlink change, I see no reason why the GETLINK api can
>> not take an the IFLA_ALT_IFNAME and return the full details of the
>> device if the altname is unique.
>>
>> So, what do the new RTM commands give you that you can not do with
>> RTM_*LINK?
>>
>
>
>To put this another way, the ALT_NAME is an attribute of an object - a
>LINK. It is *not* a separate object which requires its own set of
>commands for manipulating.
Okay, again, could you provide example of a message to add/remove
altname using existing setlink message? Thanks!
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 8:31 ` Jiri Pirko
@ 2019-08-12 15:13 ` Roopa Prabhu
2019-08-12 21:43 ` Jakub Kicinski
2019-08-13 6:51 ` Jiri Pirko
2019-08-12 15:40 ` Stephen Hemminger
2019-08-12 16:01 ` David Ahern
2 siblings, 2 replies; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-12 15:13 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Ahern, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Mon, Aug 12, 2019 at 1:31 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
> >On 8/11/19 7:34 PM, David Ahern wrote:
> >> On 8/10/19 12:30 AM, Jiri Pirko wrote:
> >>> Could you please write me an example message of add/remove?
> >>
> >> altnames are for existing netdevs, yes? existing netdevs have an id and
> >> a name - 2 existing references for identifying the existing netdev for
> >> which an altname will be added. Even using the altname as the main
> >> 'handle' for a setlink change, I see no reason why the GETLINK api can
> >> not take an the IFLA_ALT_IFNAME and return the full details of the
> >> device if the altname is unique.
> >>
> >> So, what do the new RTM commands give you that you can not do with
> >> RTM_*LINK?
> >>
> >
> >
> >To put this another way, the ALT_NAME is an attribute of an object - a
> >LINK. It is *not* a separate object which requires its own set of
> >commands for manipulating.
>
> Okay, again, could you provide example of a message to add/remove
> altname using existing setlink message? Thanks!
Will the below work ?... just throwing an example for discussion:
make the name list a nested list
IFLA_ALT_NAMES
IFLA_ALT_NAME_OP /* ADD or DEL used with setlink */
IFLA_ALT_NAME
IFLA_ALT_NAME_LIST
With RTM_NEWLINK you can specify a list to set and unset
With RTM_SETLINK you can specify an individual name with a add or del op
notifications will always be RTM_NEWLINK with the full list.
The nested attribute can be structured differently.
Only thing is i am worried about increasing the size of link dump and
notification msgs.
What is the limit on the number of names again ?
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 15:13 ` Roopa Prabhu
@ 2019-08-12 21:43 ` Jakub Kicinski
2019-08-13 0:29 ` David Ahern
2019-08-13 6:51 ` Jiri Pirko
1 sibling, 1 reply; 76+ messages in thread
From: Jakub Kicinski @ 2019-08-12 21:43 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Jiri Pirko, David Ahern, netdev, David Miller, Stephen Hemminger,
dcbw, Michal Kubecek, Andrew Lunn, parav, Saeed Mahameed, mlxsw
On Mon, 12 Aug 2019 08:13:39 -0700, Roopa Prabhu wrote:
> On Mon, Aug 12, 2019 at 1:31 AM Jiri Pirko <jiri@resnulli.us> wrote:
> > Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
> > >On 8/11/19 7:34 PM, David Ahern wrote:
> > >> On 8/10/19 12:30 AM, Jiri Pirko wrote:
> > >>> Could you please write me an example message of add/remove?
> > >>
> > >> altnames are for existing netdevs, yes? existing netdevs have an id and
> > >> a name - 2 existing references for identifying the existing netdev for
> > >> which an altname will be added. Even using the altname as the main
> > >> 'handle' for a setlink change, I see no reason why the GETLINK api can
> > >> not take an the IFLA_ALT_IFNAME and return the full details of the
> > >> device if the altname is unique.
> > >>
> > >> So, what do the new RTM commands give you that you can not do with
> > >> RTM_*LINK?
> > >
> > >To put this another way, the ALT_NAME is an attribute of an object - a
> > >LINK. It is *not* a separate object which requires its own set of
> > >commands for manipulating.
> >
> > Okay, again, could you provide example of a message to add/remove
> > altname using existing setlink message? Thanks!
>
> Will the below work ?... just throwing an example for discussion:
>
> make the name list a nested list
> IFLA_ALT_NAMES
> IFLA_ALT_NAME_OP /* ADD or DEL used with setlink */
> IFLA_ALT_NAME
> IFLA_ALT_NAME_LIST
>
> With RTM_NEWLINK you can specify a list to set and unset
> With RTM_SETLINK you can specify an individual name with a add or del op
>
> notifications will always be RTM_NEWLINK with the full list.
>
> The nested attribute can be structured differently.
>
> Only thing is i am worried about increasing the size of link dump and
> notification msgs.
Is not adding commands better because it's easier to deal with the
RTM_NEWLINK notification? I must say it's unclear from the thread why
muxing the op through RTM_SETLINK is preferable. IMHO new op is
cleaner, do we have precedent for such IFLA_.*_OP-style attributes?
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 21:43 ` Jakub Kicinski
@ 2019-08-13 0:29 ` David Ahern
2019-08-13 6:53 ` Jiri Pirko
0 siblings, 1 reply; 76+ messages in thread
From: David Ahern @ 2019-08-13 0:29 UTC (permalink / raw)
To: Jakub Kicinski, Roopa Prabhu
Cc: Jiri Pirko, netdev, David Miller, Stephen Hemminger, dcbw,
Michal Kubecek, Andrew Lunn, parav, Saeed Mahameed, mlxsw
On 8/12/19 3:43 PM, Jakub Kicinski wrote:
> Is not adding commands better because it's easier to deal with the
> RTM_NEWLINK notification? I must say it's unclear from the thread why
> muxing the op through RTM_SETLINK is preferable. IMHO new op is
> cleaner, do we have precedent for such IFLA_.*_OP-style attributes?
An alternative name for a link is not a primary object; it is only an
attribute of a link and links are manipulated through RTM_*LINK commands.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-13 0:29 ` David Ahern
@ 2019-08-13 6:53 ` Jiri Pirko
0 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-08-13 6:53 UTC (permalink / raw)
To: David Ahern
Cc: Jakub Kicinski, Roopa Prabhu, netdev, David Miller,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Tue, Aug 13, 2019 at 02:29:13AM CEST, dsahern@gmail.com wrote:
>On 8/12/19 3:43 PM, Jakub Kicinski wrote:
>> Is not adding commands better because it's easier to deal with the
>> RTM_NEWLINK notification? I must say it's unclear from the thread why
>> muxing the op through RTM_SETLINK is preferable. IMHO new op is
>> cleaner, do we have precedent for such IFLA_.*_OP-style attributes?
>
>An alternative name for a link is not a primary object; it is only an
>attribute of a link and links are manipulated through RTM_*LINK commands.
So? Still, doing the OP thing inside the message feels wrong, "primary
object" or now. Why can't the "secondary object" (whatever it is) have
separate command? What is the limitation? I'm trying to understand the
reason.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 15:13 ` Roopa Prabhu
2019-08-12 21:43 ` Jakub Kicinski
@ 2019-08-13 6:51 ` Jiri Pirko
1 sibling, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-08-13 6:51 UTC (permalink / raw)
To: Roopa Prabhu
Cc: David Ahern, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Mon, Aug 12, 2019 at 05:13:39PM CEST, roopa@cumulusnetworks.com wrote:
>On Mon, Aug 12, 2019 at 1:31 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
>> >On 8/11/19 7:34 PM, David Ahern wrote:
>> >> On 8/10/19 12:30 AM, Jiri Pirko wrote:
>> >>> Could you please write me an example message of add/remove?
>> >>
>> >> altnames are for existing netdevs, yes? existing netdevs have an id and
>> >> a name - 2 existing references for identifying the existing netdev for
>> >> which an altname will be added. Even using the altname as the main
>> >> 'handle' for a setlink change, I see no reason why the GETLINK api can
>> >> not take an the IFLA_ALT_IFNAME and return the full details of the
>> >> device if the altname is unique.
>> >>
>> >> So, what do the new RTM commands give you that you can not do with
>> >> RTM_*LINK?
>> >>
>> >
>> >
>> >To put this another way, the ALT_NAME is an attribute of an object - a
>> >LINK. It is *not* a separate object which requires its own set of
>> >commands for manipulating.
>>
>> Okay, again, could you provide example of a message to add/remove
>> altname using existing setlink message? Thanks!
>
>Will the below work ?... just throwing an example for discussion:
>
>make the name list a nested list
>IFLA_ALT_NAMES
> IFLA_ALT_NAME_OP /* ADD or DEL used with setlink */
This is exacly what I tried to avoid. Providing an OP within a message
is weird. So I wanted to do it rather in the way similar to NEIGH for
example, where you have new/del commands.
> IFLA_ALT_NAME
> IFLA_ALT_NAME_LIST
>
>With RTM_NEWLINK you can specify a list to set and unset
>With RTM_SETLINK you can specify an individual name with a add or del op
>
>notifications will always be RTM_NEWLINK with the full list.
>
>The nested attribute can be structured differently.
>
>Only thing is i am worried about increasing the size of link dump and
>notification msgs.
>
>What is the limit on the number of names again ?
No limit.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 8:31 ` Jiri Pirko
2019-08-12 15:13 ` Roopa Prabhu
@ 2019-08-12 15:40 ` Stephen Hemminger
2019-08-12 16:23 ` Roopa Prabhu
2019-08-13 6:55 ` Jiri Pirko
2019-08-12 16:01 ` David Ahern
2 siblings, 2 replies; 76+ messages in thread
From: Stephen Hemminger @ 2019-08-12 15:40 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Ahern, Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Mon, 12 Aug 2019 10:31:39 +0200
Jiri Pirko <jiri@resnulli.us> wrote:
> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
> >On 8/11/19 7:34 PM, David Ahern wrote:
> >> On 8/10/19 12:30 AM, Jiri Pirko wrote:
> >>> Could you please write me an example message of add/remove?
> >>
> >> altnames are for existing netdevs, yes? existing netdevs have an id and
> >> a name - 2 existing references for identifying the existing netdev for
> >> which an altname will be added. Even using the altname as the main
> >> 'handle' for a setlink change, I see no reason why the GETLINK api can
> >> not take an the IFLA_ALT_IFNAME and return the full details of the
> >> device if the altname is unique.
> >>
> >> So, what do the new RTM commands give you that you can not do with
> >> RTM_*LINK?
> >>
> >
> >
> >To put this another way, the ALT_NAME is an attribute of an object - a
> >LINK. It is *not* a separate object which requires its own set of
> >commands for manipulating.
>
> Okay, again, could you provide example of a message to add/remove
> altname using existing setlink message? Thanks!
The existing IFALIAS takes an empty name to do deletion.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 15:40 ` Stephen Hemminger
@ 2019-08-12 16:23 ` Roopa Prabhu
2019-08-13 6:55 ` Jiri Pirko
1 sibling, 0 replies; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-12 16:23 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jiri Pirko, David Ahern, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Mon, Aug 12, 2019 at 8:40 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 12 Aug 2019 10:31:39 +0200
> Jiri Pirko <jiri@resnulli.us> wrote:
>
> > Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
> > >On 8/11/19 7:34 PM, David Ahern wrote:
> > >> On 8/10/19 12:30 AM, Jiri Pirko wrote:
> > >>> Could you please write me an example message of add/remove?
> > >>
> > >> altnames are for existing netdevs, yes? existing netdevs have an id and
> > >> a name - 2 existing references for identifying the existing netdev for
> > >> which an altname will be added. Even using the altname as the main
> > >> 'handle' for a setlink change, I see no reason why the GETLINK api can
> > >> not take an the IFLA_ALT_IFNAME and return the full details of the
> > >> device if the altname is unique.
> > >>
> > >> So, what do the new RTM commands give you that you can not do with
> > >> RTM_*LINK?
> > >>
> > >
> > >
> > >To put this another way, the ALT_NAME is an attribute of an object - a
> > >LINK. It is *not* a separate object which requires its own set of
> > >commands for manipulating.
> >
> > Okay, again, could you provide example of a message to add/remove
> > altname using existing setlink message? Thanks!
>
> The existing IFALIAS takes an empty name to do deletion.
yes, if its a single alt name, keeping it similar to IFALIAS will help
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 15:40 ` Stephen Hemminger
2019-08-12 16:23 ` Roopa Prabhu
@ 2019-08-13 6:55 ` Jiri Pirko
1 sibling, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-08-13 6:55 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Ahern, Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Mon, Aug 12, 2019 at 05:40:39PM CEST, stephen@networkplumber.org wrote:
>On Mon, 12 Aug 2019 10:31:39 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
>> >On 8/11/19 7:34 PM, David Ahern wrote:
>> >> On 8/10/19 12:30 AM, Jiri Pirko wrote:
>> >>> Could you please write me an example message of add/remove?
>> >>
>> >> altnames are for existing netdevs, yes? existing netdevs have an id and
>> >> a name - 2 existing references for identifying the existing netdev for
>> >> which an altname will be added. Even using the altname as the main
>> >> 'handle' for a setlink change, I see no reason why the GETLINK api can
>> >> not take an the IFLA_ALT_IFNAME and return the full details of the
>> >> device if the altname is unique.
>> >>
>> >> So, what do the new RTM commands give you that you can not do with
>> >> RTM_*LINK?
>> >>
>> >
>> >
>> >To put this another way, the ALT_NAME is an attribute of an object - a
>> >LINK. It is *not* a separate object which requires its own set of
>> >commands for manipulating.
>>
>> Okay, again, could you provide example of a message to add/remove
>> altname using existing setlink message? Thanks!
>
>The existing IFALIAS takes an empty name to do deletion.
Ifalias is one and one only. Woudn't work for multiple altnames...
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 8:31 ` Jiri Pirko
2019-08-12 15:13 ` Roopa Prabhu
2019-08-12 15:40 ` Stephen Hemminger
@ 2019-08-12 16:01 ` David Ahern
2019-08-13 6:56 ` Jiri Pirko
2 siblings, 1 reply; 76+ messages in thread
From: David Ahern @ 2019-08-12 16:01 UTC (permalink / raw)
To: Jiri Pirko
Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On 8/12/19 2:31 AM, Jiri Pirko wrote:
> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
>> On 8/11/19 7:34 PM, David Ahern wrote:
>>> On 8/10/19 12:30 AM, Jiri Pirko wrote:
>>>> Could you please write me an example message of add/remove?
>>>
>>> altnames are for existing netdevs, yes? existing netdevs have an id and
>>> a name - 2 existing references for identifying the existing netdev for
>>> which an altname will be added. Even using the altname as the main
>>> 'handle' for a setlink change, I see no reason why the GETLINK api can
>>> not take an the IFLA_ALT_IFNAME and return the full details of the
>>> device if the altname is unique.
>>>
>>> So, what do the new RTM commands give you that you can not do with
>>> RTM_*LINK?
>>>
>>
>>
>> To put this another way, the ALT_NAME is an attribute of an object - a
>> LINK. It is *not* a separate object which requires its own set of
>> commands for manipulating.
>
> Okay, again, could you provide example of a message to add/remove
> altname using existing setlink message? Thanks!
>
Examples from your cover letter with updates
$ ip link set dummy0 altname someothername
$ ip link set dummy0 altname someotherveryveryveryverylongname
$ ip link set dummy0 del altname someothername
$ ip link set dummy0 del altname someotherveryveryveryverylongname
This syntactic sugar to what is really happening:
RTM_NEWLINK, dummy0, IFLA_ALT_IFNAME
if you are allowing many alt names, then yes, you need a flag to say
delete this specific one which is covered by Roopa's nested suggestion.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-12 16:01 ` David Ahern
@ 2019-08-13 6:56 ` Jiri Pirko
2019-08-26 16:09 ` Jiri Pirko
0 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-08-13 6:56 UTC (permalink / raw)
To: David Ahern
Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Mon, Aug 12, 2019 at 06:01:59PM CEST, dsahern@gmail.com wrote:
>On 8/12/19 2:31 AM, Jiri Pirko wrote:
>> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
>>> On 8/11/19 7:34 PM, David Ahern wrote:
>>>> On 8/10/19 12:30 AM, Jiri Pirko wrote:
>>>>> Could you please write me an example message of add/remove?
>>>>
>>>> altnames are for existing netdevs, yes? existing netdevs have an id and
>>>> a name - 2 existing references for identifying the existing netdev for
>>>> which an altname will be added. Even using the altname as the main
>>>> 'handle' for a setlink change, I see no reason why the GETLINK api can
>>>> not take an the IFLA_ALT_IFNAME and return the full details of the
>>>> device if the altname is unique.
>>>>
>>>> So, what do the new RTM commands give you that you can not do with
>>>> RTM_*LINK?
>>>>
>>>
>>>
>>> To put this another way, the ALT_NAME is an attribute of an object - a
>>> LINK. It is *not* a separate object which requires its own set of
>>> commands for manipulating.
>>
>> Okay, again, could you provide example of a message to add/remove
>> altname using existing setlink message? Thanks!
>>
>
>Examples from your cover letter with updates
>
>$ ip link set dummy0 altname someothername
>$ ip link set dummy0 altname someotherveryveryveryverylongname
>
>$ ip link set dummy0 del altname someothername
>$ ip link set dummy0 del altname someotherveryveryveryverylongname
>
>This syntactic sugar to what is really happening:
>
>RTM_NEWLINK, dummy0, IFLA_ALT_IFNAME
>
>if you are allowing many alt names, then yes, you need a flag to say
>delete this specific one which is covered by Roopa's nested suggestion.
Yeah, so you need and op inside the message. We are on the same page,
thanks.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-13 6:56 ` Jiri Pirko
@ 2019-08-26 16:09 ` Jiri Pirko
2019-08-26 16:55 ` Jakub Kicinski
0 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-08-26 16:09 UTC (permalink / raw)
To: David Ahern
Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Tue, Aug 13, 2019 at 08:56:17AM CEST, jiri@resnulli.us wrote:
>Mon, Aug 12, 2019 at 06:01:59PM CEST, dsahern@gmail.com wrote:
>>On 8/12/19 2:31 AM, Jiri Pirko wrote:
>>> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
>>>> On 8/11/19 7:34 PM, David Ahern wrote:
>>>>> On 8/10/19 12:30 AM, Jiri Pirko wrote:
>>>>>> Could you please write me an example message of add/remove?
>>>>>
>>>>> altnames are for existing netdevs, yes? existing netdevs have an id and
>>>>> a name - 2 existing references for identifying the existing netdev for
>>>>> which an altname will be added. Even using the altname as the main
>>>>> 'handle' for a setlink change, I see no reason why the GETLINK api can
>>>>> not take an the IFLA_ALT_IFNAME and return the full details of the
>>>>> device if the altname is unique.
>>>>>
>>>>> So, what do the new RTM commands give you that you can not do with
>>>>> RTM_*LINK?
>>>>>
>>>>
>>>>
>>>> To put this another way, the ALT_NAME is an attribute of an object - a
>>>> LINK. It is *not* a separate object which requires its own set of
>>>> commands for manipulating.
>>>
>>> Okay, again, could you provide example of a message to add/remove
>>> altname using existing setlink message? Thanks!
>>>
>>
>>Examples from your cover letter with updates
>>
>>$ ip link set dummy0 altname someothername
>>$ ip link set dummy0 altname someotherveryveryveryverylongname
>>
>>$ ip link set dummy0 del altname someothername
>>$ ip link set dummy0 del altname someotherveryveryveryverylongname
>>
>>This syntactic sugar to what is really happening:
>>
>>RTM_NEWLINK, dummy0, IFLA_ALT_IFNAME
>>
>>if you are allowing many alt names, then yes, you need a flag to say
>>delete this specific one which is covered by Roopa's nested suggestion.
>
>Yeah, so you need and op inside the message. We are on the same page,
>thanks.
DaveA, Roopa. Do you insist on doing add/remove of altnames in the
existing setlist command using embedded message op attrs? I'm asking
because after some time thinking about it, it still feels wrong to me :/
If this would be a generic netlink api, we would just add another couple
of commands. What is so different we can't add commands here?
It is also much simpler code. Easy error handling, no need for
rollback, no possibly inconsistent state, etc.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 16:09 ` Jiri Pirko
@ 2019-08-26 16:55 ` Jakub Kicinski
2019-08-26 21:46 ` David Ahern
0 siblings, 1 reply; 76+ messages in thread
From: Jakub Kicinski @ 2019-08-26 16:55 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Ahern, Roopa Prabhu, netdev, David Miller,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Mon, 26 Aug 2019 18:09:16 +0200, Jiri Pirko wrote:
> DaveA, Roopa. Do you insist on doing add/remove of altnames in the
> existing setlist command using embedded message op attrs? I'm asking
> because after some time thinking about it, it still feels wrong to me :/
>
> If this would be a generic netlink api, we would just add another couple
> of commands. What is so different we can't add commands here?
> It is also much simpler code. Easy error handling, no need for
> rollback, no possibly inconsistent state, etc.
+1 the separate op feels like a better uapi to me as well.
Perhaps we could redo the iproute2 command line interface to make the
name the primary object? Would that address your concern Dave and Roopa?
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 16:55 ` Jakub Kicinski
@ 2019-08-26 21:46 ` David Ahern
2019-08-26 22:15 ` Jakub Kicinski
2019-08-27 4:55 ` Michal Kubecek
0 siblings, 2 replies; 76+ messages in thread
From: David Ahern @ 2019-08-26 21:46 UTC (permalink / raw)
To: Jakub Kicinski, Jiri Pirko
Cc: Roopa Prabhu, netdev, David Miller, Stephen Hemminger, dcbw,
Michal Kubecek, Andrew Lunn, parav, Saeed Mahameed, mlxsw
On 8/26/19 10:55 AM, Jakub Kicinski wrote:
> On Mon, 26 Aug 2019 18:09:16 +0200, Jiri Pirko wrote:
>> DaveA, Roopa. Do you insist on doing add/remove of altnames in the
>> existing setlist command using embedded message op attrs? I'm asking
>> because after some time thinking about it, it still feels wrong to me :/
>>
>> If this would be a generic netlink api, we would just add another couple
>> of commands. What is so different we can't add commands here?
>> It is also much simpler code. Easy error handling, no need for
>> rollback, no possibly inconsistent state, etc.
>
> +1 the separate op feels like a better uapi to me as well.
>
> Perhaps we could redo the iproute2 command line interface to make the
> name the primary object? Would that address your concern Dave and Roopa?
>
No, my point is exactly that a name is not a primary object. A name is
an attribute of a link - something that exists for the convenience of
userspace only. (Like the 'protocol' for routes, rules and neighbors.)
Currently, names are changed by RTM_NEWLINK/RTM_SETLINK. Aliases are
added and deleted by RTM_NEWLINK/RTM_SETLINK. Why is an alternative name
so special that it should have its own API?
If only 1 alt name was allowed, then RTM_NEWLINK/RTM_SETLINK would
suffice. Management of it would have the same semantics as an alias -
empty string means delete, non-empty string sets the value.
So really the push for new RTM commands is to handle an unlimited number
of alt names with the ability to change / delete any one of them. Has
the need for multiple alternate ifnames been fully established? (I don't
recall other than a discussion about parallels to block devices.)
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 21:46 ` David Ahern
@ 2019-08-26 22:15 ` Jakub Kicinski
2019-08-26 22:18 ` David Miller
2019-08-27 4:55 ` Michal Kubecek
1 sibling, 1 reply; 76+ messages in thread
From: Jakub Kicinski @ 2019-08-26 22:15 UTC (permalink / raw)
To: David Ahern
Cc: Jiri Pirko, Roopa Prabhu, netdev, David Miller,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Mon, 26 Aug 2019 15:46:43 -0600, David Ahern wrote:
> On 8/26/19 10:55 AM, Jakub Kicinski wrote:
> > On Mon, 26 Aug 2019 18:09:16 +0200, Jiri Pirko wrote:
> >> DaveA, Roopa. Do you insist on doing add/remove of altnames in the
> >> existing setlist command using embedded message op attrs? I'm asking
> >> because after some time thinking about it, it still feels wrong to me :/
> >>
> >> If this would be a generic netlink api, we would just add another couple
> >> of commands. What is so different we can't add commands here?
> >> It is also much simpler code. Easy error handling, no need for
> >> rollback, no possibly inconsistent state, etc.
> >
> > +1 the separate op feels like a better uapi to me as well.
> >
> > Perhaps we could redo the iproute2 command line interface to make the
> > name the primary object? Would that address your concern Dave and Roopa?
>
> No, my point is exactly that a name is not a primary object. A name is
> an attribute of a link - something that exists for the convenience of
> userspace only. (Like the 'protocol' for routes, rules and neighbors.)
>
> Currently, names are changed by RTM_NEWLINK/RTM_SETLINK. Aliases are
> added and deleted by RTM_NEWLINK/RTM_SETLINK. Why is an alternative name
> so special that it should have its own API?
My feeling is that it's better to introduce operations for this
sub-object than mux everything via setlink :( The use of netlink
in more recent APIs like devlink is much more liberal when it comes
to ops and the result is much more convenient and clean IMHO.
Weren't there multiple problems with the size of the RTM_NEWLINK
notification already? Adding multiple sizeable strings to it won't
help there either :S
Do you foresee a need for the alias to be updated atomically with other
RTM_SETLINK changes?
> If only 1 alt name was allowed, then RTM_NEWLINK/RTM_SETLINK would
> suffice. Management of it would have the same semantics as an alias -
> empty string means delete, non-empty string sets the value.
>
> So really the push for new RTM commands is to handle an unlimited number
> of alt names with the ability to change / delete any one of them. Has
> the need for multiple alternate ifnames been fully established? (I don't
> recall other than a discussion about parallels to block devices.)
I feel like I already posted this link, but here it is again:
https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
IMHO the fact that there are multiple naming schemes in systemd today
is proof enough.
To be clear my (probably over-engineered) position is that "user-
-understandable" interface names had became a dead end already long
time ago. We should move away from strings and try to build APIs or at
least user space where device can be selected based on attributes
directly. The names are nothing else than a random grab bag of
attributes sprintf()-ed together, anyway.
The naming done by udev is inherently racy and over and over again we
run into races and issues because OvS or some other piece of userspace
gets confused and e.g. enslaves ports to wrong bridges.
Longer names I just a band aid. But I'm under no illusion I can convince
people to spend time working on an attribute-based scheme.. ;)
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 22:15 ` Jakub Kicinski
@ 2019-08-26 22:18 ` David Miller
2019-08-26 22:24 ` David Ahern
2019-08-27 7:08 ` Jiri Pirko
0 siblings, 2 replies; 76+ messages in thread
From: David Miller @ 2019-08-26 22:18 UTC (permalink / raw)
To: jakub.kicinski
Cc: dsahern, jiri, roopa, netdev, sthemmin, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 26 Aug 2019 15:15:52 -0700
> Weren't there multiple problems with the size of the RTM_NEWLINK
> notification already? Adding multiple sizeable strings to it won't
> help there either :S
Indeed.
We even had situations where we had to make the information provided
in a newlink dump opt-in when we added VF info because the buffers
glibc was using at the time were too small and this broke so much
stuff.
I honestly think that the size of link dumps are out of hand as-is.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 22:18 ` David Miller
@ 2019-08-26 22:24 ` David Ahern
2019-08-26 22:25 ` David Miller
2019-08-27 7:08 ` Jiri Pirko
1 sibling, 1 reply; 76+ messages in thread
From: David Ahern @ 2019-08-26 22:24 UTC (permalink / raw)
To: David Miller, jakub.kicinski
Cc: jiri, roopa, netdev, sthemmin, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
On 8/26/19 4:18 PM, David Miller wrote:
> I honestly think that the size of link dumps are out of hand as-is.
so you are suggesting new alternate names should not appear in kernel
generated RTM_NEWLINK messages - be it a link dump or a notification on
a change?
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 22:24 ` David Ahern
@ 2019-08-26 22:25 ` David Miller
2019-08-27 0:17 ` David Ahern
0 siblings, 1 reply; 76+ messages in thread
From: David Miller @ 2019-08-26 22:25 UTC (permalink / raw)
To: dsahern
Cc: jakub.kicinski, jiri, roopa, netdev, sthemmin, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
From: David Ahern <dsahern@gmail.com>
Date: Mon, 26 Aug 2019 16:24:38 -0600
> On 8/26/19 4:18 PM, David Miller wrote:
>> I honestly think that the size of link dumps are out of hand as-is.
>
> so you are suggesting new alternate names should not appear in kernel
> generated RTM_NEWLINK messages - be it a link dump or a notification on
> a change?
I counter with the question of how much crap can we keep sticking in there
before we have to do something else to provide that information?
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 22:25 ` David Miller
@ 2019-08-27 0:17 ` David Ahern
2019-08-27 5:09 ` Michal Kubecek
0 siblings, 1 reply; 76+ messages in thread
From: David Ahern @ 2019-08-27 0:17 UTC (permalink / raw)
To: David Miller
Cc: jakub.kicinski, jiri, roopa, netdev, sthemmin, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
On 8/26/19 4:25 PM, David Miller wrote:
> From: David Ahern <dsahern@gmail.com>
> Date: Mon, 26 Aug 2019 16:24:38 -0600
>
>> On 8/26/19 4:18 PM, David Miller wrote:
>>> I honestly think that the size of link dumps are out of hand as-is.
>>
>> so you are suggesting new alternate names should not appear in kernel
>> generated RTM_NEWLINK messages - be it a link dump or a notification on
>> a change?
>
> I counter with the question of how much crap can we keep sticking in there
> before we have to do something else to provide that information?
>
Something a bit stand alone would be a better choice - like all of the
VF stuff, stats, per-device type configuration. Yes, that ship has
sailed, but as I recall that is where the overhead is.
An attribute as basic as a name is the wrong place for that split.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-27 0:17 ` David Ahern
@ 2019-08-27 5:09 ` Michal Kubecek
0 siblings, 0 replies; 76+ messages in thread
From: Michal Kubecek @ 2019-08-27 5:09 UTC (permalink / raw)
To: netdev
Cc: David Ahern, David Miller, jakub.kicinski, jiri, roopa, sthemmin,
dcbw, andrew, parav, saeedm, mlxsw
On Mon, Aug 26, 2019 at 06:17:08PM -0600, David Ahern wrote:
>
> Something a bit stand alone would be a better choice - like all of the
> VF stuff, stats, per-device type configuration. Yes, that ship has
> sailed, but as I recall that is where the overhead is.
We are not so far from the point where IFLA_VFINFO_LIST grows over 64 KB
so that it does not fit into a netlink attribute. So we will probably
have to rethink that part anyway.
Michal
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 22:18 ` David Miller
2019-08-26 22:24 ` David Ahern
@ 2019-08-27 7:08 ` Jiri Pirko
2019-08-27 8:22 ` David Miller
1 sibling, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-08-27 7:08 UTC (permalink / raw)
To: David Miller
Cc: jakub.kicinski, dsahern, roopa, netdev, sthemmin, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
Tue, Aug 27, 2019 at 12:18:19AM CEST, davem@davemloft.net wrote:
>From: Jakub Kicinski <jakub.kicinski@netronome.com>
>Date: Mon, 26 Aug 2019 15:15:52 -0700
>
>> Weren't there multiple problems with the size of the RTM_NEWLINK
>> notification already? Adding multiple sizeable strings to it won't
>> help there either :S
>
>Indeed.
>
>We even had situations where we had to make the information provided
>in a newlink dump opt-in when we added VF info because the buffers
>glibc was using at the time were too small and this broke so much
>stuff.
>
>I honestly think that the size of link dumps are out of hand as-is.
Okay, so if I understand correctly, on top of separate commands for
add/del of alternative names, you suggest also get/dump to be separate
command and don't fill this up in existing newling/getlink command.
So we'll have:
CMD to add:
RTM_NEWALTIFNAME = 108
#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
Example msg (user->kernel):
IFLA_IFNAME eth0
IFLA_ALT_IFNAME_MOD somereallyreallylongname
Example msg (user->kernel):
IFLA_ALT_IFNAME somereallyreallylongname
IFLA_ALT_IFNAME_MOD someotherreallyreallylongname
CMD to delete:
RTM_DELALTIFNAME,
#define RTM_DELALTIFNAME RTM_DELALTIFNAME
Example msg (user->kernel):
IFLA_IFNAME eth0
IFLA_ALT_IFNAME_MOD somereallyreallylongname
CMD to get/dump:
RTM_GETALTIFNAME,
#define RTM_GETALTIFNAME RTM_GETALTIFNAME
Example msg (kernel->user):
hdr (with ifindex)
IFLA_ALT_IFNAME_LIST (nest)
IFLA_ALT_IFNAME somereallyreallylongname
IFLA_ALT_IFNAME someotherreallyreallylongname
Correct?
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-27 7:08 ` Jiri Pirko
@ 2019-08-27 8:22 ` David Miller
2019-08-27 9:35 ` Jiri Pirko
0 siblings, 1 reply; 76+ messages in thread
From: David Miller @ 2019-08-27 8:22 UTC (permalink / raw)
To: jiri
Cc: jakub.kicinski, dsahern, roopa, netdev, sthemmin, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 27 Aug 2019 09:08:08 +0200
> Okay, so if I understand correctly, on top of separate commands for
> add/del of alternative names, you suggest also get/dump to be separate
> command and don't fill this up in existing newling/getlink command.
I'm not sure what to do yet.
David has a point, because the only way these ifnames are useful is
as ways to specify and choose net devices. So based upon that I'm
slightly learning towards not using separate commands.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-27 8:22 ` David Miller
@ 2019-08-27 9:35 ` Jiri Pirko
2019-08-27 15:14 ` Roopa Prabhu
0 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-08-27 9:35 UTC (permalink / raw)
To: David Miller
Cc: jakub.kicinski, dsahern, roopa, netdev, sthemmin, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
Tue, Aug 27, 2019 at 10:22:42AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Tue, 27 Aug 2019 09:08:08 +0200
>
>> Okay, so if I understand correctly, on top of separate commands for
>> add/del of alternative names, you suggest also get/dump to be separate
>> command and don't fill this up in existing newling/getlink command.
>
>I'm not sure what to do yet.
>
>David has a point, because the only way these ifnames are useful is
>as ways to specify and choose net devices. So based upon that I'm
>slightly learning towards not using separate commands.
Well yeah, one can use it to handle existing commands instead of
IFLA_NAME.
But why does it rule out separate commands? I think it is cleaner than
to put everything in poor setlink messages :/ The fact that we would
need to add "OP" to the setlink message just feels of. Other similar
needs may show up in the future and we may endup in ridiculous messages
like:
SETLINK
IFLA_NAME eth0
IFLA_ATLNAME_LIST (nest)
IFLA_ALTNAME_OP add
IFLA_ALTNAME somereallylongname
IFLA_ALTNAME_OP del
IFLA_ALTNAME somereallyreallylongname
IFLA_ALTNAME_OP add
IFLA_ALTNAME someotherreallylongname
IFLA_SOMETHING_ELSE_LIST (nest)
IFLA_SOMETHING_ELSE_OP add
...
IFLA_SOMETHING_ELSE_OP del
...
IFLA_SOMETHING_ELSE_OP add
...
I don't know what to think about it. Rollbacks are going to be pure hell :/
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-27 9:35 ` Jiri Pirko
@ 2019-08-27 15:14 ` Roopa Prabhu
2019-08-28 7:07 ` Jiri Pirko
0 siblings, 1 reply; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-27 15:14 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, Jakub Kicinski, David Ahern, netdev,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Tue, Aug 27, 2019 at 2:35 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, Aug 27, 2019 at 10:22:42AM CEST, davem@davemloft.net wrote:
> >From: Jiri Pirko <jiri@resnulli.us>
> >Date: Tue, 27 Aug 2019 09:08:08 +0200
> >
> >> Okay, so if I understand correctly, on top of separate commands for
> >> add/del of alternative names, you suggest also get/dump to be separate
> >> command and don't fill this up in existing newling/getlink command.
> >
> >I'm not sure what to do yet.
> >
> >David has a point, because the only way these ifnames are useful is
> >as ways to specify and choose net devices. So based upon that I'm
> >slightly learning towards not using separate commands.
>
> Well yeah, one can use it to handle existing commands instead of
> IFLA_NAME.
>
> But why does it rule out separate commands? I think it is cleaner than
> to put everything in poor setlink messages :/ The fact that we would
> need to add "OP" to the setlink message just feels of. Other similar
> needs may show up in the future and we may endup in ridiculous messages
> like:
>
> SETLINK
> IFLA_NAME eth0
> IFLA_ATLNAME_LIST (nest)
> IFLA_ALTNAME_OP add
> IFLA_ALTNAME somereallylongname
> IFLA_ALTNAME_OP del
> IFLA_ALTNAME somereallyreallylongname
> IFLA_ALTNAME_OP add
> IFLA_ALTNAME someotherreallylongname
> IFLA_SOMETHING_ELSE_LIST (nest)
> IFLA_SOMETHING_ELSE_OP add
> ...
> IFLA_SOMETHING_ELSE_OP del
> ...
> IFLA_SOMETHING_ELSE_OP add
> ...
>
> I don't know what to think about it. Rollbacks are going to be pure hell :/
I don't see a huge problem with the above. We need a way to solve this
anyways for other list types in the future correct ?.
The approach taken by this series will not scale if we have to add a
new msg type and header for every such list attribute in the future.
A good parallel here is bridge vlan which uses RTM_SETLINK and
RTM_DELLINK for vlan add and deletes. But it does have an advantage of
a separate
msg space under AF_BRIDGE which makes it cleaner. Maybe something
closer to that can be made to work (possibly with a msg flag) ?.
Would be good to have a consistent way to update list attributes for
future needs too.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-27 15:14 ` Roopa Prabhu
@ 2019-08-28 7:07 ` Jiri Pirko
2019-08-29 4:36 ` Roopa Prabhu
0 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-08-28 7:07 UTC (permalink / raw)
To: Roopa Prabhu
Cc: David Miller, Jakub Kicinski, David Ahern, netdev,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Tue, Aug 27, 2019 at 05:14:49PM CEST, roopa@cumulusnetworks.com wrote:
>On Tue, Aug 27, 2019 at 2:35 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Tue, Aug 27, 2019 at 10:22:42AM CEST, davem@davemloft.net wrote:
>> >From: Jiri Pirko <jiri@resnulli.us>
>> >Date: Tue, 27 Aug 2019 09:08:08 +0200
>> >
>> >> Okay, so if I understand correctly, on top of separate commands for
>> >> add/del of alternative names, you suggest also get/dump to be separate
>> >> command and don't fill this up in existing newling/getlink command.
>> >
>> >I'm not sure what to do yet.
>> >
>> >David has a point, because the only way these ifnames are useful is
>> >as ways to specify and choose net devices. So based upon that I'm
>> >slightly learning towards not using separate commands.
>>
>> Well yeah, one can use it to handle existing commands instead of
>> IFLA_NAME.
>>
>> But why does it rule out separate commands? I think it is cleaner than
>> to put everything in poor setlink messages :/ The fact that we would
>> need to add "OP" to the setlink message just feels of. Other similar
>> needs may show up in the future and we may endup in ridiculous messages
>> like:
>>
>> SETLINK
>> IFLA_NAME eth0
>> IFLA_ATLNAME_LIST (nest)
>> IFLA_ALTNAME_OP add
>> IFLA_ALTNAME somereallylongname
>> IFLA_ALTNAME_OP del
>> IFLA_ALTNAME somereallyreallylongname
>> IFLA_ALTNAME_OP add
>> IFLA_ALTNAME someotherreallylongname
>> IFLA_SOMETHING_ELSE_LIST (nest)
>> IFLA_SOMETHING_ELSE_OP add
>> ...
>> IFLA_SOMETHING_ELSE_OP del
>> ...
>> IFLA_SOMETHING_ELSE_OP add
>> ...
>>
>> I don't know what to think about it. Rollbacks are going to be pure hell :/
>
>I don't see a huge problem with the above. We need a way to solve this
>anyways for other list types in the future correct ?.
>The approach taken by this series will not scale if we have to add a
>new msg type and header for every such list attribute in the future.
Do you have some other examples in mind? So far, this was not needed.
>
>A good parallel here is bridge vlan which uses RTM_SETLINK and
>RTM_DELLINK for vlan add and deletes. But it does have an advantage of
>a separate
>msg space under AF_BRIDGE which makes it cleaner. Maybe something
>closer to that can be made to work (possibly with a msg flag) ?.
1) Not sure if AF_BRIDGE is the right example how to do things
2) See br_vlan_info(). It is not an OP-PER-VLAN. You either add or
delete all passed info, depending on the cmd (RTM_SETLINK/RTM_DETLINK).
>
>Would be good to have a consistent way to update list attributes for
>future needs too.
Okay. Do you suggest to have new set of commands to handle
adding/deleting lists of items? altNames now, others (other nests) later?
Something like:
CMD SETLISTS
IFLA_NAME eth0
IFLA_ATLNAME_LIST (nest)
IFLA_ALTNAME somereallylongname
IFLA_ALTNAME somereallyreallylongname
IFLA_ALTNAME someotherreallylongname
IFLA_SOMETHING_ELSE_LIST (nest)
IFLA_SOMETHING_ELSE
IFLA_SOMETHING_ELSE
IFLA_SOMETHING_ELSE
CMD DELLISTS
IFLA_NAME eth0
IFLA_ATLNAME_LIST (nest)
IFLA_ALTNAME somereallylongname
IFLA_ALTNAME somereallyreallylongname
IFLA_ALTNAME someotherreallylongname
IFLA_SOMETHING_ELSE_LIST (nest)
IFLA_SOMETHING_ELSE
IFLA_SOMETHING_ELSE
IFLA_SOMETHING_ELSE
How does this sound?
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-28 7:07 ` Jiri Pirko
@ 2019-08-29 4:36 ` Roopa Prabhu
2019-08-29 5:26 ` Michal Kubecek
0 siblings, 1 reply; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-29 4:36 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, Jakub Kicinski, David Ahern, netdev,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Wed, Aug 28, 2019 at 12:07 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, Aug 27, 2019 at 05:14:49PM CEST, roopa@cumulusnetworks.com wrote:
> >On Tue, Aug 27, 2019 at 2:35 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> Tue, Aug 27, 2019 at 10:22:42AM CEST, davem@davemloft.net wrote:
> >> >From: Jiri Pirko <jiri@resnulli.us>
> >> >Date: Tue, 27 Aug 2019 09:08:08 +0200
> >> >
> >> >> Okay, so if I understand correctly, on top of separate commands for
> >> >> add/del of alternative names, you suggest also get/dump to be separate
> >> >> command and don't fill this up in existing newling/getlink command.
> >> >
> >> >I'm not sure what to do yet.
> >> >
> >> >David has a point, because the only way these ifnames are useful is
> >> >as ways to specify and choose net devices. So based upon that I'm
> >> >slightly learning towards not using separate commands.
> >>
> >> Well yeah, one can use it to handle existing commands instead of
> >> IFLA_NAME.
> >>
> >> But why does it rule out separate commands? I think it is cleaner than
> >> to put everything in poor setlink messages :/ The fact that we would
> >> need to add "OP" to the setlink message just feels of. Other similar
> >> needs may show up in the future and we may endup in ridiculous messages
> >> like:
> >>
> >> SETLINK
> >> IFLA_NAME eth0
> >> IFLA_ATLNAME_LIST (nest)
> >> IFLA_ALTNAME_OP add
> >> IFLA_ALTNAME somereallylongname
> >> IFLA_ALTNAME_OP del
> >> IFLA_ALTNAME somereallyreallylongname
> >> IFLA_ALTNAME_OP add
> >> IFLA_ALTNAME someotherreallylongname
> >> IFLA_SOMETHING_ELSE_LIST (nest)
> >> IFLA_SOMETHING_ELSE_OP add
> >> ...
> >> IFLA_SOMETHING_ELSE_OP del
> >> ...
> >> IFLA_SOMETHING_ELSE_OP add
> >> ...
> >>
> >> I don't know what to think about it. Rollbacks are going to be pure hell :/
> >
> >I don't see a huge problem with the above. We need a way to solve this
> >anyways for other list types in the future correct ?.
> >The approach taken by this series will not scale if we have to add a
> >new msg type and header for every such list attribute in the future.
>
> Do you have some other examples in mind? So far, this was not needed.
yes, so far it was not needed.
No other future possible examples in mind...but I wont be surprised if
we see such cases in the future.
Having a consistent API to extend a list attribute will help.
>
>
> >
> >A good parallel here is bridge vlan which uses RTM_SETLINK and
> >RTM_DELLINK for vlan add and deletes. But it does have an advantage of
> >a separate
> >msg space under AF_BRIDGE which makes it cleaner. Maybe something
> >closer to that can be made to work (possibly with a msg flag) ?.
>
> 1) Not sure if AF_BRIDGE is the right example how to do things
> 2) See br_vlan_info(). It is not an OP-PER-VLAN. You either add or
> delete all passed info, depending on the cmd (RTM_SETLINK/RTM_DETLINK).
yes, correct. I mentioned that because I was wondering if we can
think along the same lines for this API.
eg
(a) RTM_NEWLINK always replaces the list attribute
(b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
(c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
(It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
case. I have not looked at the full dellink path if it will work
neatly..its been a busy day )
>
>
> >
> >Would be good to have a consistent way to update list attributes for
> >future needs too.
>
> Okay. Do you suggest to have new set of commands to handle
> adding/deleting lists of items? altNames now, others (other nests) later?
>
> Something like:
>
> CMD SETLISTS
> IFLA_NAME eth0
> IFLA_ATLNAME_LIST (nest)
> IFLA_ALTNAME somereallylongname
> IFLA_ALTNAME somereallyreallylongname
> IFLA_ALTNAME someotherreallylongname
> IFLA_SOMETHING_ELSE_LIST (nest)
> IFLA_SOMETHING_ELSE
> IFLA_SOMETHING_ELSE
> IFLA_SOMETHING_ELSE
>
>
> CMD DELLISTS
> IFLA_NAME eth0
> IFLA_ATLNAME_LIST (nest)
> IFLA_ALTNAME somereallylongname
> IFLA_ALTNAME somereallyreallylongname
> IFLA_ALTNAME someotherreallylongname
> IFLA_SOMETHING_ELSE_LIST (nest)
> IFLA_SOMETHING_ELSE
> IFLA_SOMETHING_ELSE
> IFLA_SOMETHING_ELSE
>
> How does this sound?
This seems fine but it does introduce a new type. If we can avoid a
new msg type with a flag that would be nice (like the NLM_F_APPEND eg
above).
The reason for that is to see if we can use it else where too (eg some
random future list attribute in the route subsystem. If a flag works
then we don't have to add a RTM_NEWROUTE variant of CMD SETLISTS and
CMD DELLISTS)
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-29 4:36 ` Roopa Prabhu
@ 2019-08-29 5:26 ` Michal Kubecek
2019-08-30 14:35 ` Roopa Prabhu
0 siblings, 1 reply; 76+ messages in thread
From: Michal Kubecek @ 2019-08-29 5:26 UTC (permalink / raw)
To: netdev
Cc: Roopa Prabhu, Jiri Pirko, David Miller, Jakub Kicinski,
David Ahern, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
>
> yes, correct. I mentioned that because I was wondering if we can
> think along the same lines for this API.
> eg
> (a) RTM_NEWLINK always replaces the list attribute
> (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
> (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
>
> (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
> case. I have not looked at the full dellink path if it will work
> neatly..its been a busy day )
AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
current code would ignore any future attribute in RTM_DELLINK message
(any kernel before the strict validation was introduced definitely will)
and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
unless I missed something, such message would result in deleting the
network device (if possible) with any kernel not implementing the
feature.
Michal
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-29 5:26 ` Michal Kubecek
@ 2019-08-30 14:35 ` Roopa Prabhu
2019-08-30 14:47 ` David Ahern
` (2 more replies)
0 siblings, 3 replies; 76+ messages in thread
From: Roopa Prabhu @ 2019-08-30 14:35 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, Jiri Pirko, David Miller, Jakub Kicinski, David Ahern,
Stephen Hemminger, dcbw, Andrew Lunn, parav, Saeed Mahameed,
mlxsw
On Wed, Aug 28, 2019 at 10:26 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
> >
> > yes, correct. I mentioned that because I was wondering if we can
> > think along the same lines for this API.
> > eg
> > (a) RTM_NEWLINK always replaces the list attribute
> > (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
> > (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
> >
> > (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
> > case. I have not looked at the full dellink path if it will work
> > neatly..its been a busy day )
>
> AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
> current code would ignore any future attribute in RTM_DELLINK message
> (any kernel before the strict validation was introduced definitely will)
> and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
> unless I missed something, such message would result in deleting the
> network device (if possible) with any kernel not implementing the
> feature.
ok, ack. yes today it does. I was hinting if that can be changed to
support list update with a flag like the RTM_DELLINK AF_BRIDGE does
for vlan list del.
so to summarize, i think we have discussed the following options to
update a netlink list attribute so far:
(a) encode an optional attribute/flag in the list attribute in
RTM_SETLINK to indicate if it is a add or del
(b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
(close to bridge vlan add/del)
(c) introduce a separate generic msg type to add/del to a list
attribute (IIUC this does need a separate msg type per subsystem or
netlink API)
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-30 14:35 ` Roopa Prabhu
@ 2019-08-30 14:47 ` David Ahern
2019-08-30 17:04 ` Jiri Pirko
2019-08-30 14:49 ` Michal Kubecek
2019-08-30 17:03 ` Jiri Pirko
2 siblings, 1 reply; 76+ messages in thread
From: David Ahern @ 2019-08-30 14:47 UTC (permalink / raw)
To: Roopa Prabhu, Michal Kubecek
Cc: netdev, Jiri Pirko, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Andrew Lunn, parav, Saeed Mahameed,
mlxsw
On 8/30/19 8:35 AM, Roopa Prabhu wrote:
> On Wed, Aug 28, 2019 at 10:26 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>>
>> On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
>>>
>>> yes, correct. I mentioned that because I was wondering if we can
>>> think along the same lines for this API.
>>> eg
>>> (a) RTM_NEWLINK always replaces the list attribute
>>> (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
>>> (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
>>>
>>> (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
>>> case. I have not looked at the full dellink path if it will work
>>> neatly..its been a busy day )
>>
>> AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
>> current code would ignore any future attribute in RTM_DELLINK message
>> (any kernel before the strict validation was introduced definitely will)
>> and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
>> unless I missed something, such message would result in deleting the
>> network device (if possible) with any kernel not implementing the
>> feature.
>
> ok, ack. yes today it does. I was hinting if that can be changed to
> support list update with a flag like the RTM_DELLINK AF_BRIDGE does
> for vlan list del.
>
> so to summarize, i think we have discussed the following options to
> update a netlink list attribute so far:
> (a) encode an optional attribute/flag in the list attribute in
> RTM_SETLINK to indicate if it is a add or del
The ALT_IFNAME attribute could also be a struct that has both the string
and a flag.
> (b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
> (close to bridge vlan add/del)
> (c) introduce a separate generic msg type to add/del to a list
> attribute (IIUC this does need a separate msg type per subsystem or
> netlink API)
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-30 14:47 ` David Ahern
@ 2019-08-30 17:04 ` Jiri Pirko
0 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-08-30 17:04 UTC (permalink / raw)
To: David Ahern
Cc: Roopa Prabhu, Michal Kubecek, netdev, David Miller,
Jakub Kicinski, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Fri, Aug 30, 2019 at 04:47:41PM CEST, dsahern@gmail.com wrote:
>On 8/30/19 8:35 AM, Roopa Prabhu wrote:
>> On Wed, Aug 28, 2019 at 10:26 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>>>
>>> On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
>>>>
>>>> yes, correct. I mentioned that because I was wondering if we can
>>>> think along the same lines for this API.
>>>> eg
>>>> (a) RTM_NEWLINK always replaces the list attribute
>>>> (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
>>>> (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
>>>>
>>>> (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
>>>> case. I have not looked at the full dellink path if it will work
>>>> neatly..its been a busy day )
>>>
>>> AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
>>> current code would ignore any future attribute in RTM_DELLINK message
>>> (any kernel before the strict validation was introduced definitely will)
>>> and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
>>> unless I missed something, such message would result in deleting the
>>> network device (if possible) with any kernel not implementing the
>>> feature.
>>
>> ok, ack. yes today it does. I was hinting if that can be changed to
>> support list update with a flag like the RTM_DELLINK AF_BRIDGE does
>> for vlan list del.
>>
>> so to summarize, i think we have discussed the following options to
>> update a netlink list attribute so far:
>> (a) encode an optional attribute/flag in the list attribute in
>> RTM_SETLINK to indicate if it is a add or del
>
>The ALT_IFNAME attribute could also be a struct that has both the string
>and a flag.
Not a struct, please :/
>
>> (b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
>> (close to bridge vlan add/del)
>> (c) introduce a separate generic msg type to add/del to a list
>> attribute (IIUC this does need a separate msg type per subsystem or
>> netlink API)
>>
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-30 14:35 ` Roopa Prabhu
2019-08-30 14:47 ` David Ahern
@ 2019-08-30 14:49 ` Michal Kubecek
2019-08-30 17:03 ` Jiri Pirko
2 siblings, 0 replies; 76+ messages in thread
From: Michal Kubecek @ 2019-08-30 14:49 UTC (permalink / raw)
To: netdev
Cc: Roopa Prabhu, Jiri Pirko, David Miller, Jakub Kicinski,
David Ahern, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Fri, Aug 30, 2019 at 07:35:23AM -0700, Roopa Prabhu wrote:
> On Wed, Aug 28, 2019 at 10:26 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
> > >
> > > yes, correct. I mentioned that because I was wondering if we can
> > > think along the same lines for this API.
> > > eg
> > > (a) RTM_NEWLINK always replaces the list attribute
> > > (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
> > > (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
> > >
> > > (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
> > > case. I have not looked at the full dellink path if it will work
> > > neatly..its been a busy day )
> >
> > AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
> > current code would ignore any future attribute in RTM_DELLINK message
> > (any kernel before the strict validation was introduced definitely will)
> > and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
> > unless I missed something, such message would result in deleting the
> > network device (if possible) with any kernel not implementing the
> > feature.
>
> ok, ack. yes today it does. I was hinting if that can be changed to
> support list update with a flag like the RTM_DELLINK AF_BRIDGE does
> for vlan list del.
What I wanted to say is that it would have to be done in a way that
would make current kernel (or even older, e.g. one with no strict
attribute checking at all) reject or at least ignore such request,
not delete the device.
Michal
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-30 14:35 ` Roopa Prabhu
2019-08-30 14:47 ` David Ahern
2019-08-30 14:49 ` Michal Kubecek
@ 2019-08-30 17:03 ` Jiri Pirko
2019-09-12 11:59 ` Jiri Pirko
2 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-08-30 17:03 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Michal Kubecek, netdev, David Miller, Jakub Kicinski,
David Ahern, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Fri, Aug 30, 2019 at 04:35:23PM CEST, roopa@cumulusnetworks.com wrote:
>On Wed, Aug 28, 2019 at 10:26 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>>
>> On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
>> >
>> > yes, correct. I mentioned that because I was wondering if we can
>> > think along the same lines for this API.
>> > eg
>> > (a) RTM_NEWLINK always replaces the list attribute
>> > (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
>> > (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
>> >
>> > (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
>> > case. I have not looked at the full dellink path if it will work
>> > neatly..its been a busy day )
>>
>> AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
>> current code would ignore any future attribute in RTM_DELLINK message
>> (any kernel before the strict validation was introduced definitely will)
>> and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
>> unless I missed something, such message would result in deleting the
>> network device (if possible) with any kernel not implementing the
>> feature.
>
>ok, ack. yes today it does. I was hinting if that can be changed to
>support list update with a flag like the RTM_DELLINK AF_BRIDGE does
>for vlan list del.
>
>so to summarize, i think we have discussed the following options to
>update a netlink list attribute so far:
>(a) encode an optional attribute/flag in the list attribute in
>RTM_SETLINK to indicate if it is a add or del
>(b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
>(close to bridge vlan add/del)
Nope, bridge vlan add/del is done according to the cmd, not any flag.
>(c) introduce a separate generic msg type to add/del to a list
>attribute (IIUC this does need a separate msg type per subsystem or
>netlink API)
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-30 17:03 ` Jiri Pirko
@ 2019-09-12 11:59 ` Jiri Pirko
2019-09-13 8:21 ` Jiri Pirko
2019-09-13 14:50 ` Jiri Pirko
0 siblings, 2 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-09-12 11:59 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Michal Kubecek, netdev, David Miller, Jakub Kicinski,
David Ahern, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Fri, Aug 30, 2019 at 07:03:42PM CEST, jiri@resnulli.us wrote:
>Fri, Aug 30, 2019 at 04:35:23PM CEST, roopa@cumulusnetworks.com wrote:
[...]
>>
>>so to summarize, i think we have discussed the following options to
>>update a netlink list attribute so far:
>>(a) encode an optional attribute/flag in the list attribute in
>>RTM_SETLINK to indicate if it is a add or del
>>(b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
>>(close to bridge vlan add/del)
>
>Nope, bridge vlan add/del is done according to the cmd, not any flag.
>
>
>>(c) introduce a separate generic msg type to add/del to a list
>>attribute (IIUC this does need a separate msg type per subsystem or
>>netlink API)
Getting back to this, sorry.
Thinking about it for some time, a,b,c have all their issues. Why can't
we have another separate cmd as I originally proposed in this RFC? Does
anyone have any argument against it? Could you please describe?
Because otherwise, I don't feel comfortable going to any of a,b,c :(
Thanks!
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-09-12 11:59 ` Jiri Pirko
@ 2019-09-13 8:21 ` Jiri Pirko
2019-09-13 14:50 ` Jiri Pirko
1 sibling, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-09-13 8:21 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Michal Kubecek, netdev, David Miller, Jakub Kicinski,
David Ahern, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Thu, Sep 12, 2019 at 01:59:42PM CEST, jiri@resnulli.us wrote:
>Fri, Aug 30, 2019 at 07:03:42PM CEST, jiri@resnulli.us wrote:
>>Fri, Aug 30, 2019 at 04:35:23PM CEST, roopa@cumulusnetworks.com wrote:
>
>[...]
>
>>>
>>>so to summarize, i think we have discussed the following options to
>>>update a netlink list attribute so far:
>>>(a) encode an optional attribute/flag in the list attribute in
>>>RTM_SETLINK to indicate if it is a add or del
If we do this, how do you imagine this is going to be used from cmdline?
ip link set ? How exactly?
Thanks!
>>>(b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
>>>(close to bridge vlan add/del)
>>
>>Nope, bridge vlan add/del is done according to the cmd, not any flag.
>>
>>
>>>(c) introduce a separate generic msg type to add/del to a list
>>>attribute (IIUC this does need a separate msg type per subsystem or
>>>netlink API)
>
>Getting back to this, sorry.
>
>Thinking about it for some time, a,b,c have all their issues. Why can't
>we have another separate cmd as I originally proposed in this RFC? Does
>anyone have any argument against it? Could you please describe?
>
>Because otherwise, I don't feel comfortable going to any of a,b,c :(
>
>Thanks!
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-09-12 11:59 ` Jiri Pirko
2019-09-13 8:21 ` Jiri Pirko
@ 2019-09-13 14:50 ` Jiri Pirko
1 sibling, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-09-13 14:50 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Michal Kubecek, netdev, David Miller, Jakub Kicinski,
David Ahern, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Thu, Sep 12, 2019 at 01:59:42PM CEST, jiri@resnulli.us wrote:
>Fri, Aug 30, 2019 at 07:03:42PM CEST, jiri@resnulli.us wrote:
>>Fri, Aug 30, 2019 at 04:35:23PM CEST, roopa@cumulusnetworks.com wrote:
>
>[...]
>
>>>
>>>so to summarize, i think we have discussed the following options to
>>>update a netlink list attribute so far:
>>>(a) encode an optional attribute/flag in the list attribute in
>>>RTM_SETLINK to indicate if it is a add or del
>>>(b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
>>>(close to bridge vlan add/del)
>>
>>Nope, bridge vlan add/del is done according to the cmd, not any flag.
>>
>>
>>>(c) introduce a separate generic msg type to add/del to a list
>>>attribute (IIUC this does need a separate msg type per subsystem or
>>>netlink API)
>
>Getting back to this, sorry.
>
>Thinking about it for some time, a,b,c have all their issues. Why can't
>we have another separate cmd as I originally proposed in this RFC? Does
>anyone have any argument against it? Could you please describe?
>
>Because otherwise, I don't feel comfortable going to any of a,b,c :(
How about this:
We have new commands, but we have them for lists of many types. In my
examples, I'm using "altname" and "color". This is very similar to
bridge vlan example Roopa pointed out. It scales and does not pollute
existing setlink/getlink messages. Also, the cmdline is aligned:
$ ip link property add eth0 altname someverylongname altname someotherlongname
$ ip link property add eth0 altname someotherveryverylongname color blue
$ ip link property del eth0 altname someverylongname color blue
$ ip link property add eth0 color red
$ ip link property show eth0
2: eth0: altname someotherlongname altname someotherveryverylongname color red
$ ip -j -p link property show eth0
[ {
"ifindex": 2,
"ifname": "eth0",
"property": [ "altname": "someotherlongname",
"altname": "someotherveryverylongname",
"color": "red"]
} ]
I call this "property" but I'm open to any other naming.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-26 21:46 ` David Ahern
2019-08-26 22:15 ` Jakub Kicinski
@ 2019-08-27 4:55 ` Michal Kubecek
2019-08-27 13:43 ` David Ahern
1 sibling, 1 reply; 76+ messages in thread
From: Michal Kubecek @ 2019-08-27 4:55 UTC (permalink / raw)
To: netdev
Cc: David Ahern, Jakub Kicinski, Jiri Pirko, Roopa Prabhu,
David Miller, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
On Mon, Aug 26, 2019 at 03:46:43PM -0600, David Ahern wrote:
> On 8/26/19 10:55 AM, Jakub Kicinski wrote:
> > On Mon, 26 Aug 2019 18:09:16 +0200, Jiri Pirko wrote:
> >> DaveA, Roopa. Do you insist on doing add/remove of altnames in the
> >> existing setlist command using embedded message op attrs? I'm asking
> >> because after some time thinking about it, it still feels wrong to me :/
> >>
> >> If this would be a generic netlink api, we would just add another couple
> >> of commands. What is so different we can't add commands here?
> >> It is also much simpler code. Easy error handling, no need for
> >> rollback, no possibly inconsistent state, etc.
> >
> > +1 the separate op feels like a better uapi to me as well.
> >
> > Perhaps we could redo the iproute2 command line interface to make the
> > name the primary object? Would that address your concern Dave and Roopa?
> >
>
> No, my point is exactly that a name is not a primary object. A name is
> an attribute of a link - something that exists for the convenience of
> userspace only. (Like the 'protocol' for routes, rules and neighbors.)
>
> Currently, names are changed by RTM_NEWLINK/RTM_SETLINK. Aliases are
> added and deleted by RTM_NEWLINK/RTM_SETLINK. Why is an alternative name
> so special that it should have its own API?
There is only one alias so that it makes perfect sense to set it like
any other attribute. But the series introduces a list of alternative
names. So IMHO better analogy would be network addresses - and we do
have RTM_NEWADDR/RTM_DELADDR for them.
> If only 1 alt name was allowed, then RTM_NEWLINK/RTM_SETLINK would
> suffice. Management of it would have the same semantics as an alias -
> empty string means delete, non-empty string sets the value.
>
> So really the push for new RTM commands is to handle an unlimited number
> of alt names with the ability to change / delete any one of them. Has
> the need for multiple alternate ifnames been fully established? (I don't
> recall other than a discussion about parallels to block devices.)
I still believe the biggest problem with so-called "predictable names"
for network devices (which turn out to be not predictable and often not
even persistent) comes from the fact that unlike for block (or
character) devices, we don't have anything like symlink for network
device. This forces udev to choose only one naming scheme with all the
unpleasant consequences. So I see this series as an opportunity to get
rid of this limitation.
Michal
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-27 4:55 ` Michal Kubecek
@ 2019-08-27 13:43 ` David Ahern
0 siblings, 0 replies; 76+ messages in thread
From: David Ahern @ 2019-08-27 13:43 UTC (permalink / raw)
To: Michal Kubecek, netdev
Cc: Jakub Kicinski, Jiri Pirko, Roopa Prabhu, David Miller,
Stephen Hemminger, dcbw, Andrew Lunn, parav, Saeed Mahameed,
mlxsw
On 8/26/19 10:55 PM, Michal Kubecek wrote:
> On Mon, Aug 26, 2019 at 03:46:43PM -0600, David Ahern wrote:
>> On 8/26/19 10:55 AM, Jakub Kicinski wrote:
>>> On Mon, 26 Aug 2019 18:09:16 +0200, Jiri Pirko wrote:
>>>> DaveA, Roopa. Do you insist on doing add/remove of altnames in the
>>>> existing setlist command using embedded message op attrs? I'm asking
>>>> because after some time thinking about it, it still feels wrong to me :/
>>>>
>>>> If this would be a generic netlink api, we would just add another couple
>>>> of commands. What is so different we can't add commands here?
>>>> It is also much simpler code. Easy error handling, no need for
>>>> rollback, no possibly inconsistent state, etc.
>>>
>>> +1 the separate op feels like a better uapi to me as well.
>>>
>>> Perhaps we could redo the iproute2 command line interface to make the
>>> name the primary object? Would that address your concern Dave and Roopa?
>>>
>>
>> No, my point is exactly that a name is not a primary object. A name is
>> an attribute of a link - something that exists for the convenience of
>> userspace only. (Like the 'protocol' for routes, rules and neighbors.)
>>
>> Currently, names are changed by RTM_NEWLINK/RTM_SETLINK. Aliases are
>> added and deleted by RTM_NEWLINK/RTM_SETLINK. Why is an alternative name
>> so special that it should have its own API?
>
> There is only one alias so that it makes perfect sense to set it like
> any other attribute. But the series introduces a list of alternative
> names. So IMHO better analogy would be network addresses - and we do
> have RTM_NEWADDR/RTM_DELADDR for them.
RTM_*ADDR manage network layer addresses. Those are anchored to a device
but not direct attributes describing the device.
The device names are just alternative (human friendly) references to a
specific device hence they should be direct link attributes.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
2019-08-09 15:40 ` Roopa Prabhu
2019-08-09 15:46 ` Michal Kubecek
2019-08-09 16:14 ` David Ahern
@ 2019-08-10 6:32 ` Jiri Pirko
2 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-08-10 6:32 UTC (permalink / raw)
To: Roopa Prabhu
Cc: netdev, David Miller, Jakub Kicinski, Stephen Hemminger,
David Ahern, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
Fri, Aug 09, 2019 at 05:40:25PM CEST, roopa@cumulusnetworks.com wrote:
>On Thu, Aug 8, 2019 at 11:25 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Fri, Aug 09, 2019 at 06:11:30AM CEST, roopa@cumulusnetworks.com wrote:
>> >On Fri, Jul 19, 2019 at 4:00 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >>
>> >> From: Jiri Pirko <jiri@mellanox.com>
>> >>
>> >> Add two commands to add and delete alternative ifnames for net device.
>> >> Each net device can have multiple alternative names.
>> >>
>> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> >> ---
>> >> include/linux/netdevice.h | 4 ++
>> >> include/uapi/linux/if.h | 1 +
>> >> include/uapi/linux/if_link.h | 1 +
>> >> include/uapi/linux/rtnetlink.h | 7 +++
>> >> net/core/dev.c | 58 ++++++++++++++++++-
>> >> net/core/rtnetlink.c | 102 +++++++++++++++++++++++++++++++++
>> >> security/selinux/nlmsgtab.c | 4 +-
>> >> 7 files changed, 175 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> >> index 74f99f127b0e..6922fdb483ca 100644
>> >> --- a/include/linux/netdevice.h
>> >> +++ b/include/linux/netdevice.h
>> >> @@ -920,10 +920,14 @@ struct tlsdev_ops;
>> >>
>> >> struct netdev_name_node {
>> >> struct hlist_node hlist;
>> >> + struct list_head list;
>> >> struct net_device *dev;
>> >> char *name;
>> >> };
>> >>
>> >> +int netdev_name_node_alt_create(struct net_device *dev, char *name);
>> >> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name);
>> >> +
>> >> /*
>> >> * This structure defines the management hooks for network devices.
>> >> * The following hooks can be defined; unless noted otherwise, they are
>> >> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
>> >> index 7fea0fd7d6f5..4bf33344aab1 100644
>> >> --- a/include/uapi/linux/if.h
>> >> +++ b/include/uapi/linux/if.h
>> >> @@ -33,6 +33,7 @@
>> >> #define IFNAMSIZ 16
>> >> #endif /* __UAPI_DEF_IF_IFNAMSIZ */
>> >> #define IFALIASZ 256
>> >> +#define ALTIFNAMSIZ 128
>> >> #include <linux/hdlc/ioctl.h>
>> >>
>> >> /* For glibc compatibility. An empty enum does not compile. */
>> >> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> >> index 4a8c02cafa9a..92268946e04a 100644
>> >> --- a/include/uapi/linux/if_link.h
>> >> +++ b/include/uapi/linux/if_link.h
>> >> @@ -167,6 +167,7 @@ enum {
>> >> IFLA_NEW_IFINDEX,
>> >> IFLA_MIN_MTU,
>> >> IFLA_MAX_MTU,
>> >> + IFLA_ALT_IFNAME_MOD, /* Alternative ifname to add/delete */
>> >> __IFLA_MAX
>> >> };
>> >>
>> >> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>> >> index ce2a623abb75..b36cfd83eb76 100644
>> >> --- a/include/uapi/linux/rtnetlink.h
>> >> +++ b/include/uapi/linux/rtnetlink.h
>> >> @@ -164,6 +164,13 @@ enum {
>> >> RTM_GETNEXTHOP,
>> >> #define RTM_GETNEXTHOP RTM_GETNEXTHOP
>> >>
>> >> + RTM_NEWALTIFNAME = 108,
>> >> +#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
>> >> + RTM_DELALTIFNAME,
>> >> +#define RTM_DELALTIFNAME RTM_DELALTIFNAME
>> >> + RTM_GETALTIFNAME,
>> >> +#define RTM_GETALTIFNAME RTM_GETALTIFNAME
>> >> +
>> >
>> >I might have missed the prior discussion, why do we need new commands
>> >?. can't this simply be part of RTM_*LINK and we use RTM_SETLINK to
>> >set alternate names ?
>>
>> How? This is to add/remove. How do you suggest to to add/remove by
>> setlink?
>
>to that point, I am also not sure why we have a new API For multiple
>names. I mean why support more than two names (existing old name and
Please see the previous discussion in the rfc. The point is, udev can
provide multiple names according to multiple naming scheme (mac,
pciaddr, etc).
>a new name to remove the length limitation) ?
>
>Your patch series addresses a very important problem (we run into this
>limitation all the time and its hard to explain it to network
>operators) and
> its already unfortunate that we have to have more than one name
>because we cannot resize the existing one.
>
>The best we can do for simpler transition/management from user-space
>is to keep the api simple..
>ie keep it close to the management of existing link attributes. Hence
>the question.
>
>I assumed this would be like alias. A single new field that can be
>referenced in lieu of the old one.
>
>Your series is very useful to many of us...but when i think about
>changing our network manager to accommodate this, I am worried about
>how many apps will have to change.
>I agree they have to change regardless but now they will have to
>listen to yet another notification and msg format for names ?
>
>(apologies for joining the thread late and if i missed prior discussion on this)
>
>
>>
>>
>> >
>> >
>> >
>> >> __RTM_MAX,
>> >> #define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
>> >> };
>> >> diff --git a/net/core/dev.c b/net/core/dev.c
>> >> index ad0d42fbdeee..2a3be2b279d3 100644
>> >> --- a/net/core/dev.c
>> >> +++ b/net/core/dev.c
>> >> @@ -244,7 +244,13 @@ static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
>> >> static struct netdev_name_node *
>> >> netdev_name_node_head_alloc(struct net_device *dev)
>> >> {
>> >> - return netdev_name_node_alloc(dev, dev->name);
>> >> + struct netdev_name_node *name_node;
>> >> +
>> >> + name_node = netdev_name_node_alloc(dev, dev->name);
>> >> + if (!name_node)
>> >> + return NULL;
>> >> + INIT_LIST_HEAD(&name_node->list);
>> >> + return name_node;
>> >> }
>> >>
>> >> static void netdev_name_node_free(struct netdev_name_node *name_node)
>> >> @@ -288,6 +294,55 @@ static struct netdev_name_node *netdev_name_node_lookup_rcu(struct net *net,
>> >> return NULL;
>> >> }
>> >>
>> >> +int netdev_name_node_alt_create(struct net_device *dev, char *name)
>> >> +{
>> >> + struct netdev_name_node *name_node;
>> >> + struct net *net = dev_net(dev);
>> >> +
>> >> + name_node = netdev_name_node_lookup(net, name);
>> >> + if (name_node)
>> >> + return -EEXIST;
>> >> + name_node = netdev_name_node_alloc(dev, name);
>> >> + if (!name_node)
>> >> + return -ENOMEM;
>> >> + netdev_name_node_add(net, name_node);
>> >> + /* The node that holds dev->name acts as a head of per-device list. */
>> >> + list_add_tail(&name_node->list, &dev->name_node->list);
>> >> +
>> >> + return 0;
>> >> +}
>> >> +EXPORT_SYMBOL(netdev_name_node_alt_create);
>> >> +
>> >> +static void __netdev_name_node_alt_destroy(struct netdev_name_node *name_node)
>> >> +{
>> >> + list_del(&name_node->list);
>> >> + netdev_name_node_del(name_node);
>> >> + kfree(name_node->name);
>> >> + netdev_name_node_free(name_node);
>> >> +}
>> >> +
>> >> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
>> >> +{
>> >> + struct netdev_name_node *name_node;
>> >> + struct net *net = dev_net(dev);
>> >> +
>> >> + name_node = netdev_name_node_lookup(net, name);
>> >> + if (!name_node)
>> >> + return -ENOENT;
>> >> + __netdev_name_node_alt_destroy(name_node);
>> >> +
>> >> + return 0;
>> >> +}
>> >> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
>> >> +
>> >> +static void netdev_name_node_alt_flush(struct net_device *dev)
>> >> +{
>> >> + struct netdev_name_node *name_node, *tmp;
>> >> +
>> >> + list_for_each_entry_safe(name_node, tmp, &dev->name_node->list, list)
>> >> + __netdev_name_node_alt_destroy(name_node);
>> >> +}
>> >> +
>> >> /* Device list insertion */
>> >> static void list_netdevice(struct net_device *dev)
>> >> {
>> >> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
>> >> dev_uc_flush(dev);
>> >> dev_mc_flush(dev);
>> >>
>> >> + netdev_name_node_alt_flush(dev);
>> >> netdev_name_node_free(dev->name_node);
>> >>
>> >> if (dev->netdev_ops->ndo_uninit)
>> >> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> >> index 1ee6460f8275..7a2010b16e10 100644
>> >> --- a/net/core/rtnetlink.c
>> >> +++ b/net/core/rtnetlink.c
>> >> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>> >> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
>> >> [IFLA_MIN_MTU] = { .type = NLA_U32 },
>> >> [IFLA_MAX_MTU] = { .type = NLA_U32 },
>> >> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
>> >> + .len = ALTIFNAMSIZ - 1 },
>> >> };
>> >>
>> >> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
>> >> @@ -3373,6 +3375,103 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>> >> return err;
>> >> }
>> >>
>> >> +static int rtnl_newaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
>> >> + struct netlink_ext_ack *extack)
>> >> +{
>> >> + struct net *net = sock_net(skb->sk);
>> >> + struct nlattr *tb[IFLA_MAX + 1];
>> >> + struct net_device *dev;
>> >> + struct ifinfomsg *ifm;
>> >> + char *new_alt_ifname;
>> >> + int err;
>> >> +
>> >> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
>> >> + if (err)
>> >> + return err;
>> >> +
>> >> + err = rtnl_ensure_unique_netns(tb, extack, true);
>> >> + if (err)
>> >> + return err;
>> >> +
>> >> + ifm = nlmsg_data(nlh);
>> >> + if (ifm->ifi_index > 0) {
>> >> + dev = __dev_get_by_index(net, ifm->ifi_index);
>> >> + } else if (tb[IFLA_IFNAME]) {
>> >> + char ifname[IFNAMSIZ];
>> >> +
>> >> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
>> >> + dev = __dev_get_by_name(net, ifname);
>> >> + } else {
>> >> + return -EINVAL;
>> >> + }
>> >> +
>> >> + if (!dev)
>> >> + return -ENODEV;
>> >> +
>> >> + if (!tb[IFLA_ALT_IFNAME_MOD])
>> >> + return -EINVAL;
>> >> +
>> >> + new_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
>> >> + if (!new_alt_ifname)
>> >> + return -ENOMEM;
>> >> +
>> >> + err = netdev_name_node_alt_create(dev, new_alt_ifname);
>> >> + if (err)
>> >> + goto out_free_new_alt_ifname;
>> >> +
>> >> + return 0;
>> >> +
>> >> +out_free_new_alt_ifname:
>> >> + kfree(new_alt_ifname);
>> >> + return err;
>> >> +}
>> >> +
>> >> +static int rtnl_delaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
>> >> + struct netlink_ext_ack *extack)
>> >> +{
>> >> + struct net *net = sock_net(skb->sk);
>> >> + struct nlattr *tb[IFLA_MAX + 1];
>> >> + struct net_device *dev;
>> >> + struct ifinfomsg *ifm;
>> >> + char *del_alt_ifname;
>> >> + int err;
>> >> +
>> >> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
>> >> + if (err)
>> >> + return err;
>> >> +
>> >> + err = rtnl_ensure_unique_netns(tb, extack, true);
>> >> + if (err)
>> >> + return err;
>> >> +
>> >> + ifm = nlmsg_data(nlh);
>> >> + if (ifm->ifi_index > 0) {
>> >> + dev = __dev_get_by_index(net, ifm->ifi_index);
>> >> + } else if (tb[IFLA_IFNAME]) {
>> >> + char ifname[IFNAMSIZ];
>> >> +
>> >> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
>> >> + dev = __dev_get_by_name(net, ifname);
>> >> + } else {
>> >> + return -EINVAL;
>> >> + }
>> >> +
>> >> + if (!dev)
>> >> + return -ENODEV;
>> >> +
>> >> + if (!tb[IFLA_ALT_IFNAME_MOD])
>> >> + return -EINVAL;
>> >> +
>> >> + del_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
>> >> + if (!del_alt_ifname)
>> >> + return -ENOMEM;
>> >> +
>> >> + err = netdev_name_node_alt_destroy(dev, del_alt_ifname);
>> >> + kfree(del_alt_ifname);
>> >> +
>> >> + return err;
>> >> +}
>> >> +
>> >> static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
>> >> {
>> >> struct net *net = sock_net(skb->sk);
>> >> @@ -5331,6 +5430,9 @@ void __init rtnetlink_init(void)
>> >> rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, 0);
>> >> rtnl_register(PF_UNSPEC, RTM_GETNETCONF, NULL, rtnl_dump_all, 0);
>> >>
>> >> + rtnl_register(PF_UNSPEC, RTM_NEWALTIFNAME, rtnl_newaltifname, NULL, 0);
>> >> + rtnl_register(PF_UNSPEC, RTM_DELALTIFNAME, rtnl_delaltifname, NULL, 0);
>> >> +
>> >> rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, 0);
>> >> rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, 0);
>> >> rtnl_register(PF_BRIDGE, RTM_GETNEIGH, rtnl_fdb_get, rtnl_fdb_dump, 0);
>> >> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
>> >> index 58345ba0528e..a712b54c666c 100644
>> >> --- a/security/selinux/nlmsgtab.c
>> >> +++ b/security/selinux/nlmsgtab.c
>> >> @@ -83,6 +83,8 @@ static const struct nlmsg_perm nlmsg_route_perms[] =
>> >> { RTM_NEWNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> >> { RTM_DELNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> >> { RTM_GETNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_READ },
>> >> + { RTM_NEWALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> >> + { RTM_DELALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> >> };
>> >>
>> >> static const struct nlmsg_perm nlmsg_tcpdiag_perms[] =
>> >> @@ -166,7 +168,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
>> >> * structures at the top of this file with the new mappings
>> >> * before updating the BUILD_BUG_ON() macro!
>> >> */
>> >> - BUILD_BUG_ON(RTM_MAX != (RTM_NEWNEXTHOP + 3));
>> >> + BUILD_BUG_ON(RTM_MAX != (RTM_NEWALTIFNAME + 3));
>> >> err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
>> >> sizeof(nlmsg_route_perms));
>> >> break;
>> >> --
>> >> 2.21.0
>> >>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [patch net-next rfc 4/7] net: rtnetlink: put alternative names to getlink message
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
` (2 preceding siblings ...)
2019-07-19 11:00 ` [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames Jiri Pirko
@ 2019-07-19 11:00 ` Jiri Pirko
2019-07-20 3:59 ` Jakub Kicinski
2019-07-19 11:00 ` [patch net-next rfc 5/7] net: rtnetlink: unify the code in __rtnl_newlink get dev with the rest Jiri Pirko
` (5 subsequent siblings)
9 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:00 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Extend exiting getlink info message with list of alternative names.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/uapi/linux/if_link.h | 2 ++
net/core/rtnetlink.c | 41 ++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 92268946e04a..038361f9847b 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -168,6 +168,8 @@ enum {
IFLA_MIN_MTU,
IFLA_MAX_MTU,
IFLA_ALT_IFNAME_MOD, /* Alternative ifname to add/delete */
+ IFLA_ALT_IFNAME_LIST, /* nest */
+ IFLA_ALT_IFNAME, /* Alternative ifname */
__IFLA_MAX
};
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 7a2010b16e10..f11a2367037d 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -980,6 +980,18 @@ static size_t rtnl_xdp_size(void)
return xdp_size;
}
+static size_t rtnl_alt_ifname_list_size(const struct net_device *dev)
+{
+ struct netdev_name_node *name_node;
+ size_t size = nla_total_size(0);
+
+ if (list_empty(&dev->name_node->list))
+ return 0;
+ list_for_each_entry(name_node, &dev->name_node->list, list)
+ size += nla_total_size(ALTIFNAMSIZ);
+ return size;
+}
+
static noinline size_t if_nlmsg_size(const struct net_device *dev,
u32 ext_filter_mask)
{
@@ -1027,6 +1039,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
+ nla_total_size(4) /* IFLA_CARRIER_DOWN_COUNT */
+ nla_total_size(4) /* IFLA_MIN_MTU */
+ nla_total_size(4) /* IFLA_MAX_MTU */
+ + rtnl_alt_ifname_list_size(dev)
+ 0;
}
@@ -1584,6 +1597,31 @@ static int rtnl_fill_link_af(struct sk_buff *skb,
return 0;
}
+static int rtnl_fill_alt_ifnames(struct sk_buff *skb,
+ const struct net_device *dev)
+{
+ struct netdev_name_node *name_node;
+ struct nlattr *alt_ifname_list;
+
+ if (list_empty(&dev->name_node->list))
+ return 0;
+
+ alt_ifname_list = nla_nest_start_noflag(skb, IFLA_ALT_IFNAME_LIST);
+ if (!alt_ifname_list)
+ return -EMSGSIZE;
+
+ list_for_each_entry(name_node, &dev->name_node->list, list)
+ if (nla_put_string(skb, IFLA_ALT_IFNAME, name_node->name))
+ goto nla_put_failure;
+
+ nla_nest_end(skb, alt_ifname_list);
+ return 0;
+
+nla_put_failure:
+ nla_nest_cancel(skb, alt_ifname_list);
+ return -EMSGSIZE;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb,
struct net_device *dev, struct net *src_net,
int type, u32 pid, u32 seq, u32 change,
@@ -1697,6 +1735,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
goto nla_put_failure_rcu;
rcu_read_unlock();
+ if (rtnl_fill_alt_ifnames(skb, dev))
+ goto nla_put_failure;
+
nlmsg_end(skb, nlh);
return 0;
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 4/7] net: rtnetlink: put alternative names to getlink message
2019-07-19 11:00 ` [patch net-next rfc 4/7] net: rtnetlink: put alternative names to getlink message Jiri Pirko
@ 2019-07-20 3:59 ` Jakub Kicinski
2019-07-20 7:17 ` Jiri Pirko
0 siblings, 1 reply; 76+ messages in thread
From: Jakub Kicinski @ 2019-07-20 3:59 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
On Fri, 19 Jul 2019 13:00:26 +0200, Jiri Pirko wrote:
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 7a2010b16e10..f11a2367037d 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -980,6 +980,18 @@ static size_t rtnl_xdp_size(void)
> return xdp_size;
> }
>
> +static size_t rtnl_alt_ifname_list_size(const struct net_device *dev)
> +{
> + struct netdev_name_node *name_node;
> + size_t size = nla_total_size(0);
> +
> + if (list_empty(&dev->name_node->list))
> + return 0;
Nit: it would make the intent a tiny bit clearer if
size = nla_total_size(0);
was after this early return.
> + list_for_each_entry(name_node, &dev->name_node->list, list)
> + size += nla_total_size(ALTIFNAMSIZ);
Since we have the structure I wonder if it would be worthwhile to store
the exact size in it?
> + return size;
> +}
> +
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 4/7] net: rtnetlink: put alternative names to getlink message
2019-07-20 3:59 ` Jakub Kicinski
@ 2019-07-20 7:17 ` Jiri Pirko
0 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-20 7:17 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
Sat, Jul 20, 2019 at 05:59:14AM CEST, jakub.kicinski@netronome.com wrote:
>On Fri, 19 Jul 2019 13:00:26 +0200, Jiri Pirko wrote:
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 7a2010b16e10..f11a2367037d 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -980,6 +980,18 @@ static size_t rtnl_xdp_size(void)
>> return xdp_size;
>> }
>>
>> +static size_t rtnl_alt_ifname_list_size(const struct net_device *dev)
>> +{
>> + struct netdev_name_node *name_node;
>> + size_t size = nla_total_size(0);
>> +
>> + if (list_empty(&dev->name_node->list))
>> + return 0;
>
>Nit: it would make the intent a tiny bit clearer if
>
> size = nla_total_size(0);
>
>was after this early return.
Sure.
>
>> + list_for_each_entry(name_node, &dev->name_node->list, list)
>> + size += nla_total_size(ALTIFNAMSIZ);
>
>Since we have the structure I wonder if it would be worthwhile to store
Which structure?
>the exact size in it?
>
>> + return size;
>> +}
>> +
^ permalink raw reply [flat|nested] 76+ messages in thread
* [patch net-next rfc 5/7] net: rtnetlink: unify the code in __rtnl_newlink get dev with the rest
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
` (3 preceding siblings ...)
2019-07-19 11:00 ` [patch net-next rfc 4/7] net: rtnetlink: put alternative names to getlink message Jiri Pirko
@ 2019-07-19 11:00 ` Jiri Pirko
2019-07-19 11:00 ` [patch net-next rfc 6/7] net: rtnetlink: introduce helper to get net_device instance by ifname Jiri Pirko
` (4 subsequent siblings)
9 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:00 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
__rtnl_newlink() code flow is a bit different around tb[IFLA_IFNAME]
processing comparing to the other places. Change that to be unified with
the rest.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/core/rtnetlink.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index f11a2367037d..8994dc858ae0 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3067,12 +3067,10 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
- else {
- if (ifname[0])
- dev = __dev_get_by_name(net, ifname);
- else
- dev = NULL;
- }
+ else if (tb[IFLA_IFNAME])
+ dev = __dev_get_by_name(net, ifname);
+ else
+ dev = NULL;
if (dev) {
master_dev = netdev_master_upper_dev_get(dev);
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* [patch net-next rfc 6/7] net: rtnetlink: introduce helper to get net_device instance by ifname
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
` (4 preceding siblings ...)
2019-07-19 11:00 ` [patch net-next rfc 5/7] net: rtnetlink: unify the code in __rtnl_newlink get dev with the rest Jiri Pirko
@ 2019-07-19 11:00 ` Jiri Pirko
2019-07-19 11:00 ` [patch net-next rfc 7/7] net: rtnetlink: add possibility to use alternative names as message handle Jiri Pirko
` (3 subsequent siblings)
9 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:00 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Introduce helper function rtnl_get_dev() that gets net_device structure
instance pointer according to passed ifname or ifname attribute.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/core/rtnetlink.c | 57 ++++++++++++++++++++++----------------------
1 file changed, 29 insertions(+), 28 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 8994dc858ae0..1fa30d514e3f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2765,6 +2765,23 @@ static int do_setlink(const struct sk_buff *skb,
return err;
}
+static struct net_device *rtnl_dev_get(struct net *net,
+ struct nlattr *ifname_attr,
+ char *ifname)
+{
+ char buffer[IFNAMSIZ];
+
+ if (!ifname) {
+ ifname = buffer;
+ if (ifname_attr)
+ nla_strlcpy(ifname, ifname_attr, IFNAMSIZ);
+ else
+ return NULL;
+ }
+
+ return __dev_get_by_name(net, ifname);
+}
+
static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
struct netlink_ext_ack *extack)
{
@@ -2794,7 +2811,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
else if (tb[IFLA_IFNAME])
- dev = __dev_get_by_name(net, ifname);
+ dev = rtnl_dev_get(net, NULL, ifname);
else
goto errout;
@@ -2867,7 +2884,6 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh,
struct net *tgt_net = net;
struct net_device *dev = NULL;
struct ifinfomsg *ifm;
- char ifname[IFNAMSIZ];
struct nlattr *tb[IFLA_MAX+1];
int err;
int netnsid = -1;
@@ -2881,9 +2897,6 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err < 0)
return err;
- if (tb[IFLA_IFNAME])
- nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
-
if (tb[IFLA_TARGET_NETNSID]) {
netnsid = nla_get_s32(tb[IFLA_TARGET_NETNSID]);
tgt_net = rtnl_get_net_ns_capable(NETLINK_CB(skb).sk, netnsid);
@@ -2896,7 +2909,7 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(tgt_net, ifm->ifi_index);
else if (tb[IFLA_IFNAME])
- dev = __dev_get_by_name(tgt_net, ifname);
+ dev = rtnl_dev_get(net, tb[IFLA_IFNAME], NULL);
else if (tb[IFLA_GROUP])
err = rtnl_group_dellink(tgt_net, nla_get_u32(tb[IFLA_GROUP]));
else
@@ -3068,7 +3081,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
else if (tb[IFLA_IFNAME])
- dev = __dev_get_by_name(net, ifname);
+ dev = rtnl_dev_get(net, NULL, ifname);
else
dev = NULL;
@@ -3350,7 +3363,6 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
struct net *net = sock_net(skb->sk);
struct net *tgt_net = net;
struct ifinfomsg *ifm;
- char ifname[IFNAMSIZ];
struct nlattr *tb[IFLA_MAX+1];
struct net_device *dev = NULL;
struct sk_buff *nskb;
@@ -3373,9 +3385,6 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
return PTR_ERR(tgt_net);
}
- if (tb[IFLA_IFNAME])
- nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
-
if (tb[IFLA_EXT_MASK])
ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
@@ -3384,7 +3393,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(tgt_net, ifm->ifi_index);
else if (tb[IFLA_IFNAME])
- dev = __dev_get_by_name(tgt_net, ifname);
+ dev = rtnl_dev_get(tgt_net, tb[IFLA_IFNAME], NULL);
else
goto out;
@@ -3433,16 +3442,12 @@ static int rtnl_newaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
return err;
ifm = nlmsg_data(nlh);
- if (ifm->ifi_index > 0) {
+ if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
- } else if (tb[IFLA_IFNAME]) {
- char ifname[IFNAMSIZ];
-
- nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
- dev = __dev_get_by_name(net, ifname);
- } else {
+ else if (tb[IFLA_IFNAME])
+ dev = rtnl_dev_get(net, tb[IFLA_IFNAME], NULL);
+ else
return -EINVAL;
- }
if (!dev)
return -ENODEV;
@@ -3484,16 +3489,12 @@ static int rtnl_delaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
return err;
ifm = nlmsg_data(nlh);
- if (ifm->ifi_index > 0) {
+ if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
- } else if (tb[IFLA_IFNAME]) {
- char ifname[IFNAMSIZ];
-
- nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
- dev = __dev_get_by_name(net, ifname);
- } else {
+ else if (tb[IFLA_IFNAME])
+ dev = rtnl_dev_get(net, tb[IFLA_IFNAME], NULL);
+ else
return -EINVAL;
- }
if (!dev)
return -ENODEV;
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* [patch net-next rfc 7/7] net: rtnetlink: add possibility to use alternative names as message handle
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
` (5 preceding siblings ...)
2019-07-19 11:00 ` [patch net-next rfc 6/7] net: rtnetlink: introduce helper to get net_device instance by ifname Jiri Pirko
@ 2019-07-19 11:00 ` Jiri Pirko
2019-07-20 3:59 ` Jakub Kicinski
2019-07-19 11:03 ` [patch iproute2 rfc 1/2] ip: add support for alternative name addition/deletion/list Jiri Pirko
` (2 subsequent siblings)
9 siblings, 1 reply; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:00 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Extend the basic rtnetlink commands to use alternative interface names
as a handle instead of ifindex and ifname.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/core/rtnetlink.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1fa30d514e3f..68ad12a7fc4d 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1793,6 +1793,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
[IFLA_MAX_MTU] = { .type = NLA_U32 },
[IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
.len = ALTIFNAMSIZ - 1 },
+ [IFLA_ALT_IFNAME] = { .type = NLA_STRING,
+ .len = ALTIFNAMSIZ - 1 },
};
static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -2767,14 +2769,17 @@ static int do_setlink(const struct sk_buff *skb,
static struct net_device *rtnl_dev_get(struct net *net,
struct nlattr *ifname_attr,
+ struct nlattr *altifname_attr,
char *ifname)
{
- char buffer[IFNAMSIZ];
+ char buffer[ALTIFNAMSIZ];
if (!ifname) {
ifname = buffer;
if (ifname_attr)
nla_strlcpy(ifname, ifname_attr, IFNAMSIZ);
+ else if (altifname_attr)
+ nla_strlcpy(ifname, altifname_attr, ALTIFNAMSIZ);
else
return NULL;
}
@@ -2810,8 +2815,8 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
- else if (tb[IFLA_IFNAME])
- dev = rtnl_dev_get(net, NULL, ifname);
+ else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
+ dev = rtnl_dev_get(net, NULL, tb[IFLA_ALT_IFNAME], ifname);
else
goto errout;
@@ -2908,8 +2913,9 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(tgt_net, ifm->ifi_index);
- else if (tb[IFLA_IFNAME])
- dev = rtnl_dev_get(net, tb[IFLA_IFNAME], NULL);
+ else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
+ dev = rtnl_dev_get(net, tb[IFLA_IFNAME],
+ tb[IFLA_ALT_IFNAME], NULL);
else if (tb[IFLA_GROUP])
err = rtnl_group_dellink(tgt_net, nla_get_u32(tb[IFLA_GROUP]));
else
@@ -3080,8 +3086,8 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
- else if (tb[IFLA_IFNAME])
- dev = rtnl_dev_get(net, NULL, ifname);
+ else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
+ dev = rtnl_dev_get(net, NULL, tb[IFLA_ALT_IFNAME], ifname);
else
dev = NULL;
@@ -3345,6 +3351,7 @@ static int rtnl_valid_getlink_req(struct sk_buff *skb,
switch (i) {
case IFLA_IFNAME:
+ case IFLA_ALT_IFNAME:
case IFLA_EXT_MASK:
case IFLA_TARGET_NETNSID:
break;
@@ -3392,8 +3399,9 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(tgt_net, ifm->ifi_index);
- else if (tb[IFLA_IFNAME])
- dev = rtnl_dev_get(tgt_net, tb[IFLA_IFNAME], NULL);
+ else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
+ dev = rtnl_dev_get(tgt_net, tb[IFLA_IFNAME],
+ tb[IFLA_ALT_IFNAME], NULL);
else
goto out;
@@ -3444,8 +3452,9 @@ static int rtnl_newaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
- else if (tb[IFLA_IFNAME])
- dev = rtnl_dev_get(net, tb[IFLA_IFNAME], NULL);
+ else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
+ dev = rtnl_dev_get(net, tb[IFLA_IFNAME],
+ tb[IFLA_ALT_IFNAME], NULL);
else
return -EINVAL;
@@ -3491,8 +3500,9 @@ static int rtnl_delaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index);
- else if (tb[IFLA_IFNAME])
- dev = rtnl_dev_get(net, tb[IFLA_IFNAME], NULL);
+ else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
+ dev = rtnl_dev_get(net, tb[IFLA_IFNAME],
+ tb[IFLA_ALT_IFNAME], NULL);
else
return -EINVAL;
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 7/7] net: rtnetlink: add possibility to use alternative names as message handle
2019-07-19 11:00 ` [patch net-next rfc 7/7] net: rtnetlink: add possibility to use alternative names as message handle Jiri Pirko
@ 2019-07-20 3:59 ` Jakub Kicinski
2019-07-20 7:22 ` Jiri Pirko
0 siblings, 1 reply; 76+ messages in thread
From: Jakub Kicinski @ 2019-07-20 3:59 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
On Fri, 19 Jul 2019 13:00:29 +0200, Jiri Pirko wrote:
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 1fa30d514e3f..68ad12a7fc4d 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1793,6 +1793,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
> [IFLA_MAX_MTU] = { .type = NLA_U32 },
> [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
> .len = ALTIFNAMSIZ - 1 },
> + [IFLA_ALT_IFNAME] = { .type = NLA_STRING,
> + .len = ALTIFNAMSIZ - 1 },
What's the disadvantage of just letting IFLA_IFNAME to get longer
on input? Is it just that the handling would be asymmetrical?
> };
>
> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 7/7] net: rtnetlink: add possibility to use alternative names as message handle
2019-07-20 3:59 ` Jakub Kicinski
@ 2019-07-20 7:22 ` Jiri Pirko
0 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-20 7:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
Sat, Jul 20, 2019 at 05:59:27AM CEST, jakub.kicinski@netronome.com wrote:
>On Fri, 19 Jul 2019 13:00:29 +0200, Jiri Pirko wrote:
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 1fa30d514e3f..68ad12a7fc4d 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -1793,6 +1793,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>> [IFLA_MAX_MTU] = { .type = NLA_U32 },
>> [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
>> .len = ALTIFNAMSIZ - 1 },
>> + [IFLA_ALT_IFNAME] = { .type = NLA_STRING,
>> + .len = ALTIFNAMSIZ - 1 },
>
>What's the disadvantage of just letting IFLA_IFNAME to get longer
>on input? Is it just that the handling would be asymmetrical?
Hmm, that might work. But the problem is that sometimes the IFLA_IFNAME
is used as a handle, but sometimes it is used to carry the ifname
(newlink, changename). That might be a bit confusing and the code would
be harder to follow. I don't know...
>
>> };
>>
>> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [patch iproute2 rfc 1/2] ip: add support for alternative name addition/deletion/list
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
` (6 preceding siblings ...)
2019-07-19 11:00 ` [patch net-next rfc 7/7] net: rtnetlink: add possibility to use alternative names as message handle Jiri Pirko
@ 2019-07-19 11:03 ` Jiri Pirko
2019-07-19 11:03 ` [patch iproute2 rfc 2/2] ip: allow to use alternative names as handle Jiri Pirko
2019-07-19 16:31 ` [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Stephen Hemminger
9 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:03 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/uapi/linux/if_link.h | 3 ++
include/uapi/linux/rtnetlink.h | 7 +++
include/utils.h | 1 +
ip/ipaddress.c | 14 ++++++
ip/iplink.c | 81 ++++++++++++++++++++++++++++++++++
lib/utils.c | 19 +++++---
6 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d36919fb4024..2386a3e94082 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -167,6 +167,9 @@ enum {
IFLA_NEW_IFINDEX,
IFLA_MIN_MTU,
IFLA_MAX_MTU,
+ IFLA_ALT_IFNAME_MOD, /* Alternative ifname to add/delete */
+ IFLA_ALT_IFNAME_LIST, /* nest */
+ IFLA_ALT_IFNAME, /* Alternative ifname */
__IFLA_MAX
};
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 358e83ee134d..38a4f5b55e17 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -164,6 +164,13 @@ enum {
RTM_GETNEXTHOP,
#define RTM_GETNEXTHOP RTM_GETNEXTHOP
+ RTM_NEWALTIFNAME = 108,
+#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
+ RTM_DELALTIFNAME,
+#define RTM_DELALTIFNAME RTM_DELALTIFNAME
+ RTM_GETALTIFNAME,
+#define RTM_GETALTIFNAME RTM_GETALTIFNAME
+
__RTM_MAX,
#define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
};
diff --git a/include/utils.h b/include/utils.h
index 794d36053634..2dd6443fc6ff 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -196,6 +196,7 @@ void duparg(const char *, const char *) __attribute__((noreturn));
void duparg2(const char *, const char *) __attribute__((noreturn));
int nodev(const char *dev);
int check_ifname(const char *);
+int check_altifname(const char *);
int get_ifname(char *, const char *);
const char *get_ifname_rta(int ifindex, const struct rtattr *rta);
bool matches(const char *prefix, const char *string);
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index bc8f5ba13c33..8060161dcf1a 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1139,6 +1139,20 @@ int print_linkinfo(struct nlmsghdr *n, void *arg)
close_json_array(PRINT_JSON, NULL);
}
+ if (tb[IFLA_ALT_IFNAME_LIST]) {
+ struct rtattr *i, *alt_ifname_list = tb[IFLA_ALT_IFNAME_LIST];
+ int rem = RTA_PAYLOAD(alt_ifname_list);
+
+ open_json_array(PRINT_JSON, "altifnames");
+ for (i = RTA_DATA(alt_ifname_list); RTA_OK(i, rem);
+ i = RTA_NEXT(i, rem)) {
+ print_nl();
+ print_string(PRINT_ANY, NULL,
+ " altname %s", rta_getattr_str(i));
+ }
+ close_json_array(PRINT_JSON, NULL);
+ }
+
print_string(PRINT_FP, NULL, "%s", "\n");
fflush(fp);
return 1;
diff --git a/ip/iplink.c b/ip/iplink.c
index 212a088535da..45f975f1dce9 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1617,6 +1617,84 @@ static int iplink_afstats(int argc, char **argv)
return 0;
}
+static int iplink_altname_mod(int argc, char **argv, struct iplink_req *req)
+{
+ char *name = NULL;
+ char *dev = NULL;
+
+ while (argc > 0) {
+ if (matches(*argv, "name") == 0) {
+ NEXT_ARG();
+ if (check_altifname(*argv))
+ invarg("not a valid altifname", *argv);
+ name = *argv;
+ } else if (matches(*argv, "help") == 0) {
+ usage();
+ } else {
+ if (strcmp(*argv, "dev") == 0)
+ NEXT_ARG();
+ if (dev)
+ duparg2("dev", *argv);
+ if (check_altifname(*argv))
+ invarg("\"dev\" not a valid altifname", *argv);
+ dev = *argv;
+ }
+ argv++; argc--;
+ }
+
+ if (!dev) {
+ fprintf(stderr, "Not enough of information: \"dev\" argument is required.\n");
+ exit(-1);
+ }
+
+ if (!name) {
+ if (req->n.nlmsg_type != RTM_NEWALTIFNAME) {
+ name = dev;
+ } else {
+ fprintf(stderr, "Not enough of information: \"name\" argument is required.\n");
+ exit(-1);
+ }
+ }
+
+ addattr_l(&req->n, sizeof(*req), IFLA_ALT_IFNAME,
+ dev, strlen(dev) + 1);
+ addattr_l(&req->n, sizeof(*req), IFLA_ALT_IFNAME_MOD,
+ name, strlen(name) + 1);
+
+ if (rtnl_talk(&rth, &req->n, NULL) < 0)
+ return -2;
+
+ return 0;
+}
+
+static int iplink_altname(int argc, char **argv)
+{
+ struct iplink_req req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .i.ifi_family = preferred_family,
+ };
+
+ if (argc <= 0) {
+ usage();
+ exit(-1);
+ }
+
+ if (matches(*argv, "add") == 0) {
+ req.n.nlmsg_flags |= NLM_F_EXCL | NLM_F_CREATE | NLM_F_APPEND;
+ req.n.nlmsg_type = RTM_NEWALTIFNAME;
+ } else if (matches(*argv, "del") == 0) {
+ req.n.nlmsg_flags |= RTM_DELLINK;
+ req.n.nlmsg_type = RTM_DELALTIFNAME;
+ } else if (matches(*argv, "help") == 0) {
+ usage();
+ } else {
+ fprintf(stderr, "Operator required\n");
+ exit(-1);
+ }
+ return iplink_altname_mod(argc - 1, argv + 1, &req);
+}
+
static void do_help(int argc, char **argv)
{
struct link_util *lu = NULL;
@@ -1674,6 +1752,9 @@ int do_iplink(int argc, char **argv)
return 0;
}
+ if (matches(*argv, "altname") == 0)
+ return iplink_altname(argc-1, argv+1);
+
if (matches(*argv, "help") == 0) {
do_help(argc-1, argv+1);
return 0;
diff --git a/lib/utils.c b/lib/utils.c
index 9ea21fa16503..80e025e96a5a 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -824,14 +824,10 @@ int nodev(const char *dev)
return -1;
}
-int check_ifname(const char *name)
+static int __check_ifname(const char *name)
{
- /* These checks mimic kernel checks in dev_valid_name */
if (*name == '\0')
return -1;
- if (strlen(name) >= IFNAMSIZ)
- return -1;
-
while (*name) {
if (*name == '/' || isspace(*name))
return -1;
@@ -840,6 +836,19 @@ int check_ifname(const char *name)
return 0;
}
+int check_ifname(const char *name)
+{
+ /* These checks mimic kernel checks in dev_valid_name */
+ if (strlen(name) >= IFNAMSIZ)
+ return -1;
+ return __check_ifname(name);
+}
+
+int check_altifname(const char *name)
+{
+ return __check_ifname(name);
+}
+
/* buf is assumed to be IFNAMSIZ */
int get_ifname(char *buf, const char *name)
{
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* [patch iproute2 rfc 2/2] ip: allow to use alternative names as handle
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
` (7 preceding siblings ...)
2019-07-19 11:03 ` [patch iproute2 rfc 1/2] ip: add support for alternative name addition/deletion/list Jiri Pirko
@ 2019-07-19 11:03 ` Jiri Pirko
2019-07-19 16:31 ` [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Stephen Hemminger
9 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 11:03 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek, andrew,
parav, saeedm, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
ip/iplink.c | 5 +++--
lib/ll_map.c | 41 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index 45f975f1dce9..ad1e67761dd8 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -929,7 +929,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
NEXT_ARG();
if (dev != name)
duparg2("dev", *argv);
- if (check_ifname(*argv))
+ if (check_altifname(*argv))
invarg("\"dev\" not a valid ifname", *argv);
dev = *argv;
}
@@ -1104,7 +1104,8 @@ int iplink_get(char *name, __u32 filt_mask)
if (name) {
addattr_l(&req.n, sizeof(req),
- IFLA_IFNAME, name, strlen(name) + 1);
+ check_ifname(name) ? IFLA_IFNAME : IFLA_ALT_IFNAME,
+ name, strlen(name) + 1);
}
addattr32(&req.n, sizeof(req), IFLA_EXT_MASK, filt_mask);
diff --git a/lib/ll_map.c b/lib/ll_map.c
index e0ed54bf77c9..04dfb0f2320b 100644
--- a/lib/ll_map.c
+++ b/lib/ll_map.c
@@ -70,7 +70,7 @@ static struct ll_cache *ll_get_by_name(const char *name)
struct ll_cache *im
= container_of(n, struct ll_cache, name_hash);
- if (strncmp(im->name, name, IFNAMSIZ) == 0)
+ if (strcmp(im->name, name) == 0)
return im;
}
@@ -240,6 +240,43 @@ int ll_index_to_flags(unsigned idx)
return im ? im->flags : -1;
}
+static int altnametoindex(const char *name)
+{
+ struct {
+ struct nlmsghdr n;
+ struct ifinfomsg ifm;
+ char buf[1024];
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETLINK,
+ };
+ struct rtnl_handle rth = {};
+ struct nlmsghdr *answer;
+ struct ifinfomsg *ifm;
+ int rc = 0;
+
+ if (rtnl_open(&rth, 0) < 0)
+ return 0;
+
+ addattr32(&req.n, sizeof(req), IFLA_EXT_MASK,
+ RTEXT_FILTER_VF | RTEXT_FILTER_SKIP_STATS);
+ addattr_l(&req.n, sizeof(req), IFLA_ALT_IFNAME, name, strlen(name) + 1);
+
+ if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n, &answer) < 0)
+ goto out;
+
+ ifm = NLMSG_DATA(answer);
+ rc = ifm->ifi_index;
+
+ free(answer);
+
+ rtnl_close(&rth);
+out:
+ return rc;
+}
+
+
unsigned ll_name_to_index(const char *name)
{
const struct ll_cache *im;
@@ -257,6 +294,8 @@ unsigned ll_name_to_index(const char *name)
idx = if_nametoindex(name);
if (idx == 0)
idx = ll_idx_a2n(name);
+ if (idx == 0)
+ idx = altnametoindex(name);
return idx;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 0/7] net: introduce alternative names for network interfaces
2019-07-19 11:00 [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Jiri Pirko
` (8 preceding siblings ...)
2019-07-19 11:03 ` [patch iproute2 rfc 2/2] ip: allow to use alternative names as handle Jiri Pirko
@ 2019-07-19 16:31 ` Stephen Hemminger
2019-07-19 19:16 ` Jiri Pirko
9 siblings, 1 reply; 76+ messages in thread
From: Stephen Hemminger @ 2019-07-19 16:31 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
On Fri, 19 Jul 2019 13:00:22 +0200
Jiri Pirko <jiri@resnulli.us> wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> In the past, there was repeatedly discussed the IFNAMSIZ (16) limit for
> netdevice name length. Now when we have PF and VF representors
> with port names like "pfXvfY", it became quite common to hit this limit:
> 0123456789012345
> enp131s0f1npf0vf6
> enp131s0f1npf0vf22
>
> Udev cannot rename these interfaces out-of-the-box and user needs to
> create custom rules to handle them.
>
> Also, udev has multiple schemes of netdev names. From udev code:
> * Type of names:
> * b<number> - BCMA bus core number
> * c<bus_id> - bus id of a grouped CCW or CCW device,
> * with all leading zeros stripped [s390]
> * o<index>[n<phys_port_name>|d<dev_port>]
> * - on-board device index number
> * s<slot>[f<function>][n<phys_port_name>|d<dev_port>]
> * - hotplug slot index number
> * x<MAC> - MAC address
> * [P<domain>]p<bus>s<slot>[f<function>][n<phys_port_name>|d<dev_port>]
> * - PCI geographical location
> * [P<domain>]p<bus>s<slot>[f<function>][u<port>][..][c<config>][i<interface>]
> * - USB port number chain
> * v<slot> - VIO slot number (IBM PowerVM)
> * a<vendor><model>i<instance> - Platform bus ACPI instance id
> * i<addr>n<phys_port_name> - Netdevsim bus address and port name
>
> One device can be often renamed by multiple patterns at the
> same time (e.g. pci address/mac).
>
> This patchset introduces alternative names for network interfaces.
> Main goal is to:
> 1) Overcome the IFNAMSIZ limitation
> 2) Allow to have multiple names at the same time (multiple udev patterns)
> 3) Allow to use alternative names as handle for commands
>
> See following examples.
>
> $ ip link
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 3: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
>
> -> Add alternative names for dummy0:
>
> $ ip link altname add dummy0 name someothername
> $ ip link altname add dummy0 name someotherveryveryveryverylongname
> $ ip link
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 3: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
> altname someothername
> altname someotherveryveryveryverylongname
>
> -> Add bridge brx, add it's alternative name and use alternative names to
> do enslavement.
>
> $ ip link add name brx type bridge
> $ ip link altname add brx name mypersonalsuperspecialbridge
> $ ip link set someotherveryveryveryverylongname master mypersonalsuperspecialbridge
> $ ip link
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 3: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop master brx state DOWN mode DEFAULT group default qlen 1000
> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
> altname someothername
> altname someotherveryveryveryverylongname
> 4: brx: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
> altname mypersonalsuperspecialbridge
>
> -> Add ipv4 address to the bridge using alternative name:
>
> $ ip addr add 192.168.0.1/24 dev mypersonalsuperspecialbridge
> $ ip addr show mypersonalsuperspecialbridge
> 4: brx: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
> altname mypersonalsuperspecialbridge
> inet 192.168.0.1/24 scope global brx
> valid_lft forever preferred_lft forever
>
> -> Delete one of dummy0 alternative names:
>
> $ ip link altname del someotherveryveryveryverylongname
> $ ip link
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 3: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop master brx state DOWN mode DEFAULT group default qlen 1000
> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
> altname someothername
> 4: brx: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
> altname mypersonalsuperspecialbridge
>
> TODO:
> - notifications for alternative names add/removal
> - sanitization of add/del cmds (similar to get link)
> - test more usecases and write selftests
> - extend support for other netlink ifaces (ovs for example)
> - add sysfs symlink altname->basename?
>
> Jiri Pirko (7):
> net: procfs: use index hashlist instead of name hashlist
> net: introduce name_node struct to be used in hashlist
> net: rtnetlink: add commands to add and delete alternative ifnames
> net: rtnetlink: put alternative names to getlink message
> net: rtnetlink: unify the code in __rtnl_newlink get dev with the rest
> net: rtnetlink: introduce helper to get net_device instance by ifname
> net: rtnetlink: add possibility to use alternative names as message
> handle
>
> include/linux/netdevice.h | 14 ++-
> include/uapi/linux/if.h | 1 +
> include/uapi/linux/if_link.h | 3 +
> include/uapi/linux/rtnetlink.h | 7 ++
> net/core/dev.c | 152 ++++++++++++++++++++++----
> net/core/net-procfs.c | 4 +-
> net/core/rtnetlink.c | 192 +++++++++++++++++++++++++++++----
> security/selinux/nlmsgtab.c | 4 +-
> 8 files changed, 334 insertions(+), 43 deletions(-)
>
You might want to add altname/ object property in sysfs?
I.e
/sys/class/net/eth0/altname/
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [patch net-next rfc 0/7] net: introduce alternative names for network interfaces
2019-07-19 16:31 ` [patch net-next rfc 0/7] net: introduce alternative names for network interfaces Stephen Hemminger
@ 2019-07-19 19:16 ` Jiri Pirko
0 siblings, 0 replies; 76+ messages in thread
From: Jiri Pirko @ 2019-07-19 19:16 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
Fri, Jul 19, 2019 at 06:31:35PM CEST, stephen@networkplumber.org wrote:
>On Fri, 19 Jul 2019 13:00:22 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> In the past, there was repeatedly discussed the IFNAMSIZ (16) limit for
>> netdevice name length. Now when we have PF and VF representors
>> with port names like "pfXvfY", it became quite common to hit this limit:
>> 0123456789012345
>> enp131s0f1npf0vf6
>> enp131s0f1npf0vf22
>>
>> Udev cannot rename these interfaces out-of-the-box and user needs to
>> create custom rules to handle them.
>>
>> Also, udev has multiple schemes of netdev names. From udev code:
>> * Type of names:
>> * b<number> - BCMA bus core number
>> * c<bus_id> - bus id of a grouped CCW or CCW device,
>> * with all leading zeros stripped [s390]
>> * o<index>[n<phys_port_name>|d<dev_port>]
>> * - on-board device index number
>> * s<slot>[f<function>][n<phys_port_name>|d<dev_port>]
>> * - hotplug slot index number
>> * x<MAC> - MAC address
>> * [P<domain>]p<bus>s<slot>[f<function>][n<phys_port_name>|d<dev_port>]
>> * - PCI geographical location
>> * [P<domain>]p<bus>s<slot>[f<function>][u<port>][..][c<config>][i<interface>]
>> * - USB port number chain
>> * v<slot> - VIO slot number (IBM PowerVM)
>> * a<vendor><model>i<instance> - Platform bus ACPI instance id
>> * i<addr>n<phys_port_name> - Netdevsim bus address and port name
>>
>> One device can be often renamed by multiple patterns at the
>> same time (e.g. pci address/mac).
>>
>> This patchset introduces alternative names for network interfaces.
>> Main goal is to:
>> 1) Overcome the IFNAMSIZ limitation
>> 2) Allow to have multiple names at the same time (multiple udev patterns)
>> 3) Allow to use alternative names as handle for commands
>>
>> See following examples.
>>
>> $ ip link
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> 3: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
>> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
>>
>> -> Add alternative names for dummy0:
>>
>> $ ip link altname add dummy0 name someothername
>> $ ip link altname add dummy0 name someotherveryveryveryverylongname
>> $ ip link
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> 3: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
>> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
>> altname someothername
>> altname someotherveryveryveryverylongname
>>
>> -> Add bridge brx, add it's alternative name and use alternative names to
>> do enslavement.
>>
>> $ ip link add name brx type bridge
>> $ ip link altname add brx name mypersonalsuperspecialbridge
>> $ ip link set someotherveryveryveryverylongname master mypersonalsuperspecialbridge
>> $ ip link
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> 3: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop master brx state DOWN mode DEFAULT group default qlen 1000
>> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
>> altname someothername
>> altname someotherveryveryveryverylongname
>> 4: brx: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
>> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
>> altname mypersonalsuperspecialbridge
>>
>> -> Add ipv4 address to the bridge using alternative name:
>>
>> $ ip addr add 192.168.0.1/24 dev mypersonalsuperspecialbridge
>> $ ip addr show mypersonalsuperspecialbridge
>> 4: brx: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
>> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
>> altname mypersonalsuperspecialbridge
>> inet 192.168.0.1/24 scope global brx
>> valid_lft forever preferred_lft forever
>>
>> -> Delete one of dummy0 alternative names:
>>
>> $ ip link altname del someotherveryveryveryverylongname
>> $ ip link
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> 3: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop master brx state DOWN mode DEFAULT group default qlen 1000
>> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
>> altname someothername
>> 4: brx: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
>> link/ether 7e:a2:d4:b8:91:7a brd ff:ff:ff:ff:ff:ff
>> altname mypersonalsuperspecialbridge
>>
>> TODO:
>> - notifications for alternative names add/removal
>> - sanitization of add/del cmds (similar to get link)
>> - test more usecases and write selftests
>> - extend support for other netlink ifaces (ovs for example)
>> - add sysfs symlink altname->basename?
>>
>> Jiri Pirko (7):
>> net: procfs: use index hashlist instead of name hashlist
>> net: introduce name_node struct to be used in hashlist
>> net: rtnetlink: add commands to add and delete alternative ifnames
>> net: rtnetlink: put alternative names to getlink message
>> net: rtnetlink: unify the code in __rtnl_newlink get dev with the rest
>> net: rtnetlink: introduce helper to get net_device instance by ifname
>> net: rtnetlink: add possibility to use alternative names as message
>> handle
>>
>> include/linux/netdevice.h | 14 ++-
>> include/uapi/linux/if.h | 1 +
>> include/uapi/linux/if_link.h | 3 +
>> include/uapi/linux/rtnetlink.h | 7 ++
>> net/core/dev.c | 152 ++++++++++++++++++++++----
>> net/core/net-procfs.c | 4 +-
>> net/core/rtnetlink.c | 192 +++++++++++++++++++++++++++++----
>> security/selinux/nlmsgtab.c | 4 +-
>> 8 files changed, 334 insertions(+), 43 deletions(-)
>>
>
>You might want to add altname/ object property in sysfs?
>I.e
> /sys/class/net/eth0/altname/
Sysfs representation is one point on my TODO list.
I didn't look much into this, but yeah, that would work.
I also want to have symlinks altname->basename.
>
^ permalink raw reply [flat|nested] 76+ messages in thread