From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick Ruddy Subject: Re: [PATCH net-next v3 1/2] netlink: ipv4 igmp join notifications Date: Tue, 18 Sep 2018 14:12:48 +0100 Message-ID: References: <20180830093545.29465-2-pruddy@vyatta.att-mail.com> <20180906091056.21109-1-pruddy@vyatta.att-mail.com> Reply-To: pruddy@vyatta.att-mail.com Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: netdev , =?UTF-8?Q?Ji=C5=99=C3=AD_P=C3=ADrko?= , Stephen Hemminger , Nikolay Aleksandrov To: Roopa Prabhu Return-path: Received: from mx0a-00191d01.pphosted.com ([67.231.149.140]:55524 "EHLO mx0a-00191d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729763AbeIRSpj (ORCPT ); Tue, 18 Sep 2018 14:45:39 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 2018-09-13 at 10:03 -0700, Roopa Prabhu wrote: > On Thu, Sep 6, 2018 at 8:40 PM, Roopa Prabhu wrote: > > On Thu, Sep 6, 2018 at 2:10 AM, Patrick Ruddy > > wrote: > > > Some userspace applications need to know about IGMP joins from the > > > kernel for 2 reasons: > > > 1. To allow the programming of multicast MAC filters in hardware > > > 2. To form a multicast FORUS list for non link-local multicast > > > groups to be sent to the kernel and from there to the interested > > > party. > > > (1) can be fulfilled but simply sending the hardware multicast MAC > > > address to be programmed but (2) requires the L3 address to be sent > > > since this cannot be constructed from the MAC address whereas the > > > reverse translation is a standard library function. > > > > > > This commit provides addition and deletion of multicast addresses > > > using the RTM_NEWMDB and RTM_DELMDB messages with AF_INET. It also > > > provides the RTM_GETMDB extension to allow multicast join state to > > > be read from the kernel. > > > > > > Signed-off-by: Patrick Ruddy > > > --- > > > v3 rework to use RTM_***MDB messages as per review comments. > > > > Patrick, this version seems to be using RTM_***MDB msgs with the > > RTM_*ADDR format. > > We cant do that...because existing RTM_MDB users will be confused. > > > > My request was to evaluate RTM_***MDB msg format. see > > nlmsg_populate_mdb_fill for details. > > > > If you can wait a day or two I can share some experimental code that > > moves high level RTM_*MDB msg handling into net/core/rtnetlink.c > > similar to RTM_*FDB > > > > I was trying to get a default per interface (non bridge) RTM_*MDB > working, but realized that the dev->mc > entries are already getting dumped as part of RTM_*FDB msgs instead of > RTM_*MDB. (see net/core/rtnetlink.c:ndo_dflt_fdb_dump). > This adds another wrench. > > so, that puts us back to your use of RTM_NEWADDR. > Instead of using IFA_ADDRESS, you could introduce a new one > IFA_IGMP_MULTICAST (since IFA_MULTICAST is already taken). > > > To keep existing users of RTM_NEWADDR unaffected. I think you can use > the IPMR family with RTM_NEWADDR. > We can introduce new notification group. (We can choose to add a new > family too, but that seems unnecessary) > > since you only need dumps: > rtnl_register(RTNL_FAMILY_IPMR, RTM_GETADDR, NULL, igmp_rtm_dumpaddrs, 0); > > For notifications, since we already have many variants for routes, I > don't see a problem adding similar addr variants > RTNLGRP_IPV4_MCADDR > > (Others on the list may have more feedback). I've hit a small snag with adding the new groups. The number of defined groups currently sits at 31 so I can only add one before hitting the limit defined by the 32 bit groups bitmask in socakddr_nl. I can use 1 group for both v4 and v6 notifications which seems like the sensible options since the AF is carried separately, but it breaks the precedent where there are separate IPV4 and IPV6 groups for IFADDR. I have the combined group patches ready and can share them if that's the preference. Has there been any previous discussion about extending the number of availabel groups?