Re: [net-next,v6,1/2] selftests/net/openvswitch: add SCTP flow key parsing to ovs-dpctl.py

[Aaron Conole]

Minxi Hou <houminxi@xxxxxxxxx> writes:

> The ovskey flow-string parser has no OVS_KEY_ATTR_SCTP entry, so a
> flow string containing sctp(src=.../dst=...) parses without error but
> silently drops the L4 key. The resulting flow carries only
> ipv4(proto=132), and the kernel rejects it: match_validate() in
> flow_netlink.c requires OVS_KEY_ATTR_SCTP when the IP protocol is
> IPPROTO_SCTP and returns -EINVAL for the missing key.
> 
> Register OVS_KEY_ATTR_SCTP in the parse table and add unit tests
> (runnable via ovs-dpctl.py --test) covering SCTP and the neighboring
> TCP/UDP/ICMP/IPv4 key parsers.
> 
> Signed-off-by: Minxi Hou <houminxi@xxxxxxxxx>
> ---
>  .../selftests/net/openvswitch/ovs-dpctl.py    | 129 ++++++++++++++++++
>  1 file changed, 129 insertions(+)

[...]

> @@ -3165,5 +3171,128 @@ def main(argv):
>      return 0
>
>
> +def _init_ovskey_nlas():
> +    """Initialize required NLA classes for ovskey parsing."""
> +    nlmsg_atoms.encap_ovskey = encap_ovskey
> +    nlmsg_atoms.ovskey = ovskey
> +    nlmsg_atoms.ovsactions = ovsactions
> +
> +
> +def _parse_flow(flowstr):
> +    """Parse a flow string and return the key."""
> +    key = ovskey()
> +    key["attrs"] = []
> +    key.parse(flowstr)
> +    return key
> +
> +
> +def _find_attr(key, attr_name):
> +    """Find an attribute in parsed key."""
> +    for attr in key["attrs"]:
> +        if attr[0] == attr_name:
> +            return attr[1]
> +    return None
> +
> +
> +class TestOvsKeyParse(unittest.TestCase):
> +    """Unit tests for ovskey.parse() flow string parsing."""

[...]

I don't think we need unit tests for the unit test... That seems like
extra working making tests all the way down.

Maybe someone disagrees, but to me a test case here feels a bit
excessive.