Date:   Mon, 3 Oct 2016 10:42:55 +0000
From:   Patrick Ruddy <pruddy@...cade.com>
To:     "jiri@...nulli.us" <jiri@...nulli.us>
CC:     "stephen@...workplumber.org" <stephen@...workplumber.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Luca Boccassi <lboccass@...cade.com>,
        "alexander.h.duyck@...el.com" <alexander.h.duyck@...el.com>,
        "roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
        "Sven-Thorsten Dietrich" <sven@...cade.com>
Subject: Re: [net-next PATCH] net: netlink messages for HW addr programming

On Tue, 2016-09-20 at 07:49 +0200, Jiri Pirko wrote:
> Tue, Sep 20, 2016 at 07:31:27AM CEST, roopa@...ulusnetworks.com wrote:
> >On 9/19/16, 7:46 AM, Patrick Ruddy wrote:
> >> On Sun, 2016-09-18 at 07:51 -0700, Roopa Prabhu wrote:
> >>> On 9/15/16, 9:48 AM, Patrick Ruddy wrote:
> >>>> Add RTM_NEWADDR and RTM_DELADDR netlink messages with family
> >>>> AF_UNSPEC to indicate interest in specific unicast and multicast
> >>>> hardware addresses. These messages are sent when addresses are
> >>>> added or deleted from the appropriate interface driver.
> >>>> Added AF_UNSPEC GETADDR function to allow the netlink notifications
> >>>> to be replayed to avoid loss of state due to application start
> >>>> ordering or restart.
> >>>>
> >>>> Signed-off-by: Patrick Ruddy <pruddy@...cade.com>
> >>>> ---
> >>> RTM_NEWADDR and RTM_DELADDR are not used to add these entries to the kernel.
> >>> so, it seems a bit wrong to use RTM_NEWADDR and RTM_DELADDR to notify them to
> >>> userspace and also to request a special dump of these addresses.
> >>>
> >>> This could just be a new nested netlink attribute in the existing link dump ?
> >> Hi Roopa
> >>
> >> Thanks for the review. I did initially code this using NEW/DEL/GET_LINK
> >> messages but was asked to change to to ADDR messages by Stephen
> >> Hemminger (cc'd). 
> >>
> >> However I agree that these addresses fall between the LINK and ADDR
> >> areas so I'm happy to change this if we can reach some consensus on the
> >> format.
> >>
> >ok, thanks for the history. yes, they do lie in a weird spot.
> 
> They are l2 addresses, they should be threated accordingly. Am I missing
> something?

In looking to rework this I remembered something. One of the plus sides
of using the ADDR messages to associate addresses with the device is
that we can use the NEW/DELADDR messages to signify the addition and
deletion of these addresses individually as they come and go, which they
do all the time e.g with IGMP joins and leaves. If we embed these
addresses within the LINK message we cannot use NEWLINK and DELLINK to
add and remove individual addresses as that would signify the link
coming and going which is not the case. SETLINK might work but then the
onus is on the netlink client to be able to work out the address
additions/deletions based on it's own state.  

> 
> >the general convention for other rtnl registrations seems to be
> >AF_UNSPEC family means include all supported families. thats where this seems a bit odd.
> >
> >On the other hand, one reason I see where using RTM_*ADDR will be useful for this is if we wanted
> >to provide a way to add these uc and mc address via ip addr add in the future.
> >ip addr add <lladdr> dev eth0
> >
> >Does this patch allow that in the future ?
> 
> This shoul go under ip link I believe. "ip addr" is for l3.
> 
> 
> >
> >also, will these l2 addresses now show up in 'ip addr show' output ?.
> >
I think the us ones will display in ip addr show and the mutlicast ones
in ip maddr show - this is the case today (i.e. without my changes)

thanks

-pr

Powered by blists - more mailing lists