Hi Mike, Good point. I believe this was discussed previously when this parsing issue was first addressed [1].
IIUC the reasoning is that igmp fields are not supported by OpenFlow, and other similar protocols like lldp do not print special keyword fields. It would be more consistent with OF/lldp if the keyword is removed all together. Thanks, Sal [1] https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/ On Wed, Nov 17, 2021 at 11:17 AM Mike Pattrick <[email protected]> wrote: > > Hello Salvatore, > > Why remove support for printing igmp instead of adding support for parsing > igmp? > > -M > > > On Thu, Nov 4, 2021 at 3:40 PM Salvatore Daniele <[email protected]> wrote: > > > > From: Adrian Moreno <[email protected]> > > > > The match keyword "igmp" is not supported in ofp-parse, which means > > that flow dumps cannot be restored. Previously a workaround was > > added to ovs-save to avoid changing output in stable branches. > > > > This patch changes the output to print igmp match in the accepted > > ofp-parse format (ip,nw_proto=2) and print igmp_type/code as generic > > tp_src/dst. Tests are added, and NEWS is updated to reflect this change. > > > > The workaround in ovs-save is still included to ensure that flows > > can be restored when upgrading an older ovs-vswitchd. This workaround > > should be removed in later versions. > > > > Signed-off-by: Adrian Moreno <[email protected]> > > Signed-off-by: Salvatore Daniele <[email protected]> > > Co-authored-by: Salvatore Daniele <[email protected]> > > --- > > NEWS | 2 ++ > > lib/match.c | 6 ------ > > tests/ovs-ofctl.at | 6 ++++++ > > 3 files changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index 1a1fed613..27c589aff 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -103,6 +103,8 @@ v2.16.0 - 16 Aug 2021 > > space in order to resolve a number of issues with per-vport > > dispatch. > > * New vswitchd unixctl command `dpif-netlink/dispatch-mode` will > > return > > the current dispatch mode for each datapath. > > + - ovs-ofctl dump-flows no longer prints "igmp". Instead the flag > > + "ip,nw_proto=2" is used. > > > > > > v2.15.0 - 15 Feb 2021 > > diff --git a/lib/match.c b/lib/match.c > > index ba716579d..2ad03e044 100644 > > --- a/lib/match.c > > +++ b/lib/match.c > > @@ -1556,8 +1556,6 @@ match_format(const struct match *match, > > skip_proto = true; > > if (f->nw_proto == IPPROTO_ICMP) { > > ds_put_format(s, "%sicmp%s,", colors.value, > > colors.end); > > - } else if (f->nw_proto == IPPROTO_IGMP) { > > - ds_put_format(s, "%sigmp%s,", colors.value, > > colors.end); > > } else if (f->nw_proto == IPPROTO_TCP) { > > ds_put_format(s, "%stcp%s,", colors.value, colors.end); > > } else if (f->nw_proto == IPPROTO_UDP) { > > @@ -1761,10 +1759,6 @@ match_format(const struct match *match, > > f->nw_proto == IPPROTO_ICMP) { > > format_be16_masked(s, "icmp_type", f->tp_src, wc->masks.tp_src); > > format_be16_masked(s, "icmp_code", f->tp_dst, wc->masks.tp_dst); > > - } else if (dl_type == htons(ETH_TYPE_IP) && > > - f->nw_proto == IPPROTO_IGMP) { > > - format_be16_masked(s, "igmp_type", f->tp_src, wc->masks.tp_src); > > - format_be16_masked(s, "igmp_code", f->tp_dst, wc->masks.tp_dst); > > } else if (dl_type == htons(ETH_TYPE_IPV6) && > > f->nw_proto == IPPROTO_ICMPV6) { > > format_be16_masked(s, "icmp_type", f->tp_src, wc->masks.tp_src); > > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at > > index 604f15c2d..ea74dcc64 100644 > > --- a/tests/ovs-ofctl.at > > +++ b/tests/ovs-ofctl.at > > @@ -192,6 +192,7 @@ actions=note:41.42.43,note:00.01.02.03.04.05.06.07,note > > ip,actions=set_field:10.4.3.77->ip_src,mod_nw_ecn:2 > > sctp actions=drop > > sctp actions=drop > > +ip,nw_proto=2 actions=drop > > in_port=0 actions=resubmit:0 > > > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678) > > > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,ingress) > > @@ -226,6 +227,7 @@ OFPT_FLOW_MOD: ADD > > actions=note:41.42.43.00.00.00,note:00.01.02.03.04.05.06.07.0 > > OFPT_FLOW_MOD: ADD ip > > actions=mod_nw_src:10.4.3.77,load:0x2->NXM_NX_IP_ECN[] > > OFPT_FLOW_MOD: ADD sctp actions=drop > > OFPT_FLOW_MOD: ADD sctp actions=drop > > +OFPT_FLOW_MOD: ADD ip,nw_proto=2 actions=drop > > OFPT_FLOW_MOD: ADD in_port=0 actions=resubmit:0 > > OFPT_FLOW_MOD: ADD > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678) > > OFPT_FLOW_MOD: ADD > > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,ingress) > > @@ -257,6 +259,7 @@ udp,nw_src=192.168.0.3,tp_dst=53 > > actions=mod_nw_ecn:2,output:1 > > cookie=0x123456789abcdef hard_timeout=10 priority=60000 actions=controller > > actions=note:41.42.43,note:00.01.02.03.04.05.06.07,note > > ip,actions=mod_nw_ttl:1,set_field:10.4.3.77->ip_src > > +ip,nw_proto=2 actions=drop > > sctp actions=drop > > sctp actions=drop > > in_port=0 actions=resubmit:0 > > @@ -277,6 +280,7 @@ OFPT_FLOW_MOD (OF1.1): ADD > > udp,nw_src=192.168.0.3,tp_dst=53 actions=mod_nw_ecn:2 > > OFPT_FLOW_MOD (OF1.1): ADD priority=60000 cookie:0x123456789abcdef hard:10 > > actions=CONTROLLER:65535 > > OFPT_FLOW_MOD (OF1.1): ADD > > actions=note:41.42.43.00.00.00,note:00.01.02.03.04.05.06.07.00.00.00.00.00.00,note:00.00.00.00.00.00 > > OFPT_FLOW_MOD (OF1.1): ADD ip actions=mod_nw_ttl:1,mod_nw_src:10.4.3.77 > > +OFPT_FLOW_MOD (OF1.1): ADD ip,nw_proto=2 actions=drop > > OFPT_FLOW_MOD (OF1.1): ADD sctp actions=drop > > OFPT_FLOW_MOD (OF1.1): ADD sctp actions=drop > > OFPT_FLOW_MOD (OF1.1): ADD in_port=0 actions=resubmit:0 > > @@ -300,6 +304,7 @@ actions=note:41.42.43,note:00.01.02.03.04.05.06.07,note > > ipv6,actions=set_field:fe80:0123:4567:890a:a6ba:dbff:fefe:59fa->ipv6_src > > sctp actions=set_field:3334->sctp_src > > sctp actions=set_field:4445->sctp_dst > > +ip,nw_proto=2 actions=drop > > tcp actions=mod_tp_dst:1234 > > udp actions=mod_tp_src:1111 > > ip > > actions=mod_nw_src:10.1.1.2,mod_nw_dst:192.168.10.1,mod_nw_ttl:1,mod_nw_tos:16,mod_nw_ecn:2 > > @@ -326,6 +331,7 @@ OFPT_FLOW_MOD (OF1.2): ADD > > actions=note:41.42.43.00.00.00,note:00.01.02.03.04.05 > > OFPT_FLOW_MOD (OF1.2): ADD ipv6 > > actions=set_field:fe80:123:4567:890a:a6ba:dbff:fefe:59fa->ipv6_src > > OFPT_FLOW_MOD (OF1.2): ADD sctp actions=set_field:3334->sctp_src > > OFPT_FLOW_MOD (OF1.2): ADD sctp actions=set_field:4445->sctp_dst > > +OFPT_FLOW_MOD (OF1.2): ADD ip,nw_proto=2 actions=drop > > OFPT_FLOW_MOD (OF1.2): ADD tcp actions=set_field:1234->tcp_dst > > OFPT_FLOW_MOD (OF1.2): ADD udp actions=set_field:1111->udp_src > > OFPT_FLOW_MOD (OF1.2): ADD ip > > actions=set_field:10.1.1.2->ip_src,set_field:192.168.10.1->ip_dst,mod_nw_ttl:1,set_field:4->ip_dscp,set_field:2->nw_ecn > > -- > > 2.31.1 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
