netdev - Re: [PATCH RFC 0/8] LLDP implementation for Linux

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4FE8B535.70900@intel.com>
Date:	Mon, 25 Jun 2012 12:00:05 -0700
From:	John Fastabend <john.r.fastabend@...el.com>
To:	Eldad Zack <eldad@...refinery.com>
CC:	netdev@...r.kernel.org
Subject: Re: [PATCH RFC 0/8] LLDP implementation for Linux

On 6/25/2012 11:28 AM, Eldad Zack wrote:
> Hi all,
>
> This series of patches provides a partial LLDP (IEEE Std 802.1ab)
> implementation.
>
> I'd really appreciate a review of it.
>
> LLDP is a simple discovery protocol which advertises the identification and
> other info (such as MTU or capabilities) of device on the link. It can also
> help debug misconfigurations on the link layer (wrong MTU, wrong VLAN).
>

Why do you want to implement this in the kernel? We've been doing this
in user space for sometime without any issues and has some noted
advantages. Reduces kernel bloat, easier to add extend, etc.

Take a look at

http://www.open-lldp.org/open-lldp

What are we missing? Can we address any deficiencies here rather than
embedded this into the kernel.

> Some notes/questions:
>
> * Applies against net-next and mainline.
>
> * Included in this series is only LLDP output code.
>    This is not an issue since input and output are decoupled in LLDP anyway.
>    I'm working on the input code as well and will post it at some point in the
>    future.
>
> * Sysctl is used to do (some) configuration. This is done globally right now.
>    Before I add per-device sysctls: is it at all appropriate to use sysctl
>    here?

No, netlink is much nicer for these types of things. User space
can register for events and stay in sync and also all the other
relevant events are using netlink UP/DOWN/DORMANT events for
example.

>
> * By default, transmission is suppressed. To arm it, set
> 	/proc/sys/net/lldp/lldp_op_mode
>    to 1.
>
> * I've tested it on x86_64 and (qemu'd) x86.
>
> * I've tested it on my machine and it works with Ethernet and WLAN.
>
> * The last patch ("8021q/vlan: process NETDEV_GOING_DOWN") is needed
>    to be able to send shutdown PDU on VLAN interfaces, but has otherwise
>    no effect.
>

Do this in user space and you can handle all these events without
changing existing infrastructure.

> * Is there a better way to deal with 16-bit endianness other than masking and
>    shifting, when one field is more than a byte long (in this case 7/9)?
>
> * I usually only work on it on weekends, so I might be slow in responding back.

Please consider user space implementations (you don't have to use
mine) or let us know why we _need_ this in the kernel.

>
> Thanks in advance if you're taking the time to review it or test it!
>

I'll scan the patches shortly.

Thanks,
John
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html