Commit bd128f62 authored by Aaron Conole's avatar Aaron Conole Committed by Jakub Kicinski
Browse files

selftests: openvswitch: Add validation for the recursion test 



Add a test case into the netlink checks that will show the number of
nested action recursions won't exceed 16.  Going to 17 on a small
clone call isn't enough to exhaust the stack on (most) systems, so
it should be safe to run even on systems that don't have the fix
applied.

Signed-off-by: default avatarAaron Conole <aconole@redhat.com>
Reviewed-by: default avatarSimon Horman <horms@kernel.org>
Link: https://lore.kernel.org/r/20240207132416.1488485-3-aconole@redhat.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 6e2f90d3

tools/testing/selftests/net/openvswitch/openvswitch.sh

+13 −0
Original line number Diff line number Diff line
@@ -502,7 +502,20 @@ test_netlink_checks () { 
	    wc -l) == 2 ] || \

	      return 1



	info "Checking clone depth"

	ERR_MSG="Flow actions may not be safe on all matching packets"

	PRE_TEST=$(dmesg | grep -c "${ERR_MSG}")

	ovs_add_flow "test_netlink_checks" nv0 \

		'in_port(1),eth(),eth_type(0x800),ipv4()' \

		'clone(clone(clone(clone(clone(clone(clone(clone(clone(clone(clone(clone(clone(clone(clone(clone(clone(drop)))))))))))))))))' \

		>/dev/null 2>&1 && return 1

	POST_TEST=$(dmesg | grep -c "${ERR_MSG}")



	if [ "$PRE_TEST" == "$POST_TEST" ]; then

		info "failed - clone depth too large"

		return 1

	fi



	PRE_TEST=$(dmesg | grep -c "${ERR_MSG}")

	ovs_add_flow "test_netlink_checks" nv0 \

		'in_port(1),eth(),eth_type(0x0806),arp()' 'drop(0),2' \

tools/testing/selftests/net/openvswitch/ovs-dpctl.py

+56 −15
Original line number Diff line number Diff line
@@ -299,7 +299,7 @@ class ovsactions(nla): 
        ("OVS_ACTION_ATTR_PUSH_NSH", "none"),

        ("OVS_ACTION_ATTR_POP_NSH", "flag"),

        ("OVS_ACTION_ATTR_METER", "none"),

        ("OVS_ACTION_ATTR_CLONE", "none"),

        ("OVS_ACTION_ATTR_CLONE", "recursive"),

        ("OVS_ACTION_ATTR_CHECK_PKT_LEN", "none"),

        ("OVS_ACTION_ATTR_ADD_MPLS", "none"),

        ("OVS_ACTION_ATTR_DEC_TTL", "none"),
@@ -465,16 +465,27 @@ class ovsactions(nla): 
                    print_str += "pop_mpls"

            else:

                datum = self.get_attr(field[0])

                if field[0] == "OVS_ACTION_ATTR_CLONE":

                    print_str += "clone("

                    print_str += datum.dpstr(more)

                    print_str += ")"

                else:

                    print_str += datum.dpstr(more)



        return print_str



    def parse(self, actstr):

        totallen = len(actstr)

        while len(actstr) != 0:

            parsed = False

            parencount = 0

            if actstr.startswith("drop"):

                # If no reason is provided, the implicit drop is used (i.e no

                # action). If some reason is given, an explicit action is used.

                reason = None

                if actstr.startswith("drop("):

                    parencount += 1



                    actstr, reason = parse_extract_field(

                        actstr,

                        "drop(",
@@ -483,11 +494,13 @@ class ovsactions(nla): 
                        False,

                        None,

                    )



                if reason is not None:

                    self["attrs"].append(["OVS_ACTION_ATTR_DROP", reason])

                    parsed = True

                else:

                    return

                    actstr = actstr[len("drop"): ]

                    return (totallen - len(actstr))



            elif parse_starts_block(actstr, "^(\d+)", False, True):

                actstr, output = parse_extract_field(
@@ -504,6 +517,7 @@ class ovsactions(nla): 
                    False,

                    0,

                )

                parencount += 1

                self["attrs"].append(["OVS_ACTION_ATTR_RECIRC", recircid])

                parsed = True
@@ -516,12 +530,22 @@ class ovsactions(nla): 


            for flat_act in parse_flat_map:

                if parse_starts_block(actstr, flat_act[0], False):

                    actstr += len(flat_act[0])

                    actstr = actstr[len(flat_act[0]):]

                    self["attrs"].append([flat_act[1]])

                    actstr = actstr[strspn(actstr, ", ") :]

                    parsed = True



            if parse_starts_block(actstr, "ct(", False):

            if parse_starts_block(actstr, "clone(", False):

                parencount += 1

                subacts = ovsactions()

                actstr = actstr[len("clone("):]

                parsedLen = subacts.parse(actstr)

                lst = []

                self["attrs"].append(("OVS_ACTION_ATTR_CLONE", subacts))

                actstr = actstr[parsedLen:]

                parsed = True

            elif parse_starts_block(actstr, "ct(", False):

                parencount += 1

                actstr = actstr[len("ct(") :]

                ctact = ovsactions.ctact()
@@ -553,6 +577,7 @@ class ovsactions(nla): 
                        natact = ovsactions.ctact.natattr()



                        if actstr.startswith("("):

                            parencount += 1

                            t = None

                            actstr = actstr[1:]

                            if actstr.startswith("src"):
@@ -607,15 +632,29 @@ class ovsactions(nla): 
                                    actstr = actstr[strspn(actstr, ", ") :]



                        ctact["attrs"].append(["OVS_CT_ATTR_NAT", natact])

                        actstr = actstr[strspn(actstr, ",) ") :]

                        actstr = actstr[strspn(actstr, ", ") :]



                self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])

                parsed = True



            actstr = actstr[strspn(actstr, "), ") :]

            actstr = actstr[strspn(actstr, ", ") :]

            while parencount > 0:

                parencount -= 1

                actstr = actstr[strspn(actstr, " "):]

                if len(actstr) and actstr[0] != ")":

                    raise ValueError("Action str: '%s' unbalanced" % actstr)

                actstr = actstr[1:]



            if len(actstr) and actstr[0] == ")":

                return (totallen - len(actstr))



            actstr = actstr[strspn(actstr, ", ") :]



            if not parsed:

                raise ValueError("Action str: '%s' not supported" % actstr)



        return (totallen - len(actstr))





class ovskey(nla):

    nla_flags = NLA_F_NESTED
@@ -2111,6 +2150,8 @@ def main(argv): 
    ovsflow = OvsFlow()

    ndb = NDB()



    sys.setrecursionlimit(100000)



    if hasattr(args, "showdp"):

        found = False

        for iface in ndb.interfaces: