No, you have made the good point here. Patch looks good. Acked-by: Mike Pattrick <[email protected]>
On Wed, Nov 17, 2021 at 11:28 AM Salvatore Daniele <[email protected]> wrote: > > 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
