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
| ||
|
Date: Wed, 27 Feb 2013 05:42:21 +0000 From: Jitendra Kalsaria <jitendra.kalsaria@...gic.com> To: John Fastabend <john.r.fastabend@...el.com>, "vyasevic@...hat.com" <vyasevic@...hat.com> CC: John Fastabend <john.fastabend@...il.com>, David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org> Subject: Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses On 2/26/13 9:01 PM, "John Fastabend" <john.r.fastabend@...el.com> wrote: >On 2/26/2013 6:27 PM, Vlad Yasevich wrote: >> On 02/26/2013 09:07 PM, John Fastabend wrote: >>> On Tue, Feb 26, 2013 at 05:15:03PM -0500, David Miller wrote: >>>> From: John Fastabend <john.r.fastabend@...el.com> >>>> Date: Tue, 26 Feb 2013 13:58:39 -0800 >>>> >>>>> [...] >>>>> >>>>>>>> >>>>>>>> >>>>>>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional >>>>>>>>hw >>>>>>>> addresses. >>>>>>>> >>>>>>>> Add an ability to add and remove HW addresses to the device >>>>>>>> unicast and multicast address lists. Right now, we only have >>>>>>>> an ioctl() to manage the multicast addresses and there is no >>>>>>>> way the manage the unicast list. >>>>>>>> >>>>>>>> Signed-off-by: Vlad Yasevich <vyasevic@...hat.com> >>>>>>> >>>>>>> This is a step in the right direction, and you're right that there >>>>>>>is >>>>>>> a difficulty in detecting whether support exists or not. >>>>>>> >>>>>>> I am so surprised that we've have ->set_rx_mode() support for >>>>>>> multiple >>>>>>> unicast MAC addresses in so many drivers all this time, yet no way >>>>>>> outside of FDB to make use of it at all. >>>>>> >>>>>> And even that is not always available. In most drivers it requires >>>>>> module parameters or other explicit configuration steps. Meanwhile >>>>>> set_rx_mode() doesn't seem to depend on any of those and just does >>>>>>the >>>>>> right thing. >>>>>> >>>>>> For what I was trying to do ioctl() was a really easy way out for >>>>>>both >>>>>> kernel and user space implementation, so I gave is shot. >>>>>> >>>>>> -vlad >>>>>> >>>>> >>>>> Don't we already support this with >>>> >>>> The whole point is that these multiple-unicast-address configuration >>>> facilities are inaccessible without FDB, and there is no reason >>>> whatsoever for that. >>> >>> Yes I see now sorry I was behind the thread. >>> >>> We could just use the fdb hooks and add a dflt routine to handle the >>> case where the netdev doesn't provide a specific ndo_op for us. This >>> boils down to calling set_rx_mode anyways through this call chain, >>> >>> ndo_fdb_add >>> dev_uc_add_excl >>> __dev_set_rx_mode >>> ndo_set_rx_mode >>> >>> As long as folks don't think this is too much of an abuse of the fdb >>> hooks. Here's an example I only compile tested this so take it as >>> an example only. It probably needs some cleanups and the dump routine >>> needs some thought not sure you want to dump all MACs always. >> >> I thought about doing, but this becomes broken on the drivers that >> support ndo_fdb_* functions. Those drivers mainly require additional >> configuration that is not necessary for dev_uc_add_excl() to work. >> >> For example, ixgbe and melanox requires VF to be on, qlogic needs a >> module parameter, macvlan has to be in passthrough. However, >> ndo_set_rx_mode() in most cases doesn't care about those settings and >> just works. >> > >The ixgbe piece could just use the dflt routines I put below. The >SR-IOV check is only there because when I wrote that I didn't consider >adding additional addresses without VFs. This was a poor example. > >The mlx4 card is checking if the device is a slave or master before >adding/removing addresses. My guess is this is the same type of check >as ixgbe and would work just fine without it. > >I'm not sure macvlan supports unicast hw addresses either way but >there is no reason we couldn't. The idea was we would come back and >write it later. Like many things I haven't got to it yet. > >Calling set_rx_mode() on qlcnic doesn't look to help you at all either >it appears to ignore the uc_list either way. Even though we use module parameter for fdb but it would be good it have set_rx_mode(). We don't ignore uc_list but it never called set_rx_mode() when I use dev_uc_add_excl when added support and thought that's how it might designed. -Jiten > >is that all the callers? looks like it. > >> So fdb will not always work even if the driver has proper support for >> IFF_UNICAST_FLT. >> > >The fdb could work if we fix the drivers it seems to me most >the cases where it wouldn't work are just lack of foresight or the code >isn't there yet. I think it might be valuable to have a single API for >both use cases. > >.John > >> -vlad >> >-- >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 > -- 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
Powered by blists - more mailing lists