From: Benoît Ganne Date: Fri, 19 Feb 2021 15:39:13 +0000 (+0100) Subject: classify: fix multiple filters support X-Git-Tag: v21.10-rc0~448 X-Git-Url: https://gerrit.fd.io/r/gitweb?p=vpp.git;a=commitdiff_plain;h=8c45e5109522cf9bbc98785283cd4c923f486fe6 classify: fix multiple filters support This fix the classify filter if we attach several different filters. This also fix some issues with l3 and l4 parsing. Type: fix Change-Id: I9dc6c55049a3bbc0110d1097b40d9da27633626b Signed-off-by: Benoît Ganne --- diff --git a/src/vnet/classify/vnet_classify.c b/src/vnet/classify/vnet_classify.c index 72ab33e9210..d36d93b5f31 100644 --- a/src/vnet/classify/vnet_classify.c +++ b/src/vnet/classify/vnet_classify.c @@ -911,7 +911,7 @@ unformat_l4_mask (unformat_input_t * input, va_list * args) else if (unformat (input, "dst_port")) dst_port = 0xFFFF; else - return 0; + break; } if (!src_port && !dst_port) @@ -980,6 +980,7 @@ unformat_ip4_mask (unformat_input_t * input, va_list * args) break; } + found_something = version + hdr_length; #define _(a) found_something += a; foreach_ip4_proto_field; #undef _ @@ -1886,13 +1887,13 @@ classify_filter_command_fn (vlib_main_t * vm, break; } - if (is_add && mask == 0 && table_index == ~0) + if (is_add && mask == 0) err = clib_error_return (0, "Mask required"); - else if (is_add && skip == ~0 && table_index == ~0) + else if (is_add && skip == ~0) err = clib_error_return (0, "skip count required"); - else if (is_add && match == ~0 && table_index == ~0) + else if (is_add && match == ~0) err = clib_error_return (0, "match count required"); else if (sw_if_index == ~0 && pkt_trace == 0 && pcap == 0) @@ -1936,20 +1937,30 @@ classify_filter_command_fn (vlib_main_t * vm, table_index = classify_get_pcap_chain (cm, sw_if_index); if (table_index != ~0) - table_index = classify_lookup_chain (table_index, mask, skip, match); + { + /* + * look for a compatible table in the existing chain + * - if a compatible table is found, table_index is updated with it + * - if not, table_index is updated to ~0 (aka nil) and because of that + * we are going to create one (see below). We save the original head + * in next_table_index so we can chain it with the newly created + * table + */ + next_table_index = table_index; + table_index = classify_lookup_chain (table_index, mask, skip, match); + } /* * When no table is found, make one. */ if (table_index == ~0) { + u32 new_head_index; + /* * Matching table wasn't found, so create a new one at the * head of the next_table_index chain. */ - next_table_index = table_index; - table_index = ~0; - rv = vnet_classify_add_del_table (cm, mask, nbuckets, memory_size, skip, match, next_table_index, miss_next_index, &table_index, @@ -1968,16 +1979,16 @@ classify_filter_command_fn (vlib_main_t * vm, /* * Reorder tables such that masks are most-specify to least-specific. */ - table_index = classify_sort_table_chain (cm, table_index); + new_head_index = classify_sort_table_chain (cm, table_index); /* * Put first classifier table in chain in a place where * other data structures expect to find and use it. */ if (pkt_trace) - classify_set_trace_chain (cm, table_index); + classify_set_trace_chain (cm, new_head_index); else - classify_set_pcap_chain (cm, sw_if_index, table_index); + classify_set_pcap_chain (cm, sw_if_index, new_head_index); } vec_free (mask); diff --git a/test/framework.py b/test/framework.py index 9bb3e01c31e..4d0f45621b4 100644 --- a/test/framework.py +++ b/test/framework.py @@ -824,9 +824,11 @@ class VppTestCase(unittest.TestCase): cls.sleep(0.1) @classmethod - def pg_start(cls): + def pg_start(cls, trace=True): """ Enable the PG, wait till it is done, then clean up """ - cls.vapi.cli("trace add pg-input 1000") + if trace: + cls.vapi.cli("clear trace") + cls.vapi.cli("trace add pg-input 1000") cls.vapi.cli('packet-generator enable') # PG, when starts, runs to completion - # so let's avoid a race condition, @@ -1192,11 +1194,10 @@ class VppTestCase(unittest.TestCase): "Finished sleep (%s) - slept %es (wanted %es)", remark, after - before, timeout) - def pg_send(self, intf, pkts, worker=None): - self.vapi.cli("clear trace") + def pg_send(self, intf, pkts, worker=None, trace=True): intf.add_stream(pkts, worker=worker) self.pg_enable_capture(self.pg_interfaces) - self.pg_start() + self.pg_start(trace=trace) def send_and_assert_no_replies(self, intf, pkts, remark="", timeout=None): self.pg_send(intf, pkts) @@ -1207,10 +1208,11 @@ class VppTestCase(unittest.TestCase): i.assert_nothing_captured(remark=remark) timeout = 0.1 - def send_and_expect(self, intf, pkts, output, n_rx=None, worker=None): + def send_and_expect(self, intf, pkts, output, n_rx=None, worker=None, + trace=True): if not n_rx: n_rx = len(pkts) - self.pg_send(intf, pkts, worker=worker) + self.pg_send(intf, pkts, worker=worker, trace=trace) rx = output.get_capture(n_rx) return rx diff --git a/test/test_trace_filter.py b/test/test_trace_filter.py index a9f28787eda..89ab3648169 100644 --- a/test/test_trace_filter.py +++ b/test/test_trace_filter.py @@ -26,10 +26,12 @@ class TestTracefilter(VppTestCase): def setUp(self): super(TestTracefilter, self).setUp() - self.create_pg_interfaces(range(1)) + self.create_pg_interfaces(range(2)) + self.pg0.generate_remote_hosts(11) for i in self.pg_interfaces: i.admin_up() i.config_ip4() + i.resolve_arp() def tearDown(self): super(TestTracefilter, self).tearDown() @@ -40,74 +42,70 @@ class TestTracefilter(VppTestCase): def cli(self, cmd): r = self.vapi.cli_return_response(cmd) if r.retval != 0: - if hasattr(r, 'reply'): - self.logger.info(cmd + " FAIL reply " + r.reply) - else: - self.logger.info(cmd + " FAIL retval " + str(r.retval)) + s = "reply '%s'" % r.reply if hasattr( + r, "reply") else "retval '%s'" % r.retval + raise RuntimeError("cli command '%s' FAIL with %s" % (cmd, s)) return r # check number of hits for classifier def assert_hits(self, n): r = self.cli("show classify table verbose 2") - self.assertTrue(r.retval == 0) - self.assertTrue(hasattr(r, 'reply')) self.assertTrue(r.reply.find("hits %i" % n) != -1) - def test_mactime_unitTest(self): + def add_filter(self, mask, match): + r = self.cli("classify filter trace mask %s match %s" % (mask, match)) + self.vapi.cli("clear trace") + r = self.cli("trace add pg-input 1000 filter") + + def del_all_filters(self): + self.cli("classify filter trace del") + r = self.cli("show classify filter") + s = "packet tracer: first table none" + self.assertTrue(r.reply.find(s) != -1) + + def test_basic(self): """ Packet Tracer Filter Test """ - cmds = ["loopback create", - "set int ip address loop0 192.168.1.1/24", - "set int state loop0 up", - "packet-generator new {\n" - " name classifyme\n" - " limit 100\n" - " size 300-300\n" - " interface loop0\n" - " node ethernet-input\n" - " data { \n" - " IP4: 1.2.3 -> 4.5.6\n" - " UDP: 192.168.1.10 - 192.168.1.20 -> 192.168.2.10\n" - " UDP: 1234 -> 2345\n" - " incrementing 286\n" - " }\n" - "}\n", - "classify filter trace mask l3 ip4 src" - " match l3 ip4 src 192.168.1.15", - "trace add pg-input 100 filter", - "pa en classifyme"] - - for cmd in cmds: - self.cli(cmd) - - # Check for 9 classifier hits, which is the right answer + self.add_filter( + "l3 ip4 src", + "l3 ip4 src %s" % + self.pg0.remote_hosts[5].ip4) + self.add_filter( + "l3 ip4 proto l4 src_port", + "l3 ip4 proto 17 l4 src_port 2345") + # the packet we are trying to match + p = list() + for i in range(100): + src = self.pg0.remote_hosts[i % len(self.pg0.remote_hosts)].ip4 + p.append((Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) / + IP(src=src, dst=self.pg1.remote_ip4) / + UDP(sport=1234, dport=2345) / Raw('\xa5' * 100))) + for i in range(17): + p.append((Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) / + IP(src=self.pg0.remote_hosts[0].ip4, + dst=self.pg1.remote_ip4) / + UDP(sport=2345, dport=1234) / Raw('\xa5' * 100))) + + self.send_and_expect(self.pg0, p, self.pg1, trace=False) + + # Check for 9 and 17 classifier hits, which is the right answer self.assert_hits(9) + self.assert_hits(17) - # cleanup - self.cli("pa de classifyme") - self.cli("classify filter trace del mask l3 ip4 src") + self.del_all_filters() # install a classify rule, inject traffic and check for hits def assert_classify(self, mask, match, packets, n=None): - r = self.cli( - "classify filter trace mask hex %s match hex %s" % - (mask, match)) - self.assertTrue(r.retval == 0) - r = self.cli("trace add pg-input %i filter" % len(packets)) - self.assertTrue(r.retval == 0) - self.pg0.add_stream(packets) - self.cli("pa en") + self.add_filter("hex %s" % mask, "hex %s" % match) + self.send_and_expect(self.pg0, packets, self.pg1, trace=False) self.assert_hits(n if n is not None else len(packets)) - self.cli("clear trace") - self.cli( - "classify filter trace del mask hex %s" % - (mask)) + self.del_all_filters() def test_encap(self): """ Packet Tracer Filter Test with encap """ # the packet we are trying to match p = (Ether(src=self.pg0.remote_mac, dst=self.pg0.local_mac) / - IP(src=self.pg0.remote_ip4, dst=self.pg0.local_ip4) / + IP(src=self.pg0.remote_ip4, dst=self.pg1.remote_ip4) / UDP() / VXLAN() / Ether() /