Page MenuHomeFreeBSD

netgraph/ng_vlan_rotate: IEEE 802.1ad VLAN manipulation netgraph node type (new type)
ClosedPublic

Authored by donner on Oct 18 2019, 1:50 PM.

Details

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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
sys/netgraph/ng_vlan_rotate.c
473

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.

Updated to revision 358170.

Updated to revision 358500.

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

gbe added a subscriber: gbe.

LGTM

Updated to revision 367040.

LGTM.

share/man/man4/ng_vlan_rotate.4
2

The "All rights reserved." should be dropped.

sys/netgraph/ng_vlan_rotate.c
5

The "All rights reserved." should be dropped.

sys/netgraph/ng_vlan_rotate.h
5

The "All rights reserved." should be dropped.

afedorov added inline comments.
share/man/man4/ng_vlan_rotate.4
2

While we're here, maybe it should be added: "SPDX-License-Identifier: BSD-2-Clause-FreeBSD"

donner marked 3 inline comments as done.

Updated to latest revision and fixed copyright annotations.

Updated to revision 368146.

Updated to revision 368820.

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

sys/modules/netgraph/vlan_rotate/Makefile
1

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
29

Same.

165

This line is (just) too long.

192

Or maybe just 'else' in the above if/else cascade?

237

Line length.

372

Line length.

424

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
29

Same.

donner marked 7 inline comments as done.
  • Converted from subversion
  • Switch to counter framework for statistics
  • Fix style(9) errors
  • Bump dates
donner added inline comments.
sys/modules/netgraph/vlan_rotate/Makefile
1

So I'll leave it.

sys/netgraph/ng_vlan_rotate.c
424

They are not locked automatically (which is good).
So you are right, I've to switch to the counter framework.

Approved by: kp (mentor)

Just two small comments, looks good otherwise.

share/man/man4/ng_vlan_rotate.4
70

Stray comma after stack.

253

This can simply be .Nm

This revision is now accepted and ready to land.Jan 26 2021, 3:00 PM
This revision now requires review to proceed.Jan 26 2021, 3:07 PM
share/man/man4/ng_vlan_rotate.4
253

You fixed a different .Nm than the one I pointed out ;-) This one in HISTORY can be simply .Nm as well.

donner marked 2 inline comments as done.
  • Fix man page nits.
  • Removed false history from man page.
This revision is now accepted and ready to land.Jan 26 2021, 3:57 PM