diff mbox

iproute2: hide devices starting with period by default

Message ID 20170223195028.16388-1-stephen@networkplumber.org
State RFC, archived
Delegated to: stephen hemminger
Headers show

Commit Message

Stephen Hemminger Feb. 23, 2017, 7:50 p.m. UTC
Some use cases create Linux networking devices which are not intended for use
by normal networking. This is an enhancement to ip command to hide network
devices starting with period (like files in normal directory).  Interfaces whose
name start with "." are not shown by default, and the -a (or -all) flag must
be used to show these devices.

Example:
$ ip -brief link show
lo               UNKNOWN        00:00:00:00:00:00 <LOOPBACK,UP,LOWER_UP>
eth0             UP             00:25:90:86:b3:6b <BROADCAST,MULTICAST,UP,LOWER_UP>
eth1             DOWN           00:25:90:86:b3:6a <NO-CARRIER,BROADCAST,MULTICAST,UP>
$ ip -all -brief link show
lo               UNKNOWN        00:00:00:00:00:00 <LOOPBACK,UP,LOWER_UP>
eth0             UP             00:25:90:86:b3:6b <BROADCAST,MULTICAST,UP,LOWER_UP>
eth1             DOWN           00:25:90:86:b3:6a <NO-CARRIER,BROADCAST,MULTICAST,UP>
.eth2            DOWN           00:1b:21:a0:e7:06 <BROADCAST,MULTICAST>

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 include/utils.h |  1 +
 ip/ip.c         |  5 ++++-
 ip/ipaddress.c  | 31 ++++++++++++++++++++++++-------
 man/man8/ip.8   |  5 +++++
 4 files changed, 34 insertions(+), 8 deletions(-)

Comments

David Ahern Feb. 23, 2017, 11:39 p.m. UTC | #1
On 2/23/17 12:50 PM, Stephen Hemminger wrote:
> Some use cases create Linux networking devices which are not intended for use
> by normal networking. This is an enhancement to ip command to hide network
> devices starting with period (like files in normal directory).  Interfaces whose
> name start with "." are not shown by default, and the -a (or -all) flag must
> be used to show these devices.

Agree that some devices need to be hidden by default -- not just from
users but also other processes.

This solution is very narrow, only affecting iproute2 users. Any other
programs that use netlink or /proc files will continue to see those devices.

I started a patch a year ago that allows devices to marked as invisible
(attribute can be toggled at any time). Invisible devices do not show up
in netlink dumps, proc files or notifications. Netlink dumps can request
invisible devices to be included in a link dump. While it is more
intrusive, it is also more complete covering all of the paths in which
the device is shows up.

Also, changing the default behavior for iproute2 could break existing
users that have such device names.
Stephen Hemminger Feb. 24, 2017, 12:30 a.m. UTC | #2
On Thu, 23 Feb 2017 16:39:52 -0700
David Ahern <dsa@cumulusnetworks.com> wrote:

> On 2/23/17 12:50 PM, Stephen Hemminger wrote:
> > Some use cases create Linux networking devices which are not intended for use
> > by normal networking. This is an enhancement to ip command to hide network
> > devices starting with period (like files in normal directory).  Interfaces whose
> > name start with "." are not shown by default, and the -a (or -all) flag must
> > be used to show these devices.  
> 
> Agree that some devices need to be hidden by default -- not just from
> users but also other processes.
> 
> This solution is very narrow, only affecting iproute2 users. Any other
> programs that use netlink or /proc files will continue to see those devices.

I want solution that works broadly. And this works for sysfs already.


> I started a patch a year ago that allows devices to marked as invisible
> (attribute can be toggled at any time). Invisible devices do not show up
> in netlink dumps, proc files or notifications. Netlink dumps can request
> invisible devices to be included in a link dump. While it is more
> intrusive, it is also more complete covering all of the paths in which
> the device is shows up.
> 
> Also, changing the default behavior for iproute2 could break existing
> users that have such device names.

