[ovs-dev] [PATCH] ovs-dpctl-top: Flow fields display content with mask applied.
Gurucharan Shetty
shettyg at nicira.com
Mon Nov 4 16:49:53 UTC 2013
On Fri, Oct 25, 2013 at 4:10 PM, Mark Hamilton <mhamilton at nicira.com> wrote:
> From: Mark Hamilton <mark_lee_hamilton at att.net>
>
> For example, if the value is ipv4(src=192.168.0.1/255.255.0.0)
> the output from top is ipv4(src=192.168.0.0/255.255.0.0). Prior
> to this change, the original flow field content was shown
> unaltered even though flow statistics where aggregated among
> several flows.
>
> If the flow field value matches any value meaning mask is all
> zeroes then the keyword ANY is displayed abbreviating output.
>
> Changes were tested on Ubuntu 12.04 with Python 2.7 and
> XenServer 6.0 with Python 2.4.3 distribution.
>
> Signed-off-by: Mark Hamilton <mhamilton at nicira.com>
I think the expectation is that From and Signed-off-by should be the same.
I see how it can be helpful with masks applied on ipv4 and ipv6.
I personally feel it a little inconvenient when L4 fields are shown
with masks applied. But others may have a different opinion.
I also think that the L4 port masks are displayed in hex. Does your
code handle that? I see the tests assuming it to be displayed in
decimal.
There are also a couple of lines that have been commented out. Can you
just get rid of them?
Thanks,
Guru
> ---
> utilities/ovs-dpctl-top.8.in | 4 +-
> utilities/ovs-dpctl-top.in | 243 ++++++++++++++++++++++++++++++++++--------
> 2 files changed, 201 insertions(+), 46 deletions(-)
>
> diff --git a/utilities/ovs-dpctl-top.8.in b/utilities/ovs-dpctl-top.8.in
> index 410e999..4d582a2 100644
> --- a/utilities/ovs-dpctl-top.8.in
> +++ b/utilities/ovs-dpctl-top.8.in
> @@ -35,7 +35,9 @@ of packets, total bytes and occurrence of the following fields:
> .
> .SS "Output shows four values:"
> .IP
> -\- FIELDS: the flow fields for example in_port(1).
> +\- FIELDS: the flow fields for example in_port(1). If a mask is present and
> +matches any value then the keyword ANY is shown for example the kernel flow
> +with the field ipv4(src=192.168.0.1/0.0.0.0) is abbreviated to ipv4(src=ANY).
> .IP
> \- COUNT: the number of lines in the dump\-flow output contain the flow field.
> .IP
> diff --git a/utilities/ovs-dpctl-top.in b/utilities/ovs-dpctl-top.in
> index 7f0f1f8..629e534 100755
> --- a/utilities/ovs-dpctl-top.in
> +++ b/utilities/ovs-dpctl-top.in
> @@ -46,7 +46,10 @@ of packets, total bytes and occurrence of the following fields:
>
>
> Output shows four values:
> - - FIELDS: the flow fields for example in_port(1).
> + - FIELDS: the flow fields for example in_port(1). If a mask is present and
> + matches any value then the keyword ANY is shown for example the kernel flow
> + with the field ipv4(src=192.168.0.1/0.0.0.0) is abbreviated to
> + ipv4(src=ANY).
>
> - PACKETS: the total number of packets containing the flow field.
>
> @@ -158,6 +161,35 @@ import socket
>
>
> ##
> +def network_split(network_str):
> + """
> + Split value and mask.
> + @return (value, mask) or (value, None) if mask is not present.
> + """
> + values = network_str.split("/")
> + if (len(values) == 1):
> + return (network_str, None)
> + elif (len(values) == 2):
> + port = values[0]
> + mask = values[1]
> + # value does contain a mask. Return port with mask applied.
> + return (port, mask)
> + else:
> + # This does not make sense. Return ValueError
> + raise ValueError("flow port parsed error.")
> +
> +
> +def ipv4_is_any(ipv4_mask):
> + """ return true if ipv4_mask is zero meaning matching any ipv4 address. """
> + return len([ii for ii in ipv4_mask if (ii not in [".", "0"])]) == 0
> +
> +
> +def ipv6_is_any(ipv4_mask):
> + """ return true if ipv4_mask is zero meaning matching any ipv6 address. """
> + return len([ii for ii in ipv4_mask if (ii not in [".", ":", "0"])]) == 0
> +
> +
> +##
> # The following two definitions provide the necessary netaddr functionality.
> # Python netaddr module is not part of the core installation. Packaging
> # netaddr was involved and seems inappropriate given that only two
> @@ -167,12 +199,9 @@ def ipv4_to_network(ip_str):
> If a mask is not present simply return ip_str.
> """
> pack_length = '!HH'
> - try:
> - (ip, mask) = ip_str.split("/")
> - except ValueError:
> - # just an ip address no mask.
> - return ip_str
> -
> + (ip, mask) = network_split(ip_str)
> + if (mask is None):
> + return ip
> ip_p = socket.inet_pton(socket.AF_INET, ip)
> ip_t = struct.unpack(pack_length, ip_p)
> mask_t = struct.unpack(pack_length, socket.inet_pton(socket.AF_INET, mask))
> @@ -187,12 +216,10 @@ def ipv6_to_network(ip_str):
> If a mask is not present simply return ip_str.
> """
> pack_length = '!HHHHHHHH'
> - try:
> - (ip, mask) = ip_str.split("/")
> - except ValueError:
> - # just an ip address no mask.
> - return ip_str
>
> + (ip, mask) = network_split(ip_str)
> + if (mask is None):
> + return ip
> ip_p = socket.inet_pton(socket.AF_INET6, ip)
> ip_t = struct.unpack(pack_length, ip_p)
> mask_t = struct.unpack(pack_length,
> @@ -207,6 +234,71 @@ def ipv6_to_network(ip_str):
> network_n[6], network_n[7]))
>
>
> +def port_mask_is_any(port_mask):
> + """ return true if port_mask is zero matching any port. """
> + return int(port_mask) == 0
> +
> +
> +def port_to_network(port_str):
> + """ Apply a mask to tcp and udp ports.
> + If mask is not present simply return port_str.
> + """
> +
> + (port, mask) = network_split(port_str)
> + if (mask is None):
> + # value does not contain a mask. Return port_str
> + return port
> + else:
> + # value does contain a mask. Return port with mask applied.
> + return str(int(port) & int(mask))
> +
> +
> +def port_to_key(port_str):
> + """ Return key of a given port and mask. """
> +
> + try:
> + (port, mask) = network_split(port_str)
> + if (mask is None):
> + return port
> + elif (port_mask_is_any(mask)):
> + # mask refers to ANY so just show the field_type
> + return "ANY"
> + else:
> + # else mask is not masking out the entire value.
> + #return "%s/%s" % (port_mask_split(port_str), mask)
> + return port_str
> + except ValueError:
> + return port_str
> +
> +
> +def ipv4_to_key(ip_str):
> + """ Return flow_key of the ip string. """
> + try:
> + (ip, mask) = network_split(ip_str)
> + if (mask is None):
> + return ip
> + elif (ipv4_is_any(mask)):
> + return "ANY"
> + else:
> + return "%s/%s" % (ipv4_to_network(ip_str), mask)
> + except ValueError:
> + return ip_str
> +
> +
> +def ipv6_to_key(ip_str):
> + """ Return flow_key of the ip string. """
> + try:
> + (ip, mask) = network_split(ip_str)
> + if (mask is None):
> + return ip
> + elif (ipv6_is_any(mask)):
> + return "ANY"
> + else:
> + return "%s/%s" % (ipv6_to_network(ip_str), mask)
> + except ValueError:
> + return ip_str
> +
> +
> ##
> # columns displayed
> ##
> @@ -247,13 +339,15 @@ def element_eth_get(field_type, element, stats_dict):
> def element_ipv4_get(field_type, element, stats_dict):
> """ Extract src and dst from a dump-flow element."""
> fmt = "%s(src=%s,dst=%s)"
> - element_show = fmt % (field_type, element["src"], element["dst"])
>
> + #element_show = fmt % (field_type, element["src"], element["dst"])
Instead of commenting out lines, can you just get rid of them.
> + element_show = fmt % (field_type, ipv4_to_key(element["src"]),
> + ipv4_to_key(element["dst"]))
> element_key = fmt % (field_type, ipv4_to_network(element["src"]),
> ipv4_to_network(element["dst"]))
>
> return SumData(field_type, element_show, stats_dict["packets"],
> - stats_dict["bytes"], element_key)
> + stats_dict["bytes"], element_key)
>
>
> def element_tunnel_get(field_type, element, stats_dict):
> @@ -265,27 +359,39 @@ def element_ipv6_get(field_type, element, stats_dict):
> """ Extract src and dst from a dump-flow element."""
>
> fmt = "%s(src=%s,dst=%s)"
> - element_show = fmt % (field_type, element["src"], element["dst"])
> + #element_show = fmt % (field_type, element["src"], element["dst"])
> +
> + element_show = fmt % (field_type, ipv6_to_key(element["src"]),
> + ipv6_to_key(element["dst"]))
>
> element_key = fmt % (field_type, ipv6_to_network(element["src"]),
> ipv6_to_network(element["dst"]))
>
> return SumData(field_type, element_show, stats_dict["packets"],
> - stats_dict["bytes"], element_key)
> + stats_dict["bytes"], element_key)
>
>
> +##
> +# Convert port and mask to display.
> +# udp(dst=1100/5) => udp(dst=1100/5)
> +# udp(dst=1100/0) = udp(dst=ANY)
> +# udp(dst=1100) = udp(dst=1100)
> def element_dst_port_get(field_type, element, stats_dict):
> """ Extract src and dst from a dump-flow element."""
> - element_key = "%s(dst=%s)" % (field_type, element["dst"])
> - return SumData(field_type, element_key, stats_dict["packets"],
> + fmt = "%s(dst=%s)"
> +
> + element_show = fmt % (field_type, port_to_key(element["dst"]))
> + element_key = fmt % (field_type, port_to_network(element["dst"]))
> +
> + return SumData(field_type, element_show, stats_dict["packets"],
> stats_dict["bytes"], element_key)
>
>
> def element_passthrough_get(field_type, element, stats_dict):
> """ Extract src and dst from a dump-flow element."""
> element_key = "%s(%s)" % (field_type, element)
> - return SumData(field_type, element_key,
> - stats_dict["packets"], stats_dict["bytes"], element_key)
> + return SumData(field_type, element_key, stats_dict["packets"],
> + stats_dict["bytes"], element_key)
>
>
> # pylint: disable-msg=R0903
> @@ -1333,7 +1439,7 @@ elif __name__ == 'ovs-dpctl-top':
> self.assertEqual(flow_dict["bytes"], "92")
>
> def test_flow_sum(self):
> - """ test_flow_sum. """
> + """ test_flow_sum """
> line = "in_port(4),eth(src=00:50:56:b4:4e:f8,"\
> "dst=33:33:00:01:00:03),eth_type(0x86dd),"\
> "ipv6(src=fe80::55bf:fe42:bc96:2812,dst=ff02::1:3,"\
> @@ -1384,7 +1490,7 @@ elif __name__ == 'ovs-dpctl-top':
> self.assert_(sum_value[0].bytes == 2 * 92)
>
> def test_assoc_list(self):
> - """ test_assoc_list. """
> + """ test_assoc_list """
> line = "in_port(4),eth(src=00:50:56:b4:4e:f8,"\
> "dst=33:33:00:01:00:03),eth_type(0x86dd),"\
> "ipv6(src=fe80::55bf:fe42:bc96:2812,dst=ff02::1:3,"\
> @@ -1440,7 +1546,7 @@ elif __name__ == 'ovs-dpctl-top':
> self.assertEqual(approximate_size(value), "1.1 GiB")
>
> def test_flow_line_split(self):
> - """ Splitting a flow line is not trivial.
> + """ test_flow_line_split Splitting a flow line is not trivial.
> There is no clear delimiter. Comma is used liberally."""
> expected_fields = ["in_port(4)",
> "eth(src=00:50:56:b4:4e:f8,dst=33:33:00:01:00:03)",
> @@ -1499,30 +1605,39 @@ elif __name__ == 'ovs-dpctl-top':
> self.assertEqual(flow_db.flow_stats_get()["flow_total"], 0)
> timer.stop()
>
> + def test_accumulate_ipv4(self):
> + """ test_accumulate_ipv4 test accumulate ipv4. """
> + lines = ["in_port(1),eth_type(0x0800),"
> + "ipv4(src=192.168.2.1/255.255.255.255,"
> + "dst=192.168.2.2/0.0.0.0),udp(src=68/0,dst=67/0), "
> + "packets:1, bytes:10, used:never, actions:342",
> + "in_port(1),eth_type(0x0800),"
> + "ipv4(src=192.168.2.1/255.255.255.255,"
> + "dst=192.168.2.2/255.255.255.255,proto=17/0xff,"
> + "tos=0/0,ttl=64/0,frag=no/0x2),udp(src=68/0,dst=67/0), "
> + "packets:2, bytes:20, used:never, actions:342"]
> +
> + flow_db = FlowDB(True)
> + flow_db.begin()
> + for line in lines:
> + flow_db.flow_line_add(line)
> + sum_values = flow_db.field_values_in_order("all", 1)
> + in_ports = [ii for ii in sum_values if (repr(ii) == "in_port(1)")]
> + self.assertEqual(len(in_ports), 1)
> + self.assertEqual(in_ports[0].packets, 3)
> + self.assertEqual(in_ports[0].bytes, 30)
> + self.assertEqual(in_ports[0].count, 2)
> +
> + ip_field = "ipv4(src=192.168.2.1,dst=192.168.2.2)"
> + ip_stats = [ii for ii in sum_values if (repr(ii) == ip_field)]
> + self.assertEqual(len(ip_stats), 1)
> + self.assertEqual(ip_stats[0].packets, 2)
> + self.assertEqual(ip_stats[0].bytes, 20)
> + self.assertEqual(ip_stats[0].count, 1)
> +
> def test_accumulate(self):
> """ test_accumulate test that FlowDB supports accumulate. """
>
> - lines = ["in_port(1),eth(src=00:50:56:4f:dc:3b,"
> - "dst=ff:ff:ff:ff:ff:ff),"
> - "eth_type(0x0806),arp(sip=10.24.105.107/255.255.255.255,"
> - "tip=10.24.104.230/255.255.255.255,op=1/0xff,"
> - "sha=00:50:56:4f:dc:3b/00:00:00:00:00:00,"
> - "tha=00:00:00:00:00:00/00:00:00:00:00:00), "
> - "packets:1, bytes:120, used:0.004s, actions:1",
> - "in_port(2),"
> - "eth(src=68:ef:bd:25:ef:c0,dst=33:33:00:00:00:66),"
> - "eth_type(0x86dd),ipv6(src=fe80::6aef:bdff:fe25:efc0/::,"
> - "dst=ff02::66/::,label=0/0,proto=17/0xff,tclass=0xe0/0,"
> - "hlimit=255/0,frag=no/0),udp(src=2029,dst=2029), "
> - "packets:2, bytes:5026, used:0.348s, actions:1",
> - "in_port(1),eth(src=ee:ee:ee:ee:ee:ee,"
> - "dst=ff:ff:ff:ff:ff:ff),"
> - "eth_type(0x0806),arp(sip=10.24.105.107/255.255.255.255,"
> - "tip=10.24.104.230/255.255.255.255,op=1/0xff,"
> - "sha=00:50:56:4f:dc:3b/00:00:00:00:00:00,"
> - "tha=00:00:00:00:00:00/00:00:00:00:00:00), packets:2, "
> - "bytes:240, used:0.004s, actions:1"]
> -
> lines = [
> "in_port(1),eth_type(0x0806), packets:1, bytes:120, actions:1",
> "in_port(2),eth_type(0x0806), packets:2, bytes:126, actions:1",
> @@ -1628,7 +1743,7 @@ elif __name__ == 'ovs-dpctl-top':
> self.assertEqual(in_ports[0].count, 1)
>
> def test_parse_character_errors(self):
> - """ test_parsing errors.
> + """ test_parsing_character_errors.
> The flow parses is purposely loose. Its not designed to validate
> input. Merely pull out what it can but there are situations
> that a parse error can be detected.
> @@ -1706,6 +1821,15 @@ elif __name__ == 'ovs-dpctl-top':
> ("1::192:168:0:1/::", "::")
> ]
>
> + ports = [
> + ("34312/0", "0"),
> + ("5/4", "4"),
> + ("5", "5")
> + ]
> +
> + for (port_test, port_check) in ports:
> + self.assertEqual(port_to_network(port_test), port_check)
> +
> for (ipv4_test, ipv4_check) in ipv4s:
> self.assertEqual(ipv4_to_network(ipv4_test), ipv4_check)
>
> @@ -1720,3 +1844,32 @@ elif __name__ == 'ovs-dpctl-top':
> self.assertEqual(top_render._field_type_select_get(), "in_port")
> self.assertEqual(script_render._field_type_select_get(), "all")
> #pylint: enable=W0212
> +
> + def test_print_any(self):
> + """ test_print_any test when ANY should be printed. """
> + ipv4s = [
> + ("192.168.0.1", "192.168.0.1"),
> + ("192.168.0.1/255.0.0.0", "192.0.0.0/255.0.0.0"),
> + ("192.168.0.1/0.0.0.0", "ANY"),
> + ]
> +
> + ipv6s = [
> + ("1::192:168:0:1", "1::192:168:0:1"),
> + ("1::192:168:0:1/1::0:0:0:0", "1::/1::0:0:0:0"),
> + ("1::192:168:0:1/::", "ANY")
> + ]
> +
> + ports = [
> + ("34312/0", "ANY"),
> + ("5/4", "5/4"),
> + ("5", "5")
> + ]
> +
> + for (port_test, port_check) in ports:
> + self.assertEqual(port_to_key(port_test), port_check)
> +
> + for (ipv4_test, ipv4_check) in ipv4s:
> + self.assertEqual(ipv4_to_key(ipv4_test), ipv4_check)
> +
> + for (ipv6_test, ipv6_check) in ipv6s:
> + self.assertEqual(ipv6_to_key(ipv6_test), ipv6_check)
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list