Bug 219390 - [ixgbe] [patch] ixgbe stripping vlan_tag by default
Summary: [ixgbe] [patch] ixgbe stripping vlan_tag by default
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 11.1-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Sean Bruno
URL: https://reviews.freebsd.org/D12795
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-05-18 21:28 UTC by Charles Goncalves
Modified: 2019-01-23 02:32 UTC (History)
8 users (show)

See Also:


Attachments
if_ix.c is missing vlan hwtagging check (419 bytes, patch)
2017-05-18 21:28 UTC, Charles Goncalves
no flags Details | Diff
Patch for FreeBSD 11.1-STABLE (420 bytes, patch)
2017-10-18 13:54 UTC, Charles Goncalves
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Charles Goncalves 2017-05-18 21:28:36 UTC
Created attachment 182723 [details]
if_ix.c is missing vlan hwtagging check

There is a conditional check missing in if_ix.c file in ixgbe_init_locked() function before ixgbe_setup_vlan_hw_support() call. It lets to strip vlan tag (vlanhwtag) even if the option is disabled on interface.

My patch attached fix this issue.
Comment 1 Cassiano Peixoto 2017-05-19 11:54:53 UTC
Hi,

It's a long time issue. I'm glad you sent a patch, i've tried and worked well. Could some committer take care of this?
Comment 2 Cassiano Peixoto 2017-10-18 11:55:50 UTC
Please, can some committer take care of this? It's a quite simple patch.

Thanks.
Comment 3 Jeff Pieper 2017-10-18 13:08:31 UTC
This is against 10.3-STABLE. The driver has been refactored quite a bit since this was filed. Have you retested this against a current -STABLE branch?
Comment 4 Charles Goncalves 2017-10-18 13:54:41 UTC
Created attachment 187278 [details]
Patch for FreeBSD 11.1-STABLE
Comment 5 Charles Goncalves 2017-10-18 14:02:20 UTC
(In reply to Jeff Pieper from comment #3)

Hello Jeff!

I have tested on FreeBSD 11.1-STABLE and the same error happened. In fact I updated my patch to FreeBSD 11.1-STABLE.

Can you check that?
Comment 6 Cassiano Peixoto 2017-10-18 15:34:24 UTC
(In reply to Charles Goncalves from comment #5)
Same here. I just tested and the patch still needed.
Comment 7 Krzysztof Galazka 2017-10-20 16:44:03 UTC
(In reply to Charles Goncalves from comment #5)

Hi Charles!

Thank you for the patch. Could you provide me the reproduction steps and what NIC do you use? The ixgbe_setup_vlan_hw_support() is used also in ixgbe_(un)register_vlan() event handlers so I'd like to check if it covers all use cases.

Thanks!
Comment 8 Charles Goncalves 2017-10-20 17:37:10 UTC
(In reply to Krzysztof Galazka from comment #7)
Hello Krzysztof!

I'm using: Intel 82599ES 10-Gigabit SFI/SFP+ (chip=0x10fb8086)

I have two machines with crossover connection.

test01 <-> test02

On test01 I have ixgbe driver patched and on test02 I don't have.

On both machines there is a vlan10 over an ix NIC.

Step 1: on machine test02 do a ping to test01 like: ping 10.0.0.1
Step 2: on machine test01 do a tcpdump like: tcpdump -eni netmap:ix0

On Step 2 I'm seeing tcpdump output like: ethertype 802.1Q (0x8100). This is an expected behavior.

Step 3: on machine test01 do a ping to test02 like: ping 10.0.0.2
Step 4: on machine test02 do a tcpdump like: tcpdump -eni netmap:ix0
On Step 4 I'm seeing tcpdump output like: ethertype IPv4 (0x0800). This is a strange behavior.

My inspiration is from em1000 driver: https://github.com/freebsd/freebsd/blob/bb993c697ac4f18efa742ac69db425a5e0351117/sys/dev/e1000/if_em.c#L1246+L1249

I hope these steps are clear. If you have any further doubts feel free to contact me.
Comment 9 Piotr Pietruszewski 2017-10-26 10:56:02 UTC
(In reply to Charles Goncalves from comment #8)

Thank you for reproduction steps. We discovered that your patch doesn't cover all scenarios. Fixed patch is now under review ( https://reviews.freebsd.org/D12795 ).
Comment 10 Charles Goncalves 2017-10-26 11:44:08 UTC
(In reply to Piotr Pietruszewski from comment #9)

Hello Piotr!

Thank you for your answer. I have tested your patch in my test scenario and it worked as expected. 

Thank you very much!
Comment 11 commit-hook freebsd_committer freebsd_triage 2017-11-06 18:09:16 UTC
A commit references this bug:

Author: sbruno
Date: Mon Nov  6 18:09:00 UTC 2017
New revision: 325492
URL: https://svnweb.freebsd.org/changeset/base/325492

Log:
  Fix ixgbe(4) support for ifconfig's vlanhwtag flag.  Disabling this flag
  will now prevent the driver from stripping vlan tags from packets.

  PR:		219390
  Submitted by:	Piotr Pietruszewski <piotr.pietruszewski@intel.com>
  Reported by:	Charles Goncalves <halfling@halfling.com.br>
  Obtained from:	1 week
  Sponsored by:	Intel Corporation
  Differential Revision:	https://reviews.freebsd.org/D12795

Changes:
  head/sys/dev/ixgbe/if_ix.c
Comment 12 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2019-01-21 19:33:48 UTC
There is a commit referencing this PR, but it's still not closed and has been inactive for some time. Closing the PR as fixed but feel free to re-open it if the issue hasn't been completely resolved.

Thanks
Comment 13 Kubilay Kocak freebsd_committer freebsd_triage 2019-01-23 02:32:43 UTC
Assign to committer that resolved, who can (in addition to the original reporter) re-open if necessary