On Wed, Mar 18, 2026 at 12:02:59AM +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. Group entries also have a link to ofproto > with the bridge name. > > 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. > > Signed-off-by: Ilya Maximets <[email protected]>
Thanks! retis likes this patch :-) Reviewed-by: Adrian Moreno <[email protected]> > --- > > Version 2: > * Fixed missing bridge name for groups. [Adrian] > > Version 3: > * Fixed a race condition in the test where packet could > be sent to ovs-ofctl while it is still connected. > > ofproto/ofproto-dpif-xlate-cache.c | 19 ++++++++++ > tests/system-traffic.at | 59 ++++++++++++++++++++++++------ > utilities/checkpatch_dict.txt | 1 + > 3 files changed, 67 insertions(+), 12 deletions(-) > > diff --git a/ofproto/ofproto-dpif-xlate-cache.c > b/ofproto/ofproto-dpif-xlate-cache.c > index c6d935cf0..62df8ecba 100644 > --- a/ofproto/ofproto-dpif-xlate-cache.c > +++ b/ofproto/ofproto-dpif-xlate-cache.c > @@ -306,22 +306,41 @@ void > xlate_xcache_format(struct ds *s, const struct xlate_cache *xcache) > { > struct ofpbuf entries = xcache->entries; > + struct ofproto *last_ofproto = NULL; > struct xc_entry *entry; > struct ofgroup *ofg; > > 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: > ofg = &entry->group.group->up; > + if (last_ofproto != ofg->ofproto) { > + ds_put_format(s, "%s\n", ofg->ofproto->name); > + last_ofproto = ofg->ofproto; > + } > + ds_put_cstr(s, " "); > 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->up) { > + ds_put_format(s, "%s\n", entry->table.ofproto->up.name); > + last_ofproto = &entry->table.ofproto->up; > + } > + 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..eb956f7f5 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,25 +2754,38 @@ > 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 > ]) > > AT_CHECK([ovs-appctl revalidator/wait]) > -# Add an unrelated OF change to populate the xcache during the next reval > run. > -AT_CHECK([ovs-ofctl add-flow br0 table=4,ip,action=drop]) > +dnl Add an unrelated OF change to populate the xcache during the next > +dnl revlaidation run. Using OpenFlow 1.3 to avoid sending packet to > ovs-ofctl > +dnl on a miss in case it is still attached during revalidation. > +AT_CHECK([ovs-ofctl -OOpenFlow13 add-flow br0 table=15,ip,action=drop]) > AT_CHECK([ovs-appctl revalidator/wait]) > > AT_CHECK([ovs-appctl revalidator/pause]) > +on_exit 'ovs-appctl revalidator/resume' > > AT_CHECK([ovs-appctl dpctl/dump-flows], [0], [stdout]) > > @@ -2782,8 +2800,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 +2809,49 @@ > 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) > +br0 > + > group_id=1,type=select,selection_method=dp_hash,bucket=bucket_id:0,weight:100,actions=ct(commit,table=2) > ]) > > 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, n_packets=3, n_bytes=294, 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
