On Fri, Mar 06, 2026 at 11:22:51AM +0100, Ilya Maximets wrote: > Current output of ofproto/detrace doesn't provide information on what > bridge the rules and groups belong to. This may be confusing in > setups with multiple bridges, especially when packets are resubmitted > multiple times through different patch ports. > > Translation cache does have the bridge information though as every > time we lookup the rule we add the XC_TABLE entry that has the bridge > information. We also know when the rule lookup was executed but > didn't result in a match. We can report that as well, so it's easier > to follow the resubmits. > > Note: On a miss, the packet hits the internal drop rule in table 254. > By reporting the fact that there was no match it's easier to > understand why that internal rule is getting hit. > > There is no extra overhead for providing the bridge information as the > data is already there, we just need to print it out. Retis, that is > collecting the detrace output, should also be able to take advantage > of this enhancement.
Tested this on Retis and it "just works" :-) > > Signed-off-by: Ilya Maximets <[email protected]> > --- > ofproto/ofproto-dpif-xlate-cache.c | 15 +++++++++ > tests/system-traffic.at | 51 ++++++++++++++++++++++++------ > utilities/checkpatch_dict.txt | 1 + > 3 files changed, 57 insertions(+), 10 deletions(-) > > diff --git a/ofproto/ofproto-dpif-xlate-cache.c > b/ofproto/ofproto-dpif-xlate-cache.c > index c6d935cf0..a225a1b70 100644 > --- a/ofproto/ofproto-dpif-xlate-cache.c > +++ b/ofproto/ofproto-dpif-xlate-cache.c > @@ -305,6 +305,7 @@ xlate_cache_steal_entries(struct xlate_cache *dst, struct > xlate_cache *src) > void > xlate_xcache_format(struct ds *s, const struct xlate_cache *xcache) > { > + struct ofproto_dpif *last_ofproto = NULL; > struct ofpbuf entries = xcache->entries; > struct xc_entry *entry; > struct ofgroup *ofg; > @@ -312,16 +313,30 @@ xlate_xcache_format(struct ds *s, const struct > xlate_cache *xcache) > XC_ENTRY_FOR_EACH (entry, &entries) { > switch (entry->type) { > case XC_RULE: > + ds_put_cstr(s, " "); > ofproto_rule_stats_ds(s, &entry->rule->up, true); > break; > + > case XC_GROUP: > + ds_put_cstr(s, " "); This results in a slightly weird output because groups with dp_hash will result in a recirculation with frozen_actions. In these cases, after thrawing there is no rule lookup and therefore, no XC_TABLE entry, but the action contains the OFP_GROUP which generates the XC_GROUP entry. The result is ofproto/detrace shows an indented" group entry with no "parent" bridge. The unit tests is a perfect example of this. Maybe something like this would help: --- i/ofproto/ofproto-dpif-xlate-cache.c +++ w/ofproto/ofproto-dpif-xlate-cache.c @@ -318,8 +318,14 @@ xlate_xcache_format(struct ds *s, const struct xlate_cache *xcache) break; case XC_GROUP: - ds_put_cstr(s, " "); ofg = &entry->group.group->up; + struct ofproto_dpif *group_ofproto = + CONTAINER_OF(ofg->ofproto, struct ofproto_dpif, up); + if (last_ofproto != group_ofproto) { + ds_put_format(s, "%s\n", group_ofproto->up.name); + last_ofproto = group_ofproto; + } + ds_put_cstr(s, " "); ofputil_group_format(s, ofg->group_id, ofg->type, entry->group.bucket, &ofg->buckets, &ofg->props, OFP15_VERSION, > ofg = &entry->group.group->up; > ofputil_group_format(s, ofg->group_id, ofg->type, > entry->group.bucket, &ofg->buckets, > &ofg->props, OFP15_VERSION, > false, NULL, NULL); > break; > + > case XC_TABLE: > + if (last_ofproto != entry->table.ofproto) { > + ds_put_format(s, "%s\n", entry->table.ofproto->up.name); > + last_ofproto = entry->table.ofproto; > + } > + if (!entry->table.match) { > + ds_put_format(s, " table_id=%"PRIu8", no match\n", > + entry->table.id); > + } > + break; > + > case XC_BOND: > case XC_NETDEV: > case XC_NETFLOW: > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index 8f4fdf8b1..ea0dc5ec3 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -2733,7 +2733,12 @@ OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > AT_SETUP([datapath - Dump OF rules corresponding to UFID]) > -OVS_TRAFFIC_VSWITCHD_START() > +OVS_TRAFFIC_VSWITCHD_START( > + [_ADD_BR([br1]) -- \ > + add-port br0 patch1 -- set interface patch1 type=patch \ > + options:peer=patch0 ofport_request=3 -- \ > + add-port br1 patch0 -- set interface patch0 type=patch \ > + options:peer=patch1 ofport_request=4 --]) > > ADD_NAMESPACES(at_ns0, at_ns1) > > @@ -2749,14 +2754,24 @@ > group_id=1,type=select,selection_method=dp_hash,bucket=bucket_id:0,weight:100,ac > ]) > AT_DATA([flows.txt], [dnl > table=0,arp,actions=NORMAL > +table=0,priority=100,cookie=0x1234abcd,in_port=patch1,ip,nw_dst=10.0.0.2,actions=resubmit(,4) > > table=0,priority=100,cookie=0x12345678,in_port=ovs-p1,ip,nw_dst=10.0.0.2,actions=resubmit(,1) > > table=0,priority=100,cookie=0xabcedf,in_port=ovs-p2,ip,nw_dst=10.0.0.1,actions=ct(table=3) > table=1,priority=200,ip,actions=group:1 > -table=2,ip,actions=ovs-p2 > +table=2,ip,actions=ovs-p2,patch1 > table=3,ip,actions=ovs-p1 > +table=4,ip,actions=drop > +]) > +AT_DATA([flows1.txt], [dnl > +table=0,arp,actions=drop > +table=0,actions=clone(resubmit(,1)),resubmit(,2) > +table=1,ip,actions=set_field:10.0.0.3->nw_dst,in_port > +table=2,ip,actions=in_port > ]) > + > AT_CHECK([ovs-ofctl add-groups br0 groups.txt]) > AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > +AT_CHECK([ovs-ofctl add-flows br1 flows1.txt]) > > NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.1 -W 2 10.0.0.2 | FORMAT_PING], > [0], [dnl > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > @@ -2782,8 +2797,8 @@ AT_CHECK([ovs-appctl dpctl/dump-flows --names | grep > "ipv4" | strip_used | strip > strip_duration | strip_dp_hash | strip_recirc | sed > 's/:never/:0.0s/' | \ > strip_ptype | strip_eth | sort], [0], [dnl > > recirc_id(<recirc>),in_port(ovs-p1),eth_type(0x0800),ipv4(dst=10.0.0.2,frag=no), > packets:0, bytes:0, used:0.0s, actions:hash(l4(0)),recirc(<recirc>) > +recirc_id(<recirc>),in_port(ovs-p1),eth_type(0x0800),ipv4(dst=10.0.0.2,proto=1,frag=no), > packets:0, bytes:0, used:0.0s, actions:ovs-p2 > recirc_id(<recirc>),in_port(ovs-p1),eth_type(0x0800),ipv4(frag=no), > packets:0, bytes:0, used:0.0s, actions:ct(commit),recirc(<recirc>) > -recirc_id(<recirc>),in_port(ovs-p1),eth_type(0x0800),ipv4(frag=no), > packets:0, bytes:0, used:0.0s, actions:ovs-p2 > > recirc_id(<recirc>),in_port(ovs-p2),eth_type(0x0800),ipv4(dst=10.0.0.1,frag=no), > packets:0, bytes:0, used:0.0s, actions:ct,recirc(<recirc>) > recirc_id(<recirc>),in_port(ovs-p2),eth_type(0x0800),ipv4(frag=no), > packets:0, bytes:0, used:0.0s, actions:ovs-p1 > ]) > @@ -2791,32 +2806,48 @@ > recirc_id(<recirc>),in_port(ovs-p2),eth_type(0x0800),ipv4(frag=no), > packets:0, b > ufid=$(ovs-appctl dpctl/dump-flows -m > filter='recirc_id(0),in_port(ovs-p1),ipv4' | parse_ufid) > AT_CHECK([ovs-appctl ofproto/detrace $ufid $pmd_id | ofctl_strip | \ > sed -E 's/n_bytes=2(66|80),/n_bytes=294,/'], [0], [dnl > -cookie=0x12345678, n_packets=3, n_bytes=294, > priority=100,ip,in_port=1,nw_dst=10.0.0.2,actions=resubmit(,1) > -table_id=1, n_packets=3, n_bytes=294, priority=200,ip,actions=group:1 > +br0 > + cookie=0x12345678, n_packets=3, n_bytes=294, > priority=100,ip,in_port=1,nw_dst=10.0.0.2,actions=resubmit(,1) > + table_id=1, n_packets=3, n_bytes=294, priority=200,ip,actions=group:1 > ]) > > ufid=$(ovs-appctl dpctl/dump-flows -m filter='in_port(ovs-p1),ipv4' | grep > "ct(commit)" | parse_ufid) > AT_CHECK([ovs-appctl ofproto/detrace $ufid $pmd_id | ofctl_strip | \ > sed -E 's/n_bytes=2(66|80),/n_bytes=294,/'], [0], [dnl > -group_id=1,type=select,selection_method=dp_hash,bucket=bucket_id:0,weight:100,actions=ct(commit,table=2) > + > group_id=1,type=select,selection_method=dp_hash,bucket=bucket_id:0,weight:100,actions=ct(commit,table=2) This is the weird output I was refering to in the above comment. > ]) > > ufid=$(ovs-appctl dpctl/dump-flows -m filter='in_port(ovs-p1),ipv4' | grep > "actions:ovs-p2" | parse_ufid) > AT_CHECK([ovs-appctl ofproto/detrace $ufid $pmd_id | ofctl_strip | \ > - sed -E 's/n_bytes=2(66|80),/n_bytes=294,/'], [0], [dnl > -table_id=2, n_packets=3, n_bytes=294, ip,actions=output:2 > + sed -E 's/n_bytes=2(66|80),/n_bytes=294,/' | \ > + sed 's/table_id=254, n_packets=[[0-9]]*, > n_bytes=[[0-9]]*,/table_id=254,/'], [0], [dnl > +br0 > + table_id=2, n_packets=3, n_bytes=294, ip,actions=output:2,output:3 > +br1 > + n_packets=3, n_bytes=294, ,actions=clone(resubmit(,1)),resubmit(,2) > + table_id=1, n_packets=3, n_bytes=294, > ip,actions=mod_nw_dst:10.0.0.3,IN_PORT > +br0 > + table_id=0, no match > + table_id=254, priority=0,reg0=0x2,actions=drop > +br1 > + table_id=2, n_packets=3, n_bytes=294, ip,actions=IN_PORT > +br0 > + cookie=0x1234abcd, n_packets=3, n_bytes=294, > priority=100,ip,in_port=3,nw_dst=10.0.0.2,actions=resubmit(,4) > + table_id=4, ip,actions=drop > ]) > > ufid=$(ovs-appctl dpctl/dump-flows -m > filter='recirc_id(0),in_port(ovs-p2),ipv4' | grep "ct," | parse_ufid) > AT_CHECK([ovs-appctl ofproto/detrace $ufid $pmd_id | ofctl_strip | \ > sed -E 's/n_bytes=2(66|80),/n_bytes=294,/'], [0], [dnl > -cookie=0xabcedf, n_packets=3, n_bytes=294, > priority=100,ip,in_port=2,nw_dst=10.0.0.1,actions=ct(table=3) > +br0 > + cookie=0xabcedf, n_packets=3, n_bytes=294, > priority=100,ip,in_port=2,nw_dst=10.0.0.1,actions=ct(table=3) > ]) > > ufid=$(ovs-appctl dpctl/dump-flows -m filter='in_port(ovs-p2),ipv4' | grep > "actions:ovs-p1" | parse_ufid) > AT_CHECK([ovs-appctl ofproto/detrace $ufid $pmd_id | ofctl_strip | \ > sed -E 's/n_bytes=2(66|80),/n_bytes=294,/'], [0], [dnl > -table_id=3, n_packets=3, n_bytes=294, ip,actions=output:1 > +br0 > + table_id=3, n_packets=3, n_bytes=294, ip,actions=output:1 > ]) > > AT_CHECK([ovs-appctl revalidator/resume]) > diff --git a/utilities/checkpatch_dict.txt b/utilities/checkpatch_dict.txt > index d40194a95..c1f43e5af 100644 > --- a/utilities/checkpatch_dict.txt > +++ b/utilities/checkpatch_dict.txt > @@ -49,6 +49,7 @@ defragment > deref > dereference > dest > +detrace > dhcp > dhcpv4 > dhcpv6 > -- > 2.53.0 > Thanks. Adrián _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
