Details

Reviewers

brueffer
julian
glebius
kp

Group Reviewers

manpages
network

Commits

rGb2c14db4f8fd: netgraph/ng_vlan_rotate: IEEE 802.1ad VLAN manipulation netgraph type
rG522a0b7d029f: netgraph/ng_vlan_rotate: IEEE 802.1ad VLAN manipulation netgraph type
rGa0e2fe87031a: netgraph/ng_vlan_rotate: IEEE 802.1ad VLAN manipulation netgraph type
rGcfd6422a5217: netgraph/ng_vlan_rotate: IEEE 802.1ad VLAN manipulation netgraph type

Summary

This node is part of an A10-NSP development.

Carrier networks tend to stack three or more tags for internal purposes and therefore hiding the service tags deep inside of the stack. When decomposing such an access network frame, the processing order is typically reversed: First distinguish by service, than by other means.

This new netgragh node allows to bring the relevant VLAN in front (to the out-most position). This way other netgraph nodes (like ng_vlan) can operate on this specific type.

Test Plan

The example section of the man page contains two examples, how to use this node.
It also contains the sniffer insight of the manipulation process.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 36508
Build 33397: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

kevans added inline comments.Jan 3 2020, 1:58 PM

sys/netgraph/ng_vlan_rotate.c
474	Operator belongs to the line before it, with the wrapping starting just after.

Run indent(1) on the source files.
Reduced comments from the boilerplate.
Changed hard coded ethertypes by global definitions introduced by D21846.

Harbormaster completed remote builds in B28482: Diff 66351.Jan 4 2020, 9:09 PM

donner marked 7 inline comments as done.Jan 4 2020, 9:12 PM

Rebase to r357812

Harbormaster completed remote builds in B29305: Diff 68169.Feb 12 2020, 11:35 AM

rebase to r358008.

Herald added a subscriber: melifaro. · View Herald TranscriptFeb 16 2020, 10:57 PM

Harbormaster completed remote builds in B29434: Diff 68430.Feb 16 2020, 10:58 PM

Updated to revision 358170.

Harbormaster completed remote builds in B29512: Diff 68601.Feb 20 2020, 4:24 PM

Updated to revision 358500.

Harbormaster completed remote builds in B29706: Diff 69032.Mar 1 2020, 9:05 PM

We are going to hard production now.
Any interest in reviewing it?

@brueffer Are your concerns resolved?

Rebase to r366170.

Harbormaster completed remote builds in B33829: Diff 77548.Sep 25 2020, 7:28 PM

LGTM

donner mentioned this in D26436: Add support for stacked VLANs (IEEE 802.1ad, AKA Q-in-Q)..Oct 21 2020, 3:22 PM

Updated to revision 367040.

Harbormaster completed remote builds in B34379: Diff 78742.Oct 25 2020, 4:54 PM

LGTM.

share/man/man4/ng_vlan_rotate.4
3	The "All rights reserved." should be dropped.
sys/netgraph/ng_vlan_rotate.c
6	The "All rights reserved." should be dropped.
sys/netgraph/ng_vlan_rotate.h
6	The "All rights reserved." should be dropped.

afedorov added a subscriber: afedorov.Nov 19 2020, 8:04 PM

afedorov added inline comments.

share/man/man4/ng_vlan_rotate.4
3	While we're here, maybe it should be added: "SPDX-License-Identifier: BSD-2-Clause-FreeBSD"

afedorov added a reviewer: glebius.Nov 19 2020, 8:18 PM

Updated to latest revision and fixed copyright annotations.

Harbormaster completed remote builds in B34926: Diff 79796.Nov 20 2020, 10:47 AM

Mark annotations as done.

Updated to revision 368146.

Harbormaster completed remote builds in B35104: Diff 80115.Nov 29 2020, 1:33 PM

Updated to revision 368820.

Harbormaster completed remote builds in B35689: Diff 81162.Dec 24 2020, 10:33 PM

Couple really minor (and mostly style(9) remarks, other than those I think it's good.

sys/modules/netgraph/vlan_rotate/Makefile
2	This tag is no longer required in the git world. If you're planning to MFC this to stable/12 it may be useful to just leave it, because it does still do something in svn.
sys/netgraph/ng_vlan_rotate.c
30	Same.
166	This line is (just) too long.
193	Or maybe just 'else' in the above if/else cascade?
238	Line length.
373	Line length.
425	Are netgraph nodes implicitly locked? If not, this could potentially miscount if two cores run the function at the same time. Not a huge issue, but perhaps counter(9) would be useful here.
sys/netgraph/ng_vlan_rotate.h
30	Same.

Converted from subversion
Switch to counter framework for statistics
Fix style(9) errors
Bump dates

Harbormaster completed remote builds in B36492: Diff 82897.Jan 26 2021, 8:44 AM

Fix spelling error.

Harbormaster completed remote builds in B36493: Diff 82898.Jan 26 2021, 8:45 AM

donner marked an inline comment as done.Jan 26 2021, 8:45 AM

donner added inline comments.

sys/modules/netgraph/vlan_rotate/Makefile
2	So I'll leave it.
sys/netgraph/ng_vlan_rotate.c
425	They are not locked automatically (which is good). So you are right, I've to switch to the counter framework.