call for review: 802.11q QinQ netgraph support
Gleb Smirnoff
glebius at FreeBSD.org
Wed Feb 8 10:42:15 UTC 2012
On Tue, Feb 07, 2012 at 03:27:10PM -0800, Adrian Chadd wrote:
A> I've been working with the patch author on this and although I haven't
A> yet had time to test it out myself, he's taken my suggestions on board
A> and continued improving things.
A>
A> The patch can be found in the PR:
A>
A> http://www.freebsd.org/cgi/query-pr.cgi?pr=161908
A>
A> In summary, he's added the ability to support q-in-q tags, as well as
A> maintaining backwards compatibility for existing users.
A>
A> I'd like to commit this at the end of the week.
A>
A> He's indicated that he will take care of any issues it may break. I'll
A> back it out if it breaks things and isn't fixed.
A>
A> Ivan - thank you for being so patient!
A>
A> I would appreciate further review from network/netgraph related
A> people. I'm going to borrow a term from gnn and say "Silence implies
A> consent." :-)
I have only minor comments:
1.1) According to style(9) new code should use uintXX_t instead of u_intXX_t.
1.2) Some lines are really loooong, they need to be broken into shorter ones
accoording to style(9).
1.3) Operators at beginning of a line - also style(9) violation.
2) NETGRAPH_DEBUG wasn't designed for the things the patch is doing. But
the KASSERT was. So it'll be better to change the code under NETGRAPH_DEBUG
to KASSERTs. For example in the chunk '@@ -262,35 +322,143 @@':
KASSERT(priv->vlan_hook[EVL_VLANOFTAG(hook_data)] == hook,
("%s: NGM_VLAN_DEL_FILTER: Invalid VID for Hook = %s\n",
__func__, (char *)msg->data));
and
KASSERT(EVL_VLANOFTAG(hook_data) == vid,
("%s: NGM_VLAN_DEL_VID_FLT: Invalid VID Hook = %us, must be: %us\n",
__func__, (uint16_t )EVL_VLANOFTAG(hook_data), vid));
--
Totus tuus, Glebius.
More information about the freebsd-net
mailing list