I am less worried about this. The only people using . in name already
are probably Brocade, and they have similar thing in CLI to hide these
devices.
David Ahern Feb. 24, 2017, 1:07 a.m. UTC | #3
On 2/23/17 5:30 PM, Stephen Hemminger wrote:
> On Thu, 23 Feb 2017 16:39:52 -0700
> David Ahern <dsa@cumulusnetworks.com> wrote:
> 
>> On 2/23/17 12:50 PM, Stephen Hemminger wrote:
>>> Some use cases create Linux networking devices which are not intended for use
>>> by normal networking. This is an enhancement to ip command to hide network
>>> devices starting with period (like files in normal directory).  Interfaces whose
>>> name start with "." are not shown by default, and the -a (or -all) flag must
>>> be used to show these devices.  
>>
>> Agree that some devices need to be hidden by default -- not just from
>> users but also other processes.
>>
>> This solution is very narrow, only affecting iproute2 users. Any other
>> programs that use netlink or /proc files will continue to see those devices.
> 
> I want solution that works broadly. And this works for sysfs already.

for 'ls' maybe, but not general walking of /sys. It does not hide
devices from snmpd, from ifconfig, etc., etc.


>> I started a patch a year ago that allows devices to marked as invisible
>> (attribute can be toggled at any time). Invisible devices do not show up
>> in netlink dumps, proc files or notifications. Netlink dumps can request
>> invisible devices to be included in a link dump. While it is more
>> intrusive, it is also more complete covering all of the paths in which
>> the device is shows up.
>>
>> Also, changing the default behavior for iproute2 could break existing
>> users that have such device names.
> 
> I am less worried about this. The only people using . in name already
> are probably Brocade, and they have similar thing in CLI to hide these
> devices.


seems like a big assumption.
Stephen Hemminger Feb. 24, 2017, 1:31 a.m. UTC | #4
On Thu, 23 Feb 2017 18:07:07 -0700
David Ahern <dsa@cumulusnetworks.com> wrote:

> On 2/23/17 5:30 PM, Stephen Hemminger wrote:
> > On Thu, 23 Feb 2017 16:39:52 -0700
> > David Ahern <dsa@cumulusnetworks.com> wrote:
> >   
> >> On 2/23/17 12:50 PM, Stephen Hemminger wrote:  
> >>> Some use cases create Linux networking devices which are not intended for use
> >>> by normal networking. This is an enhancement to ip command to hide network
> >>> devices starting with period (like files in normal directory).  Interfaces whose
> >>> name start with "." are not shown by default, and the -a (or -all) flag must
> >>> be used to show these devices.    
> >>
> >> Agree that some devices need to be hidden by default -- not just from
> >> users but also other processes.
> >>
> >> This solution is very narrow, only affecting iproute2 users. Any other
> >> programs that use netlink or /proc files will continue to see those devices.  
> > 
> > I want solution that works broadly. And this works for sysfs already.  
> 
> for 'ls' maybe, but not general walking of /sys. It does not hide
> devices from snmpd, from ifconfig, etc., etc.
> 
> 
> >> I started a patch a year ago that allows devices to marked as invisible
> >> (attribute can be toggled at any time). Invisible devices do not show up
> >> in netlink dumps, proc files or notifications. Netlink dumps can request
> >> invisible devices to be included in a link dump. While it is more
> >> intrusive, it is also more complete covering all of the paths in which
> >> the device is shows up.
> >>
> >> Also, changing the default behavior for iproute2 could break existing
> >> users that have such device names.  
> > 
> > I am less worried about this. The only people using . in name already
> > are probably Brocade, and they have similar thing in CLI to hide these
> > devices.  
> 
> 
> seems like a big assumption.

Need a solution now, not something that requires kernel and command changes.
David Miller Feb. 24, 2017, 3:12 a.m. UTC | #5
From: David Ahern <dsa@cumulusnetworks.com>
Date: Thu, 23 Feb 2017 18:07:07 -0700

