[ovs-dev] [PATCH] meta-flow: Make mf_parse() report bad masks as errors.
Ben Pfaff
blp at nicira.com
Fri Nov 1 03:56:49 UTC 2013
Thanks! I applied this to master.
I apologize that I failed to add your Reviewed-by: tag. Too quick on
the "push".
On Fri, Nov 01, 2013 at 10:05:53AM +0900, Simon Horman wrote:
> Thanks Ben, I like this a lot.
>
> Reviewed-by: Simon Horman <horms at verge.net.au>
>
> On Thu, Oct 31, 2013 at 05:22:11PM -0700, Ben Pfaff wrote:
> > Reported-by: Simon Horman <horms at verge.net.au>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > lib/meta-flow.c | 37 ++++++++++++++++++++++--------
> > tests/ovs-ofctl.at | 65 ++++++++++++++++++++++++++++++++--------------------
> > 2 files changed, 67 insertions(+), 35 deletions(-)
> >
> > diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> > index 494277f..c0925e8 100644
> > --- a/lib/meta-flow.c
> > +++ b/lib/meta-flow.c
> > @@ -2609,6 +2609,8 @@ char *
> > mf_parse(const struct mf_field *mf, const char *s,
> > union mf_value *value, union mf_value *mask)
> > {
> > + char *error;
> > +
> > if (!strcmp(s, "*")) {
> > memset(value, 0, mf->n_bytes);
> > memset(mask, 0, mf->n_bytes);
> > @@ -2618,32 +2620,47 @@ mf_parse(const struct mf_field *mf, const char *s,
> > switch (mf->string) {
> > case MFS_DECIMAL:
> > case MFS_HEXADECIMAL:
> > - return mf_from_integer_string(mf, s,
> > - (uint8_t *) value, (uint8_t *) mask);
> > + error = mf_from_integer_string(mf, s,
> > + (uint8_t *) value, (uint8_t *) mask);
> > + break;
> >
> > case MFS_ETHERNET:
> > - return mf_from_ethernet_string(mf, s, value->mac, mask->mac);
> > + error = mf_from_ethernet_string(mf, s, value->mac, mask->mac);
> > + break;
> >
> > case MFS_IPV4:
> > - return mf_from_ipv4_string(mf, s, &value->be32, &mask->be32);
> > + error = mf_from_ipv4_string(mf, s, &value->be32, &mask->be32);
> > + break;
> >
> > case MFS_IPV6:
> > - return mf_from_ipv6_string(mf, s, &value->ipv6, &mask->ipv6);
> > + error = mf_from_ipv6_string(mf, s, &value->ipv6, &mask->ipv6);
> > + break;
> >
> > case MFS_OFP_PORT:
> > - return mf_from_ofp_port_string(mf, s, &value->be16, &mask->be16);
> > + error = mf_from_ofp_port_string(mf, s, &value->be16, &mask->be16);
> > + break;
> >
> > case MFS_OFP_PORT_OXM:
> > - return mf_from_ofp_port_string32(mf, s, &value->be32, &mask->be32);
> > + error = mf_from_ofp_port_string32(mf, s, &value->be32, &mask->be32);
> > + break;
> >
> > case MFS_FRAG:
> > - return mf_from_frag_string(s, &value->u8, &mask->u8);
> > + error = mf_from_frag_string(s, &value->u8, &mask->u8);
> > + break;
> >
> > case MFS_TNL_FLAGS:
> > ovs_assert(mf->n_bytes == sizeof(ovs_be16));
> > - return mf_from_tun_flags_string(s, &value->be16, &mask->be16);
> > + error = mf_from_tun_flags_string(s, &value->be16, &mask->be16);
> > + break;
> > +
> > + default:
> > + NOT_REACHED();
> > }
> > - NOT_REACHED();
> > +
> > + if (!error && !mf_is_mask_valid(mf, mask)) {
> > + error = xasprintf("%s: invalid mask for field %s", s, mf->name);
> > + }
> > + return error;
> > }
> >
> > /* Parses 's', a string value for field 'mf', into 'value'. Returns NULL if
> > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> > index 7097009..ae252b9 100644
> > --- a/tests/ovs-ofctl.at
> > +++ b/tests/ovs-ofctl.at
> > @@ -12,14 +12,11 @@ for test_case in \
> > 'tun_flags=0 none' \
> > 'tun_flags=1/1 none' \
> > 'tun_tos=0 none' \
> > - 'tun_tos=1/1 none' \
> > 'tun_ttl=0 none' \
> > - 'tun_ttl=1/1 none' \
> > 'metadata=0 NXM,OXM,OpenFlow11' \
> > 'metadata=1/1 NXM,OXM,OpenFlow11' \
> > 'in_port=1 any' \
> > 'skb_priority=0 none' \
> > - 'skb_priority=1/1 none' \
> > 'pkt_mark=1 NXM,OXM' \
> > 'pkt_mark=1/1 NXM,OXM' \
> > 'reg0=0 NXM,OXM' \
> > @@ -43,7 +40,6 @@ for test_case in \
> > 'dl_dst=00:11:22:33:44:55 any' \
> > 'dl_dst=00:11:22:33:44:55/00:ff:ff:ff:ff:ff NXM,OXM,OpenFlow11' \
> > 'dl_type=0x1234 any' \
> > - 'dl_type=0x1234/0x1 none' \
> > 'dl_type=0x0800 any' \
> > 'dl_type=0x0806 any' \
> > 'dl_type=0x86dd any' \
> > @@ -51,19 +47,13 @@ for test_case in \
> > 'vlan_tci=0x1009 any' \
> > 'vlan_tci=0x1009/0x1 NXM,OXM' \
> > 'dl_vlan=9 any' \
> > - 'dl_vlan=9/0x1 none' \
> > 'vlan_vid=11 any' \
> > 'vlan_vid=11/0x1 NXM,OXM' \
> > 'dl_vlan_pcp=6 any' \
> > - 'dl_vlan_pcp=6/0x1 none' \
> > 'vlan_pcp=5 any' \
> > - 'vlan_pcp=5/0x1 none' \
> > 'mpls,mpls_label=5 NXM,OXM,OpenFlow11' \
> > - 'mpls,mpls_label=5/0x1 none' \
> > 'mpls,mpls_tc=1 NXM,OXM,OpenFlow11' \
> > - 'mpls,mpls_tc=1/0x1 none' \
> > 'mpls,mpls_bos=0 NXM,OXM' \
> > - 'mpls,mpls_bos=1/0x1 none' \
> > 'ip,ip_src=1.2.3.4 any' \
> > 'ip,ip_src=192.168.0.0/24 any' \
> > 'ip,ip_src=192.0.168.0/255.0.255.0 NXM,OXM,OpenFlow11' \
> > @@ -77,29 +67,18 @@ for test_case in \
> > 'ipv6,ipv6_label=5 NXM,OXM' \
> > 'ipv6,ipv6_label=5/1 NXM,OXM' \
> > 'ip,nw_proto=1 any' \
> > - 'ip,nw_proto=1/1 none' \
> > 'ipv6,nw_proto=1 NXM,OXM' \
> > - 'ipv6,nw_proto=1/1 none' \
> > 'ip,nw_tos=0xf0 any' \
> > - 'ip,nw_tos=0xf0/0xf0 none' \
> > 'ipv6,nw_tos=0xf0 NXM,OXM' \
> > - 'ipv6,nw_tos=0xf0/0xf0 none' \
> > 'ip,nw_tos_shifted=0x3c any' \
> > - 'ip,nw_tos_shifted=0x3c/0xf0 none' \
> > 'ipv6,nw_tos_shifted=0x3c NXM,OXM' \
> > - 'ipv6,nw_tos_shifted=0x3c/0xf0 none' \
> > 'ip,nw_ecn=1 NXM,OXM' \
> > - 'ip,nw_ecn=1/1 none' \
> > 'ipv6,nw_ecn=1 NXM,OXM' \
> > - 'ipv6,nw_ecn=1/1 none' \
> > 'ip,nw_ttl=5 NXM,OXM' \
> > - 'ip,nw_ttl=5/1 none' \
> > 'ipv6,nw_ttl=5 NXM,OXM' \
> > - 'ipv6,nw_ttl=5/1 none' \
> > 'ip,ip_frag=no NXM,OXM' \
> > 'ipv6,ip_frag=no NXM,OXM' \
> > 'arp,arp_op=0 any' \
> > - 'arp,arp_op=0/1 none' \
> > 'arp,arp_spa=1.2.3.4 any' \
> > 'arp,arp_spa=1.2.3.4/0.0.0.1 NXM,OXM,OpenFlow11' \
> > 'arp,arp_tpa=1.2.3.4 any' \
> > @@ -125,12 +104,9 @@ for test_case in \
> > 'udp6,udp_dst=80 NXM,OXM' \
> > 'udp6,udp_dst=0x1000/0x1000 NXM,OXM' \
> > 'icmp,icmp_type=1 any' \
> > - 'icmp,icmp_type=1/1 none' \
> > 'icmp,icmp_code=2 any' \
> > - 'icmp,icmp_code=2/1 none' \
> > 'icmp6,icmpv6_type=1 NXM,OXM' \
> > - 'icmp6,icmpv6_code=2 NXM,OXM' \
> > - 'icmp6,icmpv6_code=2/1 none'
> > + 'icmp6,icmpv6_code=2 NXM,OXM'
> > do
> > set $test_case
> > echo
> > @@ -260,6 +236,45 @@ OFPT_FLOW_MOD (OF1.2): ADD table:255 actions=sample(probability=12345,collector_
> > ]])
> > AT_CLEANUP
> >
> > +AT_SETUP([ovs-ofctl parse-flow with invalid mask])
> > +for test_case in \
> > + 'tun_tos 1/1' \
> > + 'tun_ttl 1/1' \
> > + 'skb_priority 1/1' \
> > + 'eth_type 0x1234/0x1' \
> > + 'dl_vlan 9/0x1' \
> > + 'dl_vlan_pcp 6/0x1' \
> > + 'vlan_pcp 5/0x1' \
> > + 'mpls mpls_label 5/0x1' \
> > + 'mpls mpls_tc 1/0x1' \
> > + 'mpls mpls_bos 1/0x1' \
> > + 'ip nw_proto 1/1' \
> > + 'ipv6 nw_proto 1/1' \
> > + 'ip nw_tos 0xf0/0xf0' \
> > + 'ipv6 nw_tos 0xf0/0xf0' \
> > + 'ip nw_tos_shifted 0x3c/0xf0' \
> > + 'ipv6 nw_tos_shifted 0x3c/0xf0' \
> > + 'ip nw_ecn 1/1' \
> > + 'ipv6 nw_ecn 1/1' \
> > + 'ip nw_ttl 5/1' \
> > + 'ipv6 nw_ttl 5/1' \
> > + 'arp arp_op 0/1' \
> > + 'icmp icmp_type 1/1' \
> > + 'icmp icmp_code 2/1' \
> > + 'icmp6 icmpv6_code 2/1'
> > +do
> > + set $test_case
> > + if test $# = 3; then
> > + prereq=$1, field=$2 value=$3
> > + else
> > + prereq= field=$1 value=$2
> > + fi
> > + AT_CHECK_UNQUOTED([ovs-ofctl parse-flow "$prereq$field=$value,actions=drop"], [1], [],
> > +[ovs-ofctl: $value: invalid mask for field $field
> > +])
> > +done
> > +AT_CLEANUP
> > +
> > AT_SETUP([ovs-ofctl action inconsistency (OpenFlow 1.1)])
> > AT_CHECK([ovs-ofctl --protocols OpenFlow11 add-flow br0 'ip actions=mod_tp_dst:1234'
> > ], [1], [stdout], [ovs-ofctl: actions are invalid with specified match (OFPBAC_MATCH_INCONSISTENT)
> > --
> > 1.7.10.4
> >
More information about the dev
mailing list