Thanks Ben, I like this a lot. Reviewed-by: Simon Horman <[email protected]>
On Thu, Oct 31, 2013 at 05:22:11PM -0700, Ben Pfaff wrote: > Reported-by: Simon Horman <[email protected]> > Signed-off-by: Ben Pfaff <[email protected]> > --- > 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 > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
