[ovs-dev] [PATCH v5 2/2] Match: Do not print "igmp" match keyword
Salvatore Daniele
sdaniele at redhat.com
Wed Nov 17 16:28:17 UTC 2021
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/20201102232805.1960103-1-blp@ovn.org/
On Wed, Nov 17, 2021 at 11:17 AM Mike Pattrick <mkp at redhat.com> 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 <sdaniele at redhat.com> wrote:
> >
> > From: Adrian Moreno <amorenoz at redhat.com>
> >
> > 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 <amorenoz at redhat.com>
> > Signed-off-by: Salvatore Daniele <sdaniele at redhat.com>
> > Co-authored-by: Salvatore Daniele <sdaniele at redhat.com>
> > ---
> > 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
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
More information about the dev
mailing list