Bug 236584 - em(4): Cannot disable vlanhwtag
Summary: em(4): Cannot disable vlanhwtag
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 11.2-STABLE
Hardware: amd64 Any
: Normal Affects Many People
Assignee: Vincenzo Maffione
URL: https://reviews.freebsd.org/D25286
Keywords: easy, needs-qa
Depends on:
Blocks:
 
Reported: 2019-03-16 22:55 UTC by Murat
Modified: 2022-04-25 07:15 UTC (History)
6 users (show)

See Also:
koobs: maintainer-feedback? (freebsd)
erj: maintainer-feedback+
vmaffione: mfc-stable12+


Attachments
patch to fix ifconfig vlanhwcsum parameter handling for em (492 bytes, patch)
2020-06-15 16:58 UTC, Murat
no flags Details | Diff
em vlanhwtag patch tests (4.33 KB, text/plain)
2020-08-06 00:01 UTC, Murat
no flags Details
dmesg.boot file for the test PC (53.69 KB, text/plain)
2020-08-06 00:02 UTC, Murat
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Murat 2019-03-16 22:55:49 UTC
Please see the following:

[root@bsd11 ~]# uname -a
FreeBSD bsd11.2 11.2-STABLE FreeBSD 11.2-STABLE #0: Sat Dec 22 11:34:16 PST 2018     root@bsd11.2:/usr/obj/usr/src/sys/GENERIC  amd64

[root@bsd11 ~]# ifconfig em0
em0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
	options=209b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,WOL_MAGIC>
	ether 52:54:00:8a:e5:33
	hwaddr 52:54:00:8a:e5:33
	inet 0.0.0.0 netmask 0xff000000 broadcast 255.255.255.255 
	nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
	media: Ethernet autoselect (1000baseT <full-duplex>)
	status: active

[root@bsd11 ~]# ifconfig em0 -vlanhwtso -vlanhwfilter -vlanhwtag -vlanhwcsum

[root@bsd11 ~]# ifconfig em0
em0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
	options=208b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWCSUM,WOL_MAGIC>
	ether 52:54:00:8a:e5:33
	hwaddr 52:54:00:8a:e5:33
	inet 0.0.0.0 netmask 0xff000000 broadcast 255.255.255.255 
	nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
	media: Ethernet autoselect (1000baseT <full-duplex>)
	status: active

[root@bsd11 ~]# ifconfig em0 -vlanhwcsum

[root@bsd11 ~]# ifconfig em0
em0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
	options=208b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWCSUM,WOL_MAGIC>
	ether 52:54:00:8a:e5:33
	hwaddr 52:54:00:8a:e5:33
	inet 0.0.0.0 netmask 0xff000000 broadcast 255.255.255.255 
	nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
	media: Ethernet autoselect (1000baseT <full-duplex>)
	status: active
[root@bsd11 ~]#
Comment 1 Murat 2020-06-15 16:58:44 UTC
Created attachment 215584 [details]
patch to fix ifconfig vlanhwcsum parameter handling for em
Comment 2 Murat 2020-06-15 17:05:54 UTC
Hi,

It looks like this bug effects quite many FreeBSD versions. This was present in FreeBSD 11.x and I could confirm this is present in FreeBSD 12.x

Fix looks very easy. 

Impact will be huge since em(4) driver is being used by many hypervisors (QEMU, Proxmox, VirtualBox, VMware) and you can find this adapter oboard in many COTS hardware. 

Most obvious effect is you cannot run netmap(4) on em VLAN interfaces.
Comment 3 Murat 2020-06-15 17:07:18 UTC
Related netmap(4) PR:

https://github.com/luigirizzo/netmap/issues/703
Comment 4 Vincenzo Maffione freebsd_committer freebsd_triage 2020-06-15 20:13:31 UTC
Review created
https://reviews.freebsd.org/D25286
However, it's not clear if this is about the vlanhwcsum flag or the vlanhwtag flag (as stated on the github).
Comment 5 Murat 2020-06-16 00:11:42 UTC
Vincenzo,

It should be as stated in the netmap ticket: vlanhwtag
Comment 6 Murat 2020-08-05 02:59:17 UTC
Vincenzo,

It looks like the proposed diff has already been integrated into 12/STABLE. 

As of now, we have been reported that below chip ids working with VLANs if the interface is in netmap mode.

0x156f8086
0x100e8086

Below chip id still seems to be not working with VLANs:
0x10d38086