> On 2/23/17 5:30 PM, Stephen Hemminger wrote:
>> On Thu, 23 Feb 2017 16:39:52 -0700
>> David Ahern <dsa@cumulusnetworks.com> wrote:
>> 
>>> On 2/23/17 12:50 PM, Stephen Hemminger wrote:
>>>> Some use cases create Linux networking devices which are not intended for use
>>>> by normal networking. This is an enhancement to ip command to hide network
>>>> devices starting with period (like files in normal directory).  Interfaces whose
>>>> name start with "." are not shown by default, and the -a (or -all) flag must
>>>> be used to show these devices.  
>>>
>>> Agree that some devices need to be hidden by default -- not just from
>>> users but also other processes.
>>>
>>> This solution is very narrow, only affecting iproute2 users. Any other
>>> programs that use netlink or /proc files will continue to see those devices.
>> 
>> I want solution that works broadly. And this works for sysfs already.
> 
> for 'ls' maybe, but not general walking of /sys. It does not hide
> devices from snmpd, from ifconfig, etc., etc.

I agree, that this is a pretty poor assumption.

And relying upon tool specific behavior to provide this facility
is even more special purpose.

This really need to be a fundamental facility, so that it transparently
works for NetworkManager, router daemons, everything.  Not just iproute2
and "ls".
Phil Sutter Feb. 24, 2017, 3:38 p.m. UTC | #6
On Thu, Feb 23, 2017 at 05:31:14PM -0800, Stephen Hemminger wrote:
> On Thu, 23 Feb 2017 18:07:07 -0700
> David Ahern <dsa@cumulusnetworks.com> wrote:
> 
> > On 2/23/17 5:30 PM, Stephen Hemminger wrote:
> > > On Thu, 23 Feb 2017 16:39:52 -0700
> > > David Ahern <dsa@cumulusnetworks.com> wrote:
> > >   
> > >> On 2/23/17 12:50 PM, Stephen Hemminger wrote:  
> > >>> Some use cases create Linux networking devices which are not intended for use
> > >>> by normal networking. This is an enhancement to ip command to hide network
> > >>> devices starting with period (like files in normal directory).  Interfaces whose
> > >>> name start with "." are not shown by default, and the -a (or -all) flag must
> > >>> be used to show these devices.    
> > >>
> > >> Agree that some devices need to be hidden by default -- not just from
> > >> users but also other processes.
> > >>
> > >> This solution is very narrow, only affecting iproute2 users. Any other
> > >> programs that use netlink or /proc files will continue to see those devices.  
> > > 
> > > I want solution that works broadly. And this works for sysfs already.  
> > 
> > for 'ls' maybe, but not general walking of /sys. It does not hide
> > devices from snmpd, from ifconfig, etc., etc.
> > 
> > 
> > >> I started a patch a year ago that allows devices to marked as invisible
> > >> (attribute can be toggled at any time). Invisible devices do not show up
> > >> in netlink dumps, proc files or notifications. Netlink dumps can request
> > >> invisible devices to be included in a link dump. While it is more
> > >> intrusive, it is also more complete covering all of the paths in which
> > >> the device is shows up.
> > >>
> > >> Also, changing the default behavior for iproute2 could break existing
> > >> users that have such device names.  
> > > 
> > > I am less worried about this. The only people using . in name already
> > > are probably Brocade, and they have similar thing in CLI to hide these
> > > devices.  
> > 
> > 
> > seems like a big assumption.
> 
> Need a solution now, not something that requires kernel and command changes.

Why the haste? This doesn't seem like an urgent thing to fix and given
the mixed feelings this provoked giving it a second thought might not be
the worst idea, no?

Cheers, Phil
David Ahern Feb. 24, 2017, 3:52 p.m. UTC | #7
On 2/23/17 8:12 PM, David Miller wrote:
> This really need to be a fundamental facility, so that it transparently
> works for NetworkManager, router daemons, everything.  Not just iproute2
> and "ls".

