matthias
/
iproute2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

tc/skbmod: Remove misinformation about the swap action 

Currently man 8 tc-skbmod says that "...the swap action will occur after
any smac/dmac substitutions are executed, if they are present."

This is false.  In fact, trying to "set" and "swap" in a single skbmod
command causes the "set" part to be completely ignored.  As an example:

	$ tc filter add dev eth0 parent 1: protocol ip prio 10 \
		matchall action skbmod \
        	set dmac AA:AA:AA:AA:AA:AA smac BB:BB:BB:BB:BB:BB \
        	swap mac

The above command simply does a "swap", without setting DMAC or SMAC to
AA's or BB's.  The root cause of this is in the kernel, see
net/sched/act_skbmod.c:tcf_skbmod_init():

	parm = nla_data(tb[TCA_SKBMOD_PARMS]);
	index = parm->index;
	if (parm->flags & SKBMOD_F_SWAPMAC)
		lflags = SKBMOD_F_SWAPMAC;
		^^^^^^^^^^^^^^^^^^^^^^^^^^

Doing a "=" instead of "|=" clears all other "set" flags when doing a
"swap".  Discourage using "set" and "swap" in the same command by
documenting it as undefined behavior, and update the "SYNOPSIS" section
as well as tc -help text accordingly.

If one really needs to e.g. "set" DMAC to all AA's then "swap" DMAC and
SMAC, one should do two separate commands and "pipe" them together.

Reviewed-by: Cong Wang <cong.wang@bytedance.com>
Signed-off-by: Peilin Ye <peilin.ye@bytedance.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
This commit is contained in:
Peilin Ye 2021-07-20 12:21:45 -07:00 committed by Stephen Hemminger
parent 71d36000dc
commit c06d313d86
2 changed files with 15 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
man/man8/tc-skbmod.8
View File

@ -5,12 +5,12 @@ skbmod - user-friendly packet editor action
.SH SYNOPSIS
.in +8
.ti -8
.BR tc " ... " "action skbmod " "{ [ " "set "
.IR SETTABLE " ] [ "
.BR tc " ... " "action skbmod " "{ " "set "
.IR SETTABLE " | "
.BI swap " SWAPPABLE"
.RI " ] [ " CONTROL " ] [ "
.RI " } [ " CONTROL " ] [ "
.BI index " INDEX "
] }
]
.ti -8
.IR SETTABLE " := "
@ -25,6 +25,7 @@ skbmod - user-friendly packet editor action
.IR SWAPPABLE " := "
.B mac
.ti -8
.IR CONTROL " := {"
.BR reclassify " | " pipe " | " drop " | " shot " | " continue " | " pass " }"
.SH DESCRIPTION
@ -48,10 +49,7 @@ Change the source mac to the specified address.
Change the ethertype to the specified value.
.TP
.BI mac
Used to swap mac addresses. The
.B swap mac
directive is performed
after any outstanding D/SMAC changes.
Used to swap mac addresses.
.TP
.I CONTROL
The following keywords allow to control how the tree of qdisc, classes,
@ -128,9 +126,13 @@ tc filter add dev eth3 parent 1: protocol ip prio 10 \\
.EE
.RE
As mentioned above, the swap action will occur after any
.B " smac/dmac "
substitutions are executed, if they are present.
However, trying to
.B set
and
.B swap
in a single
.B skbmod
command will cause undefined behavior.
.SH SEE ALSO
.BR tc (8),

5
tc/m_skbmod.c
View File

@ -28,10 +28,9 @@
static void skbmod_explain(void)
{
fprintf(stderr,
"Usage:... skbmod {[set <SETTABLE>] [swap <SWAPABLE>]} [CONTROL] [index INDEX]\n"
"Usage:... skbmod { set <SETTABLE> | swap <SWAPPABLE> } [CONTROL] [index INDEX]\n"
"where SETTABLE is: [dmac DMAC] [smac SMAC] [etype ETYPE]\n"
"where SWAPABLE is: \"mac\" to swap mac addresses\n"
"note: \"swap mac\" is done after any outstanding D/SMAC change\n"
"where SWAPPABLE is: \"mac\" to swap mac addresses\n"
"\tDMAC := 6 byte Destination MAC address\n"
"\tSMAC := optional 6 byte Source MAC address\n"
"\tETYPE := optional 16 bit ethertype\n"