| Message ID | 20180124205520.5811-1-jeffrey.t.kirsher@intel.com |
|---|---|
| Headers | show |
| Series | 1GbE Intel Wired LAN Driver Updates 2018-01-24 | expand |
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Date: Wed, 24 Jan 2018 12:55:12 -0800 > This series contains updates to igb and e1000e only. Pulled, however: > Corinna Vinschen implements the ability to set the VF MAC to > 00:00:00:00:00:00 via RTM_SETLINK on the PF, to prevent receiving > "invlaid argument" when libvirt attempts to restore the MAC address back > to its original state of 00:00:00:00:00:00. This is really a mess and the wrong way to go about this. No interface, even a VF, should come up or ever have an invalid MAC addres like all-zeros. That's the fundamental problem and once you fix that all of this other crazy logic and workarounds no longer become necessary. Whatever it takes, just do it. We can even come up with a global MAC address range that on a Linux system is reserved for VFs to come up with. Thanks.
On Wed, Jan 24, 2018 at 1:10 PM, David Miller <davem@davemloft.net> wrote: > From: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > Date: Wed, 24 Jan 2018 12:55:12 -0800 > >> This series contains updates to igb and e1000e only. > > Pulled, however: > >> Corinna Vinschen implements the ability to set the VF MAC to >> 00:00:00:00:00:00 via RTM_SETLINK on the PF, to prevent receiving >> "invlaid argument" when libvirt attempts to restore the MAC address back >> to its original state of 00:00:00:00:00:00. > > This is really a mess and the wrong way to go about this. > > No interface, even a VF, should come up or ever have an invalid > MAC addres like all-zeros. That's the fundamental problem and > once you fix that all of this other crazy logic and workarounds > no longer become necessary. In the case of igbvf the VFs never come up with 0s in their MAC address. An all 0's MAC address basically leaves it open to VF's choice for assigning themselves a MAC address, or at least that is the way I recall coding it back in the day. There are a few issues with making changes to this at this point. The first being that this concept is pretty much baked into the VF driver logic for most drivers supporting legacy SR-IOV, and as pointed out in the patch comments the libvirt interface is writing 0's to disable the VF MAC address when it is not in use. At this point we cannot change this without breaking the libvirt userspace. One of the motivations for clearing this is to avoid having the PF misdirect traffic as having a MAC address mapped to a disabled/unassigned VF could result in traffic being dropped when it should be directed elsewhere such as a bridge on the PF, or out to some other PF that is now running the VM there. > Whatever it takes, just do it. We can even come up with a global > MAC address range that on a Linux system is reserved for VFs to > come up with. That is normally how the VFs handle this on their side. The code was setup such that if the PF provided an all 0's MAC address then the VF would assign itself a locally administered address so that it wouldn't come up with an address of 0s. If you are saying the VFs shouldn't be allowed to come up with an all 0's MAC address I believe that none of them do. I believe they either fail to come up at all or report a locally administered address for themselves. I can double check that though (at least for Intel) to verify that it is in fact a consistent behavior. In theory there isn't likely to be a VF bound to the interface anyway, usually when the MAC address is invalidated it is because a VM has been terminated and the VF driver is just in limbo since it is usually assigned to a VFIO interface which doesn't actually expose the network interface to the kernel. I suppose we could look at pushing the LAA generation up into the PF, but we would still want to maintain the all 0's address while the VF is inactive since we need to clear the stale VF addresses from the MAC address table in the event of a VM being relocated to a different server and taking the MAC address with it. The good news to all this is that this is going to be fading out and going away anyway as SwitchDev takes over for SR-IOV. > Thanks. I'll double check our VF drivers and make sure none of them are exposing a netdevice with an all 0's MAC address, and see what we can do about relocating the locally administered address generation into the PF. Thanks. - Alex
From: Alexander Duyck <alexander.duyck@gmail.com> Date: Wed, 24 Jan 2018 18:29:42 -0800 > I'll double check our VF drivers and make sure none of them are > exposing a netdevice with an all 0's MAC address, and see what we can > do about relocating the locally administered address generation into > the PF. If such netdevs are never exposed with all 0's MACs, then I'm happy. Thanks for the explanation.
This series contains updates to igb and e1000e only. Corinna Vinschen implements the ability to set the VF MAC to 00:00:00:00:00:00 via RTM_SETLINK on the PF, to prevent receiving "invlaid argument" when libvirt attempts to restore the MAC address back to its original state of 00:00:00:00:00:00. Zhang Shengju adds a new function igb_get_max_rss_queues() to get the maxium number of RSS queues and to reduce code duplication. Matt fixes an issue with e1000e where when setting HTHREST, we should only be setting bit 8. Also added a dev_info() message to alert when C-states have been disabled, to help in debugging. Jesus adds code comments to clarify the idlescope configuration constraints. Lyude Paul fixes a kernel crash on hotplug of igb, by checking to ensure that we are in the process of dismantling the netdev on hotplug events. Markus Elfring removes a duplicate message for a memory allocation failure. Daniel Hua fixes an issue where transmit timestamps will stop being put into the socket when link is repeatedly up/down due to TSYNCTXCTL's TXTT bit (Transmit timestamp valid bit) not clearing in the timeout logic of ptp_tx_work(), which in turn causes no new timestamp to be loaded to the TXSTMP register. The following are changes since commit 46410c2efa9cb5b2f40c9ce24a75d147f44aedeb: Merge branch 'pktgen-Behavior-flags-fixes' and are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 1GbE Corinna Vinschen (1): igb: Allow to remove administratively set MAC on VFs Daniel Hua (1): igb: Clear TXSTMP when ptp_tx_work() is timeout Jesus Sanchez-Palencia (1): igb: Clarify idleslope config constraints Lyude Paul (1): igb: Free IRQs when device is hotplugged Markus Elfring (1): igb: Delete an error message for a failed memory allocation in igb_enable_sriov() Matt Turner (2): e1000e: Set HTHRESH when PTHRESH is used e1000e: Alert the user that C-states will be disabled by enabling jumbo frames Zhang Shengju (1): igb: add function to get maximum RSS queues drivers/net/ethernet/intel/e1000e/netdev.c | 4 +- drivers/net/ethernet/intel/igb/igb.h | 1 + drivers/net/ethernet/intel/igb/igb_ethtool.c | 32 +------------ drivers/net/ethernet/intel/igb/igb_main.c | 72 +++++++++++++++++++++------- drivers/net/ethernet/intel/igb/igb_ptp.c | 9 ++++ 5 files changed, 70 insertions(+), 48 deletions(-)