]> git.ipfire.org Git - thirdparty/iproute2.git/commit
tc/skbmod: Remove misinformation about the swap action
authorPeilin Ye <peilin.ye@bytedance.com>
Tue, 20 Jul 2021 19:21:45 +0000 (12:21 -0700)
committerStephen Hemminger <stephen@networkplumber.org>
Thu, 22 Jul 2021 22:14:29 +0000 (15:14 -0700)
commitc06d313d86c1acb8dd72589816301853ff5a4ac4
tree7e05672cfd53fc9743c9b19b9bc2d2186d41650d
parent71d36000dc9ce8397fc45b680e0c0340df5a28e5
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>
man/man8/tc-skbmod.8
tc/m_skbmod.c