Any thoughts?
Comment 7 Vincenzo Maffione freebsd_committer freebsd_triage 2020-08-05 18:01:10 UTC
(In reply to Murat from comment #6)
I just checked in the code, and I don't see the change in the stable/12 tree, nor in HEAD.

Why did you say it has been integrated already?

If however you can do some tests on real em hardware, to check the patch does not introduce any regression, independently on netmap, I will gladly merge this into HEAD.
Comment 8 Murat 2020-08-06 00:00:53 UTC
Hi Vincenzo,

Sorry, my bad. Working with different source bases now, I had a look at another directory.

I've done some tests on an HP Compaq 6305 SFF Desktop. 

Test results and HW configuration is attached. 

Please feel free to ask, if you need more tests.
Comment 9 Murat 2020-08-06 00:01:42 UTC
Created attachment 217046 [details]
em vlanhwtag patch tests
Comment 10 Murat 2020-08-06 00:02:24 UTC
Created attachment 217047 [details]
dmesg.boot file for the test PC
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2020-08-06 01:49:02 UTC
^Triage:

- Leave Version at earliest reported/reproduced version
- Track MFC requests
- Assign to committer resolving

@Murat / Luigi To what extent might this apply to other drivers (say; other Intel network drivers) ?
Comment 12 Murat 2020-08-06 01:50:52 UTC
@Kubilay, it appears this only affects em(4).
Comment 13 Vincenzo Maffione freebsd_committer freebsd_triage 2020-08-06 20:29:55 UTC
(In reply to Murat from comment #8)
Looks good to me.
Thanks.
I'll commit to head, giving some time for people to detect possible regressions.
Comment 14 Vincenzo Maffione freebsd_committer freebsd_triage 2020-08-06 20:50:40 UTC
(In reply to Kubilay Kocak from comment #11)
Looking at the ixgbe(4) driver, I see the same VME bit, and the same code pattern as in em(4). So it is possible in principle that the same issue affect also ixgbe(4).

However, the NIC may be just different, not needing an explicit reset of the VME bit at interface initialization time. If anyone has some ixgbe(4) card and is willing to test, they could try to play with VLANs, toggling the vlanhwtag ifconfig features on and off to check that things keep working.
Comment 15 commit-hook freebsd_committer freebsd_triage 2020-08-06 21:02:22 UTC
A commit references this bug:

Author: vmaffione
Date: Thu Aug  6 21:01:27 UTC 2020
New revision: 363995
URL: https://svnweb.freebsd.org/changeset/base/363995

Log:
  em(4): honor vlanhwtag offload

  The FreeBSD em driver fails to properly reset the VME flag
  in the e1000 CTRL register oneg the following ifconfig command

  	ifconfig em1 -vlanhwtag

  Tested on the e1000 device emulated by QEMU, and on a real
  NIC (chip=0x10d38086).

  PR:	236584
  Submitted by:	 murat@sunnyvalley.io
  Reported by:	 murat@sunnyvalley.io
  MFC after:	3 weeks
  Differential Revision:	https://reviews.freebsd.org/D25286

Changes:
  head/sys/dev/e1000/if_em.c
Comment 16 Kubilay Kocak freebsd_committer freebsd_triage 2020-08-07 05:22:18 UTC
(In reply to Vincenzo Maffione from comment #14)

^Triage: Request feedback from our Intel ports (kmod) maintainers and Eric (erj)
Comment 17 Eric Joyner freebsd_committer freebsd_triage 2020-08-26 16:41:04 UTC
(In reply to Vincenzo Maffione from comment #14)

This should be something that is investigated on ixgbe(4), but I can't do it atm.

In reply to the request for feedback, I think this change is good and does what it's supposed to, looking at the documentation for the VME bit in E1000_CTRL. Is the port feedback asking if this should be added to the port?
Comment 18 commit-hook freebsd_committer freebsd_triage 2020-08-27 19:18:23 UTC
A commit references this bug:

Author: vmaffione
Date: Thu Aug 27 19:12:40 UTC 2020
New revision: 364878
URL: https://svnweb.freebsd.org/changeset/base/364878

Log:
  MFC r363995

  em(4): honor vlanhwtag offload

  The FreeBSD em driver fails to properly reset the VME flag
  in the e1000 CTRL register oneg the following ifconfig command

      ifconfig em1 -vlanhwtag

  Tested on the e1000 device emulated by QEMU, and on a real
  NIC (chip=0x10d38086).

  PR:     236584
  Submitted by:    murat@sunnyvalley.io
  Reported by:     murat@sunnyvalley.io
  Differential Revision:  https://reviews.freebsd.org/D25286

Changes:
_U  stable/12/
  stable/12/sys/dev/e1000/if_em.c
Comment 19 commit-hook freebsd_committer freebsd_triage 2020-08-27 19:18:25 UTC
A commit references this bug:

Author: vmaffione
Date: Thu Aug 27 19:15:09 UTC 2020
New revision: 364879
URL: https://svnweb.freebsd.org/changeset/base/364879

Log:
  MFC r363995

  em(4): honor vlanhwtag offload

  The FreeBSD em driver fails to properly reset the VME flag
  in the e1000 CTRL register oneg the following ifconfig command

      ifconfig em1 -vlanhwtag

  Tested on the e1000 device emulated by QEMU, and on a real
  NIC (chip=0x10d38086).

  PR:     236584
  Submitted by:    murat@sunnyvalley.io
  Reported by:     murat@sunnyvalley.io
  Differential Revision:  https://reviews.freebsd.org/D25286

Changes:
_U  stable/11/
  stable/11/sys/dev/e1000/if_em.c
Comment 20 Vincenzo Maffione freebsd_committer freebsd_triage 2022-04-25 07:15:17 UTC
Fix committed and MFCed. I think we can close.