[ovs-dev] Sync on PTAP, EXT-382 and NSH: Mon 2017-03-13, 17:00 CET

[Jan Scheurich]

Hi,

Please be invited to our next sync meeting: Mon 13 March, 5pm CET.

Agenda:
*   Review/Discussion of current patch packages, e.g.
o   Display format for packet_type field in match and flow keys
o   packet_type in struct dp_packet or struct pkt_metadata
*   Status/Planning of next work packages
*   AOB

Thank you,
Jan

Link to the Google design doc:
https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8/edit


.
--> Join Skype Meeting
  This is an online meeting for Skype for Business, the professional meetings 
and communications app formerly known as Lync.
Join by phone

+492115343925 (Germany)  English (United 
States)
89925 (Germany) English (United States)

Find a local number

Conference ID: 70849799
 Forgot your dial-in PIN? 
|Help


To join a Lync / Skype for Business meeting from an Ericsson standard video 
room, add 77 before the Conference ID (e.g. 771234567 where 1234567 is the 
conference ID).To join from a video room outside of Ericsson add one of the 
domains after 77 and Conference ID (e.g. 771234567@ .ericsson.net, where 
=emea/apac/amcs).  For assistance contact the IT Service Desk.
[!OC([1033])!]
.


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4] ovn: Modify the DHCPv4 router option to optional

[Guoshuai Li]

Co-authored-by: Dong Jun 
Signed-off-by: Guoshuai Li 
Acked-by: Numan Siddique 
---
 NEWS|  2 ++
 ovn/northd/ovn-northd.c | 10 +++---
 ovn/ovn-nb.xml  | 16 
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/NEWS b/NEWS
index ce9fe88..853b5ef 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ Post-v2.7.0
`egress_pkt_mark` OVSDB option.
- EMC insertion probability is reduced to 1% and is configurable via
  the new 'other_config:emc-insert-inv-prob' option.
+   - OVN:
+ * Modify the DHCPv4 router option to optional in northbound databases.
 
 v2.7.0 - xx xxx 
 -
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 03dc850..d844a60 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2191,11 +2191,9 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
 >nbsp->dhcpv4_options->options, "server_mac");
 const char *lease_time = smap_get(
 >nbsp->dhcpv4_options->options, "lease_time");
-const char *router = smap_get(
->nbsp->dhcpv4_options->options, "router");
 
-if (!(server_ip && server_mac && lease_time && router)) {
-/* "server_id", "server_mac", "lease_time" and "router" should be
+if (!(server_ip && server_mac && lease_time)) {
+/* "server_id", "server_mac" and "lease_time" should be
  * present in the dhcp_options. */
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 VLOG_WARN_RL(, "Required DHCPv4 options not defined for lport - %s",
@@ -2738,9 +2736,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
 >nbs->ports[i]->dhcpv4_options->options, "server_mac");
 const char *lease_time = smap_get(
 >nbs->ports[i]->dhcpv4_options->options, "lease_time");
-const char *router = smap_get(
->nbs->ports[i]->dhcpv4_options->options, "router");
-if (server_id && server_mac && lease_time && router) {
+if (server_id && server_mac && lease_time) {
 struct ds match = DS_EMPTY_INITIALIZER;
 const char *actions =
 has_stateful ? "ct_commit; next;" : "next;";
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index c5ebbea..a17d876 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -1365,14 +1365,6 @@
   The Ethernet address for the DHCP server to use.
 
 
-
-  
-The IP address of a gateway for the client to use.  This should be
-in the subnet of the offered IP.  The DHCPv4 option code for this
-option is 3.
-  
-
-
 
   
@@ -1394,6 +1386,14 @@
   DHCPv4 options and their codes.
 
 
+
+  
+The IP address of a gateway for the client to use.  This should be
+in the subnet of the offered IP.  The DHCPv4 option code for this
+option is 3.
+  
+
+
 
   
 The DHCPv4 option code for this option is 1.
-- 
2.10.1.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH V2] ovn: Modify the DHCPV4 router option to optional

[Guoshuai Li]

Co-authored-by: Dong Jun 
Signed-off-by: Guoshuai Li 
---
 NEWS|  2 ++
 ovn/northd/ovn-northd.c | 10 +++---
 ovn/ovn-nb.xml  |  3 ++-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index ce9fe88..1698d39 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ Post-v2.7.0
`egress_pkt_mark` OVSDB option.
- EMC insertion probability is reduced to 1% and is configurable via
  the new 'other_config:emc-insert-inv-prob' option.
+   - OVN:
+ * Modify the DHCPV4 router option to optional in northbound databases.
 
 v2.7.0 - xx xxx 
 -
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 03dc850..d844a60 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2191,11 +2191,9 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
 >nbsp->dhcpv4_options->options, "server_mac");
 const char *lease_time = smap_get(
 >nbsp->dhcpv4_options->options, "lease_time");
-const char *router = smap_get(
->nbsp->dhcpv4_options->options, "router");
 
-if (!(server_ip && server_mac && lease_time && router)) {
-/* "server_id", "server_mac", "lease_time" and "router" should be
+if (!(server_ip && server_mac && lease_time)) {
+/* "server_id", "server_mac" and "lease_time" should be
  * present in the dhcp_options. */
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 VLOG_WARN_RL(, "Required DHCPv4 options not defined for lport - %s",
@@ -2738,9 +2736,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
 >nbs->ports[i]->dhcpv4_options->options, "server_mac");
 const char *lease_time = smap_get(
 >nbs->ports[i]->dhcpv4_options->options, "lease_time");
-const char *router = smap_get(
->nbs->ports[i]->dhcpv4_options->options, "router");
-if (server_id && server_mac && lease_time && router) {
+if (server_id && server_mac && lease_time) {
 struct ds match = DS_EMPTY_INITIALIZER;
 const char *actions =
 has_stateful ? "ct_commit; next;" : "next;";
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index c5ebbea..9b01668 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -1369,7 +1369,8 @@
   
 The IP address of a gateway for the client to use.  This should be
 in the subnet of the offered IP.  The DHCPv4 option code for this
-option is 3.
+option is 3. This is optional and when not specified, option 3 is
+not present in DHCPv4 response.
   
 
 
-- 
2.10.1.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 2/2] ovn-northd ipam: Support IPv6 dynamic assignment

[nusiddiq]

From: Numan Siddique 

OVN will generate the IPv6 address for a logical port if requested
using the IPv6 prefix and the MAC address (as IEEE EUI64 identifier).
To generate the IPv6 address, CMS should define the IPv6 prefix in the
'Logical_switch.other_config:ipv6_prefix' column.

Signed-off-by: Numan Siddique 
---
 lib/packets.h   | 20 +++
 ovn/northd/ovn-northd.c | 66 ++---
 ovn/ovn-nb.xml  | 23 +
 tests/ovn.at| 54 
 4 files changed, 148 insertions(+), 15 deletions(-)

diff --git a/lib/packets.h b/lib/packets.h
index c4d3799..ce1e1c0 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -990,6 +990,26 @@ in6_addr_solicited_node(struct in6_addr *addr, const 
struct in6_addr *ip6)
 }
 
 /*
+ * Generates ipv6 EUI64 address from the given eth addr
+ * and prefix and stores it in 'lla'
+ */
+static inline void
+in6_generate_eui64(struct eth_addr ea, struct in6_addr *prefix,
+   struct in6_addr *lla)
+{
+union ovs_16aligned_in6_addr *taddr = (void *) lla;
+union ovs_16aligned_in6_addr *prefix_taddr = (void *) prefix;
+taddr->be16[0] = prefix_taddr->be16[0];
+taddr->be16[1] = prefix_taddr->be16[1];
+taddr->be16[2] = prefix_taddr->be16[2];
+taddr->be16[3] = prefix_taddr->be16[3];
+taddr->be16[4] = htons(((ea.ea[0] ^ 0x02) << 8) | ea.ea[1]);
+taddr->be16[5] = htons(ea.ea[2] << 8 | 0x00ff);
+taddr->be16[6] = htons(0xfe << 8 | ea.ea[3]);
+taddr->be16[7] = ea.be16[2];
+}
+
+/*
  * Generates ipv6 link local address from the given eth addr
  * with prefix 'fe80::/64' and stores it in 'lla'
  */
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 5b471e1..9ad47d8 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -377,6 +377,8 @@ struct ipam_info {
 uint32_t start_ipv4;
 size_t total_ipv4s;
 unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */
+bool ipv6_prefix_set;
+struct in6_addr ipv6_prefix;
 };
 
 /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or
@@ -509,6 +511,14 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
 }
 
 const char *subnet_str = smap_get(>nbs->other_config, "subnet");
+const char *ipv6_prefix = smap_get(>nbs->other_config, "ipv6_prefix");
+
+if (ipv6_prefix) {
+od->ipam_info = xzalloc(sizeof *od->ipam_info);
+od->ipam_info->ipv6_prefix_set = ipv6_parse(
+ipv6_prefix, >ipam_info->ipv6_prefix);
+}
+
 if (!subnet_str) {
 return;
 }
@@ -523,7 +533,9 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
 return;
 }
 
-od->ipam_info = xzalloc(sizeof *od->ipam_info);
+if (!od->ipam_info) {
+od->ipam_info = xzalloc(sizeof *od->ipam_info);
+}
 od->ipam_info->start_ipv4 = ntohl(subnet) + 1;
 od->ipam_info->total_ipv4s = ~ntohl(mask);
 od->ipam_info->allocated_ipv4s =
