On Mon, Oct 1, 2018 at 12:01 PM David Ahern <dsah...@gmail.com> wrote: > > On 10/1/18 6:44 AM, Mauricio Faria de Oliveira wrote: > >> I suspect rtnl_fdb_dump is forever stuck with the ifinfomsg struct as > >> the header if any kernel side filtering is to be done. [snip] > > > > Why exactly? I understand currently there may be little information > > to distinguish family headers, but if it comes down to certain attributes > > the function expects/uses, that can be checked if nlmsg_parse() is OK. > > > > Otherwise, if it comes down to some struct field that is not common > > between both structs, then.. well. Maybe something else/new. > > struct ifinfomsg is 16 bytes; ndmsg is 12 bytes. The difference is > ifi_change in the ifinfomsg. If you don't know which header is sent, how > can you reliably parse -- and believe -- the result of the parsing? > > Yes, iproute2 0's out the structs. So does FRR. But the general argument > is that userspace may not and the kernel has to this point happily > ignored fields it was not using.
Right, I see the point of the unknown header type, and wanted to understand whether you had more reasons in mind, as I imagined the particular users/attributes involved in this function could have provided an way out. *But*, as you pointed out, the general case is that userspace can do anything. So, now I see -- the assumption for particular cases is not an option. Thanks. > The short of it is that ifi_change may be non-0 and should not be relied > on. That's the theory anyways ... And/or ndm_ifindex, which in the ifinfomsg type cast, gives brport_idx a non-zero value I don't yet know if it's equivalent, but if not, that's a problem. (i.e., specify a non-brport thing, but it's processed as such) > <snip> > Perhaps there is a work around. IFLA_MASTER is the only supported > attribute that can be appended, and it is sent as a u32. Then the > rtnl_fdb_dump function has 4 legitimate cases: > > 1. ndmsg = size 12 bytes > 2. ndmsg + MASTER = 20 ? > 3. ifinfomsg = size 16 bytes > 4. ifinfomsg + MASTER = 24 ? > > 'ip neigh show' could send NDA_IFINDEX as an additional attribute. That > is my mistake. I should have set ndm_ifindex rather than using the > attribute, but that ship sailed 3 years ago. Anyways, that case too > might be uniquely detected. The size checks have been used in other > places, so should be ok here too. Ok, thanks for your suggestions. I'll do some research/learning on them, and give it a try for a v2. > At this point the use of ifinfomsg for dumps has created a mess > extending kernel side filtering. The point of the PROPER_HDR patch set > is to give a point in time across all dump functions where the kernel > and userspace can reliably communicate about what is wanted. Nice. I guess for this particular problem/thread, we'll won't be able to use that one, since existing/older userspace that doesn't know about it should be fixed for.. but definitely a great thing that should help in this general problem. Thank you, -- Mauricio Faria de Oliveira
