On 2019-05-20 11:26 a.m., Edward Cree wrote:
On 18/05/2019 21:39, Jamal Hadi Salim wrote:
On 2019-05-17 1:14 p.m., Edward Cree wrote:
On 17/05/2019 16:27, Edward Cree wrote:
Unless *I* missed something, I'm not changing the TC<=>user-space API at
all. If user space specifies an index, then TC will either create a new
action with that index, or find an existing one. Then flow_offload turns
that into a cookie; in the 'existing action' case it'll be the same
cookie as any previous offloads of that action, in the 'new action' case
it'll be a cookie distinct from any existing action.
That is fine then if i could do:
tc actions add action drop index 104
then
followed by for example the two filters you show below..
Is your hardware not using explicit indices into a stats table?
Drivers aren't interested in the specific index value, only in "which
other actions (counters) I've offloaded are shared with this one?", which
the cookie gives them.
With my (unreleased) driver code, I've successfully tested this with e.g.
the following rules:
tc filter add dev $vfrep parent ffff: protocol arp flower skip_sw \
action vlan push id 100 protocol 802.1q \
action mirred egress mirror dev $pf index 101 \
action vlan pop \
action drop index 104
tc filter add dev $vfrep parent ffff: protocol ipv4 flower skip_sw \
action vlan push id 100 protocol 802.1q \
action mirred egress mirror dev $pf index 102 \
action vlan pop \
action drop index 104
Then when viewing with `tc -stats filter show`, the mirreds count their
traffic separately (and with an extra 4 bytes per packet for the VLAN),
whereas the drops (index 104, shared) show the total count (and without
the 4 bytes).
Beauty. Assuming the stats are being synced to the kernel?
Test 1:
What does "tc -s actions ls action drop index 104" show?
Test 2:
Delete one of the filters above then dump actions again as above.
(From your other email)
tcfa_index + action identifier seem to be sufficiently global, no?
The reason I don't like using the action identifier is because flow_offload
slightly alters those: mirred gets split into two (FLOW_ACTION_REDIRECT
and FLOW_ACTION_MIRRED (mirror)). Technically it'll still work (a redirect
and a mirror are different actions, so can't have the same index, so it
doesn't matter if they're treated as the same action-type or not) but it
feels like a kludge.
As long as uapi semantics are not broken (which you are demonstrating it
is not) then we are good.
cheers,
jamal
-Ed