I'll rebase my patch and send out as RFC.
Andy Gospodarek Feb. 24, 2017, 5:06 p.m. UTC | #8
On Thu, Feb 23, 2017 at 11:50:28AM -0800, stephen hemminger wrote:
> Some use cases create Linux networking devices which are not intended for use
> by normal networking. This is an enhancement to ip command to hide network
> devices starting with period (like files in normal directory).  Interfaces whose
> name start with "." are not shown by default, and the -a (or -all) flag must
> be used to show these devices.
> 
> Example:
> $ ip -brief link show
> lo               UNKNOWN        00:00:00:00:00:00 <LOOPBACK,UP,LOWER_UP>
> eth0             UP             00:25:90:86:b3:6b <BROADCAST,MULTICAST,UP,LOWER_UP>
> eth1             DOWN           00:25:90:86:b3:6a <NO-CARRIER,BROADCAST,MULTICAST,UP>
> $ ip -all -brief link show
> lo               UNKNOWN        00:00:00:00:00:00 <LOOPBACK,UP,LOWER_UP>
> eth0             UP             00:25:90:86:b3:6b <BROADCAST,MULTICAST,UP,LOWER_UP>
> eth1             DOWN           00:25:90:86:b3:6a <NO-CARRIER,BROADCAST,MULTICAST,UP>
> .eth2            DOWN           00:1b:21:a0:e7:06 <BROADCAST,MULTICAST>

I've run across a time when there was a perceived need for this with an
out of tree driver that created a netdev that was never used.  I was
never a big fan of the attempt to hide it.  It seemed like the better
answer would be to try and fix the driver that is creating and
registering this unnecessary netdev so it no longer appears and that was
what we did.  

As an admin I'd be pretty frustrated if I somehow had a network issue
that I could not properly debug due to a hidden network interface.  If
there was an extra flag that was needed to a tool like iproute2 to show
the hidden device, soon admin's hands would just type 'ip -all link
show' each time to get the full picture.  

If the concensus is that we DO want to hide devices I've seen David
Ahern's set and it is much more complete than this.  It is probably the
better approach to not leave too many loose ends.

> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  include/utils.h |  1 +
>  ip/ip.c         |  5 ++++-
>  ip/ipaddress.c  | 31 ++++++++++++++++++++++++-------
>  man/man8/ip.8   |  5 +++++
>  4 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/include/utils.h b/include/utils.h
> index 22369e0b4e03..50712d27f112 100644
> --- a/include/utils.h
> +++ b/include/utils.h
> @@ -16,6 +16,7 @@ extern int human_readable;
>  extern int use_iec;
>  extern int show_stats;
>  extern int show_details;
> +extern int show_all;
>  extern int show_raw;
>  extern int resolve_hosts;
>  extern int oneline;
> diff --git a/ip/ip.c b/ip/ip.c
> index 07050b07592a..ed637b065b58 100644
> --- a/ip/ip.c
> +++ b/ip/ip.c
> @@ -30,6 +30,7 @@ int human_readable;
>  int use_iec;
>  int show_stats;
>  int show_details;
> +int show_all;
>  int resolve_hosts;
>  int oneline;
>  int brief;
> @@ -54,7 +55,7 @@ static void usage(void)
>  "                   netns | l2tp | fou | macsec | tcp_metrics | token | netconf | ila |\n"
>  "                   vrf }\n"
>  "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |\n"
> -"                    -h[uman-readable] | -iec |\n"
> +"                    -h[uman-readable] | -iec | -all |\n"
>  "                    -f[amily] { inet | inet6 | ipx | dnet | mpls | bridge | link } |\n"
>  "                    -4 | -6 | -I | -D | -B | -0 |\n"
>  "                    -l[oops] { maximum-addr-flush-attempts } | -br[ief] |\n"
> @@ -231,6 +232,8 @@ int main(int argc, char **argv)
>  			++show_stats;
>  		} else if (matches(opt, "-details") == 0) {
>  			++show_details;
> +		} else if (matches(opt, "-all") == 0) {
> +			++show_all;
>  		} else if (matches(opt, "-resolve") == 0) {
>  			++resolve_hosts;
>  		} else if (matches(opt, "-oneline") == 0) {
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 400ebb4de563..029aae100549 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -660,6 +660,7 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
>  	struct ifinfomsg *ifi = NLMSG_DATA(n);
>  	struct rtattr *tb[IFLA_MAX+1];
>  	int len = n->nlmsg_len;
> +	const char *ifname;
>  	char *name;
>  	char buf[32] = { 0, };
>  	unsigned int m_flag = 0;
> @@ -677,14 +678,22 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
>  		return -1;
>  
>  	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
> -	if (tb[IFLA_IFNAME] == NULL)
> -		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
> +	if (tb[IFLA_IFNAME])
> +		ifname = rta_getattr_str(tb[IFLA_IFNAME]);
> +	else {
> +		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n",
> +			ifi->ifi_index);
> +		ifname = "<nil>";
> +	}
>  
>  	if (filter.label &&
>  	    (!filter.family || filter.family == AF_PACKET) &&
>  	    fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
>  		return -1;
>  
> +	if (!filter.ifindex && *ifname == '.' && !show_all)
> +		return 0;
> +
>  	if (tb[IFLA_GROUP]) {
>  		int group = *(int *)RTA_DATA(tb[IFLA_GROUP]);
>  
> @@ -758,6 +767,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
>  	struct ifinfomsg *ifi = NLMSG_DATA(n);
>  	struct rtattr *tb[IFLA_MAX+1];
>  	int len = n->nlmsg_len;
> +	const char *ifname;
>  	unsigned int m_flag = 0;
>  
>  	if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
> @@ -773,12 +783,20 @@ int print_linkinfo(const struct sockaddr_nl *who,
>  		return 0;
>  
>  	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
> -	if (tb[IFLA_IFNAME] == NULL)
> -		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
> +	if (tb[IFLA_IFNAME])
> +		ifname = rta_getattr_str(tb[IFLA_IFNAME]);
> +	else {
> +		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n",
> +			ifi->ifi_index);
> +		ifname = "<nil>";
> +	}
>  
>  	if (filter.label &&
>  	    (!filter.family || filter.family == AF_PACKET) &&
> -	    fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
> +	    fnmatch(filter.label, ifname, 0))
> +		return 0;
> +
> +	if (!filter.ifindex && *ifname == '.' && !show_all)
>  		return 0;
>  
>  	if (tb[IFLA_GROUP]) {
> @@ -806,8 +824,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
>  		fprintf(fp, "Deleted ");
>  
>  	fprintf(fp, "%d: ", ifi->ifi_index);
> -	color_fprintf(fp, COLOR_IFNAME, "%s",
> -		      tb[IFLA_IFNAME] ? rta_getattr_str(tb[IFLA_IFNAME]) : "<nil>");
> +	color_fprintf(fp, COLOR_IFNAME, "%s", ifname);
>  
>  	if (tb[IFLA_LINK]) {
>  		SPRINT_BUF(b1);
> diff --git a/man/man8/ip.8 b/man/man8/ip.8
> index 8ecb1996da92..813dbec2a6f2 100644
> --- a/man/man8/ip.8
> +++ b/man/man8/ip.8
> @@ -31,6 +31,7 @@ ip \- show / manipulate routing, devices, policy routing and tunnels
>  \fB\-h\fR[\fIuman-readable\fR] |
>  \fB\-s\fR[\fItatistics\fR] |
>  \fB\-d\fR[\fIetails\fR] |
> +\fB\-a\fR[\fll\fR] |
>  \fB\-r\fR[\fIesolve\fR] |
>  \fB\-iec\fR |
>  \fB\-f\fR[\fIamily\fR] {
> @@ -84,6 +85,10 @@ As a rule, the information is statistics or some time values.
>  Output more detailed information.
>  
>  .TP
> +.BR "\-a" , " \-all"
> +Show all devices, do not ignore entries starting with period.
> +
> +.TP
>  .BR "\-l" , " \-loops " <COUNT>
>  Specify maximum number of loops the 'ip address flush' logic
>  will attempt before giving up. The default is 10.
Nicolas Dichtel May 4, 2017, 3:15 p.m. UTC | #9
Le 24/02/2017 à 16:52, David Ahern a écrit :
> On 2/23/17 8:12 PM, David Miller wrote:
>> This really need to be a fundamental facility, so that it transparently
>> works for NetworkManager, router daemons, everything.  Not just iproute2
>> and "ls".
> 
> I'll rebase my patch and send out as RFC.
> 
David, did you finally send those patches?


Thank you,
Nicolas
David Ahern May 4, 2017, 4:37 p.m. UTC | #10
On 5/4/17 9:15 AM, Nicolas Dichtel wrote:
> Le 24/02/2017 à 16:52, David Ahern a écrit :
>> On 2/23/17 8:12 PM, David Miller wrote:
>>> This really need to be a fundamental facility, so that it transparently
>>> works for NetworkManager, router daemons, everything.  Not just iproute2
>>> and "ls".
>>
>> I'll rebase my patch and send out as RFC.
>>
> David, did you finally send those patches?
> 

No, but for a few reasons.

It is easy to hide devices in a dump:

https://github.com/dsahern/linux/commit/48a80a00eac284e58bae04af10a5a932dd7aee00


But I think those devices should also not exist in sysfs or procfs which
overlaps what I would like to see for lightweight netdevices:

https://github.com/dsahern/linux/commit/70574be699cf252e77f71e3df11192438689f976


and to be complete, hidden devices should not be allowed to have a
network address or transmit packets which is the L2 only intent from
Florian:
    https://www.spinics.net/lists/netdev/msg340808.html
Florian Fainelli May 4, 2017, 7:10 p.m. UTC | #11
On 05/04/2017 09:37 AM, David Ahern wrote:
> On 5/4/17 9:15 AM, Nicolas Dichtel wrote:
>> Le 24/02/2017 à 16:52, David Ahern a écrit :
>>> On 2/23/17 8:12 PM, David Miller wrote:
>>>> This really need to be a fundamental facility, so that it transparently
>>>> works for NetworkManager, router daemons, everything.  Not just iproute2
>>>> and "ls".
>>>
>>> I'll rebase my patch and send out as RFC.
>>>
>> David, did you finally send those patches?
>>
> 
> No, but for a few reasons.
> 
> It is easy to hide devices in a dump:
> 
> https://github.com/dsahern/linux/commit/48a80a00eac284e58bae04af10a5a932dd7aee00
> 
> 
> But I think those devices should also not exist in sysfs or procfs which
> overlaps what I would like to see for lightweight netdevices:
> 
> https://github.com/dsahern/linux/commit/70574be699cf252e77f71e3df11192438689f976

Interesting that does indeed solve the same problems as the L2 only
patch set intended. I am not exactly sure if hiding the devices from
procfs/sysfs would be appropriate in my case (dumb L2 only switch that
only does 802.1q for instance), but why not.


> 
> 
> and to be complete, hidden devices should not be allowed to have a
> network address or transmit packets which is the L2 only intent from
> Florian:
>     https://www.spinics.net/lists/netdev/msg340808.html
> 

Do you plan on submitting the LWT patch set at some point?
David Ahern May 4, 2017, 7:47 p.m. UTC | #12
On 5/4/17 1:10 PM, Florian Fainelli wrote:
> On 05/04/2017 09:37 AM, David Ahern wrote:
>> On 5/4/17 9:15 AM, Nicolas Dichtel wrote:
>>> Le 24/02/2017 à 16:52, David Ahern a écrit :
>>>> On 2/23/17 8:12 PM, David Miller wrote:
>>>>> This really need to be a fundamental facility, so that it transparently
>>>>> works for NetworkManager, router daemons, everything.  Not just iproute2
>>>>> and "ls".
>>>>
>>>> I'll rebase my patch and send out as RFC.
>>>>
>>> David, did you finally send those patches?
>>>
>>
>> No, but for a few reasons.
>>
>> It is easy to hide devices in a dump:
>>
>> https://github.com/dsahern/linux/commit/48a80a00eac284e58bae04af10a5a932dd7aee00
>>
>>
>> But I think those devices should also not exist in sysfs or procfs which
>> overlaps what I would like to see for lightweight netdevices:
>>
>> https://github.com/dsahern/linux/commit/70574be699cf252e77f71e3df11192438689f976
> 
> Interesting that does indeed solve the same problems as the L2 only
> patch set intended. I am not exactly sure if hiding the devices from
> procfs/sysfs would be appropriate in my case (dumb L2 only switch that
> only does 802.1q for instance), but why not.
> 
> 
>>
>>
>> and to be complete, hidden devices should not be allowed to have a
>> network address or transmit packets which is the L2 only intent from
>> Florian:
>>     https://www.spinics.net/lists/netdev/msg340808.html
>>
> 
> Do you plan on submitting the LWT patch set at some point?

Definitely. Maybe I can find some time this weekend.
Jiri Benc May 5, 2017, 6:47 a.m. UTC | #13
On Thu, 4 May 2017 13:47:36 -0600, David Ahern wrote:
> On 5/4/17 1:10 PM, Florian Fainelli wrote:
> > On 05/04/2017 09:37 AM, David Ahern wrote:
> > Do you plan on submitting the LWT patch set at some point?
> 
> Definitely. Maybe I can find some time this weekend.

I suggest to change the name to "lwd" or so. "lwt" name is too similar
to the existing "lwtunnel" infrastructure and would be very confusing.

Thanks,

 Jiri
Nicolas Dichtel May 5, 2017, 7:42 a.m. UTC | #14
Le 04/05/2017 à 21:47, David Ahern a écrit :
> On 5/4/17 1:10 PM, Florian Fainelli wrote:
>> On 05/04/2017 09:37 AM, David Ahern wrote:
>>> On 5/4/17 9:15 AM, Nicolas Dichtel wrote:
>>>> Le 24/02/2017 à 16:52, David Ahern a écrit :
>>>>> On 2/23/17 8:12 PM, David Miller wrote:
>>>>>> This really need to be a fundamental facility, so that it transparently
>>>>>> works for NetworkManager, router daemons, everything.  Not just iproute2
>>>>>> and "ls".
>>>>>
>>>>> I'll rebase my patch and send out as RFC.
>>>>>
>>>> David, did you finally send those patches?
>>>>
>>>
>>> No, but for a few reasons.
>>>
>>> It is easy to hide devices in a dump:
>>>
>>> https://github.com/dsahern/linux/commit/48a80a00eac284e58bae04af10a5a932dd7aee00
>>>
>>>
>>> But I think those devices should also not exist in sysfs or procfs which
>>> overlaps what I would like to see for lightweight netdevices:
>>>
>>> https://github.com/dsahern/linux/commit/70574be699cf252e77f71e3df11192438689f976
>>
>> Interesting that does indeed solve the same problems as the L2 only
>> patch set intended. I am not exactly sure if hiding the devices from
>> procfs/sysfs would be appropriate in my case (dumb L2 only switch that
>> only does 802.1q for instance), but why not.
>>
>>
>>>
>>>
>>> and to be complete, hidden devices should not be allowed to have a
>>> network address or transmit packets which is the L2 only intent from
>>> Florian:
>>>     https://www.spinics.net/lists/netdev/msg340808.html
>>>
>>
>> Do you plan on submitting the LWT patch set at some point?
> 
> Definitely. Maybe I can find some time this weekend.
> 
Ok, thank you for the details.

I agree with Jiri that the name should be something different than lwt.


Regards,
Nicolas
diff mbox

Patch

diff --git a/include/utils.h b/include/utils.h
index 22369e0b4e03..50712d27f112 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -16,6 +16,7 @@  extern int human_readable;
 extern int use_iec;
 extern int show_stats;
 extern int show_details;
+extern int show_all;
 extern int show_raw;
 extern int resolve_hosts;
 extern int oneline;
diff --git a/ip/ip.c b/ip/ip.c
index 07050b07592a..ed637b065b58 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -30,6 +30,7 @@  int human_readable;
 int use_iec;
 int show_stats;
 int show_details;
+int show_all;
 int resolve_hosts;
 int oneline;
 int brief;
@@ -54,7 +55,7 @@  static void usage(void)
 "                   netns | l2tp | fou | macsec | tcp_metrics | token | netconf | ila |\n"
 "                   vrf }\n"
 "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |\n"
-"                    -h[uman-readable] | -iec |\n"
+"                    -h[uman-readable] | -iec | -all |\n"
 "                    -f[amily] { inet | inet6 | ipx | dnet | mpls | bridge | link } |\n"
 "                    -4 | -6 | -I | -D | -B | -0 |\n"
 "                    -l[oops] { maximum-addr-flush-attempts } | -br[ief] |\n"
@@ -231,6 +232,8 @@  int main(int argc, char **argv)
 			++show_stats;
 		} else if (matches(opt, "-details") == 0) {
 			++show_details;
+		} else if (matches(opt, "-all") == 0) {
+			++show_all;
 		} else if (matches(opt, "-resolve") == 0) {
 			++resolve_hosts;
 		} else if (matches(opt, "-oneline") == 0) {
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 400ebb4de563..029aae100549 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -660,6 +660,7 @@  int print_linkinfo_brief(const struct sockaddr_nl *who,
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct rtattr *tb[IFLA_MAX+1];
 	int len = n->nlmsg_len;
+	const char *ifname;
 	char *name;
 	char buf[32] = { 0, };
 	unsigned int m_flag = 0;
@@ -677,14 +678,22 @@  int print_linkinfo_brief(const struct sockaddr_nl *who,
 		return -1;
 
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-	if (tb[IFLA_IFNAME] == NULL)
-		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
+	if (tb[IFLA_IFNAME])
+		ifname = rta_getattr_str(tb[IFLA_IFNAME]);
+	else {
+		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n",
+			ifi->ifi_index);
+		ifname = "<nil>";
+	}
 
 	if (filter.label &&
 	    (!filter.family || filter.family == AF_PACKET) &&
 	    fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
 		return -1;
 
+	if (!filter.ifindex && *ifname == '.' && !show_all)
+		return 0;
+
 	if (tb[IFLA_GROUP]) {
 		int group = *(int *)RTA_DATA(tb[IFLA_GROUP]);
 
@@ -758,6 +767,7 @@  int print_linkinfo(const struct sockaddr_nl *who,
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct rtattr *tb[IFLA_MAX+1];
 	int len = n->nlmsg_len;
+	const char *ifname;
 	unsigned int m_flag = 0;
 
 	if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
@@ -773,12 +783,20 @@  int print_linkinfo(const struct sockaddr_nl *who,
 		return 0;
 
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-	if (tb[IFLA_IFNAME] == NULL)
-		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
+	if (tb[IFLA_IFNAME])
+		ifname = rta_getattr_str(tb[IFLA_IFNAME]);
+	else {
+		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n",
+			ifi->ifi_index);
+		ifname = "<nil>";
+	}
 
 	if (filter.label &&
 	    (!filter.family || filter.family == AF_PACKET) &&
-	    fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
+	    fnmatch(filter.label, ifname, 0))
+		return 0;
+
+	if (!filter.ifindex && *ifname == '.' && !show_all)
 		return 0;
 
 	if (tb[IFLA_GROUP]) {
@@ -806,8 +824,7 @@  int print_linkinfo(const struct sockaddr_nl *who,
 		fprintf(fp, "Deleted ");
 
 	fprintf(fp, "%d: ", ifi->ifi_index);
-	color_fprintf(fp, COLOR_IFNAME, "%s",
-		      tb[IFLA_IFNAME] ? rta_getattr_str(tb[IFLA_IFNAME]) : "<nil>");
+	color_fprintf(fp, COLOR_IFNAME, "%s", ifname);
 
 	if (tb[IFLA_LINK]) {
 		SPRINT_BUF(b1);
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 8ecb1996da92..813dbec2a6f2 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -31,6 +31,7 @@  ip \- show / manipulate routing, devices, policy routing and tunnels
 \fB\-h\fR[\fIuman-readable\fR] |
 \fB\-s\fR[\fItatistics\fR] |
 \fB\-d\fR[\fIetails\fR] |
+\fB\-a\fR[\fll\fR] |
 \fB\-r\fR[\fIesolve\fR] |
 \fB\-iec\fR |
 \fB\-f\fR[\fIamily\fR] {
@@ -84,6 +85,10 @@  As a rule, the information is statistics or some time values.
 Output more detailed information.
 
 .TP
+.BR "\-a" , " \-all"
+Show all devices, do not ignore entries starting with period.
+
+.TP
 .BR "\-l" , " \-loops " <COUNT>
 Specify maximum number of loops the 'ip address flush' logic
 will attempt before giving up. The default is 10.