@@ -1023,13 +1035,21 @@ static bool
 ipam_allocate_addresses(struct ovn_datapath *od, struct ovn_port *op,
 const char *addrspec)
 {
-if (!od || !op || !op->nbsp) {
+if (!od || !op || !op->nbsp || !od->ipam_info ||
+(!od->ipam_info->allocated_ipv4s && !od->ipam_info->ipv6_prefix_set)) {
 return false;
 }
 
-uint32_t ip = ipam_get_unused_ip(od);
-if (!ip) {
-return false;
+uint32_t ip = 0;
+
+if (od->ipam_info->allocated_ipv4s) {
+ip = ipam_get_unused_ip(od);
+if (!ip && !od->ipam_info->ipv6_prefix_set) {
+/* We dont want to return false for cases where IPv4 addresses
+ * are exhausted and IPv6 prefix is set.
+ */
+return false;
+}
 }
 
 struct eth_addr mac;
@@ -1049,17 +1069,33 @@ ipam_allocate_addresses(struct ovn_datapath *od, struct 
ovn_port *op,
 check_mac = false;
 }
 
-/* Add MAC to MACAM and IP to IPAM bitmap if both addresses were allocated
- * successfully. */
-ipam_insert_ip(od, ip);
-ipam_insert_mac(, check_mac);
+bool retval = false;
+struct ds new_addr = DS_EMPTY_INITIALIZER;
+ds_put_format(_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
+if (ip) {
+ipam_insert_ip(od, ip);
+ds_put_format(_addr, " "IP_FMT, IP_ARGS(htonl(ip)));
+}
 
-char *new_addr = xasprintf(ETH_ADDR_FMT" "IP_FMT,
-   ETH_ADDR_ARGS(mac), IP_ARGS(htonl(ip)));
-nbrec_logical_switch_port_set_dynamic_addresses(op->nbsp, new_addr);
-free(new_addr);
+if (od->ipam_info->ipv6_prefix_set) {
+struct in6_addr ipv6_addr;
+in6_generate_eui64(mac, >ipam_info->ipv6_prefix, _addr);
+char ipv6_addr_s[INET6_ADDRSTRLEN + 1];
+ipv6_string_mapped(ipv6_addr_s, _addr);
+ds_put_format(_addr, " %s", ipv6_addr_s);
+}
 
-return true;
+if (ip || 

[ovs-dev] [PATCH v2 1/2] ovn-northd ipam: Support 'exclude_ips' option

[nusiddiq]

From: Numan Siddique 

If the CMS wants to make use of ovn ipam it can now provide a
list of IPv4 addresses and a range of IPv4 addresses which
will be excluded from the dynamic address assignment.
To support this, a new option 'exclude_ips' is added in the
Logical_switch.other_config column.

Eg. ovn-nbctl set Logical_switch sw0
other_config:exclude_ips="10.0.0.2 10.0.0.30-10.0.0.40"

The present code, uses hash maps to store the assigned IP addresses.
In order to support this option, this patch has refactored the IPAM
assignment. It now uses a bitmap to manage the IP assignment with
each bit in the bitmap representing an IPv4 address.

Signed-off-by: Numan Siddique 
---
 ovn/northd/ovn-northd.c | 250 +++-
 ovn/ovn-nb.xml  |  24 -
 tests/ovn.at|  44 +
 3 files changed, 209 insertions(+), 109 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 03dc850..5b471e1 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -372,6 +372,13 @@ port_has_qos_params(const struct smap *opts)
 smap_get(opts, "qos_burst"));
 }
 
+
+struct ipam_info {
+uint32_t start_ipv4;
+size_t total_ipv4s;
+unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */
+};
+
 /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or
  * sb->external_ids:logical-switch. */
 struct ovn_datapath {
@@ -394,7 +401,7 @@ struct ovn_datapath {
 bool has_unknown;
 
 /* IPAM data. */
-struct hmap ipam;
+struct ipam_info *ipam_info;
 
 /* OVN northd only needs to know about the logical router gateway port for
  * NAT on a distributed router.  This "distributed gateway port" is
@@ -420,21 +427,6 @@ cleanup_macam(struct hmap *macam)
 }
 }
 
-struct ipam_node {
-struct hmap_node hmap_node;
-uint32_t ip_addr; /* Allocated IP address. */
-};
-
-static void
-destroy_ipam(struct hmap *ipam)
-{
-struct ipam_node *node;
-HMAP_FOR_EACH_POP (node, hmap_node, ipam) {
-free(node);
-}
-hmap_destroy(ipam);
-}
-
 static struct ovn_datapath *
 ovn_datapath_create(struct hmap *datapaths, const struct uuid *key,
 const struct nbrec_logical_switch *nbs,
@@ -447,7 +439,6 @@ ovn_datapath_create(struct hmap *datapaths, const struct 
uuid *key,
 od->nbs = nbs;
 od->nbr = nbr;
 hmap_init(>port_tnlids);
-hmap_init(>ipam);
 od->port_key_hint = 0;
 hmap_insert(datapaths, >key_node, uuid_hash(>key));
 return od;
@@ -462,7 +453,10 @@ ovn_datapath_destroy(struct hmap *datapaths, struct 
ovn_datapath *od)
  * use it. */
 hmap_remove(datapaths, >key_node);
 destroy_tnlids(>port_tnlids);
-destroy_ipam(>ipam);
+if (od->ipam_info) {
+bitmap_free(od->ipam_info->allocated_ipv4s);
+free(od->ipam_info);
+}
 free(od->router_ports);
 free(od);
 }
@@ -508,6 +502,89 @@ lrouter_is_enabled(const struct nbrec_logical_router 
*lrouter)
 }
 
 static void
+init_ipam_info_for_datapath(struct ovn_datapath *od)
+{
+if (!od->nbs) {
+return;
+}
+
+const char *subnet_str = smap_get(>nbs->other_config, "subnet");
+if (!subnet_str) {
+return;
+}
+
+ovs_be32 subnet, mask;
+char *error = ip_parse_masked(subnet_str, , );
+if (error || mask == OVS_BE32_MAX || !ip_is_cidr(mask)) {
+static struct vlog_rate_limit rl
+= VLOG_RATE_LIMIT_INIT(5, 1);
+VLOG_WARN_RL(, "bad 'subnet' %s", subnet_str);
+free(error);
+return;
+}
+
+od->ipam_info = xzalloc(sizeof *od->ipam_info);
+od->ipam_info->start_ipv4 = ntohl(subnet) + 1;
+od->ipam_info->total_ipv4s = ~ntohl(mask);
+od->ipam_info->allocated_ipv4s =
+bitmap_allocate(od->ipam_info->total_ipv4s);
+
+/* Mark first IP as taken */
+bitmap_set1(od->ipam_info->allocated_ipv4s, 0);
+
+/* Check if there are any reserver IPs (list) to be excluded from IPAM */
+const char *exclude_ip_list = smap_get(>nbs->other_config,
+   "exclude_ips");
+if (!exclude_ip_list) {
+return;
+}
+
+const char *buf_end = exclude_ip_list + strlen(exclude_ip_list);
+const char *buf = exclude_ip_list;
+int buf_index;
+ovs_be32 ip1, ip2;
+/* exclude_ip_list could be in the format -
+ *  "10.0.0.4,10.0.0.10,10.0.0.20-10.0.0.50,10.0.0.100-10.0.0.110".
+ */
+while (buf < buf_end) {
+buf_index = 0;
+if (ovs_scan_len(buf, _index, IP_SCAN_FMT"-"IP_SCAN_FMT,
+ IP_SCAN_ARGS(), IP_SCAN_ARGS())) {
+/* OK. */
+} else if (ovs_scan_len(buf, _index, IP_SCAN_FMT,
+IP_SCAN_ARGS())) {
+ip2 = 0;
+} else {
+static struct vlog_rate_limit rl
+= VLOG_RATE_LIMIT_INIT(5, 

[ovs-dev] [PATCH v2 0/2] ovn-northd: IPAM changes

[nusiddiq]

From: Numan Siddique 

This patch series supports 'exclude_ip's option and IPv6 dynamic
assignment.


v2 -> v1

 * P2 to support IPv6 dynamic assigment is added.


Numan Siddique (2):
  ovn-northd ipam: Support 'exclude_ips' option
  ovn-northd ipam: Support IPv6 dynamic assignment

 lib/packets.h   |  20 
 ovn/northd/ovn-northd.c | 306 +---
 ovn/ovn-nb.xml  |  47 +++-
 tests/ovn.at|  98 
 4 files changed, 352 insertions(+), 119 deletions(-)

-- 
2.9.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 16/22] odp: Support conntrack orig tuple key.

[Jarno Rajahalme]

Userspace support for datapath original direction conntrack tuple.

Signed-off-by: Jarno Rajahalme 
---
 build-aux/extract-ofp-fields|   3 +
 include/openvswitch/flow.h  |  15 ++-
 include/openvswitch/match.h |  16 +++
 include/openvswitch/meta-flow.h | 136 
 lib/conntrack.c |  43 ++--
 lib/flow.c  | 228 
 lib/flow.h  |  50 +
 lib/match.c | 110 ++-
 lib/meta-flow.c | 157 ++-
 lib/meta-flow.xml   |  92 
 lib/nx-match.c  |  18 +++-
 lib/odp-util.c  |  86 +++
 lib/odp-util.h  |   8 +-
 lib/ofp-util.c  |   2 +-
 lib/packets.h   |   5 +
 ofproto/ofproto-dpif-rid.h  |   2 +-
 ofproto/ofproto-dpif-xlate.c|  13 ++-
 ofproto/ofproto-dpif.c  |   2 +
 tests/odp.at|   2 +-
 tests/ofproto-dpif.at   |  30 +++---
 tests/ofproto.at|   7 ++
 tests/system-traffic.at | 142 +++--
 22 files changed, 1058 insertions(+), 109 deletions(-)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 498b887..a26d558 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -44,6 +44,9 @@ PREREQS = {"none": "MFP_NONE",
"IPv4": "MFP_IPV4",
"IPv6": "MFP_IPV6",
"IPv4/IPv6": "MFP_IP_ANY",
+   "CT": "MFP_CT_VALID",
+   "CTv4": "MFP_CTV4_VALID",
+   "CTv6": "MFP_CTV6_VALID",
"MPLS": "MFP_MPLS",
"TCP": "MFP_TCP",
"UDP": "MFP_UDP",
diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index 9169272..5cd78e4 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -23,7 +23,7 @@
 /* This sequence number should be incremented whenever anything involving flows
  * or the wildcarding of flows changes.  This will cause build assertion
  * failures in places which likely need to be updated. */
-#define FLOW_WC_SEQ 36
+#define FLOW_WC_SEQ 37
 
 /* Number of Open vSwitch extension 32-bit registers. */
 #define FLOW_N_REGS 16
@@ -92,7 +92,7 @@ struct flow {
 union flow_in_port in_port; /* Input port.*/
 uint32_t recirc_id; /* Must be exact match. */
 uint8_t ct_state;   /* Connection tracking state. */
-uint8_t pad0;
+uint8_t ct_nw_proto;/* CT orig tuple IP protocol. */
 uint16_t ct_zone;   /* Connection tracking zone. */
 uint32_t ct_mark;   /* Connection mark.*/
 uint8_t pad1[4];/* Pad to 64 bits. */
@@ -110,8 +110,12 @@ struct flow {
 /* L3 (64-bit aligned) */
 ovs_be32 nw_src;/* IPv4 source address or ARP SPA. */
 ovs_be32 nw_dst;/* IPv4 destination address or ARP TPA. */
+ovs_be32 ct_nw_src; /* CT orig tuple IPv4 source address. */
+ovs_be32 ct_nw_dst; /* CT orig tuple IPv4 destination address. */
 struct in6_addr ipv6_src;   /* IPv6 source address. */
 struct in6_addr ipv6_dst;   /* IPv6 destination address. */
+struct in6_addr ct_ipv6_src; /* CT orig tuple IPv6 source address. */
+struct in6_addr ct_ipv6_dst; /* CT orig tuple IPv6 destination address. */
 ovs_be32 ipv6_label;/* IPv6 flow label. */
 uint8_t nw_frag;/* FLOW_FRAG_* flags. */
 uint8_t nw_tos; /* IP ToS (including DSCP and ECN). */
@@ -126,8 +130,11 @@ struct flow {
 /* L4 (64-bit aligned) */
 ovs_be16 tp_src;/* TCP/UDP/SCTP source port/ICMP type. */
 ovs_be16 tp_dst;/* TCP/UDP/SCTP destination port/ICMP code. */
+ovs_be16 ct_tp_src; /* CT original tuple source port/ICMP type. */
+ovs_be16 ct_tp_dst; /* CT original tuple dst port/ICMP code. */
 ovs_be32 igmp_group_ip4;/* IGMP group IPv4 address.
  * Keep last for BUILD_ASSERT_DECL below. */
+ovs_be32 pad4;  /* Pad to 64 bits. */
 };
 BUILD_ASSERT_DECL(sizeof(struct flow) % sizeof(uint64_t) == 0);
 BUILD_ASSERT_DECL(sizeof(struct flow_tnl) % sizeof(uint64_t) == 0);
@@ -136,8 +143,8 @@ BUILD_ASSERT_DECL(sizeof(struct flow_tnl) % 
sizeof(uint64_t) == 0);
 
 /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
 BUILD_ASSERT_DECL(offsetof(struct flow, igmp_group_ip4) + sizeof(uint32_t)
-  == sizeof(struct flow_tnl) + 248
-  && FLOW_WC_SEQ == 36);
+  == sizeof(struct flow_tnl) + 292
+  && FLOW_WC_SEQ == 37);
 
 /* Incremental points at which flow classification may be performed in
  * segments.
diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index 0b5f050..06fa04c 100644
--- a/include/openvswitch/match.h
+++ 

[ovs-dev] [PATCH v3 22/22] tests: Add an FTP test without conntrack.

[Jarno Rajahalme]

If FTP tests with conntrack fail, it is informative to know if the
problem is with the FTP client and/or server, or with conntrack
itself.

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 tests/system-traffic.at | 29 +
 1 file changed, 29 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cad2677..ede9261 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2040,6 +2040,35 @@ 
tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([FTP - no conntrack])
+AT_SKIP_IF([test $HAVE_FTP = no])
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+AT_DATA([flows.txt], [dnl
+table=0,action=normal
+])
+
+AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows.txt])
+
+NETNS_DAEMONIZE([at_ns0], [[$PYTHON $srcdir/test-l7.py ftp]], [ftp1.pid])
+NETNS_DAEMONIZE([at_ns1], [[$PYTHON $srcdir/test-l7.py ftp]], [ftp0.pid])
+OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
+
+dnl FTP requests from p0->p1 should work fine.
+NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 --no-passive-ftp -t 3 -T 1 
--retry-connrefused -v -o wget0.log])
+
+AT_CHECK([find -name index.html], [0], [dnl
+./index.html
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([conntrack - FTP])
 AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 20/22] conntrack: Force commit.

[Jarno Rajahalme]

Userspace support for force commit.

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 include/openvswitch/ofp-actions.h |  7 +++-
 lib/conntrack.c   | 16 ++--
 lib/conntrack.h   |  2 +-
 lib/dpif-netdev.c |  8 ++--
 lib/odp-util.c| 20 -
 lib/ofp-actions.c | 29 +++--
 ofproto/ofproto-dpif-xlate.c  |  3 +-
 tests/odp.at  | 16 
 tests/ofp-actions.at  | 10 +
 tests/ofproto-dpif.at | 85 +++
 tests/system-traffic.at   | 53 
 tests/test-conntrack.c|  9 +++--
 utilities/ovs-ofctl.8.in  | 12 ++
 13 files changed, 252 insertions(+), 18 deletions(-)

diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index 5ea0763..622dd7a 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -556,9 +556,14 @@ ofpact_nest_get_action_len(const struct ofpact_nest *on)
 /* Bits for 'flags' in struct nx_action_conntrack.
  *
  * If NX_CT_F_COMMIT is set, then the connection entry is moved from the
- * unconfirmed to confirmed list in the tracker. */
+ * unconfirmed to confirmed list in the tracker.
+ * If NX_CT_F_FORCE is set, in addition to NX_CT_F_COMMIT, then the conntrack
+ * entry is replaced with a new one in case the original direction of the
+ * existing entry is opposite of the current packet direction.
+ */
 enum nx_conntrack_flags {
 NX_CT_F_COMMIT = 1 << 0,
+NX_CT_F_FORCE  = 1 << 1,
 };
 
 /* Magic value for struct nx_action_conntrack 'recirc_table' field, to specify
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 8ae501b..60a3972 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -239,12 +239,21 @@ conn_not_found(struct conntrack *ct, struct dp_packet 
*pkt,
 static struct conn *
 process_one(struct conntrack *ct, struct dp_packet *pkt,
 struct conn_lookup_ctx *ctx, uint16_t zone,
-bool commit, long long now)
+bool force, bool commit, long long now)
 {
 unsigned bucket = hash_to_bucket(ctx->hash);
 struct conn *conn = ctx->conn;
 uint16_t state = 0;
 
+/* Delete found entry if in wrong direction. 'force' implies commit. */
+if (conn && force && ctx->reply) {
+ovs_list_remove(>exp_node);
+hmap_remove(>buckets[bucket].connections, >node);
+atomic_count_dec(>n_conn);
+delete_conn(conn);
+conn = NULL;
+}
+
 if (conn) {
 if (ctx->related) {
 state |= CS_RELATED;
@@ -301,7 +310,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
  * 'setlabel' behaves similarly for the connection label.*/
 int
 conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch,
-  ovs_be16 dl_type, bool commit, uint16_t zone,
+  ovs_be16 dl_type, bool force, bool commit, uint16_t zone,
   const uint32_t *setmark,
   const struct ovs_key_ct_labels *setlabel,
   const char *helper)
@@ -364,7 +373,8 @@ conntrack_execute(struct conntrack *ct, struct 
dp_packet_batch *pkt_batch,
 
 conn_key_lookup(ctb, [j], now);
 
-conn = process_one(ct, pkts[j], [j], zone, commit, now);
+conn = process_one(ct, pkts[j], [j], zone, force, commit,
+   now);
 
 if (conn && setmark) {
 set_mark(pkts[j], conn, setmark[0], setmark[1]);
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 254f61c..0437cd3 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -65,7 +65,7 @@ void conntrack_init(struct conntrack *);
 void conntrack_destroy(struct conntrack *);
 
 int conntrack_execute(struct conntrack *, struct dp_packet_batch *,
-  ovs_be16 dl_type, bool commit,
+  ovs_be16 dl_type, bool force, bool commit,
   uint16_t zone, const uint32_t *setmark,
   const struct ovs_key_ct_labels *setlabel,
   const char *helper);
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 42196b2..ee2b691 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4734,6 +4734,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 
 case OVS_ACTION_ATTR_CT: {
 const struct nlattr *b;
+bool force = false;
 bool commit = false;
 unsigned int left;
 uint16_t zone = 0;
@@ -4747,7 +4748,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 
 switch(sub_type) {
 case OVS_CT_ATTR_FORCE_COMMIT:
-/* Not implemented yet. */
+force = true;
+/* fall through. */
 case OVS_CT_ATTR_COMMIT:
 commit = true;
 break;
@@ -4770,8 +4772,8 @@ 

[ovs-dev] [PATCH v3 17/22] actions: Add resubmit with conntrack tuple.

[Jarno Rajahalme]

Add resubmit option to use the conntrack original direction tuple
swapped with the corresponding packet header fields during the lookup.
This could allow the same ACL table be used for admitting return
and/or related traffic as is used for admitting the original direction
traffic.

Signed-off-by: Jarno Rajahalme 
---
 include/openvswitch/ofp-actions.h |   4 +-
 lib/ofp-actions.c |  82 +++--
 ofproto/ofproto-dpif-xlate.c  |  75 ---
 tests/ofp-actions.at  |   6 ++
 tests/ofproto-dpif.at |  89 +--
 tests/system-traffic.at   | 122 --
 utilities/ovs-ofctl.8.in  |  19 +-
 7 files changed, 317 insertions(+), 80 deletions(-)

diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index 53d6b44..5ea0763 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -640,11 +640,13 @@ struct ofpact_nat {
 
 /* OFPACT_RESUBMIT.
  *
- * Used for NXAST_RESUBMIT, NXAST_RESUBMIT_TABLE. */
+ * Used for NXAST_RESUBMIT, NXAST_RESUBMIT_TABLE, NXAST_RESUBMIT_TABLE_CT. */
 struct ofpact_resubmit {
 struct ofpact ofpact;
 ofp_port_t in_port;
 uint8_t table_id;
+bool with_ct_orig;   /* Resubmit with Conntrack original direction tuple
+  * fields in place of IP header fields. */
 };
 
 /* Bits for 'flags' in struct nx_action_learn.
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 2869e0f..489fdb0 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -265,6 +265,8 @@ enum ofp_raw_action_type {
 NXAST_RAW_RESUBMIT,
 /* NX1.0+(14): struct nx_action_resubmit. */
 NXAST_RAW_RESUBMIT_TABLE,
+/* NX1.0+(44): struct nx_action_resubmit. */
+NXAST_RAW_RESUBMIT_TABLE_CT,
 
 /* NX1.0+(2): uint32_t. */
 NXAST_RAW_SET_TUNNEL,
@@ -3850,19 +3852,20 @@ format_FIN_TIMEOUT(const struct ofpact_fin_timeout *a, 
struct ds *s)
 ds_put_format(s, "%s)%s", colors.paren, colors.end);
 }
 
-/* Action structures for NXAST_RESUBMIT and NXAST_RESUBMIT_TABLE.
+/* Action structures for NXAST_RESUBMIT, NXAST_RESUBMIT_TABLE, and
+ * NXAST_RESUBMIT_TABLE_CT.
  *
  * These actions search one of the switch's flow tables:
  *
- *- For NXAST_RESUBMIT_TABLE only, if the 'table' member is not 255, then
- *  it specifies the table to search.
+ *- For NXAST_RESUBMIT_TABLE and NXAST_RESUBMIT_TABLE_CT, if the
+ *  'table' member is not 255, then it specifies the table to search.
  *
- *- Otherwise (for NXAST_RESUBMIT_TABLE with a 'table' of 255, or for
- *  NXAST_RESUBMIT regardless of 'table'), it searches the current flow
- *  table, that is, the OpenFlow flow table that contains the flow from
- *  which this action was obtained.  If this action did not come from a
- *  flow table (e.g. it came from an OFPT_PACKET_OUT message), then table 0
- *  is the current table.
+ *- Otherwise (for NXAST_RESUBMIT_TABLE or NXAST_RESUBMIT_TABLE_CT with a
+ *  'table' of 255, or for NXAST_RESUBMIT regardless of 'table'), it
+ *  searches the current flow table, that is, the OpenFlow flow table that
+ *  contains the flow from which this action was obtained.  If this action
+ *  did not come from a flow table (e.g. it came from an OFPT_PACKET_OUT
+ *  message), then table 0 is the current table.
  *
  * The flow table lookup uses a flow that may be slightly modified from the
  * original lookup:
@@ -3870,9 +3873,12 @@ format_FIN_TIMEOUT(const struct ofpact_fin_timeout *a, 
struct ds *s)
  *- For NXAST_RESUBMIT, the 'in_port' member of struct nx_action_resubmit
  *  is used as the flow's in_port.
  *
- *- For NXAST_RESUBMIT_TABLE, if the 'in_port' member is not OFPP_IN_PORT,
- *  then its value is used as the flow's in_port.  Otherwise, the original
- *  in_port is used.
+ *- For NXAST_RESUBMIT_TABLE and NXAST_RESUBMIT_TABLE_CT, if the 'in_port'
+ *  member is not OFPP_IN_PORT, then its value is used as the flow's
+ *  in_port.  Otherwise, the original in_port is used.
+ *
+ *- For NXAST_RESUBMIT_TABLE_CT the Conntrack 5-tuple fields are used as
+ *  the packets IP header fields during the lookup.
  *
  *- If actions that modify the flow (e.g. OFPAT_SET_VLAN_VID) precede the
  *  resubmit action, then the flow is updated with the new values.
@@ -3905,11 +3911,12 @@ format_FIN_TIMEOUT(const struct ofpact_fin_timeout *a, 
struct ds *s)
  *  a total limit of 4,096 resubmits per flow translation (earlier versions
  *  did not impose any total limit).
  *
- * NXAST_RESUBMIT ignores 'table' and 'pad'.  NXAST_RESUBMIT_TABLE requires
- * 'pad' to be all-bits-zero.
+ * NXAST_RESUBMIT ignores 'table' and 'pad'.  NXAST_RESUBMIT_TABLE and
+ * NXAST_RESUBMIT_TABLE_CT require 'pad' to be all-bits-zero.
  *
  * Open vSwitch 1.0.1 and earlier did not support recursion.  

[ovs-dev] [PATCH v3 21/22] datapath: Add a missing comment.

[Jarno Rajahalme]

Make openvswitch.h better match upstream by adding a missing comment.

Signed-off-by: Jarno Rajahalme 
---
 datapath/linux/compat/include/linux/openvswitch.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 3112214..4e0e9a6 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -718,6 +718,8 @@ struct ovs_action_push_tnl {
  * mask. For each bit set in the mask, the corresponding bit in the value is
  * copied to the connection tracking label field in the connection.
  * @OVS_CT_ATTR_HELPER: variable length string defining conntrack ALG.
+ * @OVS_CT_ATTR_NAT: Nested OVS_NAT_ATTR_* for performing L3 network address
+ * translation (NAT) on the packet.
  * @OVS_CT_ATTR_FORCE_COMMIT: Like %OVS_CT_ATTR_COMMIT, but instead of doing
  * nothing if the connection is already committed will check that the current
  * packet is in conntrack entry's original direction.  If directionality does
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 19/22] datapath: Add force commit.

[Jarno Rajahalme]

Upstream patch:

commit dd41d33f0b033885211a5d6f3ee19e73238aa9ee
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:22:00 2017 -0800

openvswitch: Add force commit.

Stateful network admission policy may allow connections to one
direction and reject connections initiated in the other direction.
After policy change it is possible that for a new connection an
overlapping conntrack entry already exists, where the original
direction of the existing connection is opposed to the new
connection's initial packet.

Most importantly, conntrack state relating to the current packet gets
the "reply" designation based on whether the original direction tuple
or the reply direction tuple matched.  If this "directionality" is
wrong w.r.t. to the stateful network admission policy it may happen
that packets in neither direction are correctly admitted.

This patch adds a new "force commit" option to the OVS conntrack
action that checks the original direction of an existing conntrack
entry.  If that direction is opposed to the current packet, the
existing conntrack entry is deleted and a new one is subsequently
created in the correct direction.

Signed-off-by: Jarno Rajahalme 
Acked-by: Pravin B Shelar 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Signed-off-by: Jarno Rajahalme 
---
 datapath/conntrack.c  | 26 +--
 datapath/linux/compat/include/linux/openvswitch.h |  5 +
 lib/dpif-netdev.c |  2 ++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 11ebefe..4df7352 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -72,6 +72,7 @@ struct ovs_conntrack_info {
u8 commit : 1;
u8 nat : 3; /* enum ovs_ct_nat */
u8 random_fully_compat : 1; /* bool */
+   u8 force : 1;
u16 family;
struct md_mark mark;
struct md_labels labels;
@@ -647,10 +648,13 @@ static bool skb_nfct_cached(struct net *net,
 */
if (!ct && key->ct.state & OVS_CS_F_TRACKED &&
!(key->ct.state & OVS_CS_F_INVALID) &&
-   key->ct.zone == info->zone.id)
+   key->ct.zone == info->zone.id) {
ct = ovs_ct_find_existing(net, >zone, info->family, skb,
  !!(key->ct.state
 & OVS_CS_F_NAT_MASK));
+   if (ct)
+   nf_ct_get(skb, );
+   }
if (!ct)
return false;
if (!net_eq(net, read_pnet(>ct_net)))
@@ -664,6 +668,18 @@ static bool skb_nfct_cached(struct net *net,
if (help && rcu_access_pointer(help->helper) != info->helper)
return false;
}
+   /* Force conntrack entry direction to the current packet? */
+   if (info->force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
+   /* Delete the conntrack entry if confirmed, else just release
+* the reference.
+*/
+   if (nf_ct_is_confirmed(ct))
+   nf_ct_delete(ct, 0, 0);
+   else
+   nf_conntrack_put(>ct_general);
+   nf_ct_set(skb, NULL, 0);
+   return false;
+   }
 
return true;
 }
@@ -1246,6 +1262,7 @@ static int parse_nat(const struct nlattr *attr,
 
 static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
[OVS_CT_ATTR_COMMIT]= { .minlen = 0, .maxlen = 0 },
+   [OVS_CT_ATTR_FORCE_COMMIT]  = { .minlen = 0, .maxlen = 0 },
[OVS_CT_ATTR_ZONE]  = { .minlen = sizeof(u16),
.maxlen = sizeof(u16) },
[OVS_CT_ATTR_MARK]  = { .minlen = sizeof(struct md_mark),
@@ -1285,6 +1302,9 @@ static int parse_ct(const struct nlattr *attr, struct 
ovs_conntrack_info *info,
}
 
switch (type) {
+   case OVS_CT_ATTR_FORCE_COMMIT:
+   info->force = true;
+   /* fall through. */
case OVS_CT_ATTR_COMMIT:
info->commit = true;
break;
@@ -1515,7 +1535,9 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info 
*ct_info,
if (!start)
return -EMSGSIZE;
 
-   if (ct_info->commit && nla_put_flag(skb, OVS_CT_ATTR_COMMIT))
+   if (ct_info->commit && nla_put_flag(skb, ct_info->force
+   ? OVS_CT_ATTR_FORCE_COMMIT
+   : OVS_CT_ATTR_COMMIT))
return -EMSGSIZE;
if (IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES) &&
nla_put_u16(skb, OVS_CT_ATTR_ZONE, ct_info->zone.id))
diff 

[ovs-dev] [PATCH v3 18/22] compat: nf_ct_delete compat.

[Jarno Rajahalme]

Upstream commit:

commit f330a7fdbe1611104622faff7e614a246a7d20f0
Author: Florian Westphal 
Date:   Thu Aug 25 15:33:31 2016 +0200

netfilter: conntrack: get rid of conntrack timer

With stats enabled this eats 80 bytes on x86_64 per nf_conn entry, as
Eric Dumazet pointed out during netfilter workshop 2016.

Eric also says: "Another reason was the fact that Thomas was about to
change max timer range [..]" (500462a9de657f8, 'timers: Switch to
a non-cascading wheel').

Remove the timer and use a 32bit jiffies value containing timestamp until
entry is valid.

During conntrack lookup, even before doing tuple comparision, check
the timeout value and evict the entry in case it is too old.

The dying bit is used as a synchronization point to avoid races where
multiple cpus try to evict the same entry.

Because lookup is always lockless, we need to bump the refcnt once
when we evict, else we could try to evict already-dead entry that
is being recycled.

This is the standard/expected way when conntrack entries are destroyed.

Followup patches will introduce garbage colliction via work queue
and further places where we can reap obsoleted entries (e.g. during
netlink dumps), this is needed to avoid expired conntracks from hanging
around for too long when lookup rate is low after a busy period.

Signed-off-by: Florian Westphal 
Acked-by: Eric Dumazet 
Signed-off-by: Pablo Neira Ayuso 

Upstream commit f330a7fdbe16 ("netfilter: conntrack: get rid of
conntrack timer") changes the way nf_ct_delete() is called.  Prior to
commit the call pattern was like this:

   if (del_timer(>timeout))
   nf_ct_delete(ct, ...);

After this change nf_ct_delete() is called directly:

   nf_ct_delete(ct, ...);

This patch provides a replacement implementation for nf_ct_delete()
that first calls the del_timer().  This replacement is only used if
the struct nf_conn has member 'timeout' of type 'struct timer_list'.

The following patch introduces the first caller to nf_ct_delete() in
the OVS kernel module.

Linux <3.12 does not have nf_ct_delete() at all, so we inline it if it
does not exist.  The inlined code is from 3.11 death_by_timeout(),
which in later versions simply calls nf_ct_delete().

Upstream commit 02982c27ba1e1bd9f9d4747214e19ca83aa88d0e introduced
nf_ct_delete() in Linux 3.12.  This commit has the original code that
is being inlined here.

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 acinclude.m4   |  6 
 .../include/net/netfilter/nf_conntrack_core.h  | 37 ++
 2 files changed, 43 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index 44ad1c4..b53469c 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -525,6 +525,12 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netfilter_ipv6.h], [nf_ipv6_ops],
 [fragment.*sock], 
[OVS_DEFINE([HAVE_NF_IPV6_OPS_FRAGMENT])])
 
+  OVS_FIND_FIELD_IFELSE([$KSRC/include/net/netfilter/nf_conntrack.h],
+[nf_conn], [struct timer_list[[ \t]]*timeout],
+[OVS_DEFINE([HAVE_NF_CONN_TIMER])])
+  OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack.h],
+  [nf_ct_delete(], [OVS_DEFINE([HAVE_NF_CT_DELETE])])
+
   OVS_FIND_PARAM_IFELSE([$KSRC/include/net/netfilter/nf_conntrack.h],
   [nf_ct_tmpl_alloc], [nf_conntrack_zone],
   [OVS_DEFINE([HAVE_NF_CT_TMPL_ALLOC_TAKES_STRUCT_ZONE])])
diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h 
b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
index 16b57a6..715e1d5 100644
--- a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
+++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
@@ -88,4 +88,41 @@ static unsigned int rpl_nf_conntrack_in(struct net *net, 
u_int8_t pf,
 #define nf_conntrack_in rpl_nf_conntrack_in
 #endif /* < 4.10 */
 
+#ifdef HAVE_NF_CONN_TIMER
+
+#ifndef HAVE_NF_CT_DELETE
+#include 
+#endif
+
+static inline bool rpl_nf_ct_delete(struct nf_conn *ct, u32 portid, int report)
+{
+   if (del_timer(>timeout))
+#ifdef HAVE_NF_CT_DELETE
+   return nf_ct_delete(ct, portid, report);
+#else
+   {
+   struct nf_conn_tstamp *tstamp;
+
+   tstamp = nf_conn_tstamp_find(ct);
+   if (tstamp && tstamp->stop == 0)
+   tstamp->stop = ktime_to_ns(ktime_get_real());
+
+   if (!test_bit(IPS_DYING_BIT, >status) &&
+   unlikely(nf_conntrack_event(IPCT_DESTROY, ct) < 0)) {
+   /* destroy event was not delivered */
+   nf_ct_delete_from_lists(ct);
+   nf_ct_dying_timeout(ct);
+   

[ovs-dev] [PATCH v3 13/22] datapath: Add original direction conntrack tuple to sw_flow_key.

[Jarno Rajahalme]

Upstream commit:

commit 9dd7f8907c3705dc7a7a375d1c6e30b06e6daffc
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:59 2017 -0800

openvswitch: Add original direction conntrack tuple to sw_flow_key.

Add the fields of the conntrack original direction 5-tuple to struct
sw_flow_key.  The new fields are initially marked as non-existent, and
are populated whenever a conntrack action is executed and either finds
or generates a conntrack entry.  This means that these fields exist
for all packets that were not rejected by conntrack as untrackable.

The original tuple fields in the sw_flow_key are filled from the
original direction tuple of the conntrack entry relating to the
current packet, or from the original direction tuple of the master
conntrack entry, if the current conntrack entry has a master.
Generally, expected connections of connections having an assigned
helper (e.g., FTP), have a master conntrack entry.

The main purpose of the new conntrack original tuple fields is to
allow matching on them for policy decision purposes, with the premise
that the admissibility of tracked connections reply packets (as well
as original direction packets), and both direction packets of any
related connections may be based on ACL rules applying to the master
connection's original direction 5-tuple.  This also makes it easier to
make policy decisions when the actual packet headers might have been
transformed by NAT, as the original direction 5-tuple represents the
packet headers before any such transformation.

When using the original direction 5-tuple the admissibility of return
and/or related packets need not be based on the mere existence of a
conntrack entry, allowing separation of admission policy from the
established conntrack state.  While existence of a conntrack entry is
required for admission of the return or related packets, policy
changes can render connections that were initially admitted to be
rejected or dropped afterwards.  If the admission of the return and
related packets was based on mere conntrack state (e.g., connection
being in an established state), a policy change that would make the
connection rejected or dropped would need to find and delete all
conntrack entries affected by such a change.  When using the original
direction 5-tuple matching the affected conntrack entries can be
allowed to time out instead, as the established state of the
connection would not need to be the basis for packet admission any
more.

It should be noted that the directionality of related connections may
be the same or different than that of the master connection, and
neither the original direction 5-tuple nor the conntrack state bits
carry this information.  If needed, the directionality of the master
connection can be stored in master's conntrack mark or labels, which
are automatically inherited by the expected related connections.

The fact that neither ARP nor ND packets are trackable by conntrack
allows mutual exclusion between ARP/ND and the new conntrack original
tuple fields.  Hence, the IP addresses are overlaid in union with ARP
and ND fields.  This allows the sw_flow_key to not grow much due to
this patch, but it also means that we must be careful to never use the
new key fields with ARP or ND packets.  ARP is easy to distinguish and
keep mutually exclusive based on the ethernet type, but ND being an
ICMPv6 protocol requires a bit more attention.

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

This patch squashes in minimal amount of OVS userspace code to not
break the build. Later patches contain the full userspace support.

Signed-off-by: Jarno Rajahalme 
---
 datapath/actions.c|  2 +
 datapath/conntrack.c  | 86 +--
 datapath/conntrack.h  | 10 ++-
 datapath/flow.c   | 34 +++--
 datapath/flow.h   | 49 ++---
 datapath/flow_netlink.c   | 85 --
 datapath/flow_netlink.h   |  7 +-
 datapath/linux/compat/include/linux/openvswitch.h | 18 +
 lib/odp-execute.c |  4 ++
 lib/odp-util.c| 38 ++
 ofproto/ofproto-dpif-sflow.c  |  2 +
 11 files changed, 289 insertions(+), 46 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index abb6637..06080b2 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -1058,6 +1058,8 @@ static int execute_masked_set_action(struct sk_buff 

[ovs-dev] [PATCH v3 15/22] ofp-util: Ignore unknown fields in ofputil_decode_packet_in2().

[Jarno Rajahalme]

The decoder of packet_in messages should not fail on encountering
unknown metadata fields.  This allows the switch to add new features
without breaking controllers.  The controllers should, however, copy
the metadata fields from the packet_int to packet_out so that the
switch gets back the full metadata.  OVN is already doing this.

Signed-off-by: Jarno Rajahalme 
---
 lib/nx-match.c | 25 -
 lib/nx-match.h |  4 ++--
 lib/ofp-util.c |  5 +++--
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 95516a1..bcc1347 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -504,6 +504,9 @@ nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie,
 return 0;
 }
 
+/* Prerequisites will only be checked when 'strict' is 'true'.  This allows
+ * decoding conntrack original direction 5-tuple IP addresses without the
+ * ethertype being present, when decoding metadata only. */
 static enum ofperr
 nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict,
 struct match *match, ovs_be64 *cookie, ovs_be64 *cookie_mask,
@@ -539,7 +542,7 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, bool 
strict,
 *cookie = value.be64;
 *cookie_mask = mask.be64;
 }
-} else if (!mf_are_match_prereqs_ok(field, match)) {
+} else if (strict && !mf_are_match_prereqs_ok(field, match)) {
 error = OFPERR_OFPBMC_BAD_PREREQ;
 } else if (!mf_is_all_wild(field, >wc)) {
 error = OFPERR_OFPBMC_DUP_FIELD;
@@ -607,7 +610,8 @@ nx_pull_match(struct ofpbuf *b, unsigned int match_len, 
struct match *match,
 }
 
 /* Behaves the same as nx_pull_match(), but skips over unknown NXM headers,
- * instead of failing with an error. */
+ * instead of failing with an error, and does not check for field
+ * prerequisities. */
 enum ofperr
 nx_pull_match_loose(struct ofpbuf *b, unsigned int match_len,
 struct match *match,
@@ -664,8 +668,9 @@ oxm_pull_match(struct ofpbuf *b, const struct tun_table 
*tun_table,
 return oxm_pull_match__(b, true, tun_table, match);
 }
 
-/* Behaves the same as oxm_pull_match() with one exception.  Skips over unknown
- * OXM headers instead of failing with an error when they are encountered. */
+/* Behaves the same as oxm_pull_match() with two exceptions.  Skips over
+ * unknown OXM headers instead of failing with an error when they are
+ * encountered, and does not check for field prerequisities. */
 enum ofperr
 oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table,
  struct match *match)
@@ -676,14 +681,16 @@ oxm_pull_match_loose(struct ofpbuf *b, const struct 
tun_table *tun_table,
 /* Parses the OXM match description in the 'oxm_len' bytes in 'oxm'.  Stores
  * the result in 'match'.
  *
- * Fails with an error when encountering unknown OXM headers.
+ * Returns 0 if successful, otherwise an OpenFlow error code.
  *
- * Returns 0 if successful, otherwise an OpenFlow error code. */
+ * Encountering unknown OXM headers or missing field prerequisites are not
+ * considered as error conditions.
+ */
 enum ofperr
-oxm_decode_match(const void *oxm, size_t oxm_len,
- const struct tun_table *tun_table, struct match *match)
+oxm_decode_match_loose(const void *oxm, size_t oxm_len,
+   const struct tun_table *tun_table, struct match *match)
 {
-return nx_pull_raw(oxm, oxm_len, true, match, NULL, NULL, tun_table);
+return nx_pull_raw(oxm, oxm_len, false, match, NULL, NULL, tun_table);
 }
 
 /* Verify an array of OXM TLVs treating value of each TLV as a mask,
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 631ab48..b599731 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -61,8 +61,8 @@ enum ofperr oxm_pull_match(struct ofpbuf *, const struct 
tun_table *,
struct match *);
 enum ofperr oxm_pull_match_loose(struct ofpbuf *, const struct tun_table *,
  struct match *);
-enum ofperr oxm_decode_match(const void *, size_t, const struct tun_table *,
- struct match *);
+enum ofperr oxm_decode_match_loose(const void *, size_t,
+   const struct tun_table *, struct match *);
 enum ofperr oxm_pull_field_array(const void *, size_t fields_len,
  struct field_array *);
 
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 7d40cbb..ed66dd1 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3397,8 +3397,9 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool 
loose,
 }
 
 case NXPINT_METADATA:
-error = oxm_decode_match(payload.msg, ofpbuf_msgsize(),
- tun_table, >flow_metadata);
+error = oxm_decode_match_loose(payload.msg,
+   ofpbuf_msgsize(),
+   

[ovs-dev] [PATCH v3 14/22] flow: Make room after ct_state.

[Jarno Rajahalme]

'ct_state' currently only needs 8 bits, so we can make room for a new
CT field introduced in the next patch.

Signed-off-by: Jarno Rajahalme 
---
 include/openvswitch/flow.h | 3 ++-
 lib/flow.c | 3 ++-
 lib/match.c| 8 
 lib/packets.h  | 2 +-
 ofproto/ofproto-dpif.c | 2 +-
 tests/ovs-ofctl.at | 4 ++--
 6 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index df80dfe..9169272 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -91,7 +91,8 @@ struct flow {
  * computation is opaque to the user space. */
 union flow_in_port in_port; /* Input port.*/
 uint32_t recirc_id; /* Must be exact match. */
-uint16_t ct_state;  /* Connection tracking state. */
+uint8_t ct_state;   /* Connection tracking state. */
+uint8_t pad0;
 uint16_t ct_zone;   /* Connection tracking zone. */
 uint32_t ct_mark;   /* Connection mark.*/
 uint8_t pad1[4];/* Pad to 64 bits. */
diff --git a/lib/flow.c b/lib/flow.c
index fb7bfeb..0c95b75 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -593,7 +593,8 @@ miniflow_extract(struct dp_packet *packet, struct miniflow 
*dst)
 miniflow_push_uint32(mf, in_port, odp_to_u32(md->in_port.odp_port));
 if (md->recirc_id || md->ct_state) {
 miniflow_push_uint32(mf, recirc_id, md->recirc_id);
-miniflow_push_uint16(mf, ct_state, md->ct_state);
+miniflow_push_uint8(mf, ct_state, md->ct_state);
+miniflow_push_uint8(mf, pad0, 0);
 miniflow_push_uint16(mf, ct_zone, md->ct_zone);
 }
 
diff --git a/lib/match.c b/lib/match.c
index 3fcaec5..882bf0c 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -340,8 +340,8 @@ match_set_ct_state(struct match *match, uint32_t ct_state)
 void
 match_set_ct_state_masked(struct match *match, uint32_t ct_state, uint32_t 
mask)
 {
-match->flow.ct_state = ct_state & mask & UINT16_MAX;
-match->wc.masks.ct_state = mask & UINT16_MAX;
+match->flow.ct_state = ct_state & mask & UINT8_MAX;
+match->wc.masks.ct_state = mask & UINT8_MAX;
 }
 
 void
@@ -,7 +,7 @@ match_format(const struct match *match, struct ds *s, int 
priority)
 }
 
 if (wc->masks.ct_state) {
-if (wc->masks.ct_state == UINT16_MAX) {
+if (wc->masks.ct_state == UINT8_MAX) {
 ds_put_format(s, "%sct_state=%s", colors.param, colors.end);
 if (f->ct_state) {
 format_flags(s, ct_state_to_string, f->ct_state, '|');
@@ -1120,7 +1120,7 @@ match_format(const struct match *match, struct ds *s, int 
priority)
 }
 } else {
 format_flags_masked(s, "ct_state", ct_state_to_string,
-f->ct_state, wc->masks.ct_state, UINT16_MAX);
+f->ct_state, wc->masks.ct_state, UINT8_MAX);
 }
 ds_put_char(s, ',');
 }
diff --git a/lib/packets.h b/lib/packets.h
index c4d3799..f7e1d82 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -99,7 +99,7 @@ struct pkt_metadata {
action. */
 uint32_t skb_priority;  /* Packet priority for QoS. */
 uint32_t pkt_mark;  /* Packet mark. */
-uint16_t ct_state;  /* Connection state. */
+uint8_t  ct_state;  /* Connection state. */
 uint16_t ct_zone;   /* Connection zone. */
 uint32_t ct_mark;   /* Connection mark. */
 ovs_u128 ct_label;  /* Connection label. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 1e1b107..0bf3786 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3996,7 +3996,7 @@ check_mask(struct ofproto_dpif *ofproto, const struct 
miniflow *flow)
 uint32_t ct_mark;
 
 support = >backer->support.odp;
-ct_state = MINIFLOW_GET_U16(flow, ct_state);
+ct_state = MINIFLOW_GET_U8(flow, ct_state);
 if (support->ct_state && support->ct_zone && support->ct_mark
 && support->ct_label && support->ct_state_nat) {
 return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0;
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 7e26735..bea3cf4 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -1213,9 +1213,9 @@ NXM_NX_REG0(a0e0d050)
 
 # Connection tracking fields,
 dnl
-dnl When re-serialising, bits 16-31 are wildcarded, because current OVS 
userspace
+dnl When re-serialising, bits 8-31 are wildcarded, because current OVS 
userspace
 dnl doesn't understand (or store) those bits.
-NXM_OF_ETH_TYPE(0800), NXM_NX_CT_STATE_W(0020/)
+NXM_OF_ETH_TYPE(0800), NXM_NX_CT_STATE_W(0020/00ff)
 nx_pull_match() returned error OFPBMC_BAD_VALUE
 NXM_OF_ETH_TYPE(0800), NXM_NX_CT_STATE_W(0020/0020)
 NXM_OF_ETH_TYPE(0800), NXM_NX_CT_STATE_W(0020/00f0)
-- 
2.1.4

[ovs-dev] [PATCH v3 12/22] lib: Check match and action prerequisities with 'match'.

[Jarno Rajahalme]

Supply the match mask to prerequisities checking when available.  This
allows checking for zero-valued matches.  Non-zero valued matches
imply the presense of corresponding mask bits, but for zero valued
matches we must explicitly check the mask, too.

This is required now only for conntrack validity checking due to the
conntrack state having and 'invalid' bit, but not 'valid' bit.  One
way to match an valid conntrack state is to match on the 'tracked' bit
being one and 'invalid' bit being zero.  The latter requires the
corresponding mask bit be verified.

Signed-off-by: Jarno Rajahalme 
---
 include/openvswitch/meta-flow.h   |  5 ++--
 include/openvswitch/ofp-actions.h |  4 ++--
 lib/bundle.c  |  4 ++--
 lib/bundle.h  |  3 ++-
 lib/learn.c   | 15 ++--
 lib/learn.h   |  3 ++-
 lib/meta-flow.c   | 38 ++---
 lib/multipath.c   |  4 ++--
 lib/multipath.h   |  3 ++-
 lib/nx-match.c| 17 ++---
 lib/nx-match.h|  6 ++---
 lib/ofp-actions.c | 50 ---
 lib/ofp-parse.c   |  2 +-
 lib/ofp-util.c|  2 +-
 ofproto/ofproto-dpif-trace.c  | 13 +-
 ofproto/ofproto.c |  5 ++--
 utilities/ovs-ofctl.c |  6 ++---
 17 files changed, 105 insertions(+), 75 deletions(-)

diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 83e2599..aac9945 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -1898,6 +1898,7 @@ void mf_get_mask(const struct mf_field *, const struct 
flow_wildcards *,
 /* Prerequisites. */
 bool mf_are_prereqs_ok(const struct mf_field *mf, const struct flow *flow,
struct flow_wildcards *wc);
+bool mf_are_match_prereqs_ok(const struct mf_field *, const struct match *);
 
 static inline bool
 mf_is_l3_or_higher(const struct mf_field *mf)
@@ -1959,8 +1960,8 @@ void mf_subfield_swap(const struct mf_subfield *,
   const struct mf_subfield *,
   struct flow *flow, struct flow_wildcards *);
 
-enum ofperr mf_check_src(const struct mf_subfield *, const struct flow *);
-enum ofperr mf_check_dst(const struct mf_subfield *, const struct flow *);
+enum ofperr mf_check_src(const struct mf_subfield *, const struct match *);
+enum ofperr mf_check_dst(const struct mf_subfield *, const struct match *);
 
 /* Parsing and formatting. */
 char *mf_parse(const struct mf_field *, const char *,
diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index 88f573d..53d6b44 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -954,11 +954,11 @@ ofpacts_pull_openflow_instructions(struct ofpbuf 
*openflow,
const struct vl_mff_map *vl_mff_map,
struct ofpbuf *ofpacts);
 enum ofperr ofpacts_check(struct ofpact[], size_t ofpacts_len,
-  struct flow *, ofp_port_t max_ports,
+  struct match *, ofp_port_t max_ports,
   uint8_t table_id, uint8_t n_tables,
   enum ofputil_protocol *usable_protocols);
 enum ofperr ofpacts_check_consistency(struct ofpact[], size_t ofpacts_len,
-  struct flow *, ofp_port_t max_ports,
+  struct match *, ofp_port_t max_ports,
   uint8_t table_id, uint8_t n_tables,
   enum ofputil_protocol usable_protocols);
 enum ofperr ofpact_check_output_port(ofp_port_t port, ofp_port_t max_ports);
diff --git a/lib/bundle.c b/lib/bundle.c
index 70a743b..620318e 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -105,13 +105,13 @@ bundle_execute(const struct ofpact_bundle *bundle,
 
 enum ofperr
 bundle_check(const struct ofpact_bundle *bundle, ofp_port_t max_ports,
- const struct flow *flow)
+ const struct match *match)
 {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 size_t i;
 
 if (bundle->dst.field) {
-enum ofperr error = mf_check_dst(>dst, flow);
+enum ofperr error = mf_check_dst(>dst, match);
 if (error) {
 return error;
 }
diff --git a/lib/bundle.h b/lib/bundle.h
index f5ce321..48b9b79 100644
--- a/lib/bundle.h
+++ b/lib/bundle.h
@@ -29,6 +29,7 @@
 struct ds;
 struct flow;
 struct flow_wildcards;
+struct match;
 struct ofpact_bundle;
 struct ofpbuf;
 
@@ -43,7 +44,7 @@ ofp_port_t bundle_execute(const struct ofpact_bundle *, const 
struct flow *,
 bool (*slave_enabled)(ofp_port_t ofp_port, void *aux),
 void *aux);
 enum ofperr bundle_check(const struct 

[ovs-dev] [PATCH v3 11/22] netlink: Simplify nl_msg_start_nested().

[Jarno Rajahalme]

Since there is no data to copy nl_msg_put_unspec_uninit() may be used
directly, rather than via nl_msg_put_unspec().

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 lib/netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netlink.c b/lib/netlink.c
index ae4c72a..3da22a1 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -454,7 +454,7 @@ size_t
 nl_msg_start_nested(struct ofpbuf *msg, uint16_t type)
 {
 size_t offset = msg->size;
-nl_msg_put_unspec(msg, type, NULL, 0);
+nl_msg_put_unspec_uninit(msg, type, 0);
 return offset;
 }
 
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 09/22] datapath: Refactor labels initialization.

[Jarno Rajahalme]

Upstream commit:

Refactoring conntrack labels initialization makes changes in later
patches easier to review.

Signed-off-by: Jarno Rajahalme 
Acked-by: Pravin B Shelar 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Signed-off-by: Jarno Rajahalme 
---
 datapath/conntrack.c | 120 +++
 1 file changed, 64 insertions(+), 56 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 2d095b8..a56fe07 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -136,15 +136,6 @@ static u32 ovs_ct_get_mark(const struct nf_conn *ct)
 #endif
 }
 
-static size_t ovs_ct_get_labels_len(struct nf_conn_labels *cl)
-{
-#ifdef HAVE_NF_CONN_LABELS_WITH_WORDS
-   return cl->words * sizeof(long);
-#else
-   return sizeof(cl->bits);
-#endif
-}
-
 /* Guard against conntrack labels max size shrinking below 128 bits. */
 #if NF_CT_LABELS_MAX_SIZE < 16
 #error NF_CT_LABELS_MAX_SIZE must be at least 16 bytes
@@ -243,19 +234,12 @@ int ovs_ct_put_key(const struct sw_flow_key *key, struct 
sk_buff *skb)
return 0;
 }
 
-static int ovs_ct_set_mark(struct sk_buff *skb, struct sw_flow_key *key,
+static int ovs_ct_set_mark(struct nf_conn *ct, struct sw_flow_key *key,
   u32 ct_mark, u32 mask)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
-   enum ip_conntrack_info ctinfo;
-   struct nf_conn *ct;
u32 new_mark;
 
-   /* The connection could be invalid, in which case set_mark is no-op. */
-   ct = nf_ct_get(skb, );
-   if (!ct)
-   return 0;
-
new_mark = ct_mark | (ct->mark & ~(mask));
if (ct->mark != new_mark) {
ct->mark = new_mark;
@@ -270,18 +254,9 @@ static int ovs_ct_set_mark(struct sk_buff *skb, struct 
sw_flow_key *key,
 #endif
 }
 
-static int ovs_ct_set_labels(struct sk_buff *skb, struct sw_flow_key *key,
-const struct ovs_key_ct_labels *labels,
-const struct ovs_key_ct_labels *mask)
+static struct nf_conn_labels *ovs_ct_get_conn_labels(struct nf_conn *ct)
 {
-   enum ip_conntrack_info ctinfo;
struct nf_conn_labels *cl;
-   struct nf_conn *ct;
-
-   /* The connection could be invalid, in which case set_label is no-op.*/
-   ct = nf_ct_get(skb, );
-   if (!ct)
-   return 0;
 
cl = nf_ct_labels_find(ct);
if (!cl) {
@@ -289,37 +264,59 @@ static int ovs_ct_set_labels(struct sk_buff *skb, struct 
sw_flow_key *key,
cl = nf_ct_labels_find(ct);
}
 
-   if (!cl || ovs_ct_get_labels_len(cl) < OVS_CT_LABELS_LEN)
+   return cl;
+}
+
+/* Initialize labels for a new, yet to be committed conntrack entry.  Note that
+ * since the new connection is not yet confirmed, and thus no-one else has
+ * access to it's labels, we simply write them over.
+ */
+static int ovs_ct_init_labels(struct nf_conn *ct, struct sw_flow_key *key,
+ const struct ovs_key_ct_labels *labels,
+ const struct ovs_key_ct_labels *mask)
+{
+   struct nf_conn_labels *cl;
+   u32 *dst;
+   int i;
+
+   cl = ovs_ct_get_conn_labels(ct);
+   if (!cl)
return -ENOSPC;
 
-   if (nf_ct_is_confirmed(ct)) {
-   /* Triggers a change event, which makes sense only for
-* confirmed connections.
-*/
-   int err = nf_connlabels_replace(ct, labels->ct_labels_32,
-   mask->ct_labels_32,
-   OVS_CT_LABELS_LEN_32);
-   if (err)
-   return err;
-   } else {
-   u32 *dst = (u32 *)cl->bits;
-   const u32 *msk = mask->ct_labels_32;
-   const u32 *lbl = labels->ct_labels_32;
-   int i;
+   dst = (u32 *)cl->bits;
+   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
+   dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
+   (labels->ct_labels_32[i] & mask->ct_labels_32[i]);
 
-   /* No-one else has access to the non-confirmed entry, copy
-* labels over, keeping any bits we are not explicitly setting.
-*/
-   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
-   dst[i] = (dst[i] & ~msk[i]) | (lbl[i] & msk[i]);
+   /* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
+* IPCT_LABEL bit it set in the event cache.
+*/
+   nf_conntrack_event_cache(IPCT_LABEL, ct);
 
-   /* Labels are included in the IPCTNL_MSG_CT_NEW event only if
-* the IPCT_LABEL bit it set in the event cache.
-*/
-   nf_conntrack_event_cache(IPCT_LABEL, ct);
-   }
+   memcpy(>ct.labels, cl->bits, 

[ovs-dev] [PATCH v3 10/22] datapath: Inherit master's labels.

[Jarno Rajahalme]

Upstream commit:

commit 09aa98ad496d6b11a698b258bc64d7f64c55d682
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:58 2017 -0800

openvswitch: Inherit master's labels.

We avoid calling into nf_conntrack_in() for expected connections, as
that would remove the expectation that we want to stick around until
we are ready to commit the connection.  Instead, we do a lookup in the
expectation table directly.  However, after a successful expectation
lookup we have set the flow key label field from the master
connection, whereas nf_conntrack_in() does not do this.  This leads to
master's labels being inherited after an expectation lookup, but those
labels not being inherited after the corresponding conntrack action
with a commit flag.

This patch resolves the problem by changing the commit code path to
also inherit the master's labels to the expected connection.
Resolving this conflict in favor of inheriting the labels allows more
information be passed from the master connection to related
connections, which would otherwise be much harder if the 32 bits in
the connmark are not enough.  Labels can still be set explicitly, so
this change only affects the default values of the labels in presense
of a master connection.

Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
Signed-off-by: Jarno Rajahalme 
Acked-by: Pravin B Shelar 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Fixes: a94ebc39996b ("datapath: Add conntrack action")
Signed-off-by: Jarno Rajahalme 
---
 datapath/conntrack.c | 45 +++--
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index a56fe07..9428eb2 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -80,6 +80,8 @@ struct ovs_conntrack_info {
 #endif
 };
 
+static bool labels_nonzero(const struct ovs_key_ct_labels *labels);
+
 static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info);
 
 static u16 key_to_nfproto(const struct sw_flow_key *key)
@@ -275,18 +277,32 @@ static int ovs_ct_init_labels(struct nf_conn *ct, struct 
sw_flow_key *key,
  const struct ovs_key_ct_labels *labels,
  const struct ovs_key_ct_labels *mask)
 {
-   struct nf_conn_labels *cl;
-   u32 *dst;
-   int i;
+   struct nf_conn_labels *cl, *master_cl;
+   bool have_mask = labels_nonzero(mask);
+
+   /* Inherit master's labels to the related connection? */
+   master_cl = ct->master ? nf_ct_labels_find(ct->master) : NULL;
+
+   if (!master_cl && !have_mask)
+   return 0;   /* Nothing to do. */
 
cl = ovs_ct_get_conn_labels(ct);
if (!cl)
return -ENOSPC;
 
-   dst = (u32 *)cl->bits;
-   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
-   dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
-   (labels->ct_labels_32[i] & mask->ct_labels_32[i]);
+   /* Inherit the master's labels, if any. */
+   if (master_cl)
+   *cl = *master_cl;
+
+   if (have_mask) {
+   u32 *dst = (u32 *)cl->bits;
+   int i;
+
+   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
+   dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
+   (labels->ct_labels_32[i]
+& mask->ct_labels_32[i]);
+   }
 
/* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
 * IPCT_LABEL bit it set in the event cache.
@@ -943,13 +959,14 @@ static int ovs_ct_commit(struct net *net, struct 
sw_flow_key *key,
if (err)
return err;
}
-   if (labels_nonzero(>labels.mask)) {
-   if (!nf_ct_is_confirmed(ct))
-   err = ovs_ct_init_labels(ct, key, >labels.value,
->labels.mask);
-   else
-   err = ovs_ct_set_labels(ct, key, >labels.value,
-   >labels.mask);
+   if (!nf_ct_is_confirmed(ct)) {
+   err = ovs_ct_init_labels(ct, key, >labels.value,
+>labels.mask);
+   if (err)
+   return err;
+   } else if (labels_nonzero(>labels.mask)) {
+   err = ovs_ct_set_labels(ct, key, >labels.value,
+   >labels.mask);
if (err)
return err;
}
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 08/22] datapath: Simplify labels length logic.

[Jarno Rajahalme]

Upstream commit:

commit b87cec3814ccc7f6afb0a1378ee7e5110d07cdd3
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:56 2017 -0800

openvswitch: Simplify labels length logic.

Since 23014011ba42 ("netfilter: conntrack: support a fixed size of 128
distinct labels"), the size of conntrack labels extension has fixed to
128 bits, so we do not need to check for labels sizes shorter than 128
at run-time.  This patch simplifies labels length logic accordingly,
but allows the conntrack labels size to be increased in the future
without breaking the build.  In the event of conntrack labels
increasing in size OVS would still be able to deal with the 128 first
label bits.

Suggested-by: Joe Stringer 
Signed-off-by: Jarno Rajahalme 
Acked-by: Pravin B Shelar 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Signed-off-by: Jarno Rajahalme 
---
 datapath/conntrack.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index b5c80be..2d095b8 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -145,22 +145,20 @@ static size_t ovs_ct_get_labels_len(struct nf_conn_labels 
*cl)
 #endif
 }
 
+/* Guard against conntrack labels max size shrinking below 128 bits. */
+#if NF_CT_LABELS_MAX_SIZE < 16
+#error NF_CT_LABELS_MAX_SIZE must be at least 16 bytes
+#endif
+
 static void ovs_ct_get_labels(const struct nf_conn *ct,
  struct ovs_key_ct_labels *labels)
 {
struct nf_conn_labels *cl = ct ? nf_ct_labels_find(ct) : NULL;
 
-   if (cl) {
-   size_t len = ovs_ct_get_labels_len(cl);
-
-   if (len > OVS_CT_LABELS_LEN)
-   len = OVS_CT_LABELS_LEN;
-   else if (len < OVS_CT_LABELS_LEN)
-   memset(labels, 0, OVS_CT_LABELS_LEN);
-   memcpy(labels, cl->bits, len);
-   } else {
+   if (cl)
+   memcpy(labels, cl->bits, OVS_CT_LABELS_LEN);
+   else
memset(labels, 0, OVS_CT_LABELS_LEN);
-   }
 }
 
 static void __ovs_ct_update_key(struct sw_flow_key *key, u8 state,
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 07/22] datapath: Unionize ovs_key_ct_label with a u32 array.

[Jarno Rajahalme]

Upstream commit:

commit cb80d58fae76d8ea93555149b2b16e19b89a1f4f
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:55 2017 -0800

openvswitch: Unionize ovs_key_ct_label with a u32 array.

Make the array of labels in struct ovs_key_ct_label an union, adding a
u32 array of the same byte size as the existing u8 array.  It is
faster to loop through the labels 32 bits at the time, which is also
the alignment of netlink attributes.

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Jarno Rajahalme 
---
 datapath/conntrack.c  | 15 ---
 datapath/linux/compat/include/linux/openvswitch.h |  8 ++--
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index cee47b7..b5c80be 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -298,20 +298,21 @@ static int ovs_ct_set_labels(struct sk_buff *skb, struct 
sw_flow_key *key,
/* Triggers a change event, which makes sense only for
 * confirmed connections.
 */
-   int err = nf_connlabels_replace(ct, (u32 *)labels, (u32 *)mask,
-   OVS_CT_LABELS_LEN / 
sizeof(u32));
+   int err = nf_connlabels_replace(ct, labels->ct_labels_32,
+   mask->ct_labels_32,
+   OVS_CT_LABELS_LEN_32);
if (err)
return err;
} else {
u32 *dst = (u32 *)cl->bits;
-   const u32 *msk = (const u32 *)mask->ct_labels;
-   const u32 *lbl = (const u32 *)labels->ct_labels;
+   const u32 *msk = mask->ct_labels_32;
+   const u32 *lbl = labels->ct_labels_32;
int i;
 
/* No-one else has access to the non-confirmed entry, copy
 * labels over, keeping any bits we are not explicitly setting.
 */
-   for (i = 0; i < OVS_CT_LABELS_LEN / sizeof(u32); i++)
+   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
dst[i] = (dst[i] & ~msk[i]) | (lbl[i] & msk[i]);
 
/* Labels are included in the IPCTNL_MSG_CT_NEW event only if
@@ -912,8 +913,8 @@ static bool labels_nonzero(const struct ovs_key_ct_labels 
*labels)
 {
size_t i;
 
-   for (i = 0; i < sizeof(*labels); i++)
-   if (labels->ct_labels[i])
+   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
+   if (labels->ct_labels_32[i])
return true;
 
return false;
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 36f7697..e1b8138 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -472,9 +472,13 @@ struct ovs_key_nd {
__u8nd_tll[ETH_ALEN];
 };
 
-#define OVS_CT_LABELS_LEN  16
+#define OVS_CT_LABELS_LEN_32   4
+#define OVS_CT_LABELS_LEN  (OVS_CT_LABELS_LEN_32 * sizeof(__u32))
 struct ovs_key_ct_labels {
-   __u8ct_labels[OVS_CT_LABELS_LEN];
+   union {
+   __u8ct_labels[OVS_CT_LABELS_LEN];
+   __u32   ct_labels_32[OVS_CT_LABELS_LEN_32];
+   };
 };
 
 /* OVS_KEY_ATTR_CT_STATE flags */
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 05/22] datapath: Use inverted tuple in ovs_ct_find_existing() if NATted.

[Jarno Rajahalme]

Upstream commit:

commit 9ff464db50e437eef131f719cc2e9902eea9c607
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:53 2017 -0800

openvswitch: Use inverted tuple in ovs_ct_find_existing() if NATted.

The conntrack lookup for existing connections fails to invert the
packet 5-tuple for NATted packets, and therefore fails to find the
existing conntrack entry.  Conntrack only stores 5-tuples for incoming
packets, and there are various situations where a lookup on a packet
that has already been transformed by NAT needs to be made.  Looking up
an existing conntrack entry upon executing packet received from the
userspace is one of them.

This patch fixes ovs_ct_find_existing() to invert the packet 5-tuple
for the conntrack lookup whenever the packet has already been
transformed by conntrack from its input form as evidenced by one of
the NAT flags being set in the conntrack state metadata.

Fixes: 05752523e565 ("openvswitch: Interface with NAT.")
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

This patch also adds a test case to OVS system tests to verify the
behavior.

The following is a more thorough explanation of what is going on:

When we have evidence that an existing conntrack entry could exist, we
must invert the tuple if NAT has already been applied, as the current
packet headers do not match any tuple stored in conntrack.  For
example, if a packet from private address X to a public address B is
source-NATted to A, the conntrack entry will have the following tuples
(ignoring the protocol and port numbers) after the conntrack entry is
committed:

Original direction tuple: (X,B)
Reply direction tuple: (B,A)

Now, if a reply packet is already transformed back to the private
address space (e.g., with a CT(nat) action), the tuple corresponding
to the current packet headers is:

Current packet tuple: (B,X)

This does not match either of the conntrack tuples above.  Normally
this does not matter, as the conntrack lookup was already done using
the tuple (B,A), but if the current packet does not match any flow in
the OVS datapath, the packet is sent to userspace via an upcall,
during which the packet's skb is freed, and the conntrack entry
pointer in the skb is lost.  When the packet is reintroduced to the
datapath, any further conntrack action will need to perform a new
conntrack lookup to find the entry again.  Prior to this patch this
second lookup failed.  The datapath flow setup corresponding to the
upcall can succeed, however, allowing all further packets in the reply
direction to re-use the conntrack entry pointer in the skb, so
typically the lookup failure only causes a packet drop.

The solution is to invert the tuple derived from the current packet
headers in case the conntrack state stored in the packet metadata
indicates that the packet has been transformed by NAT:

Inverted tuple: (X,B)

With this the conntrack entry can be found, matching the original
direction tuple.

This same logic also works for the original direction packets:

Current packet tuple (after reverse NAT): (A,B)
Inverted tuple: (B,A)

While the current packet tuple (A,B) does not match either of the
conntrack tuples, the inverted one (B,A) does match the reply
direction tuple.

Since the inverted tuple matches the reverse direction tuple the
direction of the packet must be reversed as well.

Fixes: c5f6c06b58d6 ("datapath: Interface with NAT.")
Signed-off-by: Jarno Rajahalme 
---
 datapath/conntrack.c| 24 +--
 tests/system-traffic.at | 52 +
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 9e34af9..08e5eab 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -471,7 +471,7 @@ ovs_ct_get_info(const struct nf_conntrack_tuple_hash *h)
  */
 static struct nf_conn *
 ovs_ct_find_existing(struct net *net, const struct nf_conntrack_zone *zone,
-u8 l3num, struct sk_buff *skb)
+u8 l3num, struct sk_buff *skb, bool natted)
 {
struct nf_conntrack_l3proto *l3proto;
struct nf_conntrack_l4proto *l4proto;
@@ -494,6 +494,17 @@ ovs_ct_find_existing(struct net *net, const struct 
nf_conntrack_zone *zone,
return NULL;
}
 
+   /* Must invert the tuple if skb has been transformed by NAT. */
+   if (natted) {
+   struct nf_conntrack_tuple inverse;
+
+   if (!nf_ct_invert_tuple(, , l3proto, l4proto)) {
+   pr_debug("ovs_ct_find_existing: Inversion failed!\n");
+   return NULL;
+   }
+   tuple = inverse;
+   }
+
/* look for tuple match */
h = nf_conntrack_find_get(net, zone, );
   

[ovs-dev] [PATCH v3 02/22] datapath: add and use skb_nfct helper

[Jarno Rajahalme]

From: Florian Westphal 

Upstream commit:

commit cb9c68363efb6d1f950ec55fb06e031ee70db5fc
Author: Florian Westphal 
Date:   Mon Jan 23 18:21:56 2017 +0100

skbuff: add and use skb_nfct helper

Followup patch renames skb->nfct and changes its type so add a helper to
avoid intrusive rename change later.

Signed-off-by: Florian Westphal 
Signed-off-by: Pablo Neira Ayuso 

Signed-off-by: Jarno Rajahalme 
---
 acinclude.m4 |  1 +
 datapath/conntrack.c |  6 +++---
 datapath/linux/compat/include/linux/skbuff.h | 11 +++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 5ffb5a7..caf42cb 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -607,6 +607,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_clear_hash_if_not_l4])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_postpush_rcsum])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [lco_csum])
+  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_nfct])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/types.h], [bool],
   [OVS_DEFINE([HAVE_BOOL_TYPE])])
diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 36db32a..ec354c3 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -763,8 +763,8 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
 
/* Associate skb with specified zone. */
if (tmpl) {
-   if (skb->nfct)
-   nf_conntrack_put(skb->nfct);
+   if (skb_nfct(skb))
+   nf_conntrack_put(skb_nfct(skb));
nf_conntrack_get(>ct_general);
skb->nfct = >ct_general;
skb->nfctinfo = IP_CT_NEW;
@@ -861,7 +861,7 @@ static int ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
if (err)
return err;
 
-   ct = (struct nf_conn *)skb->nfct;
+   ct = (struct nf_conn *)skb_nfct(skb);
if (ct)
nf_ct_deliver_cached_events(ct);
}
diff --git a/datapath/linux/compat/include/linux/skbuff.h 
b/datapath/linux/compat/include/linux/skbuff.h
index 6c45939..2a6cf2f 100644
--- a/datapath/linux/compat/include/linux/skbuff.h
+++ b/datapath/linux/compat/include/linux/skbuff.h
@@ -376,4 +376,15 @@ static inline __wsum lco_csum(struct sk_buff *skb)
return csum_partial(l4_hdr, csum_start - l4_hdr, partial);
 }
 #endif
+
+#ifndef HAVE_SKB_NFCT
+static inline struct nf_conntrack *skb_nfct(const struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+   return skb->nfct;
+#else
+   return NULL;
+#endif
+}
+#endif
 #endif
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 01/22] datapath: Allow compiling against Linux 4.10

[Jarno Rajahalme]

OVS in-tree datapath compiles against Linux 4.10 kernel, so allow it.

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 acinclude.m4 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 19cffe0..5ffb5a7 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [
 AC_MSG_RESULT([$kversion])
 
 if test "$version" -ge 4; then
-   if test "$version" = 4 && test "$patchlevel" -le 9; then
+   if test "$version" = 4 && test "$patchlevel" -le 10; then
   : # Linux 4.x
else
-  AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
newer than 4.9.x is not supported (please refer to the FAQ for advice)])
+  AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
newer than 4.10.x is not supported (please refer to the FAQ for advice)])
fi
 elif test "$version" = 3 && test "$patchlevel" -ge 10; then
: # Linux 3.x
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 00/22] Conntrack enhancements

[Jarno Rajahalme]

This patch set backports the recent upstream conntrack fixes and new
features to the OVS tree kernel module, and adds the OVS userspace
support.

Patch 1/22 is an unrelated datapath backport, and patch 22/22 allows
compiling against Linux 4.10.

Each new feature is introduced in two different commits, the first is
the datapath backport, the second the corresponding userspace datapath
and non-datapath functionality, including OVS system tests.  In one
instance I have squashed the system test with the datapath backport.
Compile would fail after the first patch due to missing userspace code
for new enums.  We may decide to squash the datapath and userspace
changes together for the merge, but for now the review should be more
straightforward with the separation.

System tests have been most recently run on Linux 3.16, on which the
geneve tests fail, but that should have nothing to do with this
series.

v3: Address Joe's feedback.

Florian Westphal (2):
  datapath: add and use skb_nfct helper
  datapath: add and use nf_ct_set helper

Jarno Rajahalme (20):
  datapath: Allow compiling against Linux 4.10
  datapath: Fix comments for skb->_nfct
  datapath: Use inverted tuple in ovs_ct_find_existing() if NATted.
  datapath: Do not trigger events for unconfirmed connections.
  datapath: Unionize ovs_key_ct_label with a u32 array.
  datapath: Simplify labels length logic.
  datapath: Refactor labels initialization.
  datapath: Inherit master's labels.
  netlink: Simplify nl_msg_start_nested().
  lib: Check match and action prerequisities with 'match'.
  datapath: Add original direction conntrack tuple to sw_flow_key.
  flow: Make room after ct_state.
  ofp-util: Ignore unknown fields in ofputil_decode_packet_in2().
  odp: Support conntrack orig tuple key.
  actions: Add resubmit with conntrack tuple.
  compat: nf_ct_delete compat.
  datapath: Add force commit.
  conntrack: Force commit.
  datapath: Add a missing comment.
  tests: Add an FTP test without conntrack.

 acinclude.m4   |  13 +-
 build-aux/extract-ofp-fields   |   3 +
 datapath/actions.c |   2 +
 datapath/conntrack.c   | 295 +
 datapath/conntrack.h   |  10 +-
 datapath/flow.c|  34 +-
 datapath/flow.h|  49 ++-
 datapath/flow_netlink.c|  85 +++--
 datapath/flow_netlink.h|   7 +-
 datapath/linux/compat/include/linux/openvswitch.h  |  33 +-
 datapath/linux/compat/include/linux/skbuff.h   |  11 +
 .../compat/include/net/netfilter/nf_conntrack.h|   8 +
 .../include/net/netfilter/nf_conntrack_core.h  |  37 +++
 include/openvswitch/flow.h |  16 +-
 include/openvswitch/match.h|  16 +
 include/openvswitch/meta-flow.h| 141 +++-
 include/openvswitch/ofp-actions.h  |  15 +-
 lib/bundle.c   |   4 +-
 lib/bundle.h   |   3 +-
 lib/conntrack.c|  59 +++-
 lib/conntrack.h|   2 +-
 lib/dpif-netdev.c  |   8 +-
 lib/flow.c | 229 +
 lib/flow.h |  50 +++
 lib/learn.c|  15 +-
 lib/learn.h|   3 +-
 lib/match.c| 118 ++-
 lib/meta-flow.c| 193 ++-
 lib/meta-flow.xml  |  92 ++
 lib/multipath.c|   4 +-
 lib/multipath.h|   3 +-
 lib/netlink.c  |   2 +-
 lib/nx-match.c |  58 ++--
 lib/nx-match.h |  10 +-
 lib/odp-execute.c  |   4 +
 lib/odp-util.c | 144 -
 lib/odp-util.h |   8 +-
 lib/ofp-actions.c  | 161 +++---
 lib/ofp-parse.c|   2 +-
 lib/ofp-util.c |   9 +-
 lib/packets.h  |   7 +-
 ofproto/ofproto-dpif-rid.h |   2 +-
 ofproto/ofproto-dpif-sflow.c   |   2 +
 ofproto/ofproto-dpif-trace.c   |  13 +-
 ofproto/ofproto-dpif-xlate.c   |  91 +-
 ofproto/ofproto-dpif.c |   4 +-
 ofproto/ofproto.c  |   5 +-
 tests/odp.at   |  18 +-
 

[ovs-dev] [PATCH] ofproto-dpif-xlate: fix build.

[Jarno Rajahalme]

Recent patch 27d931da3ac ("ofproto-dpif: Enhance
execute_controller_action().") missed some new action enumerations
added previously.

Fixes: 27d931da3ac ("ofproto-dpif: Enhance execute_controller_action().")
Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif-xlate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d915ba1..eda34f0 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3859,6 +3859,8 @@ xlate_fixup_actions(struct ofpbuf *b, const struct nlattr 
*actions,
 case OVS_ACTION_ATTR_USERSPACE:
 case OVS_ACTION_ATTR_RECIRC:
 case OVS_ACTION_ATTR_CT:
+case OVS_ACTION_ATTR_PUSH_ETH:
+case OVS_ACTION_ATTR_POP_ETH:
 ofpbuf_put(b, a, nl_attr_len_pad(a, left));
 break;
 
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] ofproto-dpif: Enhance execute_controller_action().

[Andy Zhou]

On Thu, Mar 2, 2017 at 2:59 PM, Jarno Rajahalme  wrote:
> With the notes below:
>
> Acked-by: Jarno Rajahalme 

Thanks for the review, I have pushed all patches in the series with
most comments applied as suggested.
>
>> On Feb 16, 2017, at 5:11 PM, Andy Zhou  wrote:
>>
>> Allow execute_controller_action() to accept actions encoded with
>> nested netlink attributes.
>>
>> execute_controller_action() can be called during 'xlate_actions'. It
>> tries executes all actions translated so far to get the current packet
>> that needs to be sent to the controller.  This works fine until when
>> the action is enclosed within a nested netlink message, and the
>> action translation has not finished yet.
>>
>> For example;
>> A, clone(B, controller, C)
>>
>> In this case, we can not execute 'clone' since its translation has not
>> be finished (missing C), However, A still needs to be executed before
>> the packet can be sent to the controller.
>>
>> This solution is to make a copy of the odp actions translated so far,
>> and 'fix up' the copy so that it can be executed. The original odp
>> actions are left intact so that xlate can continue.
>>
>> Signed-off-by: Andy Zhou 
>> ---
>> ofproto/ofproto-dpif-xlate.c | 149 
>> +--
>> 1 file changed, 144 insertions(+), 5 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index 503a347..c4ca5d2 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -3805,13 +3805,150 @@ flood_packets(struct xlate_ctx *ctx, bool all)
>> ctx->nf_output_iface = NF_OUT_FLOOD;
>> }
>>
>> +/* Copy and reformat a partially xlated odp actions to a new
>> + * odp actions list in 'b', so that the new actions list
>> + * can be executed by odp_execute_actions.
>> + *
>> + * When xlate using nested odp actions, such as sample and clone,
>> + * The nested action created by nl_msg_start_nested() may not
>
> “the”
>
>> + * have been properly closed yet, thus can not be executed
>> + * directly.
>> + *
>> + * Since unclosed nested action has to be last action, it can be
>> + * fixed by skip the outer header, and treat the actions within
>
> “skipping”, “treating"
>
>> + * as if they are outside the nested attribute. Since the effect
>
> “, since"
>
>> + * of executing them on packet is the same.
>> + *
>> + * As an optimization, a fully closed 'sample' or 'clone' action
>> + * is skipped since their execution has no effect to the packet.
>> + *
>
> In this case the actions are executed without a datapath helper, so none of 
> the datapath dependent actions (HASH, OUTPUT, USERSPACE, RECIRC, CT) are 
> actually executed, so maybe they could be skipped as well? Same for the 
> TRUNC, as it only has an effect on OUTPUT, which will not be executed.

I did not do this because it seems to be a layer violation, The idea
of which actions needs datapath help is currently encapsulated within
the ODP layer. Since this is a slow path anyways, such optimization
may not be critical.
>
>> + * Returns true if success. 'b' contains the new actions list.
>> + * The caller is responsible for dispose 'b'.
>> + *
>
> “disposing"
>
>> + * Returns false if error, 'b' has been freed already.  */
>> +static bool
>> +xlate_fixup_actions(struct ofpbuf *b, const struct nlattr *actions,
>> +size_t actions_len)
>> +{
>> +const struct nlattr *a;
>> +unsigned int left;
>> +
>> +NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) {
>> +int type = nl_attr_type(a);
>> +
>> +switch ((enum ovs_action_attr) type) {
>> +case OVS_ACTION_ATTR_HASH:
>> +case OVS_ACTION_ATTR_PUSH_VLAN:
>> +case OVS_ACTION_ATTR_POP_VLAN:
>> +case OVS_ACTION_ATTR_PUSH_MPLS:
>> +case OVS_ACTION_ATTR_POP_MPLS:
>> +case OVS_ACTION_ATTR_SET:
>> +case OVS_ACTION_ATTR_SET_MASKED:
>> +case OVS_ACTION_ATTR_TRUNC:
>> +case OVS_ACTION_ATTR_OUTPUT:
>> +case OVS_ACTION_ATTR_TUNNEL_PUSH:
>> +case OVS_ACTION_ATTR_TUNNEL_POP:
>> +case OVS_ACTION_ATTR_USERSPACE:
>> +case OVS_ACTION_ATTR_RECIRC:
>> +case OVS_ACTION_ATTR_CT:
>> +ofpbuf_put(b, a, nl_attr_len_pad(a, left));
>> +break;
>> +
>> +case OVS_ACTION_ATTR_CLONE:
>> +/* If the clone action has been fully xlated, it can
>> + * be skipped, since any actions executed within clone
>> + * do not affect the current packet.
>> + *
>> + * When xlating actions wihtin clone, the clone action,
>
> “within”
>
>> + * because it is an nested netlink attribute, do not have
>> + * a vlaid 'nla_len'; it will be zero instead.  Skip
>
> “valid”
>
>> + * the clone heaer to find the start of the actions
>
> “header”
>
>> + * enclosed. Treat those actions as if they are written

Re: [ovs-dev] [PATCH v2] conntrack: Fix checks for TCP, UDP, and IPv6 header sizes.

[Ben Pfaff]

On Mon, Mar 06, 2017 at 11:00:05AM -0800, Daniele Di Proietto wrote:
> 2017-03-03 21:18 GMT-08:00 Ben Pfaff :
> > Otherwise a malformed packet could cause a read up to about 40 bytes past
> > the end of the packet.  The packet would still likely be dropped because
> > of checksum verification.
> >
> > Reported-by: Bhargava Shastry 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Daniele Di Proietto 

Thanks, applied to master, branch-2.7, branch-2.6.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] packaging: Make Fedora spec file CentOS compatible

[Leif Madsen]

This patch v3 is ready for review. I've tested it on various distribution
versions (
https://copr.fedorainfracloud.org/coprs/leifmadsen/ovs-master/build/522596/)
and am satisfied I've accounted for all of the feedback previously received
including:

* reverting changes to Vagrantfile because Fedora 23 still uses
non-versioned package names
* accounting for ability to build python3 on distribution versions that
don't normally have python3 [1]
* making it generally work across everything [2]
* validate I didn't break anything via rpmlint

Let me know if there is anything else I should adjust. I plan to circle
back around and fix up some of the other RPM linting issues in a follow up
patch once this gets merged down.

Thanks!
Leif.

[1] I was asking about making this dynamic in #centos-devel it doesn't
really appear that is an approach that is likely to succeed, so I added a
--with build_python3 flag
[2] Also via #centos-devel and reading up on Python 2/3 spec files, there
isn't really a defacto "support py2/3 and older/newer Fedora versions in
same spec file" method, but that the %define method I've used seems to be
sane
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3] packaging: Make Fedora spec file CentOS compatible

[Leif Madsen]

On CentOS, the package names aren't prefixed with python2, but rather
are prefixed with simply python. This change addresses that and fixes
up some documentation that was outdated, and updates the Vagrantfile
to use the proper spec file and package names.

Signed-off-by: Leif Madsen 
---
 Documentation/intro/install/fedora.rst |  4 +--
 Vagrantfile|  6 ++--
 rhel/openvswitch-fedora.spec.in| 54 --
 3 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/Documentation/intro/install/fedora.rst 
b/Documentation/intro/install/fedora.rst
index 0ecd255..ffc77a4 100644
--- a/Documentation/intro/install/fedora.rst
+++ b/Documentation/intro/install/fedora.rst
@@ -42,8 +42,8 @@ in the :doc:`general`. Specific packages (by package name) 
include:
 - rpm-build
 - autoconf automake libtool
 - systemd-units openssl openssl-devel
-- python-devel python3-devel
-- python python-twisted-core python-zope-interface python-six
+- python2-devel python3-devel
+- python2 python2-twisted python2-zope-interface python2-six
 - desktop-file-utils
 - groff graphviz
 - procps-ng
diff --git a/Vagrantfile b/Vagrantfile
index 8439918..d2b7ecb 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -9,7 +9,7 @@ $bootstrap_fedora = 

[ovs-dev] [PATCH v2] packaging: Make Fedora spec file CentOS compatible

[Leif Madsen]

On CentOS, the package names aren't prefixed with python2, but rather
are prefixed with simply python. This change addresses that and fixes
up some documentation that was outdated, and updates the Vagrantfile
to use the proper spec file and package names.

Signed-off-by: Leif Madsen 
---
 Documentation/intro/install/fedora.rst |  4 +--
 Vagrantfile|  6 ++--
 rhel/openvswitch-fedora.spec.in| 50 --
 3 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/Documentation/intro/install/fedora.rst 
b/Documentation/intro/install/fedora.rst
index 0ecd255..ffc77a4 100644
--- a/Documentation/intro/install/fedora.rst
+++ b/Documentation/intro/install/fedora.rst
@@ -42,8 +42,8 @@ in the :doc:`general`. Specific packages (by package name) 
include:
 - rpm-build
 - autoconf automake libtool
 - systemd-units openssl openssl-devel
-- python-devel python3-devel
-- python python-twisted-core python-zope-interface python-six
+- python2-devel python3-devel
+- python2 python2-twisted python2-zope-interface python2-six
 - desktop-file-utils
 - groff graphviz
 - procps-ng
diff --git a/Vagrantfile b/Vagrantfile
index 8439918..d2b7ecb 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -9,7 +9,7 @@ $bootstrap_fedora = 

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Allow sending BFD messages even when RSTP port is not forwarding

[Joe Stringer]

On 20 February 2017 at 04:27, Mika Väisänen  wrote:
> Interworking of BFD and RSTP does not work, as currently BFD messages are
> dropped if RSTP port is not in forwarding mode. To correct this problem,
> an extra check is added to allow BFD messages to be sent even when
> rstp_forward_state is false.
>
> Note: This patch is made against branch-2.5.
>
> Signed-off-by: Mika Vaisanen 
>
> ---

There's something a bit off about the formatting of the patch, but
it's simple enough that I can just make the equivalent change locally.
The test seems to show the expected behaviour well.

I see now, looking at the codepaths, the bfd packet generation link
through to the compose_output_action() goes like this:

monitor_mport_run()
ofproto_dpif_send_packet()
xlate_send_packet()
ofproto_dpif_execute_actions()
ofproto_dpif_execute_actions__()
xlate_actions()
do_xlate_actions()
xlate_output_action()
compose_output_action()

I didn't realise that when these various protocols (STP, RSTP, CFM,
BFD, etc.) send packets from OVS, it goes through the standard
translation like this. I guess it makes sense :-)

The only concern I had about this patch is if there was a way to end
up broadcasting BFD in a loop because BFD is skipping past the
STP/RSTP forwarding checks. However, I believe that on receive,
xlate_actions() will handle BFD via process_special(), so typically it
should not end up in the path where it's attempting to forward the
packet. If it somehow does get past there, the check that is being
added by this patch will ensure that the BFD packet matches the
settings for this link, or otherwise it will respect the STP/RSTP
forwarding state. So I think it's fine. I wouldn't mind bouncing it
off someone like Jarno just to double-check my reasoning.

As a matter of style, I think this diff in ofproto-dpif-xlate.c is a
little easier to follow---and would cover the CFM case as well:

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 89fc3a44a0d1..578fef168b30 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3127,6 +3127,10 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
 }
 return;
 }
+} else if ((xport->cfm && cfm_should_process_flow(xport->cfm,
flow, wc))
+   || (xport->bfd && bfd_should_process_flow(xport->bfd, flow,
+ wc))) {
+/* Pass; STP should not block link health detection. */
 } else if (!xport_stp_forward_state(xport) ||
!xport_rstp_forward_state(xport)) {
 if (ctx->xbridge->stp != NULL) {
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] dpdk: Redirect DPDK log to OVS logging subsystem.

[Aaron Conole]

Ilya Maximets  writes:

> This should be helpful for have all the logs in one place.
> 'ovs-appctl vlog' commands for 'dpdk' module can be used
> to configure the log level. Lower bound for DPDK logging
> (--log-level) still can be passed through 'dpdk-extra' field.
>
> Signed-off-by: Ilya Maximets 
> ---

Worked fine for me so far.  I'm going to keep running with this.

Acked-by: Aaron Conole 

-Aaron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] conntrack: Fix checks for TCP, UDP, and IPv6 header sizes.

[Daniele Di Proietto]

2017-03-03 21:18 GMT-08:00 Ben Pfaff :
> Otherwise a malformed packet could cause a read up to about 40 bytes past
> the end of the packet.  The packet would still likely be dropped because
> of checksum verification.
>
> Reported-by: Bhargava Shastry 
> Signed-off-by: Ben Pfaff 

Acked-by: Daniele Di Proietto 

> ---
> v1->v2: Eliminate duplicate check in extract_l3_ipv6().  Thanks Daniele!
>
>  lib/conntrack.c | 16 +++-
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 9bea3d93e4ad..677c0d2a3cdc 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -568,15 +568,15 @@ extract_l3_ipv6(struct conn_key *key, const void *data, 
> size_t size,
>  const char **new_data)
>  {
>  const struct ovs_16aligned_ip6_hdr *ip6 = data;
> -uint8_t nw_proto = ip6->ip6_nxt;
> -uint8_t nw_frag = 0;
> -
>  if (new_data) {
>  if (OVS_UNLIKELY(size < sizeof *ip6)) {
>  return false;
>  }
>  }
>
> +uint8_t nw_proto = ip6->ip6_nxt;
> +uint8_t nw_frag = 0;
> +
>  data = ip6 + 1;
>  size -=  sizeof *ip6;
>
> @@ -623,8 +623,11 @@ check_l4_tcp(const struct conn_key *key, const void 
> *data, size_t size,
>   const void *l3)
>  {
>  const struct tcp_header *tcp = data;
> -size_t tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
> +if (size < sizeof *tcp) {
> +return false;
> +}
>
> +size_t tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
>  if (OVS_UNLIKELY(tcp_len < TCP_HEADER_LEN || tcp_len > size)) {
>  return false;
>  }
> @@ -637,8 +640,11 @@ check_l4_udp(const struct conn_key *key, const void 
> *data, size_t size,
>   const void *l3)
>  {
>  const struct udp_header *udp = data;
> -size_t udp_len = ntohs(udp->udp_len);
> +if (size < sizeof *udp) {
> +return false;
> +}
>
> +size_t udp_len = ntohs(udp->udp_len);
>  if (OVS_UNLIKELY(udp_len < UDP_HEADER_LEN || udp_len > size)) {
>  return false;
>  }
> --
> 2.10.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] FAQ: Add another question about do-nothing flows.

[Ben Pfaff]

On Mon, Mar 06, 2017 at 09:53:02AM -0800, Guru Shetty wrote:
> On 6 March 2017 at 08:11, Ben Pfaff  wrote:
> 
> > Signed-off-by: Ben Pfaff 
> >
> 
> Acked-by: Gurucharan Shetty 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] FAQ: Add another question about do-nothing flows.

[Guru Shetty]

On 6 March 2017 at 08:11, Ben Pfaff  wrote:

> Signed-off-by: Ben Pfaff 
>

Acked-by: Gurucharan Shetty 

> ---
>  Documentation/faq/openflow.rst | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/faq/openflow.rst b/Documentation/faq/openflow.
> rst
> index 529e3f50aadf..376e64eb4482 100644
> --- a/Documentation/faq/openflow.rst
> +++ b/Documentation/faq/openflow.rst
> @@ -496,6 +496,27 @@ but the packets are actually being output in VLAN
> 123.  Why?
>
>  $ ovs-ofctl add-flow br0 dl_vlan=123,actions=mod_vlan_
> vid:456,output:1
>
> +See also the following question.
> +
> +Q: I added a flow to a redirect packets for TCP port 80 to port 443,
> +like so::
> +
> +$ ovs-ofctl add-flow br0 tcp,tcp_dst=123,actions=mod_tp_dst:443
> +
> +but the packets are getting dropped instead.  Why?
> +
> +A: This set of actions does change the TCP destination port to 443,
> but
> +then it does nothing more.  It doesn't, for example, say to continue
> to
> +another flow table or to output the packet.  Therefore, the packet is
> +dropped.
> +
> +To solve the problem, add an action that does something with the
> modified
> +packet.  For example::
> +
> +$ ovs-ofctl add-flow br0 tcp,tcp_dst=123,actions=mod_
> tp_dst:443,normal
> +
> +See also the preceding question.
> +
>  Q: The "learn" action can't learn the action I want, can you improve it?
>
>  A: By itself, the "learn" action can only put two kinds of actions
> into the
> --
> 2.10.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] datapath-windows: Fix GENEVE option header

[Guru Shetty]

On 6 March 2017 at 08:16, Alin Serdean 
wrote:

> From: Alin Serdean 
>
> The GENEVE option header is defined in big endian, however we support only
> little endian on Windows at the moment.
>
> This patch changes the GENEVE option header into little endian.
>
> Found while testing.
>
> Signed-off-by: Alin Gabriel Serdean 
> Acked-by: Yin Lin 
>
Applied, thanks.


> ---
> v2: Change commit title and message as suggested by:
> Guru Shetty 
> intended for: master, branch-2.7
> ---
>  datapath-windows/ovsext/Geneve.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Geneve.h b/datapath-windows/ovsext/
> Geneve.h
> index be8a834..019c0dd 100644
> --- a/datapath-windows/ovsext/Geneve.h
> +++ b/datapath-windows/ovsext/Geneve.h
> @@ -71,10 +71,10 @@ typedef struct GeneveOptionHdr {
>  UINT32   optionClass:16;
>  /* Format of data contained in the option. */
>  UINT32   type:8;
> -/* Reserved. */
> -UINT32   reserved:3;
>  /* Length of option in int32 excluding the option header. */
>  UINT32   length:5;
> +/* Reserved. */
> +UINT32   reserved:3;
>  } GeneveOptionHdr;
>
>  #define GENEVE_CRIT_OPT_TYPE (1 << 7)
> --
> 2.10.2.windows.1
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] windows: Fix uninitialized variable in netlink-socket

[Alin Serdean]

From: Alin Serdean 

The variable `request_nlmsg` was used without being initialized.

This patch assigns a value to it before being used.

Found by inspection.

Signed-off-by: Alin Gabriel Serdean 
---
v2: Change commit title
---
 lib/netlink-socket.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
index e45914c..7105b9b 100644
--- a/lib/netlink-socket.c
+++ b/lib/netlink-socket.c
@@ -886,6 +886,8 @@ nl_sock_transact_multiple__(struct nl_sock *sock,
 }
 
 if (reply_len != 0) {
+request_nlmsg = nl_msg_nlmsghdr(txn->request);
+
 if (reply_len < sizeof *reply_nlmsg) {
 nl_sock_record_errors__(transactions, n, 0);
 VLOG_DBG_RL(, "insufficient length of reply %#"PRIu32
@@ -894,7 +896,6 @@ nl_sock_transact_multiple__(struct nl_sock *sock,
 }
 
 /* Validate the sequence number in the reply. */
-request_nlmsg = nl_msg_nlmsghdr(txn->request);
 reply_nlmsg = (struct nlmsghdr *)reply_buf;
 
 if (request_nlmsg->nlmsg_seq != reply_nlmsg->nlmsg_seq) {
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] datapath-windows: Fix GENEVE option header

[Alin Serdean]

From: Alin Serdean 

The GENEVE option header is defined in big endian, however we support only
little endian on Windows at the moment.

This patch changes the GENEVE option header into little endian.

Found while testing.

Signed-off-by: Alin Gabriel Serdean 
Acked-by: Yin Lin 
---
v2: Change commit title and message as suggested by:
Guru Shetty 
intended for: master, branch-2.7
---
 datapath-windows/ovsext/Geneve.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Geneve.h b/datapath-windows/ovsext/Geneve.h
index be8a834..019c0dd 100644
--- a/datapath-windows/ovsext/Geneve.h
+++ b/datapath-windows/ovsext/Geneve.h
@@ -71,10 +71,10 @@ typedef struct GeneveOptionHdr {
 UINT32   optionClass:16;
 /* Format of data contained in the option. */
 UINT32   type:8;
-/* Reserved. */
-UINT32   reserved:3;
 /* Length of option in int32 excluding the option header. */
 UINT32   length:5;
+/* Reserved. */
+UINT32   reserved:3;
 } GeneveOptionHdr;
 
 #define GENEVE_CRIT_OPT_TYPE (1 << 7)
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn: Modify the DHCP router option to optional

[Ben Pfaff]

On Mon, Mar 06, 2017 at 02:55:20PM +0800, Guoshuai Li wrote:
> Co-authored-by: Dong Jun 
> Signed-off-by: Guoshuai Li 

Please update the documentation as well, and add an item to NEWS
mentioning the change.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] FAQ: Add another question about do-nothing flows.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 Documentation/faq/openflow.rst | 21 +
 1 file changed, 21 insertions(+)

diff --git a/Documentation/faq/openflow.rst b/Documentation/faq/openflow.rst
index 529e3f50aadf..376e64eb4482 100644
--- a/Documentation/faq/openflow.rst
+++ b/Documentation/faq/openflow.rst
@@ -496,6 +496,27 @@ but the packets are actually being output in VLAN 123.  
Why?
 
 $ ovs-ofctl add-flow br0 dl_vlan=123,actions=mod_vlan_vid:456,output:1
 
+See also the following question.
+
+Q: I added a flow to a redirect packets for TCP port 80 to port 443,
+like so::
+
+$ ovs-ofctl add-flow br0 tcp,tcp_dst=123,actions=mod_tp_dst:443
+
+but the packets are getting dropped instead.  Why?
+
+A: This set of actions does change the TCP destination port to 443, but
+then it does nothing more.  It doesn't, for example, say to continue to
+another flow table or to output the packet.  Therefore, the packet is
+dropped.
+
+To solve the problem, add an action that does something with the modified
+packet.  For example::
+
+$ ovs-ofctl add-flow br0 tcp,tcp_dst=123,actions=mod_tp_dst:443,normal
+
+See also the preceding question.
+
 Q: The "learn" action can't learn the action I want, can you improve it?
 
 A: By itself, the "learn" action can only put two kinds of actions into the
-- 
2.10.2

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Cómo Vender con Éxito + 11 temas

[Ventas - Plan Integral -descuentos de mes]

Plan Integral Indispensable para los Responsables de Ventas

12 conferencias en cada Plan, pregrabadas, inéditas, para capacitar a todo su 
personal.

Al adquirir el Plan de Capacitación, usted obtiene acceso a 12 temas enfocados 
al área de Ventas; temas especializados que han sido cuidadosamente 
seleccionados por nuestros expertos en entrenamiento ejecutivo con el fin de 
brindar a las empresas la actualización que necesitan para llevar su 
competitividad a su máximo potencial. Le brindaremos acceso a TODA la 
programación, para aprovecharla cuando quiera, todas las veces que quiera 
durante sus tres meses de acceso.

3x2 - Asegure su Programa Integral de Capacitación Online  

¿Requiere la información a la Brevedad? responda este email con la palabra: 
VENTAS. Junto con su Nombre, Empresa, Teléfono y se lo enviaremos a la brevedad.


centro telefónico: 018002129393

¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] fedora: Add python3-openvswitch split package

[Leif Madsen]

On Fri, Mar 3, 2017 at 6:55 AM, Flavio Leitner  wrote:

> Perhaps you could check if %{python3_pkgversion} or %{__python3} is
> defined instead, so in case someone install python3 support on older
> distros the spec will do the right thing.
>

I'm following some python2 to python3 porting guides, and it seems like the
big thing to do is to make sure things are split out into subpackages (like
what was being done). There are some Provides macros that I'm going to add
instead of leaving them hard coded like now (which is apparently the new
right thing to do).

I think checking for whether the python3 macros are defined is a good way
to make this a bit more dynamic for distros that don't install py3 by
default, but have the option to have it installed post-install.

Thanks for the feedback. I'm continuing to work on this today and clean it
up based on some additional documentation that I've found.

Leif.


-- 
Leif Madsen | Partner Engineer - NFV & CI
NFV Partner Engineering
Red Hat
GPG: (D670F846) BEE0 336E 5406 42BA 6194 6831 B38A 291E D670 F846
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] WordPress Design, Web Design & Web Development

[Monika Sharma]

Hello,

Hope you are well!!

I am Monika and I work with experienced IT professionals who are into:


  *   Website Design & Development
  *   Graphic Design & Flash
  *   Logo Design
  *   Brochure Design
  *   Website Templates
  *   Website Maintenance
  *   Mobile Apps Development (Android )
  *   Internet Marketing
  *   SEO Optimization
  *   Google Adwords

May I know if you are interested in any of these services?

Please let me know if you are interested and have any questions. Waiting for 
your reply!

Thanks & Regards,
Monika
Business Development Executive

© 2016 Microsoft Terms Privacy & cookies Developers English (United States)

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] dpif-netdev: Assign ports to pmds on non-local numa node.

[Jan Scheurich]

I support Billy's point here. There are a number of constraints/trade-offs that 
can lead to an OVS deployments in OpenStack with e.g. a single PMD. Without 
this change, a dual socket system is effectively rendered use-less. At least 
one socket is lost for running VMs.

BR, Jan

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of O Mahony, Billy
> Sent: Tuesday, 28 February, 2017 14:59
> To: Ilya Maximets ; d...@openvswitch.org
> Subject: Re: [ovs-dev] dpif-netdev: Assign ports to pmds on non-local numa 
> node.
> 
> Hi Ilya,
> 
> Comments below.
> 
> BR,
> Billy.
> 
> > -Original Message-
> > From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> > Sent: Tuesday, February 28, 2017 12:49 PM
> > To: O Mahony, Billy ; d...@openvswitch.org
> > Cc: Daniele Di Proietto 
> > Subject: Re: [ovs-dev] dpif-netdev: Assign ports to pmds on non-local numa
> > node.
> >
> > On 28.02.2017 14:43, O Mahony, Billy wrote:
> > > Hi Ilya,
> > >
> > > Thanks for the quick response. You make some good points.
> > >
> > > It's important to point out that local pmds are still chosen when 
> > > available.
> > > The change only operates to avoid totally non-operational/ non-polled
> > ports.
> > >
> > > This is something we came across when deploying DPDK-enabled OVS in
> > > OpenStack environments (OPNFV project). Where we had remote (both
> > > physically and administratively) multi-node labs already wired up and
> > > would have much preferred to have sub-optimal operation that a non-
> > operational OpenStack environment.
> > >
> > > Some further comments below.
> > >
> > > Best Regards,
> > > Billy
> > >
> > >> -Original Message-
> > >> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> > >> Sent: Tuesday, February 28, 2017 11:21 AM
> > >> To: O Mahony, Billy ; d...@openvswitch.org
> > >> Cc: Daniele Di Proietto 
> > >> Subject: Re: [ovs-dev] dpif-netdev: Assign ports to pmds on non-local
> > >> numa node.
> > >>
> > >> Hello.
> > >>
> > >> On 28.02.2017 13:12, Billy O'Mahony wrote:
> > >>> From: billyom 
> > >>>
> > >>> Previously if there is no available (non-isolated) pmd on the numa
> > >>> node for a port then the port is not polled at all. This can result
> > >>> in a non-operational system until such time as nics are physically
> > >>> repositioned.
> > >>
> > >> Why you can't just reconfigure your pmd-cpu-mask after NICs'
> > repositioning?
> > > [[BO'M]] The idea is to avoid having to repositioning NICs as they may
> > > be in remote data-centers administered by other organisations. Also
> > > this can be related to multi-node clusters where it won't be just one
> > > NIC on one system that needs to be moved but several NICS/nodes
> > affected.
> > >
> > >
> > >> Maybe you can use pmd-rxq-affinity to assign port on another NUMA
> > node?
> > > [[BO'M]] The low-performance assignment will only occur if there is no
> > > available PMD on the local NUMA node. Ie if possible the normal highly
> > > performant assignment is made. It is only when it is a choice between
> > > lower performance and total non-performance that the lesser of two evils
> > is chosen.
> >
> > Maybe you can include at least one core from each node in pmd-cpu-mask?
> 
> [[BO'M]] Ideally everyone using and developing  the many OpenStack deployment 
> tools would be aware of the optimal settings but we can
> expect that this will not the case in many situations. Poor configurations 
> will occur. A warning message is included in the VLOG in this case
> to give users a hint that performance will not be optimal but without 
> imposing the penalty of a broken system - which is a heavy penalty if
> you don’t even have direct access to the pmd-cpu-mask as you are driving the 
> ovs configuration via an OpenStack installation tool such as
> Fuel, RDO, etc.
> 
> >
> > >> The main concern here is that this 'remote' port will degrade
> > >> performance of other ports served by chosen PMD thread significantly.
> > > [[BO'M]] That is a good point that there are second-order consequences on
> > other ports.
> > > But certainly with a many cloud systems even if one port is
> > > non-operational it means the entire node is effectively down - for
> > > instance an OpenStack compute node with a non-working provider
> > network
> > > for it's tenant VMs is useless even though the administration and control
> > networks are still working.
> > >>
> > >> Best regards, Ilya Maximets.
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev