The 'filter' column of the Mirror table was parsed with parse_ofp_exact_flow(), which requires every field that is mentioned to be given as an exact value. Specifications like "ip,nw_src=10.0.0.0/24" were therefore rejected, even though the matching side already stores the filter as a miniflow plus minimask and unions that mask into the megaflow, so partially masked fields work end to end.
Add parse_ofp_flow_match(), which parses a flow specification into a 'struct match' using ofp_parse_field(), so each value may carry a mask. Use it for the mirror filter. parse_ofp_exact_flow() is left alone, as its other callers do want exact values. Reported-at: https://github.com/openvswitch/ovs-issues/issues/349 Reported-at: https://redhat.atlassian.net/browse/FDP-2474 Signed-off-by: Timothy Redaelli <[email protected]> --- NEWS | 3 ++ include/openvswitch/ofp-flow.h | 4 +++ lib/ofp-flow.c | 55 ++++++++++++++++++++++++++++++++++ ofproto/ofproto-dpif-mirror.c | 11 ++++--- tests/ofproto-dpif.at | 43 ++++++++++++++++++++++++++ vswitchd/vswitch.xml | 8 +++-- 6 files changed, 115 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index de1a030ad..33ab0585b 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,8 @@ Post-v4.0.0 -------------------- + - ovs-vswitchd: + * Mirror filters now accept masked fields, e.g. "ip,nw_src=10.0.0.0/24" + in the "filter" column of the Mirror table. v4.0.0 - 17 Aug 2026 diff --git a/include/openvswitch/ofp-flow.h b/include/openvswitch/ofp-flow.h index f2223d90b..6c4310b75 100644 --- a/include/openvswitch/ofp-flow.h +++ b/include/openvswitch/ofp-flow.h @@ -155,6 +155,10 @@ char *parse_ofp_exact_flow(struct flow *flow, struct flow_wildcards *wc, const struct tun_table *tun_table, const char *s, const struct ofputil_port_map *port_map); +char *parse_ofp_flow_match(struct match *match, + const struct tun_table *tun_table, const char *s, + const struct ofputil_port_map *port_map); + /* Flow stats or aggregate stats request, independent of protocol. */ struct ofputil_flow_stats_request { bool aggregate; /* Aggregate results? */ diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c index 3bc744f78..26d10012f 100644 --- a/lib/ofp-flow.c +++ b/lib/ofp-flow.c @@ -2016,3 +2016,58 @@ exit: } return error; } + +/* Parses a specification of a flow from 's' into 'match'. 's' must take the + * form FIELD=VALUE[,FIELD=VALUE]... where each FIELD is the name of an + * mf_field. Unlike parse_ofp_exact_flow(), each VALUE may include a mask + * (e.g. "nw_src=10.0.0.0/24"), so a field can be partially wildcarded. + * Fields must be specified in a natural order for satisfying prerequisites. + * If the map 'port_map' is specified, converts port names into port numbers. + * + * Returns NULL on success, otherwise a malloc()'d string that explains the + * problem. */ +char * +parse_ofp_flow_match(struct match *match, const struct tun_table *tun_table, + const char *s, const struct ofputil_port_map *port_map) +{ + enum ofputil_protocol usable_protocols = OFPUTIL_P_ANY; + char *pos, *key, *value_s; + char *error = NULL; + char *copy; + + match_init_catchall(match); + match->flow.tunnel.metadata.tab = tun_table; + + pos = copy = xstrdup(s); + while (ofputil_parse_key_value(&pos, &key, &value_s)) { + const struct ofp_protocol *p; + if (ofp_parse_protocol(key, &p)) { + match_set_dl_type(match, htons(p->dl_type)); + if (p->nw_proto) { + match_set_nw_proto(match, p->nw_proto); + } + match_set_default_packet_type(match); + } else { + const struct mf_field *mf = mf_from_name(key); + + if (!mf) { + error = xasprintf("%s: unknown field %s", s, key); + goto exit; + } + + error = ofp_parse_field(mf, value_s, port_map, match, + &usable_protocols); + if (error) { + goto exit; + } + } + } + +exit: + free(copy); + + if (error) { + match_init_catchall(match); + } + return error; +} diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c index e8a2830fb..346f0b79d 100644 --- a/ofproto/ofproto-dpif-mirror.c +++ b/ofproto/ofproto-dpif-mirror.c @@ -331,13 +331,11 @@ mirror_set(struct mbridge *mbridge, const struct ofproto *ofproto, if (ms->filter && strlen(ms->filter)) { struct ofputil_port_map map = OFPUTIL_PORT_MAP_INITIALIZER(&map); - struct flow_wildcards wc; - struct flow flow; + struct match match; char *err; ofproto_append_ports_to_map(&map, ofproto->ports); - err = parse_ofp_exact_flow(&flow, &wc, - ofproto_get_tun_tab(ofproto), + err = parse_ofp_flow_match(&match, ofproto_get_tun_tab(ofproto), ms->filter, &map); ofputil_port_map_destroy(&map); if (err) { @@ -352,14 +350,15 @@ mirror_set(struct mbridge *mbridge, const struct ofproto *ofproto, * behavior, and it would be overly complex to detect all possible * issues. So instead we attempt to extract the in_port and error * if successful. */ - if (wc.masks.in_port.ofp_port) { + if (match.wc.masks.in_port.ofp_port) { VLOG_WARN("filter is invalid due to in_port field."); mirror_destroy(mbridge, mirror->aux); return EINVAL; } mirror->filter_str = xstrdup(ms->filter); - ovsrcu_set(&mirror->filter_mask, filtermask_create(&flow, &wc)); + ovsrcu_set(&mirror->filter_mask, + filtermask_create(&match.flow, &match.wc)); } } diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index ee6ac873d..f46714432 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -5908,6 +5908,49 @@ OVS_VSWITCHD_STOP(["/filter is invalid: invalid: unknown field invalid/d /mirror mymirror configuration is invalid/d"]) AT_CLEANUP +AT_SETUP([ofproto-dpif - mirroring, filter with wildcards]) +AT_KEYWORDS([mirror mirrors mirroring]) +OVS_VSWITCHD_START +add_of_ports br0 1 2 3 +AT_CHECK([ovs-vsctl \ + set Bridge br0 mirrors=@m -- \ + --id=@p3 get Port p3 -- \ + --id=@m create Mirror name=mymirror select_all=true output_port=@p3 \ + filter="\"ip,nw_src=192.168.0.0/24\""], [0], [ignore]) + +AT_CHECK([ovs-ofctl add-flow br0 "in_port=1 actions=output:2"]) + +match_flow="eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=128,frag=no),tcp(dst=80)" +nomatch_flow="eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=10.0.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=128,frag=no),tcp(dst=80)" + +dnl A masked filter should be accepted and only matching flows mirrored. +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$match_flow"], [0], [stdout]) +AT_CHECK_UNQUOTED([tail -1 stdout], [0], + [Datapath actions: 3,2 +]) + +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$nomatch_flow"], [0], [stdout]) +AT_CHECK_UNQUOTED([tail -1 stdout], [0], + [Datapath actions: 2 +]) + +dnl A masked L4 port filter should compose with the wildcards too. +AT_CHECK([ovs-vsctl set mirror mymirror filter="\"tcp,tcp_dst=0x0050/0xfff0\""], [0]) + +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$match_flow"], [0], [stdout]) +AT_CHECK_UNQUOTED([tail -1 stdout], [0], + [Datapath actions: 3,2 +]) + +nomatch_port_flow="eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=128,frag=no),tcp(dst=443)" +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port(1),$nomatch_port_flow"], [0], [stdout]) +AT_CHECK_UNQUOTED([tail -1 stdout], [0], + [Datapath actions: 2 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - mirroring, select_all]) AT_KEYWORDS([mirror mirrors mirroring]) OVS_VSWITCHD_START diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 4eec70fe4..1da1a2f29 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -5319,9 +5319,11 @@ ovs-vsctl add-port br0 p1 -- \ When set, only packets that match <ref column="filter"/> are selected for mirroring. Packets that do not match are ignored by thie mirror. The <ref column="filter"/> syntax is described - in <code>ovs-fields</code>(7). However, the <code>in_port</code> - field is not supported; <ref column="select_src_port"/> should be - used to limit the mirror to a source port. + in <code>ovs-fields</code>(7). A field may be given with a mask, + e.g. <code>ip,nw_src=10.0.0.0/24</code>. + However, the <code>in_port</code> field is not supported; + <ref column="select_src_port"/> should be used to limit the + mirror to a source port. </p> <p> This filter is applied after <ref column="select_all"/>, <ref -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
