[ovs-dev] [PATCH] WIP OVN ND for Logical_Port

[Zong Kai LI]

From: lzklibj 

This patch tries to implement ND for Logical_Port in OVN.

Signed-off-by: lzklibj 
---
 lib/packets.c|   2 +-
 lib/packets.h|   3 +
 ovn/controller/pinctrl.c | 156 +++
 ovn/lib/actions.c|  50 +++
 ovn/lib/actions.h|   6 ++
 ovn/lib/expr.c   |  47 +-
 ovn/lib/expr.h   |  43 +
 ovn/northd/ovn-northd.c  |  30 -
 8 files changed, 266 insertions(+), 71 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 6a55d6f..ad7c389 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1301,7 +1301,7 @@ compose_arp__(struct dp_packet *b)
 
 /* This function expect packet with ethernet header with correct
  * l3 pointer set. */
-static void *
+void *
 compose_ipv6(struct dp_packet *packet, uint8_t proto, const ovs_be32 src[4],
  const ovs_be32 dst[4], uint8_t key_tc, ovs_be32 key_fl,
  uint8_t key_hl, int size)
diff --git a/lib/packets.h b/lib/packets.h
index 5945940..82d793c 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -274,6 +274,9 @@ static inline uint32_t hash_mac(const struct eth_addr ea,
 bool eth_addr_is_reserved(const struct eth_addr);
 bool eth_addr_from_string(const char *, struct eth_addr *);
 
+void *compose_ipv6(struct dp_packet *packet, uint8_t proto, const ovs_be32 
src[4],
+   const ovs_be32 dst[4], uint8_t key_tc, ovs_be32 key_fl,
+   uint8_t key_hl, int size);
 void compose_rarp(struct dp_packet *, const struct eth_addr);
 
 void eth_push_vlan(struct dp_packet *, ovs_be16 tpid, ovs_be16 tci);
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index bc57c40..5265207 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -14,6 +14,7 @@
  */
 
 #include 
+#include 
 
 #include "pinctrl.h"
 
@@ -23,6 +24,8 @@
 #include "flow.h"
 #include "lport.h"
 #include "ovn-controller.h"
+#include "lib/csum.h"
+#include "lib/packets.h"
 #include "lib/sset.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-msgs.h"
@@ -64,6 +67,14 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id,
   const struct lport_index *lports,
   struct hmap *local_datapaths);
+static void pinctrl_handle_nd(const struct flow *ip_flow,
+  const struct match *md,
+  struct ofpbuf *userdata);
+void compose_na(struct dp_packet *,
+const struct eth_addr eth_src, const struct eth_addr eth_dst,
+const struct in6_addr *, const struct in6_addr *);
+void reload_metadata(struct ofpbuf *ofpacts,
+ const struct match *md);
 
 COVERAGE_DEFINE(pinctrl_drop_put_arp);
 
@@ -153,31 +164,7 @@ pinctrl_handle_arp(const struct flow *ip_flow, const 
struct match *md,
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
 enum ofp_version version = rconn_get_version(swconn);
 
-enum mf_field_id md_fields[] = {
-#if FLOW_N_REGS == 8
-MFF_REG0,
-MFF_REG1,
-MFF_REG2,
-MFF_REG3,
-MFF_REG4,
-MFF_REG5,
-MFF_REG6,
-MFF_REG7,
-#else
-#error
-#endif
-MFF_METADATA,
-};
-for (size_t i = 0; i < ARRAY_SIZE(md_fields); i++) {
-const struct mf_field *field = mf_from_id(md_fields[i]);
-if (!mf_is_all_wild(field, &md->wc)) {
-struct ofpact_set_field *sf = ofpact_put_SET_FIELD(&ofpacts);
-sf->field = field;
-sf->flow_has_vlan = false;
-mf_get_value(field, &md->flow, &sf->value);
-memset(&sf->mask, 0xff, field->n_bytes);
-}
-}
+reload_metadata(&ofpacts, md);
 enum ofperr error = ofpacts_pull_openflow_actions(userdata, userdata->size,
   version, &ofpacts);
 if (error) {
@@ -242,6 +229,10 @@ process_packet_in(const struct ofp_header *msg)
 pinctrl_handle_put_arp(&pin.flow_metadata.flow, &headers);
 break;
 
+case ACTION_OPCODE_ND:
+pinctrl_handle_nd(&headers, &pin.flow_metadata, &userdata);
+break;
+
 default:
 VLOG_WARN_RL(&rl, "unrecognized packet-in opcode %"PRIu32,
  ntohl(ah->opcode));
@@ -734,3 +725,118 @@ send_garp_run(const struct ovsrec_bridge *br_int, const 
char *chassis_id,
 sset_destroy(&localnet_vifs);
 simap_destroy(&localnet_ofports);
 }
+
+void
+compose_na(struct dp_packet *b,
+   const struct eth_addr eth_src, const struct eth_addr eth_dst,
+   const struct in6_addr *ipv6_src, const struct in6_addr *ipv6_dst)
+{
+struct ovs_nd_msg *na;
+struct ovs_nd_opt *nd_opt;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+na = compose_ipv6(b, IPPROTO_ICMPV6,
+  ALIGNED_CAST(ovs_be32 *,

[ovs-dev] [PATCH] lib/packets: Add compose_na

[Zong Kai LI]

Add a method compose_na to compose a NA packet.
The ND feature of OVN will base on this.

Signed-off-by: Zong Kai LI 
---
 lib/packets.c | 33 +
 lib/packets.h |  4 
 2 files changed, 37 insertions(+)

diff --git a/lib/packets.c b/lib/packets.c
index 6a55d6f..cbc086e 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1355,6 +1355,39 @@ compose_nd(struct dp_packet *b, const struct eth_addr 
eth_src,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+void
+compose_na(struct dp_packet *b,
+   const struct eth_addr eth_src, const struct eth_addr eth_dst,
+   const struct in6_addr *ipv6_src, const struct in6_addr *ipv6_dst,
+   ovs_be16 rco_flags)
+{
+struct ovs_nd_msg *na;
+struct ovs_nd_opt *nd_opt;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+na = compose_ipv6(b, IPPROTO_ICMPV6,
+  ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
+  ALIGNED_CAST(ovs_be32 *, ipv6_dst->s6_addr),
+  0, 0, 255,
+  ND_MSG_LEN + ND_OPT_LEN);
+
+na->icmph.icmp6_type = ND_NEIGHBOR_ADVERT;
+na->icmph.icmp6_code = 0;
+na->rco_flags.hi = rco_flags;
+
+nd_opt = &na->options[0];
+nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
+nd_opt->nd_opt_len = 1;
+
+packet_set_nd(b, ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
+  eth_addr_zero, eth_src);
+na->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+na->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, na,
+  ND_MSG_LEN + 
ND_OPT_LEN));
+}
+
 uint32_t
 packet_csum_pseudoheader(const struct ip_header *ip)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 5945940..11b3b6d 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1069,6 +1069,10 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
  ovs_be32 arp_spa, ovs_be32 arp_tpa);
 void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
 struct in6_addr *, struct in6_addr *);
+void compose_na(struct dp_packet *,
+const struct eth_addr eth_src, const struct eth_addr eth_dst,
+const struct in6_addr *, const struct in6_addr *,
+ovs_be16 rco_flags);
 uint32_t packet_csum_pseudoheader(const struct ip_header *);
 void IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6);
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] [PATCH v1 1/2] ovn-controller: Add 'na' action in ovn-controller

[Zong Kai LI]

This patch adds a new OVN action 'na' to support ND versus ARP.

When ovn-controller received a ND packet, it frames a NA packet for
reply, with mac address parsed from userdata as eth.dst. Then it
reloads metadata info from previous packet to framed packet, and
finally send the framed packet back with left actions.

Eg. na{12:34:56:78:9a:bc; outport = inport; inport = ""; output;};

Since patch port for IPv6 router interface is not ready yet, this
patch will only try to deal with ND from VM. This patch will set
RSO flags to 011.

The next patch will add logical flows for this action.

Signed-off-by: Zong Kai LI 
---
 ovn/controller/pinctrl.c | 119 +--
 ovn/lib/actions.c|  50 
 ovn/lib/actions.h|   6 +++
 ovn/lib/expr.c   |  47 ++-
 ovn/lib/expr.h   |  43 +
 5 files changed, 196 insertions(+), 69 deletions(-)

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index bc57c40..1af9e89 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -23,6 +23,7 @@
 #include "flow.h"
 #include "lport.h"
 #include "ovn-controller.h"
+#include "lib/packets.h"
 #include "lib/sset.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-msgs.h"
@@ -64,6 +65,11 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id,
   const struct lport_index *lports,
   struct hmap *local_datapaths);
+static void pinctrl_handle_na(const struct flow *ip_flow,
+  const struct match *md,
+  struct ofpbuf *userdata);
+static void reload_metadata(struct ofpbuf *ofpacts,
+const struct match *md);
 
 COVERAGE_DEFINE(pinctrl_drop_put_arp);
 
@@ -153,31 +159,7 @@ pinctrl_handle_arp(const struct flow *ip_flow, const 
struct match *md,
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
 enum ofp_version version = rconn_get_version(swconn);
 
-enum mf_field_id md_fields[] = {
-#if FLOW_N_REGS == 8
-MFF_REG0,
-MFF_REG1,
-MFF_REG2,
-MFF_REG3,
-MFF_REG4,
-MFF_REG5,
-MFF_REG6,
-MFF_REG7,
-#else
-#error
-#endif
-MFF_METADATA,
-};
-for (size_t i = 0; i < ARRAY_SIZE(md_fields); i++) {
-const struct mf_field *field = mf_from_id(md_fields[i]);
-if (!mf_is_all_wild(field, &md->wc)) {
-struct ofpact_set_field *sf = ofpact_put_SET_FIELD(&ofpacts);
-sf->field = field;
-sf->flow_has_vlan = false;
-mf_get_value(field, &md->flow, &sf->value);
-memset(&sf->mask, 0xff, field->n_bytes);
-}
-}
+reload_metadata(&ofpacts, md);
 enum ofperr error = ofpacts_pull_openflow_actions(userdata, userdata->size,
   version, &ofpacts);
 if (error) {
@@ -242,6 +224,10 @@ process_packet_in(const struct ofp_header *msg)
 pinctrl_handle_put_arp(&pin.flow_metadata.flow, &headers);
 break;
 
+case ACTION_OPCODE_NA:
+pinctrl_handle_na(&headers, &pin.flow_metadata, &userdata);
+break;
+
 default:
 VLOG_WARN_RL(&rl, "unrecognized packet-in opcode %"PRIu32,
  ntohl(ah->opcode));
@@ -734,3 +720,86 @@ send_garp_run(const struct ovsrec_bridge *br_int, const 
char *chassis_id,
 sset_destroy(&localnet_vifs);
 simap_destroy(&localnet_ofports);
 }
+
+static void
+reload_metadata(struct ofpbuf *ofpacts, const struct match *md)
+{
+enum mf_field_id md_fields[] = {
+#if FLOW_N_REGS == 8
+MFF_REG0,
+MFF_REG1,
+MFF_REG2,
+MFF_REG3,
+MFF_REG4,
+MFF_REG5,
+MFF_REG6,
+MFF_REG7,
+#else
+#error
+#endif
+MFF_METADATA,
+};
+for (size_t i = 0; i < ARRAY_SIZE(md_fields); i++) {
+const struct mf_field *field = mf_from_id(md_fields[i]);
+if (!mf_is_all_wild(field, &md->wc)) {
+struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
+sf->field = field;
+sf->flow_has_vlan = false;
+mf_get_value(field, &md->flow, &sf->value);
+memset(&sf->mask, 0xff, field->n_bytes);
+}
+}
+}
+
+static void
+pinctrl_handle_na(const struct flow *ip_flow,
+  const struct match *md,
+  struct ofpbuf *userdata)
+{
+enum ofp_version version = rconn_get_version(swconn);
+enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
+
+const struct eth_addr *dl_na_reply = ofpbuf_try_pull(userdata,
+ 

[ovs-dev] [PATCH] [PATCH v2 2/2] ovn: add lflows for 'na' action for ND

[Zong Kai LI]

This patch adds some lflows for 'na' action to support ND versus ARP.

For ovn-northd, it will generate lflows per each IPv6 address on
echo lport, with lport mac and IPv6 addresss, with 'na' action.
e.g. match=(ip6 && nd && icmp6.type == 135 &&
nd.target == fde3:f657:aac1:0:f816:3eff:fe13:8198),
 action=(na{fa:16:3e:13:81:98; reg0 = 0x1; outport = inport;
inport = ""; output;};)
And new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
from previous ls_in_arp_rsp.

Setting reg0 = 0x1 to mention that such kind of NA packets are replied
by ovn-controller, and for these packets, they dont need conntrack.
So I also modfiy current table 32 and table 48, to make these packets
output directly.

(Will try to add test once I figure out what to test.)

Signed-off-by: Zong Kai LI 
---
 ovn/controller/physical.c | 17 +
 ovn/northd/ovn-northd.c   | 93 +++
 tutorial/OVN-Tutorial.md  |  6 +--
 3 files changed, 106 insertions(+), 10 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 576c695..df19680 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -707,6 +707,23 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id 
mff_ovn_geneve,
 put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
 ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 0, &match, &ofpacts);
 
+/* Table 32, Priority 100.
+ * ===
+ *
+ * Directly output NA packets replied by ovn-controller for NS packets
+ * come from local VMs.
+ *   - match: ip6 && icmp6 && icmp6.type == 136 && reg0 == 0x1
+ *   - action: resubmit(,48)
+ */
+match_init_catchall(&match);
+match_set_dl_type(&match, htons(0x86dd));
+match_set_nw_proto(&match, (uint8_t)58);
+match_set_icmp_type(&match, (uint8_t)136);
+match_set_reg(&match, 0, (uint32_t)1);
+ofpbuf_clear(&ofpacts);
+put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, &ofpacts);
+ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, &match, &ofpacts);
+
 /* Table 34, Priority 0.
  * ===
  *
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index a8dd2bb..0263d5c 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -93,7 +93,7 @@ enum ovn_stage {
 PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_ND,2, "ls_in_port_sec_nd") \
 PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,3, "ls_in_pre_acl")  \
 PIPELINE_STAGE(SWITCH, IN,  ACL,4, "ls_in_acl")  \
-PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,5, "ls_in_arp_rsp")  \
+PIPELINE_STAGE(SWITCH, IN,  ARP_ND_RSP, 5, "ls_in_arp_nd_rsp")  \
 PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,6, "ls_in_l2_lkup")  \
   \
 /* Logical switch egress stages. */   \
@@ -1475,6 +1475,64 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, 
struct hmap *ports)
   acl->match, "drop;");
 }
 }
+
+/* Egress Pre-ACL Table (Priority 110).
+ *
+ * Directly output RA packets replied by ovn-controller, they dont
+ * need go through conntrack.
+ */
+bool ra_allowed = false;
+/* Consider the following cases are RA allowed:
+ * - has:
+ * match: "... ip6 && icmp6",
+ * action: "allow"/"allow-related".
+ *   and has no:
+ * match: "... ip6 && icmp6 && icmp6.type == 136",
+ * action: "drop"/"reject".
+ * - has:
+ * match: "... ip6 && icmp6 && icmp6.type == 136",
+ * action: "allow"/"allow-related".
+ *   and has no:
+ * match: "... ip6 && icmp6",
+ * action: "drop"/"reject".
+ */
+for (size_t i = 0; i < od->nbs->n_acls; i++) {
+struct nbrec_acl *acl = od->nbs->acls[i];
+if (!strcmp(acl->direction, "from-lport")) {
+continue;
+}
+char *icmp6 = strstr(acl->match, "ip6 && icmp6");
+if (!icmp6) {
+continue;
+}
+/* strlen("ip6 && icmp6") == 12 */
+if (*(icmp6 + 12) == '\0') {
+if (!strcmp(acl->action, "drop")
+|| !strcmp(acl->action, "reject")) {
+ra_allowed = false;
+break;
+} else {
+ra_allowed = true;
+}
+} else {
+char *ra = strstr(icmp6 + 12, &

[ovs-dev] [PATCH] [PATCH v2 1/2] ovn-controller: Add 'na' action for ND

[Zong Kai LI]

This patch adds a new OVN action 'na' to support ND versus ARP.

When ovn-controller received a ND packet, it frames a NA packet for
reply, with mac address parsed from userdata as eth.dst. Then it
reloads metadata info from previous packet to framed packet, and
finally send the framed packet back with left actions.

Eg. na{12:34:56:78:9a:bc; reg0 = 0x1; outport = inport; inport = ""; output;};

Since patch port for IPv6 router interface is not ready yet, this
patch will only try to deal with ND from VM. This patch will set
RSO flags to 011 for NA packets.

The next patch will do logical flows works for this action.

Signed-off-by: Zong Kai LI 
---
 lib/packets.c|  33 +
 lib/packets.h|   4 ++
 ovn/controller/pinctrl.c | 119 +--
 ovn/lib/actions.c|  50 
 ovn/lib/actions.h|   6 +++
 ovn/lib/expr.c   |  47 ++-
 ovn/lib/expr.h   |  43 +
 tests/ovn.at |   3 ++
 8 files changed, 236 insertions(+), 69 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 6a55d6f..cbc086e 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1355,6 +1355,39 @@ compose_nd(struct dp_packet *b, const struct eth_addr 
eth_src,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+void
+compose_na(struct dp_packet *b,
+   const struct eth_addr eth_src, const struct eth_addr eth_dst,
+   const struct in6_addr *ipv6_src, const struct in6_addr *ipv6_dst,
+   ovs_be16 rco_flags)
+{
+struct ovs_nd_msg *na;
+struct ovs_nd_opt *nd_opt;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+na = compose_ipv6(b, IPPROTO_ICMPV6,
+  ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
+  ALIGNED_CAST(ovs_be32 *, ipv6_dst->s6_addr),
+  0, 0, 255,
+  ND_MSG_LEN + ND_OPT_LEN);
+
+na->icmph.icmp6_type = ND_NEIGHBOR_ADVERT;
+na->icmph.icmp6_code = 0;
+na->rco_flags.hi = rco_flags;
+
+nd_opt = &na->options[0];
+nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
+nd_opt->nd_opt_len = 1;
+
+packet_set_nd(b, ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
+  eth_addr_zero, eth_src);
+na->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+na->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, na,
+  ND_MSG_LEN + 
ND_OPT_LEN));
+}
+
 uint32_t
 packet_csum_pseudoheader(const struct ip_header *ip)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 5945940..11b3b6d 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1069,6 +1069,10 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
  ovs_be32 arp_spa, ovs_be32 arp_tpa);
 void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
 struct in6_addr *, struct in6_addr *);
+void compose_na(struct dp_packet *,
+const struct eth_addr eth_src, const struct eth_addr eth_dst,
+const struct in6_addr *, const struct in6_addr *,
+ovs_be16 rco_flags);
 uint32_t packet_csum_pseudoheader(const struct ip_header *);
 void IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6);
 
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index bc57c40..1af9e89 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -23,6 +23,7 @@
 #include "flow.h"
 #include "lport.h"
 #include "ovn-controller.h"
+#include "lib/packets.h"
 #include "lib/sset.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-msgs.h"
@@ -64,6 +65,11 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id,
   const struct lport_index *lports,
   struct hmap *local_datapaths);
+static void pinctrl_handle_na(const struct flow *ip_flow,
+  const struct match *md,
+  struct ofpbuf *userdata);
+static void reload_metadata(struct ofpbuf *ofpacts,
+const struct match *md);
 
 COVERAGE_DEFINE(pinctrl_drop_put_arp);
 
@@ -153,31 +159,7 @@ pinctrl_handle_arp(const struct flow *ip_flow, const 
struct match *md,
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
 enum ofp_version version = rconn_get_version(swconn);
 
-enum mf_field_id md_fields[] = {
-#if FLOW_N_REGS == 8
-MFF_REG0,
-MFF_REG1,
-MFF_REG2,
-MFF_REG3,
-MFF_REG4,
-MFF_REG5,
-MFF_REG6,
-MFF_REG7,
-#else
-#error
-#endif
-MFF_METADATA,
-};
-for (size_t i = 0; i < ARRAY_SIZ

Re: [ovs-dev] [ovn-ipv6 16/26] packets: Cleanup ND compose functions.

[Zong Kai Li]

On Tue, Jul 12, 2016 at 2:56 PM, Justin Pettit  wrote:
> Rename "compose_nd" and "compose_na" to "compose_nd_sol" and
> "compose_nd_adv", respecively, to be clearer about their functionality.
> Also change the source and destination IPv6 addresses to take
> "struct in6_addr" arguments, which are more common in the code base.
>
> Signed-off-by: Justin Pettit 
> ---

>  eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
> -na = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src, ipv6_dst, 0, 0, 255,
> -  ND_MSG_LEN + ND_OPT_LEN);
> +na = compose_ipv6(b, IPPROTO_ICMPV6,
> +  ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
> +  ALIGNED_CAST(ovs_be32 *, ipv6_dst->s6_addr),
> +  0, 0, 255, ND_MSG_LEN + ND_OPT_LEN);
>

Hi, Justin. It's quite a long time to wait support for IPv6, but I
think it worth. Thanks. :)

About using ALIGNED_CAST, would you mind checking
http://patchwork.ozlabs.org/patch/632026/ ?
Ben has a comment said "Using ALIGNED_CAST does not solve a problem,
it only suppresses a warning.".
So I'm not sure will this be a good way to cleanup.

Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovn-ipv6 14/26] ovn: Support multiple addresses on a single logical router port.

[Zong Kai Li]

> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 5102948..460d5bd 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
> -"version": "4.0.0",
> -"cksum": "2156178478 7460",
> +"version": "5.0.0",
> +"cksum": "849073644 7576",
>  "tables": {
>  "Logical_Switch": {
>  "columns": {
> @@ -123,7 +123,9 @@
>  "Logical_Router_Port": {
>  "columns": {
>  "name": {"type": "string"},
> -"network": {"type": "string"},
> +"networks": {"type": {"key": "string",
> +  "min": 1,
> +  "max": "unlimited"}},
>  "mac": {"type": "string"},
>  "peer": {"type": {"key": "string", "min": 0, "max": 1}},
>  "enabled": {"type": {"key": "boolean", "min": 0, "max": 1}},
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 1eeec22..e571eeb 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -794,11 +794,13 @@
>
>  
>
> -
> -  The IP address of the router and the netmask.  For example,
> -  192.168.0.1/24 indicates that the router's IP address is
> -  192.168.0.1 and that packets destined to 192.168.0.x should 
> be
> -  routed to this port.
> +
> +  
> +The IP addresses and netmasks of the router.  For example,
> +192.168.0.1/24 indicates that the router's IP
> +address is 192.168.0.1 and that packets destined to
> +192.168.0.x should be routed to this port.
> +  
>  
>

I just considered about this patch again in my afternoon. Let me share
my new idea about this.
My idea is, maybe we can directly parse a lsp.addresses for its
patched lrp when that lsp.type is "router".
This should be the simplest way to align lrp.network and lrp.mac to
lsp.addresses when lsp.type is "router".
We even don't need to change current OVN NB DB schema, and
extract_lsp_addresses can work for this.

A router could connect to either a switch or another router, and a
lsp.peer should either be empty or be set with another lsp.name.
So when a lrp has its nbr->peer been set, we should parse current
network and mac column for it. For peer router port still these to
build static route.
And when a lrp has empty nbr->peer, we can let lsp processing branch
in join_logical_ports to extract addresses for it.
And even, for op is lrp and op->peer->nbs is NULL case, we can
directly use ip, network, mask via op->peer, without extracting
addresses for lrp at first.

I can't figure why a patched lsp and lrp could have different
addresses, networks, masks, macs. So I think this should make sense.
:)

Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovn-ipv6 07/26] ovn: Add xxreg[01] symbols.

[Zong Kai Li]

> +  
> +The regX symbols are 32-bit integers.
> +The xxregX symbols are 128-bit integers,
> +which overlay four of the 32-bit registers: xxreg0
> +overlays reg0 through reg3, with
> +reg0 supplying the most-significant bits of
> +xxreg0 and reg3 the least-signficant.
> +xxreg1 similarly overlays reg4 through
> +reg7.
> +  
> +

I think REGBIT_CONNTRACK_** defined in ovn-northd will be covered by xxreg0.
Since you renumbered MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPUT from
MFF_REG3...MFF_REG7 to MFF_REG11...MFF_REG15. REGBIT_CONNTRACK_**
which are using MFF_REG0 should be renumbered too.

Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovn-ipv6 07/26] ovn: Add xxreg[01] symbols.

[Zong Kai Li]

>On Wed, Jul 13, 2016 at 11:24 AM, Justin Pettit  wrote:
>
>> On Jul 12, 2016, at 7:53 PM, Zong Kai Li  wrote:
>>
>>> +  
>>> +The regX symbols are 32-bit integers.
>>> +The xxregX symbols are 128-bit integers,
>>> +which overlay four of the 32-bit registers: xxreg0
>>> +overlays reg0 through reg3, with
>>> +reg0 supplying the most-significant bits of
>>> +xxreg0 and reg3 the least-signficant.
>>> +xxreg1 similarly overlays reg4 through
>>> +reg7.
>>> +  
>>> +
>>
>> I think REGBIT_CONNTRACK_** defined in ovn-northd will be covered by xxreg0.
>> Since you renumbered MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPUT from
>> MFF_REG3...MFF_REG7 to MFF_REG11...MFF_REG15. REGBIT_CONNTRACK_**
>> which are using MFF_REG0 should be renumbered too.
>
> I'm not sure I understand.  Those registers are that I renumbered are 
> physical (used by ovn-controller) and not accessible by logical flows.  The 
> REGBIT_CONNTRACK_* are only used by logical flows (in ovn-northd), so they 
> need to be in the logical registers that are accessible by xxregX and xregX.
>
> --Justin
>
>

Yes, REGBIT_CONNTRACK_* are only used by logical flows, just like
"inport", "outport".
But things are little different, since "inport" and "outport" are
symbols and they will be translated, while REGBIT_CONNTRACK_* are not,
they are "reg0[..]" already, ovn-controller will directly submit them
to ovs.

If a logical flow both use REGBIT_CONNTRACK_* and xxreg0, reg0 will be
called twice, the directly way and the symbol translation way.

Thanks,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovn-ipv6 06/26] ovn: Renumber logical field registers to the newly extended registers.

[Zong Kai Li]

Hi, Justin. I tried your patches one by one. P1~P5 are OK.
But something wrong seems happened to P6.
In my OpenStack integrated environment, after using P6, I cannot ping
VM dhcp netns. And later when I dump ovs flows, I found there is no
table 0/OFTABLE_PHY_TO_LOG.

I tried revert definition for MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPORT, and
this time, table OFTABLE_PHY_TO_LOG comes back.
I hope you can check this.

Thanks,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovn-ipv6 06/26] ovn: Renumber logical field registers to the newly extended registers.

[Zong Kai Li]

>On Wed, Jul 13, 2016 at 6:19 PM, Zong Kai Li  wrote:
> Hi, Justin. I tried your patches one by one. P1~P5 are OK.
> But something wrong seems happened to P6.
> In my OpenStack integrated environment, after using P6, I cannot ping
> VM dhcp netns. And later when I dump ovs flows, I found there is no
> table 0/OFTABLE_PHY_TO_LOG.
>
> I tried revert definition for MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPORT, and
> this time, table OFTABLE_PHY_TO_LOG comes back.
> I hope you can check this.
>
> Thanks,
> Zong Kai, LI

Sorry for previous misstatement, I forget to restart ovs-vswitchd.

Thanks,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] [PATCH v1] ovs python: add set type for from_python

[Zong Kai LI]

In method from_python, when parameter 'value' is in list type or tuple
type, it will process per each element in 'value'. And indeed, it just
needs a iterable sequence here, no matter it's a list or tuple.
So set type should be considered too, it's also a basic, common iterable
sequence type from python.

Signed-off-by: Zong Kai LI 
---
 python/ovs/db/data.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py
index 747acd5..6d87f89 100644
--- a/python/ovs/db/data.py
+++ b/python/ovs/db/data.py
@@ -521,7 +521,7 @@ class Datum(object):
 ka = Atom.from_python(type_.key, row_to_uuid(k))
 va = Atom.from_python(type_.value, row_to_uuid(v))
 d[ka] = va
-elif isinstance(value, (list, tuple)):
+elif isinstance(value, (list, set, tuple)):
 for k in value:
 ka = Atom.from_python(type_.key, row_to_uuid(k))
 d[ka] = None
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovn-ipv6 06/26] ovn: Renumber logical field registers to the newly extended registers.

[Zong Kai Li]

On Wed, Jul 20, 2016 at 4:50 AM, Justin Pettit  wrote:
>
>> On Jul 13, 2016, at 3:19 AM, Zong Kai Li  wrote:
>>
>> Hi, Justin. I tried your patches one by one. P1~P5 are OK.
>> But something wrong seems happened to P6.
>> In my OpenStack integrated environment, after using P6, I cannot ping
>> VM dhcp netns. And later when I dump ovs flows, I found there is no
>> table 0/OFTABLE_PHY_TO_LOG.
>>
>> I tried revert definition for MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPORT, and
>> this time, table OFTABLE_PHY_TO_LOG comes back.
>> I hope you can check this.
>
> I wasn't able to reproduce this.  I don't have a full OpenStack environment, 
> but I've done a fair amount of testing and don't know how it would pass any 
> traffic without those physical to logical flows.
>
> --Justin
>
>

Hi, Justin. Sorry to trouble you. I did reply in another mail to tell
it's caused by operation mistake, maybe I should add your mail address
next time, not only to ovs dev, when I try to report something.

Thanks,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] [PATCH v1] ovs-dev: add set type for ovs.idl.data.Datum.from_python

[Zong Kai LI]

ovs.db.idl.Datum.from_python fails to handle set type value, while set
type is also a common iterable sequence, just like list and tuple.
No reason IDL caller must to turn set type parameters to list or tuple
type. Otherwise, they will fail to insert data, but get no exception.

Signed-off-by: Zong Kai LI 
---
 python/ovs/db/data.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py
index 747acd5..6d87f89 100644
--- a/python/ovs/db/data.py
+++ b/python/ovs/db/data.py
@@ -521,7 +521,7 @@ class Datum(object):
 ka = Atom.from_python(type_.key, row_to_uuid(k))
 va = Atom.from_python(type_.value, row_to_uuid(v))
 d[ka] = va
-elif isinstance(value, (list, tuple)):
+elif isinstance(value, (list, set, tuple)):
 for k in value:
 ka = Atom.from_python(type_.key, row_to_uuid(k))
 d[ka] = None
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovn-ipv6 24/26] [RFC] ovn-northd: Implement basic IPv6 routing.

[Zong Kai Li]

On Wed, Jul 20, 2016 at 1:49 PM, Justin Pettit  wrote:
>
>> On Jul 13, 2016, at 1:10 PM, Ben Pfaff  wrote:
>>
>> On Mon, Jul 11, 2016 at 11:56:54PM -0700, Justin Pettit wrote:
>>> Future commits will add support for dynamic IPv6/MAC bindings.
>>>
>>> TODO:
>>>- ovn-northd man page needs updated flows.
>>>- Add unit tests.
>>
>> I hope it's OK if I defer reviewing this and the remaining patches until
>> the non-RFC version.
>
> I appreciate the quick reviews on the other patches.  I pushed most of the 
> patches that had acks on them.
>
> There are a couple that I want to revise, so I'll send out a v2 of the series 
> in a day or two.
>
> --Justin
>
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

I tested this patch on my 2 chassis environment, with manually
rebased, with ping test between VMs from different subnets attached on
the same router. And it works.

Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] [PATCH v1] ovn-northd: Fix {}-enclosed constants for ND responder

[Zong Kai LI]

It missed comma as constant seperator in match string for ND responder.

Signed-off-by: Zong Kai LI 
---
 ovn/northd/ovn-northd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index a3d1672..8e5c5c5 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2018,7 +2018,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 } else {
 ds_put_format(&match, "nd.target == {");
 for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) 
{
-ds_put_cstr(&match,
+ds_put_format(&match, "%s, ",
   op->lsp_addrs[i].ipv6_addrs[j].addr_s);
 }
 ds_chomp(&match, ' ');
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH RFC v1] ovn: add SLAAC support for IPv6

[Zong Kai LI]

This patch introduces(from north to south):
- OVN NB DB:
 * mtu(integer) column for Logical_Switch, used by Router Advertisement
   (RA) message MTU option.
 * slaac(bool) column for Logical_Router_Port to specify the IPv6 networks
   on a rotuer port is in SLAAC mode or not, if it has IPv6 networks. [1]
- ovn-northd:
   New table ls_in_ra_rsp to install RA responder flows, which matching
   Router Solicitation (RS) packets and have actions such as:
   action=(ra(fd80:1234::/64,2001:db8::/64,1450,fa:16:3e:12:34:56);
   outport = inport; inport = ""; output;) [2]
   while 'ra' is new action. The RA responder flows are built on logical
   switch pipeline per each logical switch. [3]
- ovn-controller:
   New action 'ra' has ordered parameters and uses 'continuation'.[4]
   The ordered parameters including: at least 1 IPv6 prefix, 1 MTU,
   and 1 ether address. They will be used in prefix information option(s),
   MTU option, SLL option for RA packet.
- lib/packet:
 * compose_ra to compose a RA packet with 1 prefix information
   option, 1 MTU option and 1 SLL option.
 * insert_ra_prefix_opt insert a prefix information option into a RA
   packet.

[1] A logical switch may have different IPv6 subnets using DHCPv6 or SLAAC,
so set 'slaac' as Logical_Switch column will not work well. Adding 'slaac'
as Logical_Router_Port may not be a good solution, but it works for now.

[2] SLAAC is the first aim of this patch, but not RA responder. So it wont
take care of lots of flags or options in RA packet.
A further work will try to support those flags or options.

[3] This works for case (a) all logical switch IPv6 subnets attached onto the
same router port, which OpenStack Neutron using this way. And it won't need
flows to route RS packets to router ports which is necessary for installing
RA responder flows on logical router pipeline.
A further work will replace this by installing RA responder flows on logical
router pipeline, which will support case (b) logical switch have IPv6 subnets
in SLAAC mode attached onto different router ports.

[4] Ordered parameters isn't a friendly way to use 'ra' action, it will be
fixed with [2].
From my side, it's fine to use 'continuation' for case (a) in [3]. But for
case (b) in [3], since RS packet will be dispatched several router ports,
which means ovn-controller will compose several RA packets for one RS packet,
I'm not sure whether it's good to keep use 'continuation'.

Signed-off-by: Zong Kai LI 
---
 lib/packets.c| 105 +++
 lib/packets.h|  58 ++
 ovn/controller/pinctrl.c |  87 +++
 ovn/lib/actions.c|  70 +++
 ovn/lib/actions.h|   9 
 ovn/lib/expr.c   |  11 +
 ovn/lib/expr.h   |  11 +
 ovn/northd/ovn-northd.c  |  56 +++--
 ovn/ovn-nb.ovsschema |   6 ++-
 ovn/ovn-nb.xml   |  11 +
 ovn/ovn-sb.xml   |  24 +++
 tests/ovn.at |  79 +++
 12 files changed, 512 insertions(+), 15 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 1bf887e..f4e5e4d 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -35,6 +35,7 @@
 
 const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT;
 const struct in6_addr in6addr_all_hosts = IN6ADDR_ALL_HOSTS_INIT;
+const struct in6_addr in6addr_all_routers = IN6ADDR_ALL_ROUTERS_INIT;
 
 struct in6_addr
 flow_tnl_dst(const struct flow_tnl *tnl)
@@ -1404,6 +1405,110 @@ compose_na(struct dp_packet *b,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+void
+compose_ra(struct dp_packet *b,
+   const struct eth_addr eth_src, const ovs_be32 ipv6_src[4],
+   const ovs_be32 router_prefix[4], uint8_t plen,
+   ovs_be16 hlmo_flags, ovs_be16 router_lifetime,
+   ovs_be32 reachable_time,ovs_be32 retrans_timer,
+   uint8_t la_flags, ovs_be32 valid_lifetime,
+   ovs_be32 preferred_lifetime, ovs_be16 mtu)
+{
+struct ovs_ra_msg *ra;
+struct ovs_ra_prefix *ra_prefix;
+struct ovs_ra_mtu *ra_mtu;
+struct ovs_nd_opt *ra_sll;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_addr_ip6_all_hosts, eth_src,
+ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+ra = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src,
+  ALIGNED_CAST(ovs_be32 *, in6addr_all_hosts.s6_addr),
+  0, 0, 255,
+  RA_MSG_LEN + RA_PREFIX_LEN + RA_MTU_LEN + ND_OPT_LEN);
+
+ra->icmph.icmp6_type = ND_ROUTER_ADVERT;
+ra->icmph.icmp6_code = 0;
+ra->hlmo_flags = hlmo_flags;
+ra->router_lifetime = router_lifetime;
+ra->reachable_time = reachable_time;
+ra->retrans_timer = retrans_timer;
+
+/* Set prefix

Re: [ovs-dev] [PATCH RFC v1] ovn: add SLAAC support for IPv6

[Zong Kai Li]

On Thu, Jul 28, 2016 at 2:04 PM, Ben Pfaff  wrote:
> On Wed, Jul 27, 2016 at 04:02:11PM +0800, Zong Kai LI wrote:
>> This patch introduces(from north to south):
>> - OVN NB DB:
>>  * mtu(integer) column for Logical_Switch, used by Router Advertisement
>>(RA) message MTU option.
>>  * slaac(bool) column for Logical_Router_Port to specify the IPv6 networks
>>on a rotuer port is in SLAAC mode or not, if it has IPv6 networks. [1]
>> - ovn-northd:
>>New table ls_in_ra_rsp to install RA responder flows, which matching
>>Router Solicitation (RS) packets and have actions such as:
>>action=(ra(fd80:1234::/64,2001:db8::/64,1450,fa:16:3e:12:34:56);
>>outport = inport; inport = ""; output;) [2]
>>while 'ra' is new action. The RA responder flows are built on logical
>>switch pipeline per each logical switch. [3]
>> - ovn-controller:
>>New action 'ra' has ordered parameters and uses 'continuation'.[4]
>>The ordered parameters including: at least 1 IPv6 prefix, 1 MTU,
>>and 1 ether address. They will be used in prefix information option(s),
>>MTU option, SLL option for RA packet.
>> - lib/packet:
>>  * compose_ra to compose a RA packet with 1 prefix information
>>option, 1 MTU option and 1 SLL option.
>>  * insert_ra_prefix_opt insert a prefix information option into a RA
>>packet.
>>
>> [1] A logical switch may have different IPv6 subnets using DHCPv6 or SLAAC,
>> so set 'slaac' as Logical_Switch column will not work well. Adding 'slaac'
>> as Logical_Router_Port may not be a good solution, but it works for now.
>>
>> [2] SLAAC is the first aim of this patch, but not RA responder. So it wont
>> take care of lots of flags or options in RA packet.
>> A further work will try to support those flags or options.
>>
>> [3] This works for case (a) all logical switch IPv6 subnets attached onto the
>> same router port, which OpenStack Neutron using this way. And it won't need
>> flows to route RS packets to router ports which is necessary for installing
>> RA responder flows on logical router pipeline.
>> A further work will replace this by installing RA responder flows on logical
>> router pipeline, which will support case (b) logical switch have IPv6 subnets
>> in SLAAC mode attached onto different router ports.
>>
>> [4] Ordered parameters isn't a friendly way to use 'ra' action, it will be
>> fixed with [2].
>> From my side, it's fine to use 'continuation' for case (a) in [3]. But for
>> case (b) in [3], since RS packet will be dispatched several router ports,
>> which means ovn-controller will compose several RA packets for one RS packet,
>> I'm not sure whether it's good to keep use 'continuation'.
>>
>> Signed-off-by: Zong Kai LI 
>
> The commit message here is so detailed that I can't figure out what the
> actual goal is.  What is SLAAC and how does it benefit the user or
> administrator?
>
> Justin has been working on IPv6 support so I'm adding him in case he has
> comments.
>
> Thanks,
>
> Ben.

Hi, Ben.
Sorry for unclear commit message, I've fixed some details of this RFC
patch, and will try to rewrite the commit message, to make it clear
and easy to get.

SLAAC is short for stateless address autoconfiguration, it's one the
address configuration method for IPv6 (another is DHCPv6).

SLAAC is simpler than DHCPv6. After configure an IPv6 prefix/CIDR for
a subnet, people can attach the subnet onto a router, then all
VMs/VIFs on subnet can get IPv6 address under the IPv6 prefix/CIDR. By
this people may not need DHCPv6. This is helpful and friendly for
people who is not familiar with IPv6.

SLAAC need router advertisement, so I implement it in this patch, but
just a basic version, not like what Numan did in native DHCP, many
options get supported there.

Thanks,
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v1] ovn-test: add flags.loopback to test-ovn

[Zong Kai LI]

Action "flags.loopback" is introduced recently, but symtab in test-ovn
doesn't recognize it yet. This may cause action parsing test broken.
This patch fixes that.

Signed-off-by: Zong Kai LI 
---
 tests/test-ovn.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index acb6a99..ce1e262 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -29,6 +29,7 @@
 #include "ovn/actions.h"
 #include "ovn/expr.h"
 #include "ovn/lex.h"
+#include "ovn/lib/logical-fields.h"
 #include "ovn/lib/ovn-dhcp.h"
 #include "ovs-thread.h"
 #include "ovstest.h"
@@ -146,6 +147,12 @@ create_symtab(struct shash *symtab)
 expr_symtab_add_string(symtab, "inport", MFF_REG14, NULL);
 expr_symtab_add_string(symtab, "outport", MFF_REG15, NULL);
 
+/* Flags used in logical to physical transformation. */
+expr_symtab_add_field(symtab, "flags", MFF_LOG_FLAGS, NULL, false);
+char flags_str[16];
+snprintf(flags_str, sizeof flags_str, "flags[%d]", MLF_ALLOW_LOOPBACK_BIT);
+expr_symtab_add_subfield(symtab, "flags.loopback", NULL, flags_str);
+
 expr_symtab_add_field(symtab, "xxreg0", MFF_XXREG0, NULL, false);
 expr_symtab_add_field(symtab, "xxreg1", MFF_XXREG1, NULL, false);
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH RFC v2 0/3] ovn: add SLAAC support for IPv6

[Zong Kai LI]

This serial patches try to support Stateless Address Autoconfiguration(SLAAC)
for IPv6, via Router Advertisement (RA) message responder.

User case:
  After this serial patches, a user can follow the following steps to let all
  VMs/VIFs on a logical switch has IPv6 prefix/CIDR, configure their IPv6
  addresses automatically:
   - set Logical_Switch.mtu column,
   - add lsp and lrp for Logical_Switch to make it attached onto a router, the
 lsp and lrp should have IPv6 prefix/CIDR in their addresses and networks
 columns,
   - set Logical_Router_Port.slaac column to true.
  Then, next time VM/VIF sends Router Solicitation (RS) message, a RA packet
  will be replied by ovn-controller. When VM/VIF receives that RA packet, it
  can configure its IPv6 address(es) and route(s) via that RA packet.

There will be logical flows in table ls_in_ra_rsp(28) to work as RA
responder, with:
 - match: ip6.dst == ff02::2 && nd_rs
   (nd_rs: icmp6.type == 133 && icmp6.code == 0 && ttl == 255)
 - action: ra{slaac(fdad:a0f9:a012::/64,...,1450,fa:16:3e:32:3c:e0);
   outport = inport; flags.loopback = 1; output;};
   (ra is a new action which stands for RA responder;
slaac is a new action which has the following parameters to tell
ovn-controller to compose RA packet with SLAAC flags, with these
parameters:
 - IPv6 prefix(es): such as fdad:a0f9:a012::/64.
 - MTU: logical switch MTU, such as 1450.
 - LLA: router port link-layer address, such as fa:16:3e:32:3c:e0.
After a RA packet is composed, the left nested actions will make RA packet
transmitted back to the inport, where Router Solicitation (RS) packet comes.

For more details, will be described in each following patches.

Zong Kai LI (1):
  ovn: add SLAAC support for IPv6

 lib/packets.c | 168 +-
 lib/packets.h |  86 +-
 2 files changed, 216 insertions(+), 38 deletions(-)

-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH RFC v2 1/3] ovn: add SLAAC support for IPv6

[Zong Kai LI]

This patch introduces methods to compose a Router Advertisement (RA) packet,
introduces flags for RA, renames ovs_nd_opt to ovs_nd_lla_opt to specify it's
Source/Target Link-layer Address option.

Signed-off-by: Zong Kai LI 
---
 lib/flow.c|  26 -
 lib/odp-execute.c |  20 +++
 lib/packets.c | 168 +-
 lib/packets.h |  86 
 4 files changed, 239 insertions(+), 61 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index ba4f8c7..250dc7d 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -400,8 +400,8 @@ parse_icmpv6(const void **datap, size_t *sizep, const 
struct icmp6_hdr *icmp,
 while (*sizep >= 8) {
 /* The minimum size of an option is 8 bytes, which also is
  * the size of Ethernet link-layer options. */
-const struct ovs_nd_opt *nd_opt = *datap;
-int opt_len = nd_opt->nd_opt_len * ND_OPT_LEN;
+const struct ovs_nd_lla_opt *nd_opt = *datap;
+int opt_len = nd_opt->length * ND_LLA_OPT_LEN;
 
 if (!opt_len || opt_len > *sizep) {
 return;
@@ -410,17 +410,17 @@ parse_icmpv6(const void **datap, size_t *sizep, const 
struct icmp6_hdr *icmp,
 /* Store the link layer address if the appropriate option is
  * provided.  It is considered an error if the same link
  * layer option is specified twice. */
-if (nd_opt->nd_opt_type == ND_OPT_SOURCE_LINKADDR
+if (nd_opt->type == ND_OPT_SOURCE_LINKADDR
 && opt_len == 8) {
 if (OVS_LIKELY(eth_addr_is_zero(arp_buf[0]))) {
-arp_buf[0] = nd_opt->nd_opt_mac;
+arp_buf[0] = nd_opt->lla;
 } else {
 goto invalid;
 }
-} else if (nd_opt->nd_opt_type == ND_OPT_TARGET_LINKADDR
+} else if (nd_opt->type == ND_OPT_TARGET_LINKADDR
&& opt_len == 8) {
 if (OVS_LIKELY(eth_addr_is_zero(arp_buf[1]))) {
-arp_buf[1] = nd_opt->nd_opt_mac;
+arp_buf[1] = nd_opt->lla;
 } else {
 goto invalid;
 }
@@ -2273,7 +2273,7 @@ flow_compose_l4(struct dp_packet *p, const struct flow 
*flow)
 (icmp->icmp6_type == ND_NEIGHBOR_SOLICIT ||
  icmp->icmp6_type == ND_NEIGHBOR_ADVERT)) {
 struct in6_addr *nd_target;
-struct ovs_nd_opt *nd_opt;
+struct ovs_nd_lla_opt *nd_opt;
 
 l4_len += sizeof *nd_target;
 nd_target = dp_packet_put_zeros(p, sizeof *nd_target);
@@ -2282,16 +2282,16 @@ flow_compose_l4(struct dp_packet *p, const struct flow 
*flow)
 if (!eth_addr_is_zero(flow->arp_sha)) {
 l4_len += 8;
 nd_opt = dp_packet_put_zeros(p, 8);
-nd_opt->nd_opt_len = 1;
-nd_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
-nd_opt->nd_opt_mac = flow->arp_sha;
+nd_opt->length = 1;
+nd_opt->type = ND_OPT_SOURCE_LINKADDR;
+nd_opt->lla = flow->arp_sha;
 }
 if (!eth_addr_is_zero(flow->arp_tha)) {
 l4_len += 8;
 nd_opt = dp_packet_put_zeros(p, 8);
-nd_opt->nd_opt_len = 1;
-nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
-nd_opt->nd_opt_mac = flow->arp_tha;
+nd_opt->length = 1;
+nd_opt->type = ND_OPT_TARGET_LINKADDR;
+nd_opt->lla = flow->arp_tha;
 }
 }
 }
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 5a43904..f791a33 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -185,7 +185,7 @@ odp_set_nd(struct dp_packet *packet, const struct 
ovs_key_nd *key,
const struct ovs_key_nd *mask)
 {
 const struct ovs_nd_msg *ns = dp_packet_l4(packet);
-const struct ovs_nd_opt *nd_opt = dp_packet_get_nd_payload(packet);
+const struct ovs_nd_lla_opt *nd_opt = dp_packet_get_nd_payload(packet);
 
 if (OVS_LIKELY(ns && nd_opt)) {
 int bytes_remain = dp_packet_l4_size(packet) - sizeof(*ns);
@@ -193,25 +193,25 @@ odp_set_nd(struct dp_packet *packet, const struct 
ovs_key_nd *key,
 struct eth_addr sll_buf = eth_addr_zero;
 struct eth_addr tll_buf = eth_addr_zero;
 
-while (bytes_remain >= ND_OPT_LEN && nd_opt->nd_opt_len != 0) {
-if (nd_opt->nd_opt_type == ND_OPT_SOURCE_LINKADDR
-&& nd_opt->nd_opt_len == 1) {
-sll_buf = nd_opt->nd_opt_mac;
+whi

[ovs-dev] [PATCH RFC v2 3/3] ovn: add SLAAC support for IPv6

[Zong Kai LI]

This patch tries to implement Router Advertisement (RA) responder for SLAAC
on ovn-northd side.

It tries to build lflows per each Logical Router Port, who have IPv6 networks
and set their 'slaac' column to true.

The lflows will look like:
 match=(inport == "lrp-32a71e0b-8b19-4c52-8cde-058325e4df5d" &&
ip6.dst == ff02::2 && nd_rs)
 action=(nd_ra{slaac(fd80:a123:b345::/64,1450,fa:16:3e:62:f1:e6);
 outport = inport; flags.loopback = 1; output;};)
while:
 - nd_rs is a new symbol stands for
   "icmp6.type == 133 && icmp6.code == 0 && ttl == 255"
 - slaac is a new action which accepts ordered parameter list:
 - one or more IPv6 prefixes: such as fd80:a123:b345::/64.
 - MTU: logical switch MTU, such as 1450.
 - MAC address: router port mac address, such as fa:16:3e:62:f1:e6.
 - nd_ra is a new action which stands for RA responder, it will compose a RA
   packet per parameters in slaac, and eth.src and ip6.src from packet being
   processed.

Logical_Router_Port.slaac column will only tell whether ovn should reply a RA
packet for Router solicitation packet received from the lrp port. To respond
a RA packet for other scenario will be a future work.
---
 ovn/northd/ovn-northd.c |  94 
 ovn/ovn-nb.ovsschema|   6 ++-
 ovn/ovn-nb.xml  |  11 +
 tests/ovn.at| 111 
 4 files changed, 213 insertions(+), 9 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index d6c14cf..98db819 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -126,9 +126,10 @@ enum ovn_stage {
 PIPELINE_STAGE(ROUTER, IN,  IP_INPUT,1, "lr_in_ip_input") \
 PIPELINE_STAGE(ROUTER, IN,  UNSNAT,  2, "lr_in_unsnat")   \
 PIPELINE_STAGE(ROUTER, IN,  DNAT,3, "lr_in_dnat") \
-PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  4, "lr_in_ip_routing")   \
-PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 5, "lr_in_arp_resolve")  \
-PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 6, "lr_in_arp_request")  \
+PIPELINE_STAGE(ROUTER, IN,  RA_RSP,  4, "lr_in_ra_rsp")  \
+PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  5, "lr_in_ip_routing")   \
+PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 6, "lr_in_arp_resolve")  \
+PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 7, "lr_in_arp_request")  \
   \
 /* Logical router egress stages. */   \
 PIPELINE_STAGE(ROUTER, OUT, SNAT,  0, "lr_out_snat")  \
@@ -2413,13 +2414,20 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 }
 }
 
-/* Ingress table 1 and 2: Port security - IP and ND, by default goto next.
- * (priority 0)*/
+/* Ingress table 1 and 2: Port security - IP and ND. */
 HMAP_FOR_EACH (od, key_node, datapaths) {
 if (!od->nbs) {
 continue;
 }
 
+/* Allow Router Solicitation and Router Advertisement messages pass.
+ * (priority 90)*/
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_ND, 90, "nd_rs",
+  "next;");
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_ND, 90, "nd_ra",
+  "next;");
+
+/* By default goto next. (priority 0)*/
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_ND, 0, "1", "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_IP, 0, "1", "next;");
 }
@@ -2582,6 +2590,32 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_RESPONSE, 0, "1", "next;");
 }
 
+/* Ingress table 12: Destination lookup, router solicitation handling
+ * (priority 110). */
+HMAP_FOR_EACH (od, key_node, datapaths) {
+if (!od->nbs) {
+continue;
+}
+
+if (!od->n_router_ports) {
+continue;
+}
+
+ds_clear(&actions);
+for (size_t i = 0; i != od->n_router_ports; i++) {
+op = od->router_ports[i];
+if (!op->lsp_addrs || !op->lsp_addrs->n_ipv6_addrs) {
+continue;
+}
+ds_put_format(&actions, "outport = %s; output; ", op->json_key);
+}
+if (actions.length != 0) {
+ds_chomp(&actions, ' ');
+ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110, "nd_rs",
+  ds_cstr(&actions));
+}
+}
+
 /* Ingress table 12: Destination lookup, broadcast and multicast handling
  * (priority 100). */
 HMAP_FOR_EACH (op, key_node, ports) {
@@ -3409,7 +3443,51 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
   "ip", "flags.loopback = 1; ct_dnat;");
 }
 
-/* Logical router ingress table 4: IP Routing.
+/* Logical router ingress table 5: RA responder, by default goto next.
+ * (priority 0)*/
+HMAP_FOR_EACH (

[ovs-dev] [PATCH RFC v2 2/3] ovn: add SLAAC support for IPv6

[Zong Kai LI]

This patch tries to implement Router Advertisement (RA) responder for SLAAC
on ovn-controller side.

It parses lflows which have:
 - match: inport == LRP_NAME && ip6.dst == ff02::2 && nd_rs
   (nd_rs: icmp6.type == 133 && icmp6.code == 0 && ttl == 255)
 - action: nd_ra{slaac(prefix,...,MTU, mac_address);
   outport = inport; flags.loopback = 1; output;};
   (nd_ra is a new action which stands for RA responder;
slaac is a new action which has the following parameters to tell
ovn-controller to compose RA packet with SLAAC flags, with these
parameters:
 - IPv6 prefix(es): such as fd80:a0f9:a012::/64.
 - MTU: logical switch MTU, such as 1450.
 - MAC address: router port mac address, such as fa:16:3e:12:34:56.
Beside the parameters list above, nd_ra action will use eth.src and ip6.src
from packet being processed as RA packet eth.src and ip6.src.
After a RA packet is composed, the left nested actions will make RA packet
transmitted back to the inport, where Router Solicitation (RS) packet comes.

For inner action 'slaac', as a prototype, it doesn't try to expose all RA
relevant flags and fields. As most flags are constant for in SLAAC scenario,
so it only exposed few fields user may care, such as prefixes, MTU and LLA.
It uses an ordered parameters list to hold all those values for now, but in
future, when we try to implement more feature on RA, the unexposed ones should
be exposed, and key-pair style parameter list should be used.

For outer action 'nd_ra', as a prototype, it's just a RA responder now, not a
real implement for RA, like it wont send periodic RA broadcast. This will be
harmful when routing relevant stuff changes, like user changes lrp mac,
disattach a switch from a router. However, a periodic version seems not so
necessary, but a broadcast should be sent when things change.

For lflow, this patch add prerequisites for Router Solicitation (RS) and RA
message. But it doesn't fix details of nd.target, nd.sll and nd.tll for RS
and RA, since these fields are not supported to be modified via ovs native
actions for now.
---
 include/ovn/actions.h|  15 +
 include/ovn/expr.h   |  11 
 ovn/controller/lflow.c   |   6 +-
 ovn/controller/pinctrl.c | 168 +++
 ovn/lib/actions.c|  69 +++
 ovn/lib/expr.c   |  11 +---
 ovn/ovn-sb.xml   |  32 -
 tests/ovn.at |   6 +-
 tests/test-ovn.c |   6 +-
 9 files changed, 310 insertions(+), 14 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index a395ce9..9eaa8db 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -88,6 +88,21 @@ enum action_opcode {
  * MFF_ETH_SRC = mac
  */
 ACTION_OPCODE_PUT_ND,
+
+/* "nd_ra { ...actions... }".
+ *
+ * The actions, in OpenFlow 1.3 format, follow the action_header.
+ */
+ACTION_OPCODE_ND_RA,
+
+/* slaac (prefix0, [prefix1, ...,] mtu, mac)".
+ *
+ * Arguments follow the action_header, in this format:
+ *   - One or more IPv6 prefixes.
+ *   - A 16-bit MTU.
+ *   - A 48-bit Ether address.
+ */
+ACTION_OPCODE_SLAAC,
 };
 
 /* Header. */
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index d790c49..150f2e7 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -450,6 +450,17 @@ struct expr_constant_set {
 bool in_curlies;  /* Whether the constants were in {}. */
 };
 
+/* Context maintained during expr_parse(). */
+struct expr_context {
+struct lexer *lexer;/* Lexer for pulling more tokens. */
+const struct shash *symtab; /* Symbol table. */
+const struct shash *macros; /* Table of macros. */
+char *error;/* Error, if any, otherwise NULL. */
+bool not;   /* True inside odd number of NOT operators. */
+};
+
+bool parse_constant(struct expr_context *ctx, struct expr_constant_set *cs,
+size_t *allocated_values);
 char *expr_parse_constant_set(struct lexer *, const struct shash *symtab,
   struct expr_constant_set *cs)
 OVS_WARN_UNUSED_RESULT;
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index fda10eb..18afefd 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -164,7 +164,11 @@ lflow_init(void)
 expr_symtab_add_field(&symtab, "arp.tha", MFF_ARP_THA, "arp", false);
 
 expr_symtab_add_predicate(&symtab, "nd",
-  "icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl == 255");
+  "icmp6.type == {133, 134, 135, 136} && icmp6.code == 0 && ip.ttl 
== 255");
+expr_symtab_add_predicate(&symtab, "nd_rs",
+  "icmp6.type == 133 && icmp6.code == 0 && ip.ttl == 255");
+expr_symtab_add_predicate(&symtab, "nd_ra",
+  "icmp6.type == 134 && icmp6.code == 0 && ip.ttl == 255");
 expr_symtab_add_predicate(&symtab, "nd_ns",
   "icmp6.type == 135 && ic

[ovs-dev] [PATCH RFC] ovn: Extend Address_Set to Macros_Set to support define port sets

[Zong Kai LI]

This patch aims to extend Address_Set to Macros_Set, make it more common to
accept variable set, not only address. And by that, we can skinny down ACLs,
if we use Macros_Set to defines port sets for ACL to use, since lots of ACL
entries are similar but only "inport" and "outport" fields are different.

Address_Set improves the performance of OVN control plane. But what more
important is, its implementation introduced a "$macros -- *_Set" mechanism,
it's good way to do bunch jobs for ACLs and lflows.

How does Address_Set improve OVN performance?
 - it reduces the match part length for lflows, who has addresses to match.
 - indeed, it means less tokens in expression to parse, less constants, less
   combination symbols.
 - and for CMS integration component, for its DB sync job, a ACL entry has
   less match column is easier to compare.

What changes:
  Adderss_Set   -> Macros_Set
  Adderss_Set.addresses -> Macros_Set.elements
   (addresses, or ports names)
-> Macros_Set.type
   (only "address", "port" are valid now)

Will port set defined under Macros_Set benefit in the same way as Address_Set
did? -- NO, it may be better.
 - it will not decrease lflow's length, but the number. ACL entries who have
   similar match expressions, but with particular "inport" or "outport"
   can use port set defined in Macros_Set to combine into one ACL.
 - less ACL entries, will help CMS integration component has less workload
   in DB sync job, comparing port name should be quicker then comparing whole
   ACL entry.
 - less ACL entries, mess less lflows, means less match part in lflows to
   parse, less tokens to parse, make parsing work quicker.

Port set defined under Macros_Set will improve ovn-controller performance on
installation plenty of similar ACLs/lflows, but not for port comes and leaves
scenario.

Potential drawback: not sure yet.

TODO: go check how will ovn-northd use port sets defined under Macros_Set

Signed-off-by: Zong Kai LI 
---
 include/ovn/expr.h|   2 +-
 ovn/controller/lflow.c| 176 --
 ovn/lib/expr.c|  68 +++---
 ovn/northd/ovn-northd.c   |  54 +++---
 ovn/ovn-nb.ovsschema  |  11 +--
 ovn/ovn-nb.xml|  20 --
 ovn/ovn-sb.ovsschema  |  11 +--
 ovn/ovn-sb.xml|   7 +-
 ovn/utilities/ovn-nbctl.c |   4 +-
 ovn/utilities/ovn-sbctl.c |   4 +-
 tests/ovn.at  |   8 +--
 11 files changed, 201 insertions(+), 164 deletions(-)

diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index d790c49..5020ff4 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -466,7 +466,7 @@ void expr_constant_set_destroy(struct expr_constant_set 
*cs);
  * The values that don't qualify are ignored.
  */
 
-void expr_macros_add(struct shash *macros, const char *name,
+void expr_macros_add(struct shash *macros, const char *name, bool parse_value,
  const char * const *values, size_t n_values);
 void expr_macros_remove(struct shash *macros, const char *name);
 void expr_macros_destroy(struct shash *macros);
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index fda10eb..5eae183 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -38,8 +38,8 @@ VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
-/* Contains an internal expr datastructure that represents an address set. */
-static struct shash expr_address_sets;
+/* Contains an internal expr datastructure that represents an macro set. */
+static struct shash expr_macros_sets;
 
 static bool full_flow_processing = false;
 static bool full_logical_flow_processing = false;
@@ -65,7 +65,7 @@ void
 lflow_init(void)
 {
 shash_init(&symtab);
-shash_init(&expr_address_sets);
+shash_init(&expr_macros_sets);
 
 /* Reserve a pair of registers for the logical inport and outport.  A full
  * 32-bit register each is bigger than we need, but the expression code
@@ -187,149 +187,159 @@ lflow_init(void)
 expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
 }
 
-/* Details of an address set currently in address_sets. We keep a cached
+/* Details of a macro set currently in macros_sets. We keep a cached
  * copy of sets still in their string form here to make it easier to compare
  * with the current values in the OVN_Southbound database. */
-struct address_set {
-char **addresses;
-size_t n_addresses;
+struct macro_set {
+char **elements;
+size_t n_elements;
 };
 
-/* struct address_set instances for address sets currently in the symtab,
- * hashed on the address set name. */
-static struct shash local_address_sets = 
SHASH_INITIALIZER(&local_address_

Re: [ovs-dev] [PATCH RFC] ovn: Extend Address_Set to Macros_Set to support define port sets

[Zong Kai Li]

On Wed, Aug 3, 2016 at 7:56 PM, Russell Bryant  wrote:
>
>
> On Wed, Aug 3, 2016 at 7:01 AM, Zong Kai LI  wrote:
>>
>> This patch aims to extend Address_Set to Macros_Set, make it more common
>> to
>> accept variable set, not only address. And by that, we can skinny down
>> ACLs,
>> if we use Macros_Set to defines port sets for ACL to use, since lots of
>> ACL
>> entries are similar but only "inport" and "outport" fields are different.
>
>
> I think the idea makes sense.
>
> I wonder if just "Set" would be an appropriate name.  One catch is that it
> may be too late to get into OVS 2.6.  If so, then we have to take into
> account backwards compatibility.  We won't be able to just rename the table.
> One option would be to at least rename the table now (before 2.6) to reflect
> a more broad purpose, and leave the support for other types of entries to
> later patches.
>
> Are you looking for any other specific feedback?
>
> --
> Russell Bryant

Hi, Russell.
I noticed "Macros_Set" is redundant to "Macros", but I'm not so sure,
so I left it in this version.
Most code in this patch is "rename", fewer is to extend to support
string type members, like port name. So I think it's ok to do both
rename and support string members. And left any other support and
enhance in follow patches.
Any other special feedback, yes, I hope that. To make a simple
beginning, I didn't do any changing in ovn-northd, but I think it's
possible to make ovn-northd to use it on some lflows. Not sure whether
it's worth to do that, or will it make ovn-northd code complex. So I
list this in TODO.

Thanks,
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v1] ovn: Extend Address_Set to Macros to support define port name sets

[Zong Kai LI]

This patch aims to extend Address_Set to Macros, make it more common to
accept variable set, not only address. And by that, we can skinny down ACLs,
if we use Macros to defines port name sets for ACL to use, since lots of ACL
entries are similar but only "inport" and "outport" fields are different.

Address_Set improves the performance of OVN control plane. But what more
important is, its implementation introduced a "$macros -- element_set"
mechanism, it's good way to do bunch jobs for ACLs, and even lflows.

How does Address_Set improve OVN performance?
 - it reduces the match part length for lflows, who has addresses to match.
 - indeed, it means less tokens in expression to parse, less constants, less
   combination symbols.
 - and for CMS integration component, for its DB sync job, a ACL entry has
   short match field is easier to compare.

What changes:
  Adderss_Set   -> Macros
  Adderss_Set.addresses -> Macros.elements
   (addresses, or ports names)
 + Macros.element_type
   (only "address", "port_name" are valid now)
  ACL match field, for example:
match : "outport == $ps1 && icmp4"

Will port set defined under Macros benefit in the same way as Address_Set
did? -- NO, it may be better.
 - it will not decrease ACL match field length, but the number of ACL entries.
   ACL entries who have similar match expressions, but with particular
   "inport" or "outport" can use port name set defined in Macros to combine
   into one ACL.
 - less ACL entries means less refers between ACL and Logical_Switch.acl.
 - less ACL entries, will help CMS integration component has less workload
   in DB sync job, comparing port name should be quicker then comparing whole
   ACL entry.
 - less ACL entries, means less lflows, means less match part in lflows to
   parse, less tokens to parse, make parsing work quicker.

Port name set defined under Macros will improve ovn-controller performance on
installation plenty of similar ACLs/lflows.
For port comes and leaves scenario, when there are no special ACL entries
assigned to ports, it will be the ones first come and last leave to update ACL
uuid refers in Logical_Switch.acl.

TODO:
 - go check how will ovn-northd use port sets defined under Macros.
 - find any other type need to support.

Signed-off-by: Zong Kai LI 
---
 AUTHORS   |   1 +
 include/ovn/expr.h|   2 +-
 ovn/controller/lflow.c| 173 --
 ovn/lib/expr.c|  68 ++
 ovn/northd/ovn-northd.c   |  54 ---
 ovn/ovn-nb.ovsschema  |  11 +--
 ovn/ovn-nb.xml|  27 ++--
 ovn/ovn-sb.ovsschema  |  11 +--
 ovn/ovn-sb.xml|   7 +-
 ovn/utilities/ovn-nbctl.c |   4 +-
 ovn/utilities/ovn-sbctl.c |   4 +-
 tests/ovn.at  |  14 ++--
 tests/test-ovn.c  |   6 +-
 13 files changed, 212 insertions(+), 170 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index b598f4b..e71571e 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -459,6 +459,7 @@ ZHANG Zhiming   zhangzhim...@yunshan.net.cn
 Zhangguanghui   zhang.guang...@h3c.com
 Ziyou Wang  ziy...@vmware.com
 Zoltán Balogh   zoltan.bal...@ericsson.com
+Zong Kai, LI   zealo...@gmail.com
 ankur dwivedi   ankurengg2...@gmail.com
 chen zhang  3zhangchen9...@gmail.com
 james hopperjameshop...@email.com
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index d790c49..5020ff4 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -466,7 +466,7 @@ void expr_constant_set_destroy(struct expr_constant_set 
*cs);
  * The values that don't qualify are ignored.
  */
 
-void expr_macros_add(struct shash *macros, const char *name,
+void expr_macros_add(struct shash *macros, const char *name, bool parse_value,
  const char * const *values, size_t n_values);
 void expr_macros_remove(struct shash *macros, const char *name);
 void expr_macros_destroy(struct shash *macros);
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index fda10eb..825dae7 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -38,8 +38,8 @@ VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
-/* Contains an internal expr datastructure that represents an address set. */
-static struct shash expr_address_sets;
+/* Contains an internal expr datastructure that represents an macro. */
+static struct shash expr_macros;
 
 static bool full_flow_processing = false;
 static bool full_logical_flow_processing = false;
@@ -65,7 +65,7 @@ void
 lflow_init(void)
 {
 shash_init(&symtab);
-shash_init(&expr_address_sets);
+shash_init(&expr_macros);
 
 /* Reserve a pair of regi

Re: [ovs-dev] [PATCH v1] ovn: Extend Address_Set to Macros to support define port name sets

[Zong Kai Li]

On Thu, Aug 4, 2016 at 6:31 AM, Ben Pfaff  wrote:
> On Wed, Aug 03, 2016 at 11:14:20PM +0800, Zong Kai LI wrote:
>> This patch aims to extend Address_Set to Macros, make it more common to
>> accept variable set, not only address. And by that, we can skinny down ACLs,
>> if we use Macros to defines port name sets for ACL to use, since lots of ACL
>> entries are similar but only "inport" and "outport" fields are different.
>>
>> Address_Set improves the performance of OVN control plane. But what more
>> important is, its implementation introduced a "$macros -- element_set"
>> mechanism, it's good way to do bunch jobs for ACLs, and even lflows.
>
> I'd prefer to defer this feature past the Open vSwitch 2.6 release.

Sure, it's ok to defer this, that will buy me some time to fix details
and complete TODO.
"Macros" tells the name column and "$MACROS_NAME" in lflows, "Set"
tells each row in table is a set of something, like addresses and port
names. Well, "Set" seems closer to the table, I will choose it in
future version.

Thanks and have a nice day! :)
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v1] ovn: rename Address_Set to Set to reflect a more broad purpose

[Zong Kai LI]

This patch renames table Address_Set to Set, Address_Set.addresses to
Set.members to reflect a more broad purpose, that we can define other types
of sets than address set.

Per discussion around [1] and [2], this patch only does rename work on table
Address_Set, and consider put future purpose on using table Set, to create
port set which will be used to improve and skinny ACLs, in following pacthes.

Since it only renames table Address_Set, and up to now, only address set get
supported, so this patch doesn't rename variables and methods which used for
address set, such as [3].

[1] http://openvswitch.org/pipermail/dev/2016-August/077121.html
[2] http://openvswitch.org/pipermail/dev/2016-August/077224.html
[3]
https://github.com/openvswitch/ovs/blob/master/ovn/controller/lflow.c#L41-L42
https://github.com/openvswitch/ovs/blob/master/ovn/controller/lflow.c#L62-L205

Signed-off-by: Zong Kai LI 
---
 ovn/controller/lflow.c| 22 +++---
 ovn/northd/ovn-northd.c   | 26 +-
 ovn/ovn-nb.ovsschema  | 12 ++--
 ovn/ovn-nb.xml| 13 +++--
 ovn/ovn-sb.ovsschema  | 12 ++--
 ovn/ovn-sb.xml|  6 +++---
 ovn/utilities/ovn-nbctl.c |  4 ++--
 ovn/utilities/ovn-sbctl.c |  4 ++--
 tests/ovn.at  |  2 +-
 9 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index ece464b..9faada4 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -82,20 +82,20 @@ addr_cmp(const void *p1, const void *p2)
 /* Return true if the address sets match, false otherwise. */
 static bool
 address_sets_match(const struct address_set *addr_set,
-   const struct sbrec_address_set *addr_set_rec)
+   const struct sbrec_set *addr_set_rec)
 {
 char **addrs1;
 char **addrs2;
 
-if (addr_set->n_addresses != addr_set_rec->n_addresses) {
+if (addr_set->n_addresses != addr_set_rec->n_members) {
 return false;
 }
 size_t n_addresses = addr_set->n_addresses;
 
 addrs1 = xmemdup(addr_set->addresses,
  n_addresses * sizeof addr_set->addresses[0]);
-addrs2 = xmemdup(addr_set_rec->addresses,
- n_addresses * sizeof addr_set_rec->addresses[0]);
+addrs2 = xmemdup(addr_set_rec->members,
+ n_addresses * sizeof addr_set_rec->members[0]);
 
 qsort(addrs1, n_addresses, sizeof *addrs1, addr_cmp);
 qsort(addrs2, n_addresses, sizeof *addrs2, addr_cmp);
@@ -142,8 +142,8 @@ update_address_sets(struct controller_ctx *ctx)
 
 /* Iterate address sets in the southbound database.  Create and update the
  * corresponding symtab entries as necessary. */
-const struct sbrec_address_set *addr_set_rec;
-SBREC_ADDRESS_SET_FOR_EACH (addr_set_rec, ctx->ovnsb_idl) {
+const struct sbrec_set *addr_set_rec;
+SBREC_SET_FOR_EACH (addr_set_rec, ctx->ovnsb_idl) {
 struct address_set *addr_set =
 shash_find_data(&local_address_sets, addr_set_rec->name);
 
@@ -169,13 +169,13 @@ update_address_sets(struct controller_ctx *ctx)
  * that resolves to the full set of addresses.  Store it in
  * address_sets to remember that we created this symbol. */
 addr_set = xzalloc(sizeof *addr_set);
-addr_set->n_addresses = addr_set_rec->n_addresses;
-if (addr_set_rec->n_addresses) {
-addr_set->addresses = xmalloc(addr_set_rec->n_addresses
+addr_set->n_addresses = addr_set_rec->n_members;
+if (addr_set_rec->n_members) {
+addr_set->addresses = xmalloc(addr_set_rec->n_members
   * sizeof addr_set->addresses[0]);
 size_t i;
-for (i = 0; i < addr_set_rec->n_addresses; i++) {
-addr_set->addresses[i] = 
xstrdup(addr_set_rec->addresses[i]);
+for (i = 0; i < addr_set_rec->n_members; i++) {
+addr_set->addresses[i] = xstrdup(addr_set_rec->members[i]);
 }
 }
 shash_add(&local_address_sets, addr_set_rec->name, addr_set);
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 861f872..acd9726 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3787,29 +3787,29 @@ sync_address_sets(struct northd_context *ctx)
 {
 struct shash sb_address_sets = SHASH_INITIALIZER(&sb_address_sets);
 
-const struct sbrec_address_set *sb_address_set;
-SBREC_ADDRESS_SET_FOR_EACH (sb_address_set, ctx->ovnsb_idl) {
+const struct sbrec_set *sb_address_set;
+SBREC_SET_FOR_EACH (sb_address_set, ctx->ovnsb_idl) {
 shash_add(&sb_address_sets, sb_address_set->name, sb_address_set);
 }
 
-const struct nbrec_address

Re: [ovs-dev] [PATCH v1] ovn: Extend Address_Set to Macros to support define port name sets

[Zong Kai Li]

On Thu, Aug 4, 2016 at 7:46 PM, Russell Bryant  wrote:
>
>
> On Wed, Aug 3, 2016 at 6:31 PM, Ben Pfaff  wrote:
>>
>> On Wed, Aug 03, 2016 at 11:14:20PM +0800, Zong Kai LI wrote:
>> > This patch aims to extend Address_Set to Macros, make it more common to
>> > accept variable set, not only address. And by that, we can skinny down
>> > ACLs,
>> > if we use Macros to defines port name sets for ACL to use, since lots of
>> > ACL
>> > entries are similar but only "inport" and "outport" fields are
>> > different.
>> >
>> > Address_Set improves the performance of OVN control plane. But what more
>> > important is, its implementation introduced a "$macros -- element_set"
>> > mechanism, it's good way to do bunch jobs for ACLs, and even lflows.
>>
>> I'd prefer to defer this feature past the Open vSwitch 2.6 release.
>
>
> On an earlier patch, I suggested the idea that we might want to at least
> rename the table if we see that coming, even if we don't apply the rest of
> the changes.  That way, we get the non-backwards compatible part out of the
> way before 2.6.  What do you think?
>
> --
> Russell Bryant

Hi, Russell and Ben.
I submitted another patch @ http://patchwork.ozlabs.org/patch/657589/
, it's a only rename table Address_Set version. Since we only have
address set supported for now, so I didn't renames and changes
variables and methods used for address set.

I hope this can get merged recently, and I don't get misunderstand on
Russell's suggestion.

Thanks.
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v1] ovn: rename Address_Set to Set to reflect a more broad purpose

[Zong Kai Li]

On Thu, Aug 11, 2016 at 1:51 PM, Han Zhou  wrote:
>
>
> On Tue, Aug 9, 2016 at 8:55 PM, Zong Kai LI  wrote:
>>
>> This patch renames table Address_Set to Set, Address_Set.addresses to
>> Set.members to reflect a more broad purpose, that we can define other
>> types
>> of sets than address set.
>>
>> Per discussion around [1] and [2], this patch only does rename work on
>> table
>> Address_Set, and consider put future purpose on using table Set, to create
>> port set which will be used to improve and skinny ACLs, in following
>> pacthes.
>>
>> Since it only renames table Address_Set, and up to now, only address set
>> get
>> supported, so this patch doesn't rename variables and methods which used
>> for
>> address set, such as [3].
>>
>> [1] http://openvswitch.org/pipermail/dev/2016-August/077121.html
>> [2] http://openvswitch.org/pipermail/dev/2016-August/077224.html
>> [3]
>>
>> https://github.com/openvswitch/ovs/blob/master/ovn/controller/lflow.c#L41-L42
>>
>> https://github.com/openvswitch/ovs/blob/master/ovn/controller/lflow.c#L62-L205
>>
> Hi Zongkai,
>
> The port set is a great idea! But do we really need to rename the
> Address_Set table? Can we just have 2 tables: Address_Set and Port_Set (and
> we can add more in the future if needed)? If we really want to put them in
> the same table, then a new column "type" is needed here, right?
>
> Thanks,
> Han

Haha, Han, you pointed the same thought as I think about it at the first time :D

It's expansive to create another table named Port_Set, if we don't
have any other plan for Port_Set than using in ACL, since processing
around Port_Set is so similar to Address_Set. So, to rename/extend
Address_Set to common Set, will keep things simple.

A new column "type" is a good point, I used it in my another patch
[1], but later I doubt whether it's necessary. Some facts I want to
share:
1) ovn-controller don't care what kind of data in a set, it do care
whether a set can be found by a macro, a set name.
2) it's user responsibility to keep all data in a set are in the same
logical type, just like ovn-nb.xml says "A single address set must
contain addresses of the same type.", and I also did a simple test:
ovn-nbctl list address-set as1
_uuid   : 88b2600b-fe4f-4ee0-88ee-3caa6493
addresses   : ["10.0.0.10", "10.0.0.10/24",
"fdd5:edde:a711:0:f816:3eff:fe08:e810"]
external_ids : {}
name   : "as1"

So, all behaviors are user defined. What data behind a macro/set name,
user defines that. We can add IPv4 addresses strings into set A, and
add set  A name as a macro in ip4.dst/ip4.src field in ACL, by this we
can call this set as an address set; or we can add string strings
which are port names in to set B, and add set B name as a macro in
inport/outport field in ACL, and by this, we can call this set as a
port name set.
And yes, bring "type" column can help us to add some verification on
data in a set, but it doesn't matter if we don't have it.

[1] http://patchwork.ozlabs.org/patch/655463/

Thanks and have a nice day. :)
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] [PATCH v3 1/2] ovn-controller: Add 'na' action for ND

[Zong Kai LI]

This patch adds a new OVN action 'na' to support ND versus ARP.

When ovn-controller received a ND packet, it frames a NA packet for
reply, with mac address parsed from userdata as eth.dst. Then it
reloads metadata info from previous packet to framed packet, and
finally send the framed packet back with left actions.

Eg. na{12:34:56:78:9a:bc; reg0 = 0x1; outport = inport; inport = ""; output;};

Since patch port for IPv6 router interface is not ready yet, this
patch will only try to deal with ND from VM. This patch will set
RSO flags to 011 for NA packets.

The next patch will do logical flows works for this action.

Signed-off-by: Zong Kai LI 
---
 lib/packets.c|  33 +
 lib/packets.h|   4 ++
 ovn/controller/pinctrl.c | 119 +--
 ovn/lib/actions.c|  50 
 ovn/lib/actions.h|   6 +++
 ovn/lib/expr.c   |  47 ++-
 ovn/lib/expr.h   |  43 +
 ovn/ovn-sb.xml   |  49 +++
 tests/ovn.at |  96 ++
 9 files changed, 378 insertions(+), 69 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 6a55d6f..cbc086e 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1355,6 +1355,39 @@ compose_nd(struct dp_packet *b, const struct eth_addr 
eth_src,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+void
+compose_na(struct dp_packet *b,
+   const struct eth_addr eth_src, const struct eth_addr eth_dst,
+   const struct in6_addr *ipv6_src, const struct in6_addr *ipv6_dst,
+   ovs_be16 rco_flags)
+{
+struct ovs_nd_msg *na;
+struct ovs_nd_opt *nd_opt;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+na = compose_ipv6(b, IPPROTO_ICMPV6,
+  ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
+  ALIGNED_CAST(ovs_be32 *, ipv6_dst->s6_addr),
+  0, 0, 255,
+  ND_MSG_LEN + ND_OPT_LEN);
+
+na->icmph.icmp6_type = ND_NEIGHBOR_ADVERT;
+na->icmph.icmp6_code = 0;
+na->rco_flags.hi = rco_flags;
+
+nd_opt = &na->options[0];
+nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
+nd_opt->nd_opt_len = 1;
+
+packet_set_nd(b, ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
+  eth_addr_zero, eth_src);
+na->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+na->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, na,
+  ND_MSG_LEN + 
ND_OPT_LEN));
+}
+
 uint32_t
 packet_csum_pseudoheader(const struct ip_header *ip)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 5945940..11b3b6d 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1069,6 +1069,10 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
  ovs_be32 arp_spa, ovs_be32 arp_tpa);
 void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
 struct in6_addr *, struct in6_addr *);
+void compose_na(struct dp_packet *,
+const struct eth_addr eth_src, const struct eth_addr eth_dst,
+const struct in6_addr *, const struct in6_addr *,
+ovs_be16 rco_flags);
 uint32_t packet_csum_pseudoheader(const struct ip_header *);
 void IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6);
 
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index bc57c40..1af9e89 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -23,6 +23,7 @@
 #include "flow.h"
 #include "lport.h"
 #include "ovn-controller.h"
+#include "lib/packets.h"
 #include "lib/sset.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-msgs.h"
@@ -64,6 +65,11 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id,
   const struct lport_index *lports,
   struct hmap *local_datapaths);
+static void pinctrl_handle_na(const struct flow *ip_flow,
+  const struct match *md,
+  struct ofpbuf *userdata);
+static void reload_metadata(struct ofpbuf *ofpacts,
+const struct match *md);
 
 COVERAGE_DEFINE(pinctrl_drop_put_arp);
 
@@ -153,31 +159,7 @@ pinctrl_handle_arp(const struct flow *ip_flow, const 
struct match *md,
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
 enum ofp_version version = rconn_get_version(swconn);
 
-enum mf_field_id md_fields[] = {
-#if FLOW_N_REGS == 8
-MFF_REG0,
-MFF_REG1,
-MFF_REG2,
-MFF_REG3,
-MFF_REG4,
-MFF_REG5,
-MFF_REG6,
-MFF_REG7,
-#else

[ovs-dev] [PATCH] [PATCH v3 2/2] ovn: add lflows for 'na' action for ND

[Zong Kai LI]

This patch adds some lflows for 'na' action to support ND versus ARP.

For ovn-northd, it will generate lflows per each IPv6 address on
echo lport, with lport mac and IPv6 addresss, with 'na' action.
e.g. match=(ip6 && nd && icmp6.type == 135 &&
nd.target == fde3:f657:aac1:0:f816:3eff:fe13:8198),
 action=(na{fa:16:3e:13:81:98; reg0 = 0x1; outport = inport;
inport = ""; output;};)
And new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
from previous ls_in_arp_rsp.

Setting reg0 = 0x1 to mention that such kind of NA packets are replied
by ovn-controller, and for these packets, dont do conntrack on them.
Also modfiy current table 32 and table 48, to make these packets
output directly.

Signed-off-by: Zong Kai LI 
---
 ovn/controller/physical.c |  41 ++
 ovn/northd/ovn-northd.c   | 104 +-
 tutorial/OVN-Tutorial.md  |   6 +--
 3 files changed, 137 insertions(+), 14 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 576c695..d6886da 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -14,6 +14,8 @@
  */
 
 #include 
+#include 
+#include 
 #include "byte-order.h"
 #include "flow.h"
 #include "lflow.h"
@@ -24,6 +26,7 @@
 #include "openvswitch/vlog.h"
 #include "ovn-controller.h"
 #include "ovn/lib/ovn-sb-idl.h"
+#include "ovn/lib/ovn-util.h"
 #include "physical.h"
 #include "shash.h"
 #include "simap.h"
@@ -367,6 +370,21 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id 
mff_ovn_geneve,
 put_load(zone_id, MFF_LOG_CT_ZONE, 0, 32, &ofpacts);
 }
 
+int zone_id_dnat, zone_id_snat;
+char *dnat = alloc_nat_zone_key(binding, "dnat");
+char *snat = alloc_nat_zone_key(binding, "snat");
+zone_id_dnat = simap_get(ct_zones, dnat);
+if (zone_id_dnat) {
+put_load(zone_id_dnat, MFF_LOG_DNAT_ZONE, 0, 32, &ofpacts);
+}
+free(dnat);
+
+zone_id_snat = simap_get(ct_zones, snat);
+if (zone_id_snat) {
+put_load(zone_id_snat, MFF_LOG_SNAT_ZONE, 0, 32, &ofpacts);
+}
+free(snat);
+
 /* Set MFF_LOG_DATAPATH and MFF_LOG_INPORT. */
 put_load(dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts);
 put_load(port_key, MFF_LOG_INPORT, 0, 32, &ofpacts);
@@ -403,6 +421,12 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id 
mff_ovn_geneve,
 if (zone_id) {
 put_load(zone_id, MFF_LOG_CT_ZONE, 0, 32, &ofpacts);
 }
+if (zone_id_dnat) {
+put_load(zone_id_dnat, MFF_LOG_DNAT_ZONE, 0, 32, &ofpacts);
+}
+if (zone_id_snat) {
+put_load(zone_id_snat, MFF_LOG_SNAT_ZONE, 0, 32, &ofpacts);
+}
 
 /* Resubmit to table 34. */
 put_resubmit(OFTABLE_DROP_LOOPBACK, &ofpacts);
@@ -707,6 +731,23 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id 
mff_ovn_geneve,
 put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
 ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 0, &match, &ofpacts);
 
+/* Table 32, Priority 100.
+ * ===
+ *
+ * Directly output NA packets replied by ovn-controller for NS packets
+ * come from local VMs.
+ *   - match: ip6 && icmp6 && icmp6.type == 136 && reg0 == 0x1
+ *   - action: resubmit(,48)
+ */
+match_init_catchall(&match);
+match_set_dl_type(&match, htons(ETH_P_IPV6));
+match_set_nw_proto(&match, (uint8_t)IPPROTO_ICMPV6);
+match_set_icmp_type(&match, (uint8_t)ND_NEIGHBOR_ADVERT);
+match_set_reg(&match, 0, (uint32_t)1);
+ofpbuf_clear(&ofpacts);
+put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, &ofpacts);
+ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, &match, &ofpacts);
+
 /* Table 34, Priority 0.
  * ===
  *
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index a8dd2bb..b2790bd 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -93,7 +93,7 @@ enum ovn_stage {
 PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_ND,2, "ls_in_port_sec_nd") \
 PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,3, "ls_in_pre_acl")  \
 PIPELINE_STAGE(SWITCH, IN,  ACL,4, "ls_in_acl")  \
-PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,5, "ls_in_arp_rsp")  \
+PIPELINE_STAGE(SWITCH, IN,  ARP_ND_RSP, 5, "ls_in_arp_nd_rsp")  \
 PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,6, "ls_in_l2_lkup")  

Re: [ovs-dev] [PATCH] WIP OVN ND for Logical_Port

[Zong Kai LI]

On Tue, Jun 7, 2016 at 5:08 AM, Ben Pfaff  wrote:

> On Fri, May 27, 2016 at 10:32:56AM +0800, Zong Kai LI wrote:
> > From: lzklibj 
> >
> > This patch tries to implement ND for Logical_Port in OVN.
> >
> > Signed-off-by: lzklibj 
>
> Thanks for working on this.
>
> I'm CCing Justin, who is working on related features, to bring this
> patch to his attention.
>
> "sparse" says:
> ../ovn/controller/pinctrl.c:748:25: warning: cast to restricted
> ovs_be16
> about this line:
> na->rco_flags.hi = (ovs_be16)0x60;
> Probably, you should use put_16aligned_be32().
>
> In pinctrl.c, please #include  after pinctrl.h.
> Keeping pinctrl.h first ensures that it is self-contained.
>
> The functions compose_na() and reload_metadata() in pinctrl.c should be
> declared static, unless you want to make them public (and in that case
> the prototypes should be in a header file instead).
>
> I am concerned about alignment issues.  I am not sure that the use of
> ALIGNED_CAST here is safe on RISC platforms, because IPv6 addresses
> aren't always aligned on a 32-bit boundary when they are inside Ethernet
> packets with different encapsulations.  It would probably be better to
> use a different strategy to assure alignment, rather than to just assume
> it.
>
> Please add documentation for the new action, in ovn-sb.xml.
>
> I see that there are a few notes and to-do items; I guess that's what
> makes it a work-in-progress.
>
> Thanks,
>
> Ben.
>

Hi, Ben. Thanks for your time and review.

I know Justin is working on implement router patch port for IPv6 switch,
but I'm not sure whether will Justin also work on implement ND or not.
And I've separated the WIP patch to:
 - 1) ovn-controller: Add 'na' action for ND
 - 2) ovn: add lflows for 'na' action for ND
it goes to version 3 now, and I just updated test in 1). Not sure what kind
of test should be added in 2) yet.

About the comments you gave:
 - the function compose_na(), I've moved it to lib/packets.h and
lib/packets.c, and it's public function now. And after that,
 is no longer needed in pinctrl.c.
 - the function reload_metadata, I keep it in pinctrl.c and make it static.
 - about na->rco_flags.hi, the na.rco_flags is in a struct has two ovs_be16
type variable, hi and lo. So I think may be it's OK to directly assign the
hi variable a ovs_be16 type value.
 - about ALIGNED_CAST, indeed, I copied most part of compose_nd() to
implement compose_na(), so I suppose they could work fine.

Thanks again & have a nice day! :)

Best regards,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] [PATCH v3 1/2] ovn-controller: Add 'na' action for ND

[Zong Kai LI]

On Wed, Jun 8, 2016 at 6:19 PM, Numan Siddique  wrote:

>
>> --- a/ovn/ovn-sb.xml
>> +++ b/ovn/ovn-sb.xml
>> @@ -985,6 +985,55 @@
>>Prerequisite: ip4
>>  
>>
>> +
>> +  na{A; action; ...
>> };
>> +
>> +
>> +
>> +  
>> +Temporarily replaces the IPv6 packet being processed by an NA
>> +packet and executes each nested action on the NA
>> +packet.  Actions following the na action, if any,
>> apply
>> +to the original, unmodified packet.
>> +  
>> +
>> +  
>> +The NA packet that this action operates on is initialized
>> based on
>> +the IPv6 packet being processed(with userdata), as follows:
>> +  
>> +
>> +  
>> +eth.dst copied from eth.src
>> +eth.src copied from userdata
>> +eth.type = 0x86dd
>> +ip6.dst copied from
>> ip6.src
>> +ip6.src copied from
>> nd.target
>> +icmp6.type = 136 (Neighbor
>> Advertisement)
>> +nd.target unchanged
>> +nd.sll = 00:00:00:00:00:00
>> +nd.sll copied from userdata
>> +  
>> +
>> +  
>> +These are default values that the nested actions will
>> probably want
>> +to change:
>> +  
>> +
>> +  
>> +reg0 = 0x1(Mark as replied by
>> ovn-controller)
>> +outport copied from inport
>> +inport = ""
>> +  
>> +
>> +The ND packet has the same VLAN header, if any, as the IP
>> packet
>> +it replaces.
>> +  
>> +
>> +  
>> +Prerequisite: nd&&icmp6.type ==
>> 135
>> +  
>> +
>> +
>>
>>
> ​Hi Zong Kai LI,
> I am seeing compilation errors when I apply this patch. Probably some xml
> tag above has a mismatch.
>
> ---
>   File "/usr/lib64/python2.7/xml/dom/expatbuilder.py", line 207, in
> parseFile
> parser.Parse(buffer, 0)
> xml.parsers.expat.ExpatError: mismatched tag: line 1035, column 10
> Makefile:6208: recipe for target 'ovn/ovn-sb.5' failed
>
> ​​
>
>>
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>
>
Hi, Numan.
Sorry for that mistake, the correct things should be like:
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 5189401..857105d 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1017,7 +1017,7 @@
   
 These are default values that the nested actions will probably
want
 to change:
-  
+  

           
 reg0 = 0x1(Mark as replied by
ovn-controller)
@@ -1025,6 +1025,7 @@
 inport = ""
   

+  
 The ND packet has the same VLAN header, if any, as the IP
packet
 it replaces.
   

And I noticed you add a comment in the other patch: "This patch fails to
apply on the latest master.".
I'm not sure what does that mean? Once things get clear, I will try to
submit a new patch later.

Thanks for your time and have a nice day! :)

Best regards,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] [PATCH v3 2/2] ovn: add lflows for 'na' action for ND

[Zong Kai LI]

On Thu, Jun 9, 2016 at 5:32 AM, Ben Pfaff  wrote:

> On Wed, Jun 08, 2016 at 03:28:58PM +0800, Zong Kai LI wrote:
> > This patch adds some lflows for 'na' action to support ND versus ARP.
> >
> > For ovn-northd, it will generate lflows per each IPv6 address on
> > echo lport, with lport mac and IPv6 addresss, with 'na' action.
> > e.g. match=(ip6 && nd && icmp6.type == 135 &&
> > nd.target == fde3:f657:aac1:0:f816:3eff:fe13:8198),
> >  action=(na{fa:16:3e:13:81:98; reg0 = 0x1; outport = inport;
> > inport = ""; output;};)
> > And new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
> > from previous ls_in_arp_rsp.
> >
> > Setting reg0 = 0x1 to mention that such kind of NA packets are replied
> > by ovn-controller, and for these packets, dont do conntrack on them.
> > Also modfiy current table 32 and table 48, to make these packets
> > output directly.
> >
> > Signed-off-by: Zong Kai LI 
>
> I don't understand why it is necessary to have special-case code in
> ovn-controller physical_run() for neighbor advertisements.  Nothing
> similar is needed for ARP.  It would be much better to avoid special
> cases.  Can you explain?  At any rate, ovn-controller should definitely
> not have any knowledge of what purpose the logical flows use registers
> for.
>
> This adds a Linux-specific header file to physical.c, but that should
> not be necessary.
>
> None of these casts should be necessary:
> > +match_set_nw_proto(&match, (uint8_t)IPPROTO_ICMPV6);
> > +match_set_icmp_type(&match, (uint8_t)ND_NEIGHBOR_ADVERT);
> > +match_set_reg(&match, 0, (uint32_t)1);
>
> In ovn-northd, it seems like a really bad idea to use substrings
> searches on ACLs as a basis for making decisions.
>
> Thanks,
>
> Ben.
>

Hi.Ben.

--- Explanation for special modification for ND in physical.c---
Yes, there are something different from ARP for ND.
For logical egress pipeline, before my modification:
  - for ARP packets, in OVN table 48(ls_out_pre_acl), they will match lflow
"match=(1), action=(next;)", while for ND packets will match lflow
"match=(ip), action=(ct_next;)"
  - and the same to following tables, the flows ARP packets matched will
not have conntrack actions.
  - but for NA packets generated/replied by ovn-controller, they are new to
conntrack, and will not get correct processed, and dropped by ovs-vswitch
later with warning message like "Failed to acquire udpif_key corresponding
to unexpected flow (Invalid argument)".

So for NA packets generated/replied by ovn-controller, I want to use reg0
to mark those packets, and make them directly outputted to table 64(the
output table) from table 48, not go through conntrack by tables after table
48.
So for OVN table ls_out_pre_acl, we will have a lflow with match and action
such as "match=(ip6 && icmp6 && icmp6.type == 136 && reg0 == 0x1),
action=(output;)".

It's OK for actions for ND to set reg0 in OVN
table ls_in_arp_nd_rsp(renamed from ls_in_arp_rsp).
But OVN table 34(OFTABLE_DROP_LOOPBACK) will reset regX(X=0,1,2) to 0. So I
try to modify table 32 to skip that and directly output ND packets replied
by ovn-controller to table 48.
--- End of explanation---

If above explanation make sense for you, then in physical.c, I think at
least  is needed, ND_NEIGHBOR_ADVERT will need it.

Yes, the casts you mentioned are unnecessary, I will remove them.

In ovn-northd, I will try to find a better way to do ACLs searching.

Thanks for your time and review, and have a nice day! :)

Best regards,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] [PATCH v3 1/2] ovn-controller: Add 'na' action for ND

[Zong Kai LI]

On Thu, Jun 9, 2016 at 5:25 AM, Ben Pfaff  wrote:

> On Wed, Jun 08, 2016 at 03:26:30PM +0800, Zong Kai LI wrote:
> > This patch adds a new OVN action 'na' to support ND versus ARP.
> >
> > When ovn-controller received a ND packet, it frames a NA packet for
> > reply, with mac address parsed from userdata as eth.dst. Then it
> > reloads metadata info from previous packet to framed packet, and
> > finally send the framed packet back with left actions.
> >
> > Eg. na{12:34:56:78:9a:bc; reg0 = 0x1; outport = inport; inport = "";
> output;};
> >
> > Since patch port for IPv6 router interface is not ready yet, this
> > patch will only try to deal with ND from VM. This patch will set
> > RSO flags to 011 for NA packets.
> >
> > The next patch will do logical flows works for this action.
> >
> > Signed-off-by: Zong Kai LI 
>
> Thanks for the patch!
>
> I think it would be better to retain more of the flavor of the arp
> action here.  For that action, it is the responsibility of whoever
> writes the flow to initialize anything that cannot be inferred by the
> action.  For example, "arp" does not take a parameter that is assigned
> to eth.src; instead, whoever writes the flow simply puts an assignment
> to eth.src inside the nested set of actions.  That's more flexible, too,
> because it allows eth.src to come from someplace other than an immediate
> constant.
>
> The intent with OVN logical actions, by the way, is that only actions
> should be enclosed in {}; data parameters should be enclosed in ().  It
> looks funny to me to mix both of them inside {}.  But if we drop the
> eth.src parameter then that solves the problem.
>
> The documentation for reg0, outport, and inport seems strange, because
> it implies that the action actually changes these.  It doesn't, as far
> as I can tell.
>
> I still don't see any attempt to deal with alignment issues for RISC
> machines.  Using ALIGNED_CAST does not solve a problem, it only
> suppresses a warning.
>
> I'm appending the changes that I recommend for the documentation.  My
> changes do not actually implement these changes, only document them.  My
> changes also include some stylistic fixes to better match
> CodingStyle.md.
>
> Thanks,
>
> Ben.
>
> --8<--cut here-->8--
>
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 1af9e89..79c4070 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -765,7 +765,7 @@ pinctrl_handle_na(const struct flow *ip_flow,
>  goto exit;
>  }
>
> -// Frame the NA packet with RSO=011.
> +/* Frame the NA packet with RSO=011. */
>  uint64_t packet_stub[128 / 8];
>  struct dp_packet packet;
>  dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
> @@ -774,7 +774,7 @@ pinctrl_handle_na(const struct flow *ip_flow,
> &(ip_flow->nd_target), &(ip_flow->ipv6_src),
> htons(0x6000));
>
> -// Reload previous packet metadata.
> +/* Reload previous packet metadata. */
>  uint64_t ofpacts_stub[4096 / 8];
>  struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
>  reload_metadata(&ofpacts, md);
> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index 5189401..3fd626f 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -986,7 +986,7 @@
>  
>
>  
> -  na{A; action; ...
> };
> +  na { action; ... };
>  
>
>  
> @@ -999,38 +999,29 @@
>
>
>  The NA packet that this action operates on is initialized
> based on
> -the IPv6 packet being processed(with userdata), as follows:
> +the IPv6 packet being processed, as follows.  These are
> default
> +values that the nested actions will probably want to change:
>
>
>
> -eth.dst copied from eth.src
> -eth.src copied from userdata
> +eth.dst copied from
> eth.src.
> +eth.src unchanged
>  eth.type = 0x86dd
>  ip6.dst copied from ip6.src
>  ip6.src copied from
> nd.target
>  icmp6.type = 136 (Neighbor
> Advertisement)
>  nd.target unchanged
>  nd.sll = 00:00:00:00:00:00
> -nd.sll copied from userdata
> +nd.sll unchanged
>
>
>
> -These are default values that the nested actions will
> probably want
> -to change:
> -

Re: [ovs-dev] [PATCH] WIP OVN ND for Logical_Port

[Zong Kai LI]

On Thu, Jun 9, 2016 at 12:43 AM, Justin Pettit  wrote:

>
> > On Jun 8, 2016, at 1:12 AM, Zong Kai LI  wrote:
> >
> > I know Justin is working on implement router patch port for IPv6 switch,
> but I'm not sure whether will Justin also work on implement ND or not.
>
> Yes, I'm planning to add support for neighbor discovery and router
> advertisements as part of IPv6 support.
>
> --Justin
>
>
>
Hi, Justin.
Sorry for I don't know your plans like that, no offence, I suppose you're
just trying to implement the patch port part.
Can I keep working on ND? Since I've just started working on it. :)

BTW, I also implemented some prototype code for RA, to make SLAAC can work.
Shall I keep working on it if you don't start yet.

Thanks for your time and have a nice day! :)
Best regards,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] [PATCH, v4, 1/2] ovn-controller: Add 'na' action for ND

[Zong Kai LI]

This patch adds a new OVN action 'na' to support ND versus ARP.

When ovn-controller received a ND packet, it frames a template
NA packet for reply. The NA packet will be initialized based on
ND packet, such as NA packet will use:
 - ND packet eth.src as eth.dst,
 - ND packet eth.dst as eth.src,
 - ND packet ip6.src as ip6.dst,
 - ND packet nd.target as ip6.src,
 - ND packet eth.dst as nd.tll.

Then nested actions in 'na' action will update necessary fileds
for NA packet, such as:
 - eth.src, nd.tll
 - inport, outport

Eg. na { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc;
 outport = inport; inport = ""; /* Allow sending out inport. */
 output; };

Since patch port for IPv6 router interface is not ready yet, this
patch will only try to deal with ND from VM. This patch will set
RSO flags to 011 for NA packets.

The next patch will do logical flows works for this action.

Signed-off-by: Zong Kai LI 
---
 lib/packets.c|  29 ++
 lib/packets.h|   4 ++
 ovn/controller/pinctrl.c | 134 ++-
 ovn/lib/actions.c|  43 +++
 ovn/lib/actions.h|   6 +++
 ovn/ovn-sb.xml   |  39 ++
 tests/ovn.at | 101 +++
 7 files changed, 331 insertions(+), 25 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 43b5a70..617ff66 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1355,6 +1355,35 @@ compose_nd(struct dp_packet *b, const struct eth_addr 
eth_src,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+void
+compose_na(struct dp_packet *b,
+   const struct eth_addr eth_src, const struct eth_addr eth_dst,
+   const ovs_be32 ipv6_src[4], const ovs_be32 ipv6_dst[4],
+   ovs_be16 rco_flags)
+{
+struct ovs_nd_msg *na;
+struct ovs_nd_opt *nd_opt;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+na = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src, ipv6_dst, 0, 0, 255,
+  ND_MSG_LEN + ND_OPT_LEN);
+
+na->icmph.icmp6_type = ND_NEIGHBOR_ADVERT;
+na->icmph.icmp6_code = 0;
+na->rco_flags.hi = rco_flags;
+
+nd_opt = &na->options[0];
+nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
+nd_opt->nd_opt_len = 1;
+
+packet_set_nd(b, ipv6_src, eth_addr_zero, eth_src);
+na->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+na->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, na,
+  ND_MSG_LEN + 
ND_OPT_LEN));
+}
+
 uint32_t
 packet_csum_pseudoheader(const struct ip_header *ip)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 5945940..55e2500 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1069,6 +1069,10 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
  ovs_be32 arp_spa, ovs_be32 arp_tpa);
 void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
 struct in6_addr *, struct in6_addr *);
+void compose_na(struct dp_packet *,
+const struct eth_addr eth_src, const struct eth_addr eth_dst,
+const ovs_be32 ipv6_src[4], const ovs_be32 ipv6_dst[4],
+ovs_be16 rco_flags);
 uint32_t packet_csum_pseudoheader(const struct ip_header *);
 void IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6);
 
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 116397e..00e8fde 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -23,6 +23,8 @@
 #include "flow.h"
 #include "lport.h"
 #include "ovn-controller.h"
+#include "lib/byte-order.h"
+#include "lib/packets.h"
 #include "lib/sset.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-msgs.h"
@@ -64,6 +66,11 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id,
   const struct lport_index *lports,
   struct hmap *local_datapaths);
+static void pinctrl_handle_na(const struct flow *ip_flow,
+  const struct match *md,
+  struct ofpbuf *userdata);
+static void reload_metadata(struct ofpbuf *ofpacts,
+const struct match *md);
 
 COVERAGE_DEFINE(pinctrl_drop_put_arp);
 
@@ -153,31 +160,7 @@ pinctrl_handle_arp(const struct flow *ip_flow, const 
struct match *md,
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
 enum ofp_version version = rconn_get_version(swconn);
 
-enum mf_field_id md_fields[] = {
-#if FLOW_N_REGS == 8
-MFF_REG0,
-MFF_REG1,
-MFF_REG2,
-MFF_REG3,
-MFF_REG4,
-MFF_REG5,
-MFF_REG6,
-MFF_REG7,
-#el

[ovs-dev] [PATCH] [PATCH, , v4, 2/2] ovn: add lflows for 'na' action for ND

[Zong Kai LI]

This patch adds some lflows for 'na' action to support ND versus ARP.

For ovn-northd, it will generate lflows per each lport with its IPv6
addresses and mac addresss, with 'na' action.

e.g. match=(icmp6 && icmp6.type == 135 &&
(nd.target == fd81:ce49:a948:0:f816:3eff:fe46:8a42 ||
 nd.target == fd81:ce49:b123:0:f816:3eff:fe46:8a42)),
 action=(na { eth.src = fa:16:3e:46:8a:42; nd.tll = fa:16:3e:46:8a:42;
  outport = inport;
  inport = ""; /* Allow sending out inport. */ output; };)

And new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
from previous ls_in_arp_rsp.

This patch also modified current ACL lflows for ND, not to do conntrack
on ND packets in following tables:
 - S_SWITCH_IN_PRE_ACL
 - S_SWITCH_OUT_PRE_ACL
 - S_SWITCH_IN_ACL
 - S_SWITCH_OUT_ACL

Signed-off-by: Zong Kai LI 
---
 ovn/northd/ovn-northd.c  | 57 +---
 tutorial/OVN-Tutorial.md |  6 ++---
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index d53fca9..20d8c98 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -93,7 +93,7 @@ enum ovn_stage {
 PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_ND,2, "ls_in_port_sec_nd") \
 PIPELINE_STAGE(SWITCH, IN,  PRE_ACL,3, "ls_in_pre_acl")  \
 PIPELINE_STAGE(SWITCH, IN,  ACL,4, "ls_in_acl")  \
-PIPELINE_STAGE(SWITCH, IN,  ARP_RSP,5, "ls_in_arp_rsp")  \
+PIPELINE_STAGE(SWITCH, IN,  ARP_ND_RSP, 5, "ls_in_arp_nd_rsp")  \
 PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,6, "ls_in_l2_lkup")  \
   \
 /* Logical switch egress stages. */   \
@@ -1383,6 +1383,12 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, 
struct hmap *ports)
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip", "ct_next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip", "ct_next;");
 
+/* Ingress and Egress Pre-ACL Table (Priority 110).
+ *
+ * Not to do conntrack on ND packets. */
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "nd", "next;");
+ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, "nd", "next;");
+
 /* Ingress and Egress ACL Table (Priority 1).
  *
  * By default, traffic is allowed.  This is partially handled by
@@ -1433,6 +1439,12 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, 
struct hmap *ports)
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
   "!ct.est && ct.rel && !ct.new && !ct.inv",
   "next;");
+
+/* Ingress and Egress ACL Table (Priority 65535).
+ *
+ * Not to do conntrack on ND packets. */
+ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 65535, "nd", "next;");
+ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 65535, "nd", "next;");
 }
 
 /* Ingress or Egress ACL Table (Various priorities). */
@@ -1566,13 +1578,13 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 
 if (!strcmp(op->nbs->type, "localnet")) {
 char *match = xasprintf("inport == %s", op->json_key);
-ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_RSP, 100,
+ovn_lflow_add(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 100,
   match, "next;");
 free(match);
 }
 }
 
-/* Ingress table 5: ARP responder, reply for known IPs.
+/* Ingress table 5: ARP/ND responder, reply for known IPs.
  * (priority 50). */
 HMAP_FOR_EACH (op, key_node, ports) {
 if (!op->nbs) {
@@ -1580,7 +1592,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 }
 
 /*
- * Add ARP reply flows if either the
+ * Add ARP/ND reply flows if either the
  *  - port is up or
  *  - port type is router
  */
@@ -1591,7 +1603,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 for (size_t i = 0; i < op->nbs->n_addresses; i++) {
 struct lport_addresses laddrs;
 if (!extract_lsp_addresses(op->nbs->addresses[i], &laddrs,
-   false)) {
+   true)) {
 continue;
 }
 for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
@@ -1612,24 +1624,53 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,

[ovs-dev] [PATCH] [PATCH, v5] ovn: Add 'na' action and lflow for ND

[Zong Kai LI]

This patch tries to support ND versus ARP for OVN.

It adds a new OVN action 'na' in ovn-controller side, and modify lflows
for 'na' action and relevant packets in ovn-northd.

First, for ovn-northd, it will generate lflows per each lport with its
IPv6 addresses and mac addresss, with 'na' action, such as:
  match=(icmp6 && icmp6.type == 135 &&
 (nd.target == fd81:ce49:a948:0:f816:3eff:fe46:8a42 ||
  nd.target == fd81:ce49:b123:0:f816:3eff:fe46:8a42)),
  action=(na { eth.src = fa:16:3e:46:8a:42; nd.tll = fa:16:3e:46:8a:42;
   outport = inport;
   inport = ""; /* Allow sending out inport. */ output; };)

and new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
from previous ls_in_arp_rsp.

Later, for ovn-controller, when it received a ND packet, it frames a
template NA packet for reply. The NA packet will be initialized based on
ND packet, such as NA packet will use:
 - ND packet eth.src as eth.dst,
 - ND packet eth.dst as eth.src,
 - ND packet ip6.src as ip6.dst,
 - ND packet nd.target as ip6.src,
 - ND packet eth.dst as nd.tll.

Finally, nested actions in 'na' action will update necessary fileds
for NA packet, such as:
 - eth.src, nd.tll
 - inport, outport

Since patch port for IPv6 router interface is not ready yet, this
patch will only try to deal with ND from VM. This patch will set
RSO flags to 011 for NA packets.

This patch also modified current ACL lflows for ND, not to do conntrack
on ND and NA packets in following tables:
 - S_SWITCH_IN_PRE_ACL
 - S_SWITCH_OUT_PRE_ACL
 - S_SWITCH_IN_ACL
 - S_SWITCH_OUT_ACL

Signed-off-by: Zong Kai LI 
---
 lib/packets.c|  29 ++
 lib/packets.h|   4 ++
 ovn/controller/pinctrl.c | 136 ++-
 ovn/lib/actions.c|  43 +++
 ovn/lib/actions.h|   6 +++
 ovn/northd/ovn-northd.c  |  57 +---
 ovn/ovn-sb.xml   |  39 ++
 tests/ovn.at | 101 +++
 tutorial/OVN-Tutorial.md |   6 +--
 9 files changed, 385 insertions(+), 36 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 43b5a70..617ff66 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1355,6 +1355,35 @@ compose_nd(struct dp_packet *b, const struct eth_addr 
eth_src,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+void
+compose_na(struct dp_packet *b,
+   const struct eth_addr eth_src, const struct eth_addr eth_dst,
+   const ovs_be32 ipv6_src[4], const ovs_be32 ipv6_dst[4],
+   ovs_be16 rco_flags)
+{
+struct ovs_nd_msg *na;
+struct ovs_nd_opt *nd_opt;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+na = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src, ipv6_dst, 0, 0, 255,
+  ND_MSG_LEN + ND_OPT_LEN);
+
+na->icmph.icmp6_type = ND_NEIGHBOR_ADVERT;
+na->icmph.icmp6_code = 0;
+na->rco_flags.hi = rco_flags;
+
+nd_opt = &na->options[0];
+nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
+nd_opt->nd_opt_len = 1;
+
+packet_set_nd(b, ipv6_src, eth_addr_zero, eth_src);
+na->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+na->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, na,
+  ND_MSG_LEN + 
ND_OPT_LEN));
+}
+
 uint32_t
 packet_csum_pseudoheader(const struct ip_header *ip)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 5945940..55e2500 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1069,6 +1069,10 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
  ovs_be32 arp_spa, ovs_be32 arp_tpa);
 void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
 struct in6_addr *, struct in6_addr *);
+void compose_na(struct dp_packet *,
+const struct eth_addr eth_src, const struct eth_addr eth_dst,
+const ovs_be32 ipv6_src[4], const ovs_be32 ipv6_dst[4],
+ovs_be16 rco_flags);
 uint32_t packet_csum_pseudoheader(const struct ip_header *);
 void IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6);
 
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 116397e..26aefae 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -23,6 +23,8 @@
 #include "flow.h"
 #include "lport.h"
 #include "ovn-controller.h"
+#include "lib/byte-order.h"
+#include "lib/packets.h"
 #include "lib/sset.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-msgs.h"
@@ -64,6 +66,11 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id,
   const struct lport_index *lpor

Re: [ovs-dev] [PATCH] [PATCH, v4, 1/2] ovn-controller: Add 'na' action for ND

[Zong Kai LI]

On Wed, Jun 15, 2016 at 10:17 PM, Ryan Moats  wrote:

> "dev"  wrote on 06/14/2016 10:09:37 PM:
>
> > From: Zong Kai LI 
> > To: dev@openvswitch.org
> > Cc: Zong Kai LI 
> > Date: 06/14/2016 10:11 PM
> > Subject: [ovs-dev] [PATCH] [PATCH, v4, 1/2] ovn-controller: Add 'na'
> > action for ND
> > Sent by: "dev" 
> >
> > This patch adds a new OVN action 'na' to support ND versus ARP.
> >
> > When ovn-controller received a ND packet, it frames a template
> > NA packet for reply. The NA packet will be initialized based on
> > ND packet, such as NA packet will use:
> >  - ND packet eth.src as eth.dst,
> >  - ND packet eth.dst as eth.src,
> >  - ND packet ip6.src as ip6.dst,
> >  - ND packet nd.target as ip6.src,
> >  - ND packet eth.dst as nd.tll.
> >
> > Then nested actions in 'na' action will update necessary fileds
> > for NA packet, such as:
> >  - eth.src, nd.tll
> >  - inport, outport
> >
> > Eg. na { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc;
> >  outport = inport; inport = ""; /* Allow sending out inport. */
> >  output; };
> >
> > Since patch port for IPv6 router interface is not ready yet, this
> > patch will only try to deal with ND from VM. This patch will set
> > RSO flags to 011 for NA packets.
> >
> > The next patch will do logical flows works for this action.
> >
> > Signed-off-by: Zong Kai LI 
> > ---
>
> While the patch applies cleanly, the new ND test that has been
> added to tests/ovn.at fails to pass - the output indicates
> that the NA packet is not seen by VIF1:
>
> hv1/vif1-tx.pcap
> ./ovn.at:3219: cat $i.packets
> --- expout  2016-06-15 14:02:42.486004483 +
> +++ /home/stack/git/ovs/tests/testsuite.dir/at-groups/2061/stdout
> 2016-06-15 14:02:42.486004483 +
> @@ -1 +0,0 @@
>
> -fa163e940598fa163ea1f9ae86dd60203afffd81ce49a948f8163efffea1f9aefd81ce49a948f8163efffe9405988800e9ed6000fd81ce49a948f8163efffea1f9ae0201fa163ea1f9ae
>
> Because of this, I didn't review the second patch in the series...
>
> Ryan Moats
>
Hi, Ryan.
Sorry for that mistake. I combined two patches into one now. That works now.

Thanks for your time and review, have a nice day! :)
Best regards,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] [PATCH, v6] ovn: Add 'na' action and lflow for ND

[Zong Kai LI]

This patch tries to support ND versus ARP for OVN.

It adds a new OVN action 'na' in ovn-controller side, and modify lflows
for 'na' action and relevant packets in ovn-northd.

First, for ovn-northd, it will generate lflows per each lport with its
IPv6 addresses and mac addresss, with 'na' action, such as:
  match=(icmp6 && icmp6.type == 135 &&
 (nd.target == fd81:ce49:a948:0:f816:3eff:fe46:8a42 ||
  nd.target == fd81:ce49:b123:0:f816:3eff:fe46:8a42)),
  action=(na { eth.src = fa:16:3e:46:8a:42; nd.tll = fa:16:3e:46:8a:42;
   outport = inport;
   inport = ""; /* Allow sending out inport. */ output; };)

and new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
from previous ls_in_arp_rsp.

Later, for ovn-controller, when it received a ND packet, it frames a
template NA packet for reply. The NA packet will be initialized based on
ND packet, such as NA packet will use:
 - ND packet eth.src as eth.dst,
 - ND packet eth.dst as eth.src,
 - ND packet ip6.src as ip6.dst,
 - ND packet nd.target as ip6.src,
 - ND packet eth.dst as nd.tll.

Finally, nested actions in 'na' action will update necessary fileds
for NA packet, such as:
 - eth.src, nd.tll
 - inport, outport

Since patch port for IPv6 router interface is not ready yet, this
patch will only try to deal with ND from VM. This patch will set
RSO flags to 011 for NA packets.

This patch also modified current ACL lflows for ND, not to do conntrack
on ND and NA packets in following tables:
 - S_SWITCH_IN_PRE_ACL
 - S_SWITCH_OUT_PRE_ACL
 - S_SWITCH_IN_ACL
 - S_SWITCH_OUT_ACL

Signed-off-by: Zong Kai LI 
---
 lib/packets.c|  29 ++
 lib/packets.h|   4 ++
 ovn/controller/pinctrl.c | 134 ++-
 ovn/lib/actions.c|  46 
 ovn/lib/actions.h|   6 +++
 ovn/northd/ovn-northd.c  |  65 ---
 ovn/ovn-sb.xml   |  39 ++
 tests/ovn.at | 101 +++
 tutorial/OVN-Tutorial.md |   6 +--
 9 files changed, 394 insertions(+), 36 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 43b5a70..a27264c 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1355,6 +1355,35 @@ compose_nd(struct dp_packet *b, const struct eth_addr 
eth_src,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+void
+compose_na(struct dp_packet *b,
+   const struct eth_addr eth_src, const struct eth_addr eth_dst,
+   const ovs_be32 ipv6_src[4], const ovs_be32 ipv6_dst[4],
+   ovs_be32 rco_flags)
+{
+struct ovs_nd_msg *na;
+struct ovs_nd_opt *nd_opt;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+na = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src, ipv6_dst, 0, 0, 255,
+  ND_MSG_LEN + ND_OPT_LEN);
+
+na->icmph.icmp6_type = ND_NEIGHBOR_ADVERT;
+na->icmph.icmp6_code = 0;
+put_16aligned_be32(&na->rco_flags, rco_flags);
+
+nd_opt = &na->options[0];
+nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
+nd_opt->nd_opt_len = 1;
+
+packet_set_nd(b, ipv6_src, eth_addr_zero, eth_src);
+na->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+na->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, na,
+  ND_MSG_LEN + 
ND_OPT_LEN));
+}
+
 uint32_t
 packet_csum_pseudoheader(const struct ip_header *ip)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 5945940..077ccfa 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1069,6 +1069,10 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
  ovs_be32 arp_spa, ovs_be32 arp_tpa);
 void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
 struct in6_addr *, struct in6_addr *);
+void compose_na(struct dp_packet *,
+const struct eth_addr eth_src, const struct eth_addr eth_dst,
+const ovs_be32 ipv6_src[4], const ovs_be32 ipv6_dst[4],
+ovs_be32 rco_flags);
 uint32_t packet_csum_pseudoheader(const struct ip_header *);
 void IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6);
 
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 116397e..47b890d 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -23,6 +23,8 @@
 #include "flow.h"
 #include "lport.h"
 #include "ovn-controller.h"
+#include "lib/byte-order.h"
+#include "lib/packets.h"
 #include "lib/sset.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-msgs.h"
@@ -64,6 +66,11 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id,
   const str

[ovs-dev] [PATCH] [PATCH v1] ovn: align lrp 'network' to lsp 'addresses'

[Zong Kai LI]

This patch renames Logical_Router_Port 'network' column to 'networks'
and aligns it to Logical_Switch_Port 'addresses' column for peer port
in Logical_Switch_Port which type is 'router'.

Currently, a lsp port whos type is 'router', may have multiple addresses
for IPv6 address case, but it's peer lrp port has only 1 address in
'network' column. It's inconsistent, this patch tries to fix that.

Signed-off-by: Zong Kai LI 
---
 ovn/northd/ovn-northd.c   |  5 +++--
 ovn/ovn-nb.ovsschema  |  6 --
 ovn/ovn-nb.xml|  7 +++---
 ovn/utilities/ovn-nbctl.c | 54 +++
 tests/ovn-nbctl.at|  9 
 5 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 17713ec..5851fca 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -612,11 +612,12 @@ join_logical_ports(struct northd_context *ctx,
 }
 
 ovs_be32 ip, mask;
-char *error = ip_parse_masked(nbr->network, &ip, &mask);
+/* It should have only one IPv4 address. */
+char *error = ip_parse_masked(nbr->networks[0], &ip, &mask);
 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(&rl, "bad 'network' %s", nbr->network);
+VLOG_WARN_RL(&rl, "bad 'network' %s", nbr->networks[0]);
 free(error);
 continue;
 }
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 58f04b2..4a345c8 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Northbound",
 "version": "3.1.0",
-"cksum": "1426508118 6135",
+"cksum": "2770494524 6251",
 "tables": {
 "Logical_Switch": {
 "columns": {
@@ -95,7 +95,9 @@
 "Logical_Router_Port": {
 "columns": {
 "name": {"type": "string"},
-"network": {"type": "string"},
+"networks": {"type": {"key": "string",
+  "min": 0,
+  "max": "unlimited"}},
 "mac": {"type": "string"},
 "peer": {"type": {"key": "string", "min": 0, "max": 1}},
 "enabled": {"type": {"key": "boolean", "min": 0, "max": 1}},
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 6355c44..56d82fb 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -689,11 +689,12 @@
   
 
 
-
-  The IP address of the router and the netmask.  For example,
+
+  The IP addresses of the router and the netmask.  For example,
   192.168.0.1/24 indicates that the router's IP address is
   192.168.0.1 and that packets destined to 192.168.0.x should be
-  routed to this port.
+  routed to this port. A router port should have only one IPv4 address, or
+  multiple IPv6 addresses.
 
 
 
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 7789cdd..7416100 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1523,8 +1523,21 @@ nbctl_lrp_add(struct ctl_context *ctx)
 
 const char *lrp_name = ctx->argv[2];
 const char *mac = ctx->argv[3];
-const char *network = ctx->argv[4];
-const char *peer = (ctx->argc == 6) ? ctx->argv[5] : NULL;
+char **networks = ctx->argv + 4;
+size_t n_networks = 1;
+const char *peer = NULL;
+if (ctx->argc >= 6) {
+struct in6_addr ipv6;
+unsigned int plen;
+char *error = ipv6_parse_cidr(ctx->argv[ctx->argc - 1], &ipv6, &plen);
+if (error) {
+peer = ctx->argv[ctx->argc - 1];
+n_networks += ctx->argc - 6;
+free(error);
+} else {
+n_networks += ctx->argc - 5;
+}
+}
 
 const struct nbrec_logical_router_port *lrp;
 lrp = lrp_by_name_or_uuid(ctx, lrp_name, false);
@@ -1547,9 +1560,21 @@ nbctl_lrp_add(struct ctl_context *ctx)
   lrp->mac);
 }
 
-if (strcmp(network, lrp->network)) {
-ctl_fatal("%s: port already exists with network %s", lrp_name,
-  lrp->networ

Re: [ovs-dev] [PATCH] [PATCH v1] ovn: align lrp 'network' to lsp 'addresses'

[Zong Kai LI]

On Fri, Jun 24, 2016 at 4:43 AM, Justin Pettit  wrote:

>
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index 17713ec..5851fca 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -612,11 +612,12 @@ join_logical_ports(struct northd_context *ctx,
> > }
> >
> > ovs_be32 ip, mask;
> > -char *error = ip_parse_masked(nbr->network, &ip, &mask);
> > +/* It should have only one IPv4 address. */
> > +char *error = ip_parse_masked(nbr->networks[0], &ip,
> &mask);
>
> You changed the schema to allow not specifying a network, but don't check
> if one is not defined.  I think this will cause a crash in that instance.
>
> Sure, I will try to fix this, and add a test for the case you mentioned.


> >
> > diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> > index 58f04b2..4a345c8 100644
> > --- a/ovn/ovn-nb.ovsschema
> > +++ b/ovn/ovn-nb.ovsschema
> > @@ -1,7 +1,7 @@
> > {
> > "name": "OVN_Northbound",
> > "version": "3.1.0",
> > -"cksum": "1426508118 6135",
> > +"cksum": "2770494524 6251",
>
> You need to change the version number when you modify the schema.
>
> Oops, thanks for mention that.


> > -
> > -  The IP address of the router and the netmask.  For example,
> > +
> > +  The IP addresses of the router and the netmask.  For example,
> >   192.168.0.1/24 indicates that the router's IP
> address is
> >   192.168.0.1 and that packets destined to 192.168.0.x
> should be
> > -  routed to this port.
> > +  routed to this port. A router port should have only one IPv4
> address, or
> > +  multiple IPv6 addresses.
>
> Is there a reason to limit it to one IPv4 address, but allow multiple IPv6
> addresses?  Also, this gives the impression that IPv6 routing is
> implemented, but it's not at this point.
>
> I have a patch in my tree that adds support for multiple IPv4 addresses in
> addition to multiple IPv6 addresses.  My preference would be to add
> multiple lrp network support to the ovn-nbctl command at the same time that
> ovn-northd can make use of them.  I'm planning to send out my series within
> the next couple of weeks.
>
> Thanks,
>
> --Justin
>
>
>
To be honest, I start to think about it just from IPv6 side, not IPv4. So I
may make some mistake, and afer I checked ovn-nb.xml for
Logical_Switch_Port table addresses column, I'm sure I've made a mistake.
Maybe multiple IPv4 addresses should be supported also. Good catch.
For IPv6, multiple IPv6 addresses in Logical_Router_Port network column
will benefit the implementation for RA/SLAAC, cause they're prefixes
natively. For IPv4, I'm not sure yet.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 2/2] ovn: Add address_set() support for ACLs.

[Zong Kai LI]

> > +/* Remember the names of all address sets currently in
> > expr_address_sets
> > + * so we can detect address sets that have been deleted. */
> > +struct sset cur_address_sets = SSET_INITIALIZER(&cur_address_sets);
> >

> This sset is not an address_set, but address_set names (or keys).
> How about renaming this variable to cur_address_set_names ?


I think I'm agree with Flavio.


> +if (!address_sets_match(addr_set, addr_set_rec)) {> +
> expr_macros_remove(&expr_address_sets, addr_set_rec->name);> +
> address_set_destroy(addr_set);> +addr_set = NULL;> +  
>   create_set = true;> +}


nit: is 'addr_set = NULL;' necessary here, seems it will be assigned later:


> +addr_set = xzalloc(sizeof *addr_set);
> +addr_set->n_addresses = addr_set_rec->n_addresses;
> +if (addr_set_rec->n_addresses) {
> +addr_set->addresses = xmalloc(addr_set_rec->n_addresses
> +  * sizeof 
> addr_set->addresses[0]);
> +size_t i;
> +for (i = 0; i < addr_set_rec->n_addresses; i++) {
> +addr_set->addresses[i] = 
> xstrdup(addr_set_rec->addresses[i]);
> +}
> +}
> +shash_add(&local_address_sets, addr_set_rec->name, addr_set);
> +
> +expr_macros_add(&expr_address_sets, addr_set_rec->name,
> +(const char * const *) addr_set->addresses,
> +addr_set->n_addresses);


So it's possible to create an empty Address_Set, right? What shall we
do with an empty Address_Set?


> +SSET_FOR_EACH_SAFE (cur_node, next_node, &cur_address_sets) {
> +expr_macros_remove(&expr_address_sets, cur_node);
> +
> +struct address_set *addr_set
> += shash_find_and_delete(&local_address_sets, cur_node);
> +address_set_destroy(addr_set);
> +
> +struct sset_node *sset_node = SSET_NODE_FROM_NAME(cur_node);
> +sset_delete(&cur_address_sets, sset_node);
> +}
> +
> +sset_destroy(&cur_address_sets);


I think you don't need manually call sset_delete, sset_destroy will do
it for you.


> +SHASH_FOR_EACH_SAFE (node, next, &sb_address_sets) {
> +    sbrec_address_set_delete(node->data);
> +shash_delete(&sb_address_sets, node);
> +}
> +shash_destroy(&sb_address_sets);


The same, you don't need manually call shash_delete, shash_destroy
will do it for you.


Thanks and have a nice day! :)

Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] [PATCH, v7] ovn: Add 'na' action and lflow for ND

[Zong Kai LI]

This patch tries to support ND versus ARP for OVN.

It adds a new OVN action 'na' in ovn-controller side, and modify lflows
for 'na' action and relevant packets in ovn-northd.

First, for ovn-northd, it will generate lflows per each lport with its
IPv6 addresses and mac addresss, with 'na' action, such as:
  match=(icmp6 && icmp6.type == 135 &&
 (nd.target == fd81:ce49:a948:0:f816:3eff:fe46:8a42 ||
  nd.target == fd81:ce49:b123:0:f816:3eff:fe46:8a42)),
  action=(na { eth.src = fa:16:3e:46:8a:42; nd.tll = fa:16:3e:46:8a:42;
   outport = inport;
   inport = ""; /* Allow sending out inport. */ output; };)

and new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
from previous ls_in_arp_rsp.

Later, for ovn-controller, when it received a ND packet, it frames a
template NA packet for reply. The NA packet will be initialized based on
ND packet, such as NA packet will use:
 - ND packet eth.src as eth.dst,
 - ND packet eth.dst as eth.src,
 - ND packet ip6.src as ip6.dst,
 - ND packet nd.target as ip6.src,
 - ND packet eth.dst as nd.tll.

Finally, nested actions in 'na' action will update necessary fileds
for NA packet, such as:
 - eth.src, nd.tll
 - inport, outport

Since patch port for IPv6 router interface is not ready yet, this
patch will only try to deal with ND from VM. This patch will set
RSO flags to 011 for NA packets.

This patch also modified current ACL lflows for ND, not to do conntrack
on ND and NA packets in following tables:
 - S_SWITCH_IN_PRE_ACL
 - S_SWITCH_OUT_PRE_ACL
 - S_SWITCH_IN_ACL
 - S_SWITCH_OUT_ACL

(Rebase on upstream)

Signed-off-by: Zong Kai LI 
---
 lib/packets.c|  29 ++
 lib/packets.h|   4 ++
 ovn/controller/pinctrl.c | 134 ++-
 ovn/lib/actions.c|  47 +
 ovn/lib/actions.h|   6 +++
 ovn/northd/ovn-northd.c  |  65 ---
 ovn/ovn-sb.xml   |  39 ++
 tests/ovn.at | 101 +++
 tutorial/OVN-Tutorial.md |   6 +--
 9 files changed, 395 insertions(+), 36 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 43b5a70..a27264c 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1355,6 +1355,35 @@ compose_nd(struct dp_packet *b, const struct eth_addr 
eth_src,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+void
+compose_na(struct dp_packet *b,
+   const struct eth_addr eth_src, const struct eth_addr eth_dst,
+   const ovs_be32 ipv6_src[4], const ovs_be32 ipv6_dst[4],
+   ovs_be32 rco_flags)
+{
+struct ovs_nd_msg *na;
+struct ovs_nd_opt *nd_opt;
+uint32_t icmp_csum;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+na = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src, ipv6_dst, 0, 0, 255,
+  ND_MSG_LEN + ND_OPT_LEN);
+
+na->icmph.icmp6_type = ND_NEIGHBOR_ADVERT;
+na->icmph.icmp6_code = 0;
+put_16aligned_be32(&na->rco_flags, rco_flags);
+
+nd_opt = &na->options[0];
+nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
+nd_opt->nd_opt_len = 1;
+
+packet_set_nd(b, ipv6_src, eth_addr_zero, eth_src);
+na->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+na->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, na,
+  ND_MSG_LEN + 
ND_OPT_LEN));
+}
+
 uint32_t
 packet_csum_pseudoheader(const struct ip_header *ip)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 5945940..077ccfa 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1069,6 +1069,10 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
  ovs_be32 arp_spa, ovs_be32 arp_tpa);
 void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
 struct in6_addr *, struct in6_addr *);
+void compose_na(struct dp_packet *,
+const struct eth_addr eth_src, const struct eth_addr eth_dst,
+const ovs_be32 ipv6_src[4], const ovs_be32 ipv6_dst[4],
+ovs_be32 rco_flags);
 uint32_t packet_csum_pseudoheader(const struct ip_header *);
 void IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6);
 
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 200505a..f7edfd3 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -25,6 +25,8 @@
 #include "lport.h"
 #include "nx-match.h"
 #include "ovn-controller.h"
+#include "lib/byte-order.h"
+#include "lib/packets.h"
 #include "lib/sset.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-msgs.h"
@@ -68,6 +70,11 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id

Re: [ovs-dev] [PATCH v4 1/2] Add address set support.

[Zong Kai LI]

>
>
>  static bool
> +parse_macros(struct expr_context *ctx, struct expr_constant_set *cs,
> +  size_t *allocated_values)
> +{
> +if (!ctx->macros) {
> +expr_syntax_error(ctx, "No macros defined.");
> +return false;
> +}
> +
> +struct expr_constant_set *addr_set
> += shash_find_data(ctx->macros, ctx->lexer->token.s);
> +if (!addr_set) {
> +expr_syntax_error(ctx, "Unknown macro: '%s'",
> ctx->lexer->token.s);
> +return false;
> +}
> +
> +cs->type = EXPR_C_INTEGER;
>

Hi, Babu.
It's odd you don't directly fix here with assign_constant_set_type, but in
the following patch.
Hope it doesn't matter to others.

Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 2/2] ovn: Add address_set() support for ACLs.

[Zong Kai LI]

>
> +static void
> +update_address_sets(struct controller_ctx *ctx)
> +{
> +/* Remember the names of all address sets currently in
> expr_address_sets
> + * so we can detect address sets that have been deleted. */
> +struct sset cur_addr_set_names =
> SSET_INITIALIZER(&cur_addr_set_names);
> +
> +struct shash_node *node;
> +SHASH_FOR_EACH (node, &local_address_sets) {
> +sset_add(&cur_addr_set_names, node->name);
> +}
> +
> +/* Iterate address sets in the southbound database.  Create and
> update the
> + * corresponding symtab entries as necessary. */
> +const struct sbrec_address_set *addr_set_rec;
> +SBREC_ADDRESS_SET_FOR_EACH (addr_set_rec, ctx->ovnsb_idl) {
> +node = shash_find(&local_address_sets, addr_set_rec->name);
> +struct address_set *addr_set = node ? node->data : NULL;
> +
>

How about using shash_find_data here?
struct address_set *addr_set = shash_find_data(&local_address_sets,
addr_set_rec->name);

+
> +/* Anything remaining in cur_addr_set_names refers to an address set
> that
> + * has been deleted from the southbound database.  We should delete
> + * the corresponding symtab entry. */
> +const char *cur_node, *next_node;
> +SSET_FOR_EACH_SAFE (cur_node, next_node, &cur_addr_set_names) {
> +expr_macros_remove(&expr_address_sets, cur_node);
> +
> +struct address_set *addr_set
> += shash_find_and_delete(&local_address_sets, cur_node);
> +address_set_destroy(addr_set);
> +
> +struct sset_node *sset_node = SSET_NODE_FROM_NAME(cur_node);
> +sset_delete(&cur_addr_set_names, sset_node);
>

Trivial: this seems unnecessary, sset_destroy will call sset_clear, and
sset_clear will call sset_delete.


> +}
> +
> +sset_destroy(&cur_addr_set_names);
> +}
>
> +static void
> +sync_address_sets(struct northd_context *ctx)
> +{
> +struct shash sb_address_sets = SHASH_INITIALIZER(&sb_address_sets);
> +
> +const struct sbrec_address_set *sb_address_set;
> +SBREC_ADDRESS_SET_FOR_EACH (sb_address_set, ctx->ovnsb_idl) {
> +shash_add(&sb_address_sets, sb_address_set->name, sb_address_set);
> +}
> +
> +const struct nbrec_address_set *nb_address_set;
> +NBREC_ADDRESS_SET_FOR_EACH (nb_address_set, ctx->ovnnb_idl) {
> +sb_address_set = shash_find_and_delete(&sb_address_sets,
> +   nb_address_set->name);
> +if (!sb_address_set) {
> +sb_address_set = sbrec_address_set_insert(ctx->ovnsb_txn);
> +sbrec_address_set_set_name(sb_address_set,
> nb_address_set->name);
> +}
> +
> +sbrec_address_set_set_addresses(sb_address_set,
> +/* "char **" is not compatible with "const char **" */
> +(const char **) nb_address_set->addresses,
> +nb_address_set->n_addresses);
> +}
> +
> +struct shash_node *node, *next;
> +SHASH_FOR_EACH_SAFE (node, next, &sb_address_sets) {
> +sbrec_address_set_delete(node->data);
>

Trivial: this seems unnecessary, shash_destroy will delete and free node in
sb_address_sets.

+shash_delete(&sb_address_sets, node);
> +}
> +shash_destroy(&sb_address_sets);
> +}
>
>
> +  
> +
> +  Each row in this table represents a named set of addresses.
> +  An address set may contain MAC, IPv4, or IPv6 addresses and cidrs.
> +  The address set will ultimately be used in ACLs, where a certain
> +  type of field such as ip4.src or ip6.src will be compared with the
> +  address set. So, a single address set must contain addresses of the
> +  same type.
> +
> +
>

Thanks for updating this.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] ovn-northd: Add logical flows to support native DHCPv4

[Zong Kai LI]

levant checking
in build_dhcpv4_action.
So think about it, it's unnecessary to do duplicated check when lsp has
multiple addresses.

@@ -43,6 +43,11 @@
> "max": "unlimited"}},
>  "up": {"type": {"key": "boolean", "min": 0, "max": 1}},
>  "enabled": {"type": {"key": "boolean", "min": 0, "max":
> 1}},
> +"dhcpv4_options": {"type": {"key": {"type": "uuid",
> +"refTable": "DHCP_Options",
> +"refType": "strong"},
>

Great for the refTable.

That's all what I can see now.

Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] ovn-northd: Add logical flows to support native DHCPv4

[Zong Kai LI]

>
> +static void
> +check_and_add_supported_dhcp_opts_to_sb_db(struct northd_context *ctx)
> +{
> +static bool nothing_to_add = false;
> +
> +if (nothing_to_add) {
> +return;
> +}
> +
> +struct hmap dhcp_opts_to_add = HMAP_INITIALIZER(&dhcp_opts_to_add);
> +for (size_t i = 0; (i < sizeof(supported_dhcp_opts) /
> +sizeof(supported_dhcp_opts[0])); i++) {
> +hmap_insert(&dhcp_opts_to_add, &supported_dhcp_opts[i].hmap_node,
> +dhcp_opt_hash(supported_dhcp_opts[i].name));
> +}
> +
> +const struct sbrec_dhcp_options *opt_row, *opt_row_next;
> +SBREC_DHCP_OPTIONS_FOR_EACH_SAFE(opt_row, opt_row_next,
> ctx->ovnsb_idl) {
> +struct dhcp_opts_map *dhcp_opt =
> +dhcp_opts_find(&dhcp_opts_to_add, opt_row->name);
> +if (dhcp_opt) {
> +hmap_remove(&dhcp_opts_to_add, &dhcp_opt->hmap_node);
> +}
> +else {
> +sbrec_dhcp_options_delete(opt_row);
> +}
> +}
> +
> +if (!dhcp_opts_to_add.n) {
> +nothing_to_add = true;
> +}
> +
> +struct dhcp_opts_map *opt;
> +HMAP_FOR_EACH_POP(opt, hmap_node, &dhcp_opts_to_add) {
> +struct sbrec_dhcp_options *sbrec_dhcp_option =
> +sbrec_dhcp_options_insert(ctx->ovnsb_txn);
> +sbrec_dhcp_options_set_name(sbrec_dhcp_option, opt->name);
> +sbrec_dhcp_options_set_code(sbrec_dhcp_option, opt->code);
> +sbrec_dhcp_options_set_type(sbrec_dhcp_option, opt->type);
> +}
> +
> +hmap_destroy(&dhcp_opts_to_add);
> +}
> +
>

After HMAP_FOR_EACH_POP processed, will not dhcp_opts_to_add.n be set to 0?
For actions left in dhcp_opts_to_add has been added in HMAP_FOR_EACH_POP
loop.
If so, how about move dhcp_opts_to_add checking after HMAP_FOR_EACH_POP
loop?


> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 58f04b2..9587b94 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
> -"version": "3.1.0",
> -"cksum": "1426508118 6135",
> +"version": "3.2.0",
> +"cksum": "27511920 6856",
>  "tables": {
>  "Logical_Switch": {
>  "columns": {
> @@ -43,6 +43,11 @@
> "max": "unlimited"}},
>  "up": {"type": {"key": "boolean", "min": 0, "max": 1}},
>  "enabled": {"type": {"key": "boolean", "min": 0, "max":
> 1}},
> +"dhcpv4_options": {"type": {"key": {"type": "uuid",
> +"refTable": "DHCP_Options",
> +"refType": "strong"},
> + "min": 0,
> + "max": 1}},
>

I just recognized that this new column is in table Logical_Switch_Port, why
not in table Logical_Switch? Or we prefer lsp on the same ls can have
different dhcp_options?


> +static void
> +nbctl_lsp_set_dhcpv4_options(struct ctl_context *ctx)
> +{
> +const char *id = ctx->argv[1];
> +const struct nbrec_logical_switch_port *lsp;
> +
> +lsp = lsp_by_name_or_uuid(ctx, id, true);
> +const struct nbrec_dhcp_options *dhcp_opt = NULL;
> +if (ctx->argc == 3 ) {
> +dhcp_opt = dhcp_options_get(ctx, ctx->argv[2], true);
> +}
> +
> +if (dhcp_opt) {
> +ovs_be32 ip;
> +unsigned int plen;
> +char *error = ip_parse_cidr(dhcp_opt->cidr, &ip, &plen);
> +if (error){
> +free(error);
> +VLOG_WARN("DHCP options cidr '%s' is not IPv4",
> dhcp_opt->cidr);
> +return;
>

how about using ctl_fatal here?


> +static void
> +nbctl_dhcp_options_create(struct ctl_context *ctx)
> +{
> +/* Validate the cidr */
> +ovs_be32 ip;
> +unsigned int plen;
> +char *error = ip_parse_cidr(ctx->argv[1], &ip, &plen);
> +if (error){
> +/* check if its IPv6 cidr */
> +free(error);
> +struct in6_addr ipv6;
> +error = ipv6_parse_cidr(ctx->argv[1], &ipv6, &plen);
> +if (error) {
> +free(error);
> +VLOG_WARN("Invalid cidr format '%s'", ctx->argv[1]);
> +return;
>

ditto.


> +ovn-nbctl -- --id=@d1 create DHCP_Options cidr=10.0.0.0/24 \
> +options="\"server_id\"=\"10.0.0.1\" \"server_mac\"=\"ff:10:00:00:00:01\" \
> +\"lease_time\"=\"3600\" \"router\"=\"10.0.0.1\"" \
>

how about using nbctl_dhcp_options_create here?

+-- add Logical_Switch_Port ls1-lp1 dhcpv4_options @d1 \
> +-- add Logical_Switch_Port ls1-lp2 dhcpv4_options @d1
> +
> +ovn-nbctl -- --id=@d2 create DHCP_Options cidr=30.0.0.0/24 \
> +options="\"server_id\"=\"30.0.0.1\" \"server_mac\"=\"ff:10:00:00:00:02\" \
> +\"lease_time\"=\"3600\"" -- add Logical_Switch_Port ls2-lp2
> dhcpv4_options @d2
>

ditto

Thanks,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v6 2/2] ovn: Add address_set() support for ACLs.

[Zong Kai LI]

I've tested this serial patches, it works as expected.

Only one thing I'm not sure:

diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
> index a12b247..d85efd5 100644
> --- a/ovn/utilities/ovn-sbctl.c
> +++ b/ovn/utilities/ovn-sbctl.c
> @@ -779,6 +779,10 @@ static const struct ctl_table_class tables[] = {
>   {{&sbrec_table_mac_binding, &sbrec_mac_binding_col_logical_port,
> NULL},
>{NULL, NULL, NULL}}},
>
> +{&sbrec_table_address_set,
> + {{&sbrec_table_address_set, &sbrec_address_set_col_name, NULL},
> +  {NULL, NULL, NULL}}},
> +
>  {NULL, {{NULL, NULL, NULL}, {NULL, NULL, NULL}}}
>  };
>

since Address_Set table is identical to OVN NB DB and OVN SB DB, and
ovn-northd will update data in this table from north to south. So I think
it will make less sense to open Address_Set table to ovn-sbctl.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/7] ovn-northd.8: Update documentation.

[Zong Kai LI]

>
> -  Ingress table 3 prepares flows for possible stateful ACL processing
> -  in table 4.  It contains a priority-0 flow that simply moves
> -  traffic to table 4.  If stateful ACLs are used in the logical
> -  datapath, a priority-100 flow is added that sends IP packets to
> -  the connection tracker before advancing to table 4.
> +  This table prepares flows for possible stateful ACL processing in
> +  ingress table ACLs.  It contains a priority-0 flow that
> +  simply moves traffic to the next table.  If stateful ACLs are used
> in the
> +  logical datapath, a priority-100 flow is added that sends IP
> packets to
> +  the connection tracker before advancing to ingress table
> +  ACLs.
>  


After this changing, shall we also need to update the following line,
remove "from-lport" to make table name simpler and clearer?


>  Ingress table 4: from-lport ACLs
>



> -  This is similar to ingress table 3 except for to-lport
> -  traffic.
> +  This is similar to ingress table Pre-ACLs except for
> + to-lport traffic.
>  
>
>  Egress Table 1: to-lport ACLs
>

ditto

thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/7] ovn-northd: Introduce pre-stateful table.

[Zong Kai LI]

>
> @@ -1377,13 +1381,34 @@ build_pre_acls(struct ovn_datapath *od, struct
> hmap *lflows,
>   *
>   * Regardless of whether the ACL is "from-lport" or "to-lport",
>   * we need rules in both the ingress and egress table, because
> - * the return traffic needs to be followed. */
> -ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip",
> "ct_next;");
> -ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
> "ct_next;");
> + * the return traffic needs to be followed.
> + *
> + * 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful table
> send
> + * it to conntrack for tracking and defragmentation. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 100, "ip",
> +  REGBIT_CONNTRACK_DEFRAG" = 1; next;");
> +ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
> +  REGBIT_CONNTRACK_DEFRAG" = 1; next;");
>  }
>  }
>
>  static void
> +build_pre_stateful(struct ovn_datapath *od, struct hmap *lflows)
> +{
> +/* Ingress and Egress pre-stateful Table (Priority 0): Packets are
> + * allowed by default. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 0, "1", "next;");
> +ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 0, "1", "next;");
> +
> +/* If REGBIT_CONNTRACK_DEFRAG is set as 1, then the packets should be
> + * sent to conntrack for tracking and defragmentation. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 100,
> +  REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
> +ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 100,
> +  REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
> +}
>

I hope you can also modify the default next lflow with 0 priority for
PRE_ACL, instead of using "next;" as action, try to directly resubmit to
table ACL.
Since in PRE_STATEFUL table, for non-stateful stuff, there is just another
"next;", this is not fun.

Thanks,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 4/7] ovn-northd: Introduce stateful table.

[Zong Kai LI]

>
> @@ -1429,9 +1432,9 @@ build_acls(struct ovn_datapath *od, struct hmap
> *lflows)
>   * and then its return traffic would not have an associated
>   * conntrack entry and would return "+invalid". */
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 1, "ip",
> -  "ct_commit; next;");
> +  REGBIT_CONNTRACK_COMMIT" = 1; next;");
>  ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 1, "ip",
> -  "ct_commit; next;");
> +  REGBIT_CONNTRACK_COMMIT" = 1; next;");
>
>
Just like I commented in patch 3, try to directly resubmit non-stateful
stuff to ARP_RSP table, for they will meet just another "next;" in stateful
table.

 /* Ingress and Egress ACL Table (Priority 65535).
>   *
> @@ -1484,7 +1487,9 @@ build_acls(struct ovn_datapath *od, struct hmap
> *lflows)
>   * direction may not have any stateful rules, the server's
>   * may and then its return traffic would not have an
>   * associated conntrack entry and would return "+invalid". */
> -const char *actions = has_stateful ? "ct_commit; next;" :
> "next;";
> +const char *actions = has_stateful
> +? REGBIT_CONNTRACK_COMMIT" = 1; next;"
> +: "next;";
>

ditto, try to using resubmit.


> static void
> +build_stateful(struct ovn_datapath *od, struct hmap *lflows)
> +{
> +/* Ingress and Egress stateful Table (Priority 0): Packets are
> + * allowed by default. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 0, "1", "next;");
> +ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 0, "1", "next;");
>

if we allow non-stateful stuff to skip PRE_STATEFUL and STATEFUL table, we
will don't need the default next action in the two tables.

Thanks,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 5/7] ovn-nb: Add Load-balancer table to schema.

[Zong Kai LI]

Hi, Guru.


> +  
> +
> +  Each row represents one load balancer.
> +
> +
> +
> +  
> +A map of virtual IPv4 addresses (and an optional port number
> separated
> +by :) associated with this load balancer and
> +their corresponding endpoint IPv4 addresses (and optional port
> numbers
> +separated by :) separated by commas.  If
>

I'm not sure whether will this confuse others, but it confused me.
At first, I thougth it would be:
ovn-nbctl create load-balancer vips:'"30.0.0.1:8000,40.0.0.1:5000"'='"
10.0.0.1:80,10.0.0.2:80,10.0.0.3:80"'
it inserts successfully into DB, but fails in ovn-northd for "bad ip port
for load balancer key".
Later, when I read your patch 7/7, I recognize it should be:
ovn-nbctl create load-balancer vips:'"30.0.0.1:8000"'='"10.0.0.1:80,
10.0.0.2:80,10.0.0.3:80"' vips:'"30.0.0.1:5000"'='"10.0.0.1:80,10.0.0.2:80,
10.0.0.3:80"', this time it get correctly parsed in ovn-northd.

Woud you mind to update the description here to notice it's a list of maps,
or/and give some examples?

Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 7/7] ovn-northd: Logical flows for load balancers.

[Zong Kai LI]

Hi, Guru.


> +/* 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful table
> send
> + * packet to conntrack for defragmentation. */

+const char *ip_address;
> +SSET_FOR_EACH(ip_address, &all_ips) {
> +char *match = xasprintf("ip && ip4.dst == %s", ip_address);
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
> +  100, match, REGBIT_CONNTRACK_DEFRAG" = 1;
> next;");
> +free(match);
> +}
>

For ls_in_pre_lb table, you add a same action from ls_in_pre_acl table.
Indeed, there will be lflows, such as:
table=3(   ls_in_pre_acl), priority=  100, match=(ip), action=(reg0[0] = 1;
next;) [1]
table=4(ls_in_pre_lb), priority=  100, match=(ip && ip4.dst ==
30.0.0.1), action=(reg0[0] = 1; next;) [2]

I know it will be flexible to have different tables for different purpose,
but they are too close enough.
And for me, the only different is "&& ip4.dst == ENDPOINT_IP".

What's more, it's easily to have a allow-related ACL rules for traffic
whose initiater is VM/container. By that, in the most common case, we will
have rule [1] in table ls_in_pre_acl. If so, rule [2] in table ls_in_pre_lb
seems duplicated, and rules in table ls_out_pre_lb are duplicated to ones
in table ls_out_pre_acl.

So the only case to make pre_lb tables are necessary is, logical switch
doesn't contain an "allow-related" action ACL rule. It seems possible, but
I cannot figure out why people choose to not using "allow-related" action,
that will make ACL table hard to maintain.

+
> +sset_destroy(&all_ips);
> +
> +if (vip_configured) {
> +ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB,
> +  100, "ip", REGBIT_CONNTRACK_DEFRAG" = 1;
> next;");
> +}
> +}
> +}





> +if (od->nbs->load_balancer) {
> +struct nbrec_load_balancer *lb = od->nbs->load_balancer;
> +struct smap *vips = &lb->vips;
> +struct smap_node *node;
> +
> +SMAP_FOR_EACH (node, vips) {
> +uint16_t port = 0;
> +
> +/* node->key contains IP:port or just IP. */
> +char *ip_address = NULL;
> +ip_address_and_port_from_lb_key(node->key, &ip_address,
> &port);
> +if (!ip_address) {
> +continue;
> +}
> +
> +/* New connections in Ingress table. */
> +char *action = xasprintf("ct_lb(\"%s\");", node->value);
> +struct ds match = DS_EMPTY_INITIALIZER;
> +ds_put_format(&match, "ct.new && ip && ip4.dst == %s",
> ip_address);
> +if (port) {
> +if (lb->protocol && !strcmp(lb->protocol, "udp")) {
> +ds_put_format(&match, "&& udp && udp.dst == %d",
> port);
> +} else {
> +ds_put_format(&match, "&& tcp && tcp.dst == %d",
> port);
> +}
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_LB,
> +  120, ds_cstr(&match), action);
> +} else {
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_LB,
> +  110, ds_cstr(&match), action);
> +}
>

S_SWITCH_IN_LB, I think you missed to put them into method build_lb.


> +
> +ds_destroy(&match);
> +free(action);
> +   }
> +}
>  }
>
>
Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4] ovn-northd: Add logical flows to support native DHCPv4

[Zong Kai LI]

>
> +/* Logical switch ingress table 10 and 11: DHCP options and response
> + * priority 100 flows. */
> +HMAP_FOR_EACH (op, key_node, ports) {
> +if (!op->nbs) {
> +   continue;
> +}
> +
> +if (!lsp_is_enabled(op->nbs) || !strcmp(op->nbs->type, "router"))
> {
> +/* Don't add the DHCP flows if the port is not enabled or if
> the
> + * port is a router port. */
> +continue;
> +}
> +
> +if (!op->nbs->dhcpv4_options) {
> +/* CMS has disabled native DHCPv4 for this lport. */
> +continue;
> +}
> +
> +for (size_t i = 0; i < op->nbs->n_addresses; i++) {
> +struct lport_addresses laddrs;
> +if (!extract_lsp_addresses(op->nbs->addresses[i], &laddrs,
> +   false)) {
> +continue;
> +}
> +
> +for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
> +struct ds options_action = DS_EMPTY_INITIALIZER;
> +struct ds response_action = DS_EMPTY_INITIALIZER;
> +if (build_dhcpv4_action(op, laddrs.ipv4_addrs[j].addr,
> +&options_action,
> &response_action)) {
> +struct ds match = DS_EMPTY_INITIALIZER;
> +ds_put_format(
> +&match, "inport == %s && eth.src == "ETH_ADDR_FMT
> +" && ip4.src == 0.0.0.0 && "
> +"ip4.dst == 255.255.255.255 && udp.src == 68 && "
> +"udp.dst == 67", op->json_key,
> +ETH_ADDR_ARGS(laddrs.ea));
> +
> +ovn_lflow_add(lflows, op->od,
> S_SWITCH_IN_DHCP_OPTIONS,
> +  100, ds_cstr(&match),
> +  ds_cstr(&options_action));
> +/* If REGBIT_DHCP_OPTS_RESULT is set, it means the
> + * put_dhcp_opts action  is successful */
> +ds_put_cstr(&match, " && "REGBIT_DHCP_OPTS_RESULT);
> +ovn_lflow_add(lflows, op->od,
> S_SWITCH_IN_DHCP_RESPONSE,
> +  100, ds_cstr(&match),
> +  ds_cstr(&response_action));
> +ds_destroy(&match);
> +ds_destroy(&options_action);
> +ds_destroy(&response_action);
> +break;
> +}
> +}
> +free(laddrs.ipv4_addrs);
> +}
> +}
> +
> +/* Ingress table 10 and 11: DHCP options and response, by default
> goto next.
> + * (priority 0). */
> +
> +HMAP_FOR_EACH (od, key_node, datapaths) {
> +if (!od->nbs) {
> +continue;
> +}
> +
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_OPTIONS, 0, "1",
> "next;");
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_RESPONSE, 0, "1",
> "next;");
> +}
>

Great work, but only one thing makes me feel uncomfortable.
DHCP Options flows are generated per each port, but not per each switch
datapath.
Since the resumed packet copies L2-L4 fields from original packet, and DHCP
options fields are the same for each port in the same switch. It doesn't
make sense to build DHCP Options flows for each ports.
The inport and eth.src in DHCP Options flows, I think we need logical
switch metadata field indeed.

Thanks and have a nice day.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 7/7] ovn-northd: Logical flows for load balancers.

[Zong Kai LI]

Hi, Ben and Guru. I tried to test lb feature on my OpenStack env, but
failed.
The simplest topology, three VMs(cirros) and VIP are on the same switch.
VM2 and VM3 are endpoints for the VIP.
I tried to use ping and ssh to test VIP, but things don't work.

I think it should be arp issue.
First, in table ls_in_arp_rsp, there is no flow entry to response for VIP.
Second, in table ls_in_l2_lkup, it will determine which port to output per
packet eth.dst. I'm not familiar with with conntrack, but it seems have
nothing to process on packet L2 address, when I run "conntrack -L". So I
suppose this is another place will cause load balance failure.

Thanks.
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4] ovn-northd: Add logical flows to support native DHCPv4

[Zong Kai Li]

On Wed, Jul 6, 2016 at 5:14 PM, Numan Siddique  wrote:

>
>
> On Tue, Jul 5, 2016 at 12:04 PM, Zong Kai LI  wrote:
>
>> +/* Logical switch ingress table 10 and 11: DHCP options and response
>>> + * priority 100 flows. */
>>> +HMAP_FOR_EACH (op, key_node, ports) {
>>> +if (!op->nbs) {
>>> +   continue;
>>> +}
>>> +
>>> +if (!lsp_is_enabled(op->nbs) || !strcmp(op->nbs->type,
>>> "router")) {
>>> +/* Don't add the DHCP flows if the port is not enabled or
>>> if the
>>> + * port is a router port. */
>>> +continue;
>>> +}
>>> +
>>> +if (!op->nbs->dhcpv4_options) {
>>> +/* CMS has disabled native DHCPv4 for this lport. */
>>> +continue;
>>> +}
>>> +
>>> +for (size_t i = 0; i < op->nbs->n_addresses; i++) {
>>> +struct lport_addresses laddrs;
>>> +if (!extract_lsp_addresses(op->nbs->addresses[i], &laddrs,
>>> +   false)) {
>>> +continue;
>>> +}
>>> +
>>> +for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
>>> +struct ds options_action = DS_EMPTY_INITIALIZER;
>>> +struct ds response_action = DS_EMPTY_INITIALIZER;
>>> +if (build_dhcpv4_action(op, laddrs.ipv4_addrs[j].addr,
>>> +&options_action,
>>> &response_action)) {
>>> +struct ds match = DS_EMPTY_INITIALIZER;
>>> +ds_put_format(
>>> +&match, "inport == %s && eth.src ==
>>> "ETH_ADDR_FMT
>>> +" && ip4.src == 0.0.0.0 && "
>>> +"ip4.dst == 255.255.255.255 && udp.src == 68 &&
>>> "
>>> +"udp.dst == 67", op->json_key,
>>> +ETH_ADDR_ARGS(laddrs.ea));
>>> +
>>> +ovn_lflow_add(lflows, op->od,
>>> S_SWITCH_IN_DHCP_OPTIONS,
>>> +  100, ds_cstr(&match),
>>> +  ds_cstr(&options_action));
>>> +/* If REGBIT_DHCP_OPTS_RESULT is set, it means the
>>> + * put_dhcp_opts action  is successful */
>>> +ds_put_cstr(&match, " && "REGBIT_DHCP_OPTS_RESULT);
>>> +ovn_lflow_add(lflows, op->od,
>>> S_SWITCH_IN_DHCP_RESPONSE,
>>> +  100, ds_cstr(&match),
>>> +  ds_cstr(&response_action));
>>> +ds_destroy(&match);
>>> +ds_destroy(&options_action);
>>> +ds_destroy(&response_action);
>>> +break;
>>> +}
>>> +}
>>> +free(laddrs.ipv4_addrs);
>>> +}
>>> +}
>>> +
>>> +/* Ingress table 10 and 11: DHCP options and response, by default
>>> goto next.
>>> + * (priority 0). */
>>> +
>>> +HMAP_FOR_EACH (od, key_node, datapaths) {
>>> +if (!od->nbs) {
>>> +continue;
>>> +}
>>> +
>>> +ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_OPTIONS, 0, "1",
>>> "next;");
>>> +ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_RESPONSE, 0, "1",
>>> "next;");
>>> +}
>>>
>>
>> Great work, but only one thing makes me feel uncomfortable.
>> DHCP Options flows are generated per each port, but not per each switch
>> datapath.
>> Since the resumed packet copies L2-L4 fields from original packet, and
>> DHCP options fields are the same for each port in the same switch. It
>> doesn't make sense to build DHCP Options flows for each ports.
>> The inport and eth.src in DHCP Options flows, I think we need logical
>> switch metadata field indeed.
>>
>>
> ​
> Since DHCP options are associated with logical ports, I don't see
> any other way. Also the IP address to be offered in the DHCP response
> comes from the 

[ovs-dev] [PATCH] [RFC Patch] ovn-controller: ignore lflow matching remote VM port

[Zong Kai Li]

Currently, ovn-controller will install all lflows for a logical
switch, when ovn-controller determines not to skip processing of
that logical switch.

This will install too many OVS flows. We have 11 tables for logical
switch ingress pipeline, 8 tables for logical switch egress pipeline
now, and more in futrue.

There are two kind lflows in for logical switch. One has no
inport/outport matching, such as lflows in table ls_in_arp_rsp and
ls_in_l2_lkup. The other one has, and for now, lflows in the following
tables belong to this type:
 - ls_in_port_sec_l2
 - ls_in_port_sec_ip
 - ls_in_port_sec_nd
 - ls_in_acl
 - ls_out_pre_acl
 - ls_out_acl
 - ls_out_port_sec_ip
 - ls_out_port_sec_l2

Consider how packet trip through flows in network topology
(P: port, S: switch, R: router.
 Two VM(or VIF) ports are on different chassis):
 - P-S-P: only flows matching remote inport, local VM port as "inport" and
  local VM port as "outport" will be matched. There is no chance for
  flows matching remote VM port as "inport" or "outport" to be
matched.
 - P-S-R-S-P and P-S-R...R-S-P: all these cases seem different from the
  above one, but they have the same "last jump". No matter how
  many routers(with or without switches) are used, before packet
  leaves current chassis, the next jump will be:
destination_switch_gateway -> destination_switch_port,
  so it will become a P-S-P case again.
  And sinse this patch will not change ingress pipeline for
  logical routers, so traffic between router port to router port
  will not be impacted.
So, as we can see, we don't need to install flow for a lflow with inport
or outport matching in logical switch ingress pipeline, when it tries to
match
a VM(or VIF) port that doesn't belong to current chassis.
This can help ovn-controller to avoid to install many unnecessary flows.

Signed-off-by: Zong Kai LI 
---
 ovn/controller/lflow.c  | 42
+++--
 ovn/controller/lflow.h  |  3 ++-
 ovn/controller/ovn-controller.c |  2 +-
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 05e1eaf..b0602b0 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -323,7 +323,8 @@ static void consider_logical_flow(const struct
lport_index *lports,
   const struct simap *ct_zones,
   struct hmap *dhcp_opts_p,
   uint32_t *conj_id_ofs_p,
-  struct hmap *flow_table);
+  struct hmap *flow_table,
+  const char* chassis_id);

 static bool
 lookup_port_cb(const void *aux_, const char *port_name, unsigned int
*portp)
@@ -361,7 +362,8 @@ add_logical_flows(struct controller_ctx *ctx, const
struct lport_index *lports,
   const struct hmap *local_datapaths,
   const struct hmap *patched_datapaths,
   struct group_table *group_table,
-  const struct simap *ct_zones, struct hmap *flow_table)
+  const struct simap *ct_zones, struct hmap *flow_table,
+  const char* chassis_id)
 {
 uint32_t conj_id_ofs = 1;

@@ -376,7 +378,8 @@ add_logical_flows(struct controller_ctx *ctx, const
struct lport_index *lports,
 SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
 consider_logical_flow(lports, mcgroups, lflow, local_datapaths,
   patched_datapaths, group_table, ct_zones,
-  &dhcp_opts, &conj_id_ofs, flow_table);
+  &dhcp_opts, &conj_id_ofs, flow_table,
+  chassis_id);
 }

 dhcp_opts_destroy(&dhcp_opts);
@@ -392,7 +395,8 @@ consider_logical_flow(const struct lport_index *lports,
   const struct simap *ct_zones,
   struct hmap *dhcp_opts_p,
   uint32_t *conj_id_ofs_p,
-  struct hmap *flow_table)
+  struct hmap *flow_table,
+  const char* chassis_id)
 {
 /* Determine translation of logical table IDs to physical table IDs. */
 bool ingress = !strcmp(lflow->pipeline, "ingress");
@@ -436,6 +440,30 @@ consider_logical_flow(const struct lport_index *lports,
 return;
 }
 }
+
+/* Skip logical flow when it has an 'inport' or 'outport' to match,
+ * and the port is a VM or VIF interface, but not a local port to
+ * current chassis. */
+if (strstr(lflow->match, "inport")
+|| strstr(lflow->match, "outport")) {
+struct lexer lexer;
+ 

[ovs-dev] [PATCH] [Patch v1] ovn-controller: ignore lflow matching remote VM port

[Zong Kai LI]

Currently, ovn-controller will install all lflows for a logical
switch, when ovn-controller determines not to skip processing of
that logical switch.

This will install too many OVS flows. We have 11 tables for logical
switch ingress pipeline, 8 tables for logical switch egress pipeline
now, and more in futrue.

There are two kind lflows in for logical switch. One has no
inport/outport matching, such as lflows in table ls_in_arp_rsp and
ls_in_l2_lkup. The other one has, and for now, lflows in the following
tables belong to this type:
 - ls_in_port_sec_l2
 - ls_in_port_sec_ip
 - ls_in_port_sec_nd
 - ls_in_pre_acl
 - ls_in_acl
 - ls_out_pre_acl
 - ls_out_acl
 - ls_out_port_sec_ip
 - ls_out_port_sec_l2

Consider how packet trip through flows in network topology
(P: port, S: switch, R: router.
 Two VM(or VIF) ports are on different chassis):
 - P-S-P: only flows matching remote inport, local VM port as "inport" and
  local VM port as "outport" will be matched. There is no chance for
  flows matching remote VM port as "inport" or "outport" to be matched.
 - P-S-R-S-P and P-S-R...R-S-P: all these cases seem different from the
  above one, but they have the same "last jump". No matter how
  many routers(with or without switches) are used, before packet
  leaves current chassis, the next jump will be:
destination_switch_gateway -> destination_switch_port,
  so it will become a P-S-P case again.
  And sinse this patch will not change ingress pipeline for
  logical routers, so traffic between router port to router port
  will not be impacted.
So, as we can see, we don't need to install flow for a lflow with inport
or outport matching in logical switch ingress pipeline, when it tries to match
a VM(or VIF) port that doesn't belong to current chassis.
This can help ovn-controller to avoid to install many unnecessary flows.

The following data are flows number comparison on a 2 chassis environment,
with 7 VMs(4 VMs on chassis-1(ch1), 3 VMs on chassis-2(ch2)), 3 dnsmasq
interfaces(on chassis-1), and some lsp-lrp pairs:

+---+---+-+-+
|number of flows in | upstream-code | this patch  | this patch  |
|table  |   | works on ch1| works on ch2|
+---+---+-+-+
|ls_in_port_sec_l2  |   37  |   34|   30|
|ls_in_port_sec_ip  |   114 |   91|   81|
|ls_in_port_sec_nd  |   70  |   44|   32|
|ls_in_pre_acl  |   26  |   26|   26|
|ls_in_acl  |   71  |   53|   47|
|ls_out_pre_acl |   30  |   30|   30|
|ls_out_acl |   157 |   100   |   82|
|ls_out_port_sec_ip |   57  |   35|   26|
|ls_out_port_sec_l2 |   18  |   15|   11|
+---+---+-+-+
|total  |   580 |   428   |   365   |
+---+---+-+-+
|reduce ls flows|-- |   26%   |   37%   |
+---+---+-+-+
|all chassis flows  |   767 |   627   |   552   |
+---+---+-+-+
|reduce totally |-- |   18%   |   28%   |
+---+---+-+-+

Image such a topology, 10 chassis, and 50 VMs/chassis. Let's take table
ls_in_port_sec_l2 as example, with some constant flows, current
implement will install 500 + C flows in ls_in_port_sec_l2 on each
chassis. After we ignore lflows matching remote VM inport, it will have
50 + C flows been installed in table ls_in_port_sec_l2 on each node.

Signed-off-by: Zong Kai LI 
---
 ovn/controller/lflow.c  | 42 +++--
 ovn/controller/lflow.h  |  3 ++-
 ovn/controller/ovn-controller.c |  2 +-
 ovn/northd/ovn-northd.8.xml | 31 +-
 ovn/northd/ovn-northd.c | 17 -
 ovn/ovn-nb.xml  |  3 +++
 6 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 00d1d6e..eff4d61 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -324,7 +324,8 @@ static void consider_logical_flow(const struct lport_index 
*lports,
   const struct simap *ct_zones,
   struct hmap *dhcp_opts_p,
   uint32_t *conj_id_ofs_p,
-  struct hmap *flow_table);
+  struct hmap *flow_table,
+  const char*

Re: [ovs-dev] [PATCH v1] ovn: rename Address_Set to Set to reflect a more broad purpose

[Zong Kai Li]

On Fri, Aug 12, 2016 at 1:33 AM, Russell Bryant  wrote:
>
> On Tue, Aug 9, 2016 at 11:55 PM, Zong Kai LI  wrote:
>>
>> This patch renames table Address_Set to Set, Address_Set.addresses to
>> Set.members to reflect a more broad purpose, that we can define other
>> types
>> of sets than address set.
>>
>> Per discussion around [1] and [2], this patch only does rename work on
>> table
>> Address_Set, and consider put future purpose on using table Set, to create
>> port set which will be used to improve and skinny ACLs, in following
>> pacthes.
>>
>> Since it only renames table Address_Set, and up to now, only address set
>> get
>> supported, so this patch doesn't rename variables and methods which used
>> for
>> address set, such as [3].
>>
>> [1] http://openvswitch.org/pipermail/dev/2016-August/077121.html
>> [2] http://openvswitch.org/pipermail/dev/2016-August/077224.html
>> [3]
>>
>> https://github.com/openvswitch/ovs/blob/master/ovn/controller/lflow.c#L41-L42
>>
>> https://github.com/openvswitch/ovs/blob/master/ovn/controller/lflow.c#L62-L205
>>
>> Signed-off-by: Zong Kai LI 
>> ---
>
>
> I brought this patch up for discussion in today's OVN IRC meeting.  Ben
> raised a good point:
>
>  russellb: I have questions whether port sets and address sets are
> really the same kind of beast though.  For port sets, possibly we want to
> manage them via references to rows as opposed to sets of strings.
>
> I think that's a great point, so let's just defer the rename and we'll go
> through deciding on a final approach for this after 2.6.
>
> --
> Russell Bryant

Hi, Russell, thanks for your help on this patch. But I think it gets a
little strange that we try to manage ports set via references to rows,
don't we have Logical_Switch.ports to do that? And that idea seems
will not directly work with ACL, for ACL is using port name, not port
refer, and ACL with port refer in match seems not so friendly to user
to insert.
I think we can try to support multiple variable types in Set, port
names set or port refers set can be both supported.
And to a port set has port reference in it, I'm not sure whether it's
really useful, since a port is usually belongs to a switch or a
router. I don't see a scenario that uses port refer set yet.

And sure, it's acceptable for me to defer it.

Thanks.
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v1,2/6] ovn.lib: update logical-fields for ND

[Zong Kai LI]

From: Zongkai LI 

This patch updates symbols in logical-fields for ND, such as "nd",
"nd.target", "nd.sll", "nd.tll", to describe more clear about "icmp6.type"
predicate.
And it adds new symbols:
 - "nd_rs" stands for Router Solicitation message,
 - "nd_ra" stands for Router Advertisement message,
 - "nd_rd" stands for Redirect message,
---
 ovn/lib/logical-fields.c | 21 +
 ovn/ovn-sb.xml   |  5 -
 tests/ovn.at |  2 +-
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index 6dbb4ae..e6adba3 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -174,14 +174,27 @@ ovn_init_symtab(struct shash *symtab)
 expr_symtab_add_field(symtab, "arp.tha", MFF_ARP_THA, "arp", false);
 
 expr_symtab_add_predicate(symtab, "nd",
-  "icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl == 255");
+  "icmp6.type == {133, 134, 135, 136, 137} "
+  "&& icmp6.code == 0 && ip.ttl == 255");
+expr_symtab_add_predicate(symtab, "nd_rs",
+  "icmp6.type == 133 && icmp6.code == 0 && ip.ttl == 255");
+expr_symtab_add_predicate(symtab, "nd_ra",
+  "icmp6.type == 134 && icmp6.code == 0 && ip.ttl == 255");
 expr_symtab_add_predicate(symtab, "nd_ns",
   "icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255");
 expr_symtab_add_predicate(symtab, "nd_na",
   "icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255");
-expr_symtab_add_field(symtab, "nd.target", MFF_ND_TARGET, "nd", false);
-expr_symtab_add_field(symtab, "nd.sll", MFF_ND_SLL, "nd_ns", false);
-expr_symtab_add_field(symtab, "nd.tll", MFF_ND_TLL, "nd_na", false);
+expr_symtab_add_predicate(symtab, "nd_rd",
+  "icmp6.type == 137 && icmp6.code == 0 && ip.ttl == 255");
+expr_symtab_add_field(symtab, "nd.target", MFF_ND_TARGET,
+  "icmp6.type == {135, 136, 137} "
+  "&& icmp6.code == 0 && ip.ttl == 255", false);
+expr_symtab_add_field(symtab, "nd.sll", MFF_ND_SLL,
+  "icmp6.type == {133, 134, 135} "
+  "&& icmp6.code == 0 && ip.ttl == 255", false);
+expr_symtab_add_field(symtab, "nd.tll", MFF_ND_TLL,
+  "icmp6.type == {136, 137} "
+  "&& icmp6.code == 0 && ip.ttl == 255", false);
 
 expr_symtab_add_predicate(symtab, "tcp", "ip.proto == 6");
 expr_symtab_add_field(symtab, "tcp.src", MFF_TCP_SRC, "tcp", false);
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index e119249..af48daa 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -858,9 +858,12 @@
 ip.later_frag expands to ip.frag[1]
 ip.first_frag expands to ip.is_frag && 
!ip.later_frag
 arp expands to eth.type == 0x806
-nd expands to icmp6.type == {135, 136} 
&& icmp6.code == 0 && ip.ttl == 255
+nd expands to icmp6.type == {133, 134, 135, 
136, 137} && icmp6.code == 0 && ip.ttl == 255
+nd_rs expands to icmp6.type == 133 && 
icmp6.code == 0 && ip.ttl == 255
+nd_ra expands to icmp6.type == 134 && 
icmp6.code == 0 && ip.ttl == 255
 nd_ns expands to icmp6.type == 135 && 
icmp6.code == 0 && ip.ttl == 255
 nd_na expands to icmp6.type == 136 && 
icmp6.code == 0 && ip.ttl == 255
+nd_rd expands to icmp6.type == 137 && 
icmp6.code == 0 && ip.ttl == 255
 tcp expands to ip.proto == 6
 udp expands to ip.proto == 17
 sctp expands to ip.proto == 132
diff --git a/tests/ovn.at b/tests/ovn.at
index 216bb07..70b3275 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -911,7 +911,7 @@ get_nd(xxreg0, ip6.dst);
 # put_nd
 put_nd(inport, nd.target, nd.sll);
 encodes as 
push:NXM_NX_XXREG0[],push:NXM_OF_ETH_SRC[],push:NXM_NX_ND_SLL[],push:NXM_NX_ND_TARGET[],pop:NXM_NX_XXREG0[],pop:NXM_OF_ETH_SRC[],controller(userdata=00.00.00.04.00.00.00.00),pop:NXM_OF_ETH_SRC[],pop:NXM_NX_XXREG0[]
-has prereqs ((icmp6.type == 0x87 && eth.type == 0x86dd && ip.proto == 0x3a 
&& (eth.type == 0x800 || eth.type == 0x86dd)) || (icmp6.type == 0x88 && 
eth.type == 0x86dd && ip.proto == 0x3a && (eth.type == 0x800 || eth.type == 
0x86dd))) && icmp6.code == 0 && eth.type == 0x86dd && ip.proto == 0x3a && 
(eth.type == 0x800 || eth.type == 0x86dd) && ip.ttl == 0xff && (eth.type == 
0x800 || eth.type == 0x86dd) && icmp6.type == 0x87 && eth.type == 0x86dd && 
ip.proto == 0x3a && (eth.type == 0x800 || eth.type == 0x86dd) && icmp6.code == 
0 && eth.type == 0x86dd && ip.proto == 0x3a && (eth.type == 0x800 || eth.type 
== 0x86dd) && ip.ttl == 0xff && (eth.type == 0x800 || eth.type == 0x86dd)
+has prereqs ((icmp6.type == 0x87 && eth.type == 0x86dd && ip.proto == 0x3a 
&& (eth.type == 0x800 || eth.type == 0x86dd)) || (icmp6.type == 0x88 && 
eth.type == 0x86dd && ip.proto == 0x3a && (eth.type == 0x800 || eth.type == 
0x86dd)) || (icmp6.type == 0x89 && eth.type == 0x86dd && ip.proto == 0x3a && 
(eth.type == 0x800 || eth.type == 0x86dd))) &

[ovs-dev] [PATCH v1, 1/6] ovs.packets: add compose_nd_ra and packet_put_ra_*_opt support

[Zong Kai LI]

From: Zongkai LI 

This patch introduces methods to compose a Router Advertisement (RA) packet,
introduces flags for RA, and renames ovs_nd_opt to ovs_nd_lla_opt to specify
it's Source/Target Link-layer Address option.
---
 lib/flow.c|  26 -
 lib/odp-execute.c |  20 +++
 lib/packets.c | 168 +-
 lib/packets.h |  80 ++
 4 files changed, 233 insertions(+), 61 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index ba4f8c7..b442965 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -400,8 +400,8 @@ parse_icmpv6(const void **datap, size_t *sizep, const 
struct icmp6_hdr *icmp,
 while (*sizep >= 8) {
 /* The minimum size of an option is 8 bytes, which also is
  * the size of Ethernet link-layer options. */
-const struct ovs_nd_opt *nd_opt = *datap;
-int opt_len = nd_opt->nd_opt_len * ND_OPT_LEN;
+const struct ovs_nd_lla_opt *nd_opt = *datap;
+int opt_len = nd_opt->len * ND_LLA_OPT_LEN;
 
 if (!opt_len || opt_len > *sizep) {
 return;
@@ -410,17 +410,17 @@ parse_icmpv6(const void **datap, size_t *sizep, const 
struct icmp6_hdr *icmp,
 /* Store the link layer address if the appropriate option is
  * provided.  It is considered an error if the same link
  * layer option is specified twice. */
-if (nd_opt->nd_opt_type == ND_OPT_SOURCE_LINKADDR
+if (nd_opt->type == ND_OPT_SOURCE_LINKADDR
 && opt_len == 8) {
 if (OVS_LIKELY(eth_addr_is_zero(arp_buf[0]))) {
-arp_buf[0] = nd_opt->nd_opt_mac;
+arp_buf[0] = nd_opt->mac;
 } else {
 goto invalid;
 }
-} else if (nd_opt->nd_opt_type == ND_OPT_TARGET_LINKADDR
+} else if (nd_opt->type == ND_OPT_TARGET_LINKADDR
&& opt_len == 8) {
 if (OVS_LIKELY(eth_addr_is_zero(arp_buf[1]))) {
-arp_buf[1] = nd_opt->nd_opt_mac;
+arp_buf[1] = nd_opt->mac;
 } else {
 goto invalid;
 }
@@ -2273,7 +2273,7 @@ flow_compose_l4(struct dp_packet *p, const struct flow 
*flow)
 (icmp->icmp6_type == ND_NEIGHBOR_SOLICIT ||
  icmp->icmp6_type == ND_NEIGHBOR_ADVERT)) {
 struct in6_addr *nd_target;
-struct ovs_nd_opt *nd_opt;
+struct ovs_nd_lla_opt *nd_opt;
 
 l4_len += sizeof *nd_target;
 nd_target = dp_packet_put_zeros(p, sizeof *nd_target);
@@ -2282,16 +2282,16 @@ flow_compose_l4(struct dp_packet *p, const struct flow 
*flow)
 if (!eth_addr_is_zero(flow->arp_sha)) {
 l4_len += 8;
 nd_opt = dp_packet_put_zeros(p, 8);
-nd_opt->nd_opt_len = 1;
-nd_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
-nd_opt->nd_opt_mac = flow->arp_sha;
+nd_opt->len = 1;
+nd_opt->type = ND_OPT_SOURCE_LINKADDR;
+nd_opt->mac = flow->arp_sha;
 }
 if (!eth_addr_is_zero(flow->arp_tha)) {
 l4_len += 8;
 nd_opt = dp_packet_put_zeros(p, 8);
-nd_opt->nd_opt_len = 1;
-nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
-nd_opt->nd_opt_mac = flow->arp_tha;
+nd_opt->len = 1;
+nd_opt->type = ND_OPT_TARGET_LINKADDR;
+nd_opt->mac = flow->arp_tha;
 }
 }
 }
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 5a43904..8156ba9 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -185,7 +185,7 @@ odp_set_nd(struct dp_packet *packet, const struct 
ovs_key_nd *key,
const struct ovs_key_nd *mask)
 {
 const struct ovs_nd_msg *ns = dp_packet_l4(packet);
-const struct ovs_nd_opt *nd_opt = dp_packet_get_nd_payload(packet);
+const struct ovs_nd_lla_opt *nd_opt = dp_packet_get_nd_payload(packet);
 
 if (OVS_LIKELY(ns && nd_opt)) {
 int bytes_remain = dp_packet_l4_size(packet) - sizeof(*ns);
@@ -193,25 +193,25 @@ odp_set_nd(struct dp_packet *packet, const struct 
ovs_key_nd *key,
 struct eth_addr sll_buf = eth_addr_zero;
 struct eth_addr tll_buf = eth_addr_zero;
 
-while (bytes_remain >= ND_OPT_LEN && nd_opt->nd_opt_len != 0) {
-if (nd_opt->nd_opt_type == ND_OPT_SOURCE_LINKADDR
-&& nd_opt->nd_opt_len == 1) {
-sll_buf = nd_opt->nd_opt_mac;
+while (bytes_remain >= ND_LLA_OPT_LEN && nd_opt->len != 0) {
+if (nd_opt->type == ND_OPT_SOURCE_LINKADDR
+&& nd_opt->len == 1) {
+sll_

[ovs-dev] [PATCH v1, 0/6] ovn: add 'nd_ra' action and SLAAC support for

[Zong Kai LI]

From: Zongkai LI 

This serial patches try to support Stateless Address Autoconfiguration(SLAAC)
for IPv6, via Router Solicitation (RS) message responder, which will reply
Router Advertisement (RA) message.

User case:
  After this serial patches, a user can follow the following steps to let all
  VMs/VIFs on a logical switch has IPv6 prefix/CIDR, configure their IPv6
  addresses automatically:
   - add MTU "mtu=VAL" into Logical_Switch.other_config column,
   - add lsp and lrp for Logical_Switch to make it attached onto a router, the
 lsp and lrp should have IPv6 prefix/CIDR in their addresses and networks
 columns,
  Then, next time VM/VIF sends Router Solicitation (RS) message, a RA packet
  will be replied by ovn-controller. When VM/VIF receives that RA packet, it
  can configure its IPv6 address(es) and route(s) via that RA packet.

There will be logical flows in table ls_in_ra_rsp(28) to work as RA
responder, with:
 - match: ip6.dst == ff02::2 && nd_rs
   (nd_rs: icmp6.type == 133 && icmp6.code == 0 && ttl == 255)
 - action: nd_ra{put_nd_ra(64,0,10800,0,0);
 put_nd_ra_opt_sll(12:34:56:78:9a:bc);
 put_nd_ra_opt_mtu(1450);
 put_nd_ra_opt_prefix(64,192,10800,10800,fdad:a0f9:a012::);
 eth.src = 12:34:56:78:9a:bc;
 ip6.src = fe80::f334:56ff:fe78:9abc;
 outport = inport; flags.loopback = 1; output;};
   (nd_ra is a new action which stands for RS responder;
put_nd_ra is a new specific inner action for nd_ra to set cur_hop_limit,
  "Managed address configuration" and "Other configuration" flags, router
  lifetime, reachable time and retrans timer in RA packet;
put_nd_ra_opt_sll is a new specific inner action for nd_ra to append Source
  Link-layer Address Option for RA message with given ethernet address,
  such as 12:34:56:78:9a:bc;
put_nd_ra_opt_mtu is a new specific inner action for nd_ra to append MTU
  Option for RA message with given integer value, such as 1450;
put_nd_ra_opt_prefix is a new specific inner action for nd_ra to append
  Prefix Information Option for RA message with inner parameters. The inner
  parameters will set prefix length, on-link flag and autonomous
  address-configuration flag, valid lifetime, preferred lifetime, prefix
  in Prefix Information Option.
After a RA packet is composed, the left nested actions will make RA packet
transmitted back to the inport, where Router Solicitation (RS) packet comes.

Zongkai LI(6):
  ovs.packets: add compose_nd_ra and packet_put_ra_*_opt support
  ovn.lib: update logical-fields for ND
  ovs.lib.flow: extend is_nd for all ND messages types
  ovn-controller: add 'nd_ra' action against 'nd_na' action
  ovn-controller: add 'put_nd_ra' and 'put_nd_opt_*' actions support
  ovn-northd: add flow support for SLAAC

 include/ovn/actions.h |  86 ++-
 lib/flow.c|  26 ++---
 lib/flow.h|   5 +-
 lib/nx-match.c|   4 +-
 lib/odp-execute.c |  20 ++--
 lib/odp-util.c|   6 +-
 lib/packets.c | 168 -
 lib/packets.h |  80 +++---
 ovn/controller/pinctrl.c  | 217 +
 ovn/lib/actions.c | 269 +-
 ovn/lib/logical-fields.c  |  21 +++-
 ovn/northd/ovn-northd.c   |  97 +++--
 ovn/ovn-nb.ovsschema  |   5 +-
 ovn/ovn-nb.xml|  11 ++
 ovn/ovn-sb.xml|  85 ++-
 ovn/utilities/ovn-trace.c |  23 ++--
 tests/ovn.at  | 134 ++-
 17 files changed, 1149 insertions(+), 108 deletions(-)

-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v1, 3/6] ovs.lib.flow: extend is_nd for all ND messages types

[Zong Kai LI]

From: Zongkai LI 

This patch updates method "is_nd" to let type ND_ROUTER_SOLICIT,
ND_ROUTER_ADVERT, ND_REDIRECT can be recoginzed.
---
 lib/flow.h   |  5 -
 lib/nx-match.c   |  4 +++-
 lib/odp-util.c   |  6 +-
 ovn/controller/pinctrl.c | 10 ++
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/lib/flow.h b/lib/flow.h
index 5b83695..56b4f40 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -937,7 +937,10 @@ static inline bool is_nd(const struct flow *flow,
 memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
 }
 return (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
-flow->tp_src == htons(ND_NEIGHBOR_ADVERT));
+flow->tp_src == htons(ND_NEIGHBOR_ADVERT) ||
+flow->tp_src == htons(ND_ROUTER_SOLICIT) ||
+flow->tp_src == htons(ND_ROUTER_ADVERT) ||
+flow->tp_src == htons(ND_REDIRECT));
 }
 return false;
 }
diff --git a/lib/nx-match.c b/lib/nx-match.c
index b03ccf2..115147a 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -879,7 +879,9 @@ nxm_put_ip(struct ofpbuf *b, const struct match *match, 
enum ofp_version oxm)
 nxm_put_8(b, MFF_ICMPV6_CODE, oxm,
   ntohs(flow->tp_dst));
 }
-if (is_nd(flow, NULL)) {
+if (is_nd(flow, NULL)
+&& (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT)
+|| flow->tp_src == htons(ND_NEIGHBOR_ADVERT))) {
 nxm_put_ipv6(b, MFF_ND_TARGET, oxm,
  &flow->nd_target, &match->wc.masks.nd_target);
 if (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT)) {
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 6d29b67..9fadfcd 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -4429,6 +4429,8 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms 
*parms,
 icmpv6_key->icmpv6_code = ntohs(data->tp_dst);
 
 if (is_nd(flow, NULL)
+&& (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT)
+|| flow->tp_src == htons(ND_NEIGHBOR_ADVERT))
 /* Even though 'tp_src' and 'tp_dst' are 16 bits wide, ICMP
  * type and code are 8 bits wide.  Therefore, an exact match
  * looks like htons(0xff), not htons(0x).  See
@@ -4963,7 +4965,9 @@ parse_l2_5_onward(const struct nlattr 
*attrs[OVS_KEY_ATTR_MAX + 1],
 flow->tp_src = htons(icmpv6_key->icmpv6_type);
 flow->tp_dst = htons(icmpv6_key->icmpv6_code);
 expected_bit = OVS_KEY_ATTR_ICMPV6;
-if (is_nd(src_flow, NULL)) {
+if (is_nd(src_flow, NULL)
+&& (src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT)
+|| src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT))) {
 if (!is_mask) {
 expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
 }
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 2737467..725b711 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1355,11 +1355,13 @@ static void
 pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
  struct ofpbuf *userdata)
 {
-/* This action only works for IPv6 ND packets, and the switch should only
- * send us ND packets this way, but check here just to be sure. */
-if (!is_nd(ip_flow, NULL)) {
+/* This action only works for IPv6 ND Neighbor Solicitation packets,
+ * and the switch should only send us such packets this way, but check
+ * here just to be sure. */
+if (!is_nd(ip_flow, NULL)
+|| ip_flow->tp_src != htons(ND_NEIGHBOR_SOLICIT)) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-VLOG_WARN_RL(&rl, "NA action on non-ND packet");
+VLOG_WARN_RL(&rl, "NA action on non-ND-NS packet");
 return;
 }
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v1,6/6] ovn-northd: add flow support for SLAAC

[Zong Kai LI]

From: Zongkai LI 

This patch adds a new logical flow table to support Router Advertisement
responder. And it also adds flow support for SLAAC.
---
 ovn/northd/ovn-northd.c |  97 +---
 ovn/ovn-nb.ovsschema|   5 ++-
 ovn/ovn-nb.xml  |  11 +
 tests/ovn.at| 114 
 4 files changed, 219 insertions(+), 8 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 0874a9c..5c7806f 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -123,9 +123,10 @@ enum ovn_stage {
 PIPELINE_STAGE(ROUTER, IN,  IP_INPUT,1, "lr_in_ip_input") \
 PIPELINE_STAGE(ROUTER, IN,  UNSNAT,  2, "lr_in_unsnat")   \
 PIPELINE_STAGE(ROUTER, IN,  DNAT,3, "lr_in_dnat") \
-PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  4, "lr_in_ip_routing")   \
-PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 5, "lr_in_arp_resolve")  \
-PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 6, "lr_in_arp_request")  \
+PIPELINE_STAGE(ROUTER, IN,  RS_RSP,  4, "lr_in_rs_rsp")  \
+PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  5, "lr_in_ip_routing")   \
+PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 6, "lr_in_arp_resolve")  \
+PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 7, "lr_in_arp_request")  \
   \
 /* Logical router egress stages. */   \
 PIPELINE_STAGE(ROUTER, OUT, SNAT,  0, "lr_out_snat")  \
@@ -2721,6 +2722,32 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_RESPONSE, 0, "1", "next;");
 }
 
+/* Ingress table 12: Destination lookup, router solicitation handling
+ * (priority 110). */
+HMAP_FOR_EACH (od, key_node, datapaths) {
+if (!od->nbs) {
+continue;
+}
+
+if (!od->n_router_ports) {
+continue;
+}
+
+ds_clear(&actions);
+for (size_t i = 0; i != od->n_router_ports; i++) {
+op = od->router_ports[i];
+if (!op->lsp_addrs || !op->lsp_addrs->n_ipv6_addrs) {
+continue;
+}
+ds_put_format(&actions, "outport = %s; output; ", op->json_key);
+}
+if (actions.length != 0) {
+ds_chomp(&actions, ' ');
+ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110, "nd_rs",
+  ds_cstr(&actions));
+}
+}
+
 /* Ingress table 12: Destination lookup, broadcast and multicast handling
  * (priority 100). */
 HMAP_FOR_EACH (op, key_node, ports) {
@@ -3548,7 +3575,65 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
   "ip", "flags.loopback = 1; ct_dnat;");
 }
 
-/* Logical router ingress table 4: IP Routing.
+/* Logical router ingress table 4: RS responder, by default goto next.
+ * (priority 0)*/
+HMAP_FOR_EACH (od, key_node, datapaths) {
+if (!od->nbr) {
+continue;
+}
+
+ovn_lflow_add(lflows, od, S_ROUTER_IN_RS_RSP, 0, "1", "next;");
+}
+
+/* Logical router ingress table 4: RS responder, reply for 'slaac' enabled
+ * router port. (priority 50)*/
+HMAP_FOR_EACH (op, key_node, ports) {
+if (!op->nbrp || op->nbrp->peer
+|| !op->peer
+|| !op->nbrp->slaac
+|| !*op->nbrp->slaac) {
+continue;
+}
+
+ds_clear(&match);
+ds_put_format(&match, "inport == %s && ip6.dst == ff02::2 && nd_rs",
+  op->json_key);
+ds_clear(&actions);
+const char *mtu_s = smap_get(&op->peer->od->nbs->other_config, "mtu");
+uint32_t mtu = (!mtu_s || atoi(mtu_s) < 1280) ? 1280 : atoi(mtu_s);
+ds_put_format(&actions, "nd_ra{put_nd_ra(64,0,10800,0,0); "
+"put_nd_opt_sll(%s); "
+"put_nd_opt_mtu(%u); ",
+op->lrp_networks.ea_s,
+mtu);
+size_t actions_len = actions.length;
+for (size_t i = 0; i != op->lrp_networks.n_ipv6_addrs; i++) {
+if (in6_is_lla(&op->lrp_networks.ipv6_addrs[i].network)) {
+continue;
+}
+ds_put_format(&actions,
+  "put_nd_opt_prefix(%u,%u,10800,10800,%s); ",
+  op->lrp_networks.ipv6_addrs[i].plen,
+  (ND_PREFIX_ON_LINK | ND_PREFIX_AUTONOMOUS_ADDRESS),
+  op->lrp_networks.ipv6_addrs[i].network_s);
+}
+if (actions.length != actions_len) {
+char ip6_str[INET6_ADDRSTRLEN + 1];
+struct in6_addr lla;
+in6_generate_lla(op->lrp_networks.ea, &lla);
+memset(ip6_str, 0, sizeof(ip6_str));
+ipv6_string_mapped(ip6_st

[ovs-dev] [PATCH v1, 4/6] ovn-controller: add 'nd_ra' action against 'nd_na' action

[Zong Kai LI]

From: Zongkai LI 

This patchs add a new action 'nd_ra' against current 'nd_na' action, to work as
a Router Solicitation packet responder. The inner actions to set fields, flags,
options in RA packet will be introduced in the following patch.
---
 include/ovn/actions.h |  7 ++
 ovn/controller/pinctrl.c  | 59 +++
 ovn/lib/actions.c | 28 ++
 ovn/ovn-sb.xml| 41 
 ovn/utilities/ovn-trace.c | 19 ---
 tests/ovn.at  |  6 +
 6 files changed, 132 insertions(+), 28 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index e2a716a..c1dda2f 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -62,6 +62,7 @@ struct simap;
 OVNACT(CT_LB, ovnact_ct_lb) \
 OVNACT(ARP,   ovnact_nest)  \
 OVNACT(ND_NA, ovnact_nest)  \
+OVNACT(ND_RA, ovnact_nest)  \
 OVNACT(GET_ARP,   ovnact_get_mac_bind)  \
 OVNACT(PUT_ARP,   ovnact_put_mac_bind)  \
 OVNACT(GET_ND,ovnact_get_mac_bind)  \
@@ -353,6 +354,12 @@ enum action_opcode {
  *   - Any number of DHCPv6 options.
  */
 ACTION_OPCODE_PUT_DHCPV6_OPTS,
+
+/* "nd_ra { ...actions... }".
+ *
+ * The actions, in OpenFlow 1.3 format, follow the action_header.
+ */
+ACTION_OPCODE_ND_RA,
 };
 
 /* Header. */
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 725b711..464c90d 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -71,9 +71,9 @@ static void send_garp_run(const struct ovsrec_bridge *,
   const char *chassis_id,
   const struct lport_index *lports,
   struct hmap *local_datapaths);
-static void pinctrl_handle_nd_na(const struct flow *ip_flow,
- const struct match *md,
- struct ofpbuf *userdata);
+static void pinctrl_handle_nd(const struct flow *ip_flow,
+  const struct match *md,
+  struct ofpbuf *userdata);
 static void reload_metadata(struct ofpbuf *ofpacts,
 const struct match *md);
 
@@ -698,7 +698,8 @@ process_packet_in(const struct ofp_header *msg)
 break;
 
 case ACTION_OPCODE_ND_NA:
-pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata);
+case ACTION_OPCODE_ND_RA:
+pinctrl_handle_nd(&headers, &pin.flow_metadata, &userdata);
 break;
 
 case ACTION_OPCODE_PUT_ND:
@@ -1352,33 +1353,51 @@ reload_metadata(struct ofpbuf *ofpacts, const struct 
match *md)
 }
 
 static void
-pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
- struct ofpbuf *userdata)
+pinctrl_handle_nd(const struct flow *ip_flow, const struct match *md,
+  struct ofpbuf *userdata)
 {
-/* This action only works for IPv6 ND Neighbor Solicitation packets,
- * and the switch should only send us such packets this way, but check
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+/* This action only works for IPv6 ND packets, and only Neighbor
+ * Solicitation and Router Solicitation packets are supported.
+ * The switch should only send us ND packets this way, but check
  * here just to be sure. */
 if (!is_nd(ip_flow, NULL)
-|| ip_flow->tp_src != htons(ND_NEIGHBOR_SOLICIT)) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-VLOG_WARN_RL(&rl, "NA action on non-ND-NS packet");
+|| (ip_flow->tp_src != htons(ND_NEIGHBOR_SOLICIT)
+&& ip_flow->tp_src != htons(ND_ROUTER_SOLICIT))) {
+VLOG_WARN_RL(&rl, "ND action on invalid or unsupported packet");
 return;
 }
 
 enum ofp_version version = rconn_get_version(swconn);
 enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
 
-uint64_t packet_stub[128 / 8];
+bool as_nd_na = ip_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT);
+uint64_t packet_stub[256 / 8];
 struct dp_packet packet;
 dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
 
-/* xxx These flags are not exactly correct.  Look at section 7.2.4
- * xxx of RFC 4861.  For example, we need to set ND_RSO_ROUTER for
- * xxx router's interfaces and ND_RSO_SOLICITED only if it was
- * xxx requested. */
-compose_nd_na(&packet, ip_flow->dl_dst, ip_flow->dl_src,
-  &ip_flow->nd_target, &ip_flow->ipv6_src,
-  htonl(ND_RSO_SOLICITED | ND_RSO_OVERRIDE));
+if (as_nd_na) {
+/* xxx These flags are not exactly correct.  Look at section 7.2.4
+ * xxx of RFC 4861.  For example, we need to set ND_RSO_ROUTER for
+ * xxx router's interfaces and ND_RSO_SOLICITED only if it was
+ 

[ovs-dev] [PATCH v1, 5/6] ovn-controller: add 'put_nd_ra' and 'put_nd_opt_*' actions support

[Zong Kai LI]

From: Zongkai LI 

This patch adds specific inner actions 'put_nd_ra', 'put_nd_opt_prefix',
'put_nd_opt_mtu', 'put_nd_opt_sll' for action 'nd_ra'.

Action put_nd_ra will set cur_hop_limit(8-bit unsigned integer),
"Managed address configuration" and "Other configuration" flags(8-bit
unsigned integer, with 6-bit 0 in low endian), router lifetime(16-bit
unsigned integer), reachable time(32-bit unsigned integer), retrans timer
(32-bit unsigned integer) in RA packet.
e.g. put_nd_ra(64,0,10800,0,0);

Action put_nd_ra_opt_sll will append Source Link-layer Address Option for RA
message with inner ethernet address parameter.
e.g. put_nd_ra_opt_sll(12:34:56:78:9a:bc);

Action put_nd_ra_opt_mtu will append MTU Option for RA message with inner
integer value(32-bit unsigned integer).
e.g. put_nd_ra_opt_mtu(1450);

Action put_nd_ra_opt_prefix will append Prefix Information Option for RA
message with inner parameters. The inner parameters will set prefix length
(8-bit unsigned integer), on-link flag and autonomous address-configuration
flag(8-bit unsigned integer, with 6-bit 0 in low endian), valid lifetime
(32-bit unsigned integer), preferred lifetime(32-bit unsigned integer),
prefix(128-bit IPv6 address prefix) in Prefix Information Option.
e.g. put_nd_ra_opt_prefix(64,192,10800,10800,fdad:a0f9:a012::);
---
 include/ovn/actions.h |  79 ++-
 ovn/controller/pinctrl.c  | 160 +-
 ovn/lib/actions.c | 243 +-
 ovn/ovn-sb.xml|  39 
 ovn/utilities/ovn-trace.c |   4 +
 tests/ovn.at  |  16 ++-
 6 files changed, 534 insertions(+), 7 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index c1dda2f..fb30821 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -68,7 +68,11 @@ struct simap;
 OVNACT(GET_ND,ovnact_get_mac_bind)  \
 OVNACT(PUT_ND,ovnact_put_mac_bind)  \
 OVNACT(PUT_DHCPV4_OPTS, ovnact_put_dhcp_opts)   \
-OVNACT(PUT_DHCPV6_OPTS, ovnact_put_dhcp_opts)
+OVNACT(PUT_DHCPV6_OPTS, ovnact_put_dhcp_opts)   \
+OVNACT(PUT_ND_RA,   ovnact_put_nd_ra)   \
+OVNACT(PUT_ND_OPT_SLL,  ovnact_put_nd_opt_sll)  \
+OVNACT(PUT_ND_OPT_MTU,  ovnact_put_nd_opt_mtu)  \
+OVNACT(PUT_ND_OPT_PREFIX, ovnact_put_nd_opt_prefix)
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -228,6 +232,38 @@ struct ovnact_put_dhcp_opts {
 size_t n_options;
 };
 
+/* OVNACT_PUt_ND_RA. */
+struct ovnact_put_nd_ra {
+struct ovnact ovnact;
+uint8_t  cur_hop_limit;
+uint8_t  mo_flags;
+uint16_t router_lifetime;
+uint32_t reachable_time;
+uint32_t retrans_timer;
+};
+
+/* OVNACT_PUT_ND_OPT_SLL. */
+struct ovnact_put_nd_opt_sll {
+struct ovnact ovnact;
+struct eth_addr mac;
+};
+
+/* OVNACT_PUT_ND_OPT_MTU. */
+struct ovnact_put_nd_opt_mtu {
+struct ovnact ovnact;
+uint32_t mtu;
+};
+
+/* OVNACT_PUT_ND_OPT_PREFIX. */
+struct ovnact_put_nd_opt_prefix {
+struct ovnact ovnact;
+uint8_t prefix_len;
+uint8_t la_flags;
+uint32_t valid_lifetime;
+uint32_t preferred_lifetime;
+struct in6_addr prefix;
+};
+
 /* Internal use by the helpers below. */
 void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
 void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
@@ -360,6 +396,44 @@ enum action_opcode {
  * The actions, in OpenFlow 1.3 format, follow the action_header.
  */
 ACTION_OPCODE_ND_RA,
+
+/* "put_nd_ra(c, mo, router_lt, reach_t, retrans_t)".
+ *
+ * Specific inner action for ACTION_OPCODE_ND_RA. Arguments in this format:
+ *   - c: 8-bit unsigned integer for current hop limit.
+ *   - mo: 8-bit unsigned integer for Managed address configuration flag
+ * and Other configuration flag. (including 6-bit reserved 0)
+ *   - router_lt: 16-bit unsigned integer for router lifetime.
+ *   - reach_t: 32-bit unisgned integer for reachable time.
+ *   - retrans_t: 32-bit unsigned integer for retrans timer.
+ */
+ACTION_OPCODE_PUT_ND_RA,
+
+/* "put_nd_opt_sll(mac)".
+ *
+ * Specific inner action for ACTION_OPCODE_ND_RA. Arguments in this format:
+ *   - mac: 48-bit ethernet address.
+ */
+ACTION_OPCODE_PUT_ND_OPT_SLL,
+
+/* "put_nd_opt_mtu(mtu)".
+ *
+ * Specific inner action for ACTION_OPCODE_ND_RA. Arguments in this format:
+ *   - mtu: 32-bit unsigned integer MTU.
+ */
+ACTION_OPCODE_PUT_ND_OPT_MTU,
+
+/* "put_nd_opt_prefix(plen, la, valid_lt, preferred_lt, prefix)".
+ *
+ * Specific inner action for ACTION_OPCODE_ND_RA. Arguments in this format:
+ *   - plen: 8-bit unsigned integer for prefix length.
+ *   - la: 8-bit unsigned integer for on-link flag and autonomous
+ * address-configuration flag. (including 6-bit reserved 0)
+ *   - valid_lt: 32-bit unsigned integer for valid

[ovs-dev] [PATCH v1] ovn-northd: allow DHCPv6 respond multiple IA Address Options

[Zong Kai LI]

From: Zongkai LI 

This patch tries to allow OVN native DHCPv6 responder to respond multiple
IA Address Options in a DHCPv6 reply message, this will help a lswitch port
to configure all IPv6 addresses belongs to it in DHCPv6 processing.

This fixes issue lswitch port can only get one IPv6 address, when it has
addresses looks like:
addresses : ["fa:16:3e:66:31:ac 10.0.0.6 fd55:cb39:f835:0:f816:3eff:fe66:31ac
  fdad:1234:5678:0:f816:3eff:fe66:31ac"]
and dhcpv6_options it refers has cidr "fd55:cb39:f835::/64". The lswitch port
will get only IPv6 address "fd55:cb39:f835:0:f816:3eff:fe66:31ac" in DHCPv6
processing routine.

Signed-off-by: Zongkai LI 
---
 ovn/northd/ovn-northd.c | 79 +++--
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 19f3fb1..d8ca91e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1852,7 +1852,7 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
 }
 
 static bool
-build_dhcpv6_action(struct ovn_port *op, struct in6_addr *offer_ip,
+build_dhcpv6_action(struct ovn_port *op, struct lport_addresses *offer_ips,
 struct ds *options_action, struct ds *response_action)
 {
 if (!op->nbsp->dhcpv6_options) {
@@ -1868,11 +1868,20 @@ build_dhcpv6_action(struct ovn_port *op, struct 
in6_addr *offer_ip,
 free(error);
 return false;
 }
-struct in6_addr ip6_mask = ipv6_addr_bitxor(offer_ip, &host_ip);
-ip6_mask = ipv6_addr_bitand(&ip6_mask, &mask);
-if (!ipv6_mask_is_any(&ip6_mask)) {
-/* offer_ip doesn't belongs to the cidr defined in lport's DHCPv6
- * options.*/
+
+bool dhcpv6_option_match = false;
+for (size_t i = 0; i != offer_ips->n_ipv6_addrs; i++) {
+struct in6_addr ip6_mask = ipv6_addr_bitxor(
+&offer_ips->ipv6_addrs[i].addr, &host_ip);
+ip6_mask = ipv6_addr_bitand(&ip6_mask, &mask);
+if (ipv6_mask_is_any(&ip6_mask)) {
+dhcpv6_option_match = true;
+break;
+}
+}
+if (!dhcpv6_option_match) {
+/* None of lswitch port ipv6 addresses belongs to the cidr defined in
+ * lswitch port's DHCPv6 options.*/
 return false;
 }
 
@@ -1896,11 +1905,12 @@ build_dhcpv6_action(struct ovn_port *op, struct 
in6_addr *offer_ip,
 ipv6_string_mapped(server_ip, &lla);
 
 char ia_addr[INET6_ADDRSTRLEN + 1];
-ipv6_string_mapped(ia_addr, offer_ip);
+ds_put_cstr(options_action, REGBIT_DHCP_OPTS_RESULT" = put_dhcpv6_opts(");
+for (size_t i = 0; i != offer_ips->n_ipv6_addrs; i++) {
+ipv6_string_mapped(ia_addr, &offer_ips->ipv6_addrs[i].addr);
+ds_put_format(options_action, "ia_addr = %s, ", ia_addr);
+}
 
-ds_put_format(options_action,
-  REGBIT_DHCP_OPTS_RESULT" = put_dhcpv6_opts(ia_addr = %s, ",
-  ia_addr);
 struct smap_node *node;
 SMAP_FOR_EACH (node, &op->nbsp->dhcpv6_options->options) {
 ds_put_format(options_action, "%s = %s, ", node->key, node->value);
@@ -2700,33 +2710,30 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 }
 }
 
-for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) {
-struct ds options_action = DS_EMPTY_INITIALIZER;
-struct ds response_action = DS_EMPTY_INITIALIZER;
-if (build_dhcpv6_action(
-op, &op->lsp_addrs[i].ipv6_addrs[j].addr,
-&options_action, &response_action)) {
-struct ds match = DS_EMPTY_INITIALIZER;
-ds_put_format(
-&match, "inport == %s && eth.src == %s"
-" && ip6.dst == ff02::1:2 && udp.src == 546 &&"
-" udp.dst == 547", op->json_key,
-op->lsp_addrs[i].ea_s);
-
-ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_OPTIONS, 
100,
-  ds_cstr(&match), ds_cstr(&options_action));
-
-/* If REGBIT_DHCP_OPTS_RESULT is set to 1, it means the
- * put_dhcpv6_opts action is successful */
-ds_put_cstr(&match, " && "REGBIT_DHCP_OPTS_RESULT);
-ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_RESPONSE, 
100,
-  ds_cstr(&match), ds_cstr(&response_action));
-ds_destroy(&match);
-ds_destroy(&options_action);
-ds_destroy(&response_action);
-break;
-}
+struct ds options_action = DS_EMPTY_INITIALIZER;
+struct ds response_action = DS_EMPTY_INITIALIZER;
+if (build_dhcpv6_action(
+op, &op->lsp_addrs[i],
+&options_action, &response_action)) {
+struct ds 

[ovs-dev] [PATCH RFC v1] ovn-northd: add default_dhcpvx_options for Logical_Switch

[Zong Kai LI]

From: Zongkai LI 

This patch adds default_dhcpv4_options and default_dhcpv6_options columns for
Logical_Switch, which should help CMS not to calculate and set dhcpv4_options
and dhcpv6_options columns for lswitch ports on lswitchs one by one, when
most of lswitch ports on the same lswitch are using the DHCPv4_Options and
DHCPv6_Options. Default DHCP(v4 and v6) Options should benefit in case
scalling up and DB synchronization between CMS and OVN NB.
---
 ovn/northd/ovn-northd.c | 87 +
 ovn/ovn-nb.ovsschema| 18 --
 ovn/ovn-nb.xml  | 26 +++
 tests/ovn.at|  6 ++--
 4 files changed, 95 insertions(+), 42 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 19f3fb1..aa17676 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1784,14 +1784,27 @@ static bool
 build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
 struct ds *options_action, struct ds *response_action)
 {
+char *cidr = NULL;
+struct smap dhcpv4_options = SMAP_INITIALIZER(&dhcpv4_options);
 if (!op->nbsp->dhcpv4_options) {
-/* CMS has disabled native DHCPv4 for this lport. */
-return false;
+if ((!op->nbsp->use_default_dhcpv4 || *op->nbsp->use_default_dhcpv4)
+&& op->od->nbs->default_dhcpv4_options) {
+/* Use lswitch default DHCPv4 options for this lport. */
+cidr = op->od->nbs->default_dhcpv4_options->cidr;
+smap_clone(&dhcpv4_options,
+   &op->od->nbs->default_dhcpv4_options->options);
+} else {
+/* CMS has disabled native DHCPv4 for this lport. */
+smap_destroy(&dhcpv4_options);
+return false;
+}
+} else {
+cidr = op->nbsp->dhcpv4_options->cidr;
+smap_clone(&dhcpv4_options, &op->nbsp->dhcpv4_options->options);
 }
 
 ovs_be32 host_ip, mask;
-char *error = ip_parse_masked(op->nbsp->dhcpv4_options->cidr, &host_ip,
-  &mask);
+char *error = ip_parse_masked(cidr, &host_ip, &mask);
 if (error || ((offer_ip ^ host_ip) & mask)) {
/* Either
 *  - cidr defined is invalid or
@@ -1802,14 +1815,10 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
 return false;
 }
 
-const char *server_ip = smap_get(
-&op->nbsp->dhcpv4_options->options, "server_id");
-const char *server_mac = smap_get(
-&op->nbsp->dhcpv4_options->options, "server_mac");
-const char *lease_time = smap_get(
-&op->nbsp->dhcpv4_options->options, "lease_time");
-const char *router = smap_get(
-&op->nbsp->dhcpv4_options->options, "router");
+const char *server_ip = smap_get(&dhcpv4_options, "server_id");
+const char *server_mac = smap_get(&dhcpv4_options, "server_mac");
+const char *lease_time = smap_get(&dhcpv4_options, "lease_time");
+const char *router = smap_get(&dhcpv4_options, "router");
 
 if (!(server_ip && server_mac && lease_time && router)) {
 /* "server_id", "server_mac", "lease_time" and "router" should be
@@ -1820,8 +1829,8 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
 return false;
 }
 
-struct smap dhcpv4_options = SMAP_INITIALIZER(&dhcpv4_options);
-smap_clone(&dhcpv4_options, &op->nbsp->dhcpv4_options->options);
+char server_mac_str[ETH_ADDR_STRLEN + 1];
+strcpy(server_mac_str, server_mac);
 
 /* server_mac is not DHCPv4 option, delete it from the smap. */
 smap_remove(&dhcpv4_options, "server_mac");
@@ -1845,7 +1854,7 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
   "ip4.dst = "IP_FMT"; ip4.src = %s; udp.src = 67; "
   "udp.dst = 68; outport = inport; flags.loopback = 1; "
   "output;",
-  server_mac, IP_ARGS(offer_ip), server_ip);
+  server_mac_str, IP_ARGS(offer_ip), server_ip);
 
 smap_destroy(&dhcpv4_options);
 return true;
@@ -1855,15 +1864,28 @@ static bool
 build_dhcpv6_action(struct ovn_port *op, struct in6_addr *offer_ip,
 struct ds *options_action, struct ds *response_action)
 {
+char *cidr = NULL;
+struct smap dhcpv6_options = SMAP_INITIALIZER(&dhcpv6_options);
 if (!op->nbsp->dhcpv6_options) {
-/* CMS has disabled native DHCPv6 for this lport. */
-return false;
+if ((!op->nbsp->use_default_dhcpv6 || *op->nbsp->use_default_dhcpv6)
+&& op->od->nbs->default_dhcpv6_options) {
+/* Use lswitch default DHCPv6 options for this lport. */
+cidr = op->od->nbs->default_dhcpv6_options->cidr;
+smap_clone(&dhcpv6_options,
+   &op->od->nbs->default_dhcpv6_options->options);
+} else {
+/* CMS has disabled native DHCPv6 for this lport. */
+smap_destroy(&dhcpv6_options)

[ovs-dev] please help review patch "allow DHCPv6 respond multiple IA Address Options"

[Zong Kai Li]

Hi, Numan and Justin.
I wish you can help review patch "allow DHCPv6 respond multiple IA
Address Options".
http://patchwork.ozlabs.org/patch/663493/

For current DHCPv6 implementation, it will respond one IPv6 address
for a lswitch port in a DHCPv6 Reply message. The "one IPv6 address",
I mean, one in the same address string, like "MAC IPv4-Address
IPv6-Address-1 IPv6-Address-2 ...".

I don't think it's a good solution, since we can set multiple IA
Address Options in a DHCPv6 Reply message, and by that, we can support
multiple IPv6 addresses in the same address string for VIF port to
configure. It will give us a lflow with action=(reg0[3] =
put_dhcpv6_opts(ia_addr = OFFER_IP1, ia_addr = OFFER_IP2, ... ,
server_id = MAC, OTHER_OPTS...); next;). This can work.

In the patch mentioned above, I kept the CIDR checking, to make sure
at least one IPv6 address owned by lswitch port can reach DNS server
list in put_dhcpv6_opts action.

Thanks and have a nice day! :)
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] please help review patch "allow DHCPv6 respond multiple IA Address Options"

[Zong Kai Li]

On Tue, Aug 30, 2016 at 8:28 PM, Numan Siddique  wrote:
>
>
> On Tue, Aug 30, 2016 at 9:40 AM, Ben Pfaff  wrote:
>>
>> The patch will get reviewed, there's no need to ask again.
>>
>> On Tue, Aug 30, 2016 at 11:09:23AM +0800, Zong Kai Li wrote:
>> > Hi, Numan and Justin.
>> > I wish you can help review patch "allow DHCPv6 respond multiple IA
>> > Address Options".
>> > http://patchwork.ozlabs.org/patch/663493/
>> >
>> > For current DHCPv6 implementation, it will respond one IPv6 address
>> > for a lswitch port in a DHCPv6 Reply message. The "one IPv6 address",
>> > I mean, one in the same address string, like "MAC IPv4-Address
>> > IPv6-Address-1 IPv6-Address-2 ...".
>> >
>> > I don't think it's a good solution, since we can set multiple IA
>> > Address Options in a DHCPv6 Reply message, and by that, we can support
>> > multiple IPv6 addresses in the same address string for VIF port to
>> > configure. It will give us a lflow with action=(reg0[3] =
>> > put_dhcpv6_opts(ia_addr = OFFER_IP1, ia_addr = OFFER_IP2, ... ,
>> > server_id = MAC, OTHER_OPTS...); next;). This can work.
>> >
>> > In the patch mentioned above, I kept the CIDR checking, to make sure
>> > at least one IPv6 address owned by lswitch port can reach DNS server
>> > list in put_dhcpv6_opts action.
>> >
>
>
> I haven't reviewed the patch. But I have few comments.
> A logical port can have multiple IPv6 addresses, out of which some can be
> from a stateless subnet and some from stateful subnet. This doesn't handle
> it right ? Correct me if I am wrong here.
>
> May be CMS can store the multiple IA_ADDR options in the
> DHCP_Options.options so that ovn-northd doesn't need to bother about it.
>
> Thanks
> Numan
>

Yes, you're right, this changing doesn't handle that stateful and
stateless addresses case. I didn't have a confident solution, so I
made this changing aims to let all addresses belong to VM/VIF port can
be get at first.

I think store the multiple IA_ADDR options in the DHCP_Options.options
is a good solution for ovn-northd, but not to CMS, it will lead
DHCP_Options.options has higher maintain workload.

Another idea is, allow Logical_Switch_Port.dhcpv6_options can have
multiple refers than just one. Using this way to tell IPv6 addresses
match DHCP_Options.cidr are stateful. It may be also possible to
combine them into one, since dns server and domain in DHCP_Options
referred by Logical_Switch_Port could be the same.

That's all idea I get for now.
Thanks and have a nice day. :)
Zongkai LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] please help review patch "allow DHCPv6 respond multiple IA Address Options"

[Zong Kai Li]

On Thu, Sep 1, 2016 at 6:05 PM, Numan Siddique  wrote:
>
>
> On Wed, Aug 31, 2016 at 8:44 AM, Zong Kai Li  wrote:
>>
>> On Tue, Aug 30, 2016 at 8:28 PM, Numan Siddique 
>> wrote:
>> >
>> >
>> > On Tue, Aug 30, 2016 at 9:40 AM, Ben Pfaff  wrote:
>> >>
>> >> The patch will get reviewed, there's no need to ask again.
>> >>
>> >> On Tue, Aug 30, 2016 at 11:09:23AM +0800, Zong Kai Li wrote:
>> >> > Hi, Numan and Justin.
>> >> > I wish you can help review patch "allow DHCPv6 respond multiple IA
>> >> > Address Options".
>> >> > http://patchwork.ozlabs.org/patch/663493/
>> >> >
>> >> > For current DHCPv6 implementation, it will respond one IPv6 address
>> >> > for a lswitch port in a DHCPv6 Reply message. The "one IPv6 address",
>> >> > I mean, one in the same address string, like "MAC IPv4-Address
>> >> > IPv6-Address-1 IPv6-Address-2 ...".
>> >> >
>> >> > I don't think it's a good solution, since we can set multiple IA
>> >> > Address Options in a DHCPv6 Reply message, and by that, we can
>> >> > support
>> >> > multiple IPv6 addresses in the same address string for VIF port to
>> >> > configure. It will give us a lflow with action=(reg0[3] =
>> >> > put_dhcpv6_opts(ia_addr = OFFER_IP1, ia_addr = OFFER_IP2, ... ,
>> >> > server_id = MAC, OTHER_OPTS...); next;). This can work.
>> >> >
>> >> > In the patch mentioned above, I kept the CIDR checking, to make sure
>> >> > at least one IPv6 address owned by lswitch port can reach DNS server
>> >> > list in put_dhcpv6_opts action.
>> >> >
>> >
>> >
>> > I haven't reviewed the patch. But I have few comments.
>> > A logical port can have multiple IPv6 addresses, out of which some can
>> > be
>> > from a stateless subnet and some from stateful subnet. This doesn't
>> > handle
>> > it right ? Correct me if I am wrong here.
>> >
>> > May be CMS can store the multiple IA_ADDR options in the
>> > DHCP_Options.options so that ovn-northd doesn't need to bother about it.
>> >
>> > Thanks
>> > Numan
>> >
>>
>> Yes, you're right, this changing doesn't handle that stateful and
>> stateless addresses case. I didn't have a confident solution, so I
>> made this changing aims to let all addresses belong to VM/VIF port can
>> be get at first.
>>
>> I think store the multiple IA_ADDR options in the DHCP_Options.options
>> is a good solution for ovn-northd, but not to CMS, it will lead
>> DHCP_Options.options has higher maintain workload.
>
>
> I would personally prefer this approach. As it is cleaner. I think it should
> be fine for CMS to take care of such things. Else it could become openstack
> specific. My 2 cents.
>

Sounds fair enough, ok, I will try to check multiple IA_ADDR options
in the DHCP_Options.options.
Thanks,
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] please help review serial patches supporting Router Solicitation responder

[Zong Kai Li]

hi, guys, please help review serial patches supporting Router
Solicitation responder.

http://patchwork.ozlabs.org/patch/665380/ [ovs-dev,v2,1/8] packets:
add compose_nd_ra to compose Router Advertisement packet
http://patchwork.ozlabs.org/patch/665381/ [ovs-dev,v2,2/8] lib: rename
ovs_nd_opt to ovs_nd_lla_opt
http://patchwork.ozlabs.org/patch/665382/ [ovs-dev,v2,3/8] ovn: extend
expr symbols for ND
http://patchwork.ozlabs.org/patch/665383/ [ovs-dev,v2,4/8] flow:
update is_nd for all ND msg types
http://patchwork.ozlabs.org/patch/665384/ [ovs-dev,v2,5/8]
ovn-controller: add 'nd_ra' action
http://patchwork.ozlabs.org/patch/665385/ [ovs-dev,v2,6/8]
ovn-controller: add 'put_nd_ra' and 'put_nd_opt_*' actions support
http://patchwork.ozlabs.org/patch/665386/ [ovs-dev,v2,7/8] ovn-northd:
add Router Advertisement packet forward flow
http://patchwork.ozlabs.org/patch/665387/ [ovs-dev,v2,8/8] ovn-northd:
add IPv6 RS responder support

thanks.
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 3/8] ovn: extend expr symbols for ND

[Zong Kai Li]

On Thu, Sep 8, 2016 at 9:13 AM, Justin Pettit  wrote:
>
>> On Sep 2, 2016, at 10:09 PM, Zongkai LI  wrote:
>>
>> This patch updates symbols in logical-fields for ND, such as "nd",
>> "nd.target", "nd.sll", "nd.tll", to describe more clear about "icmp6.type"
>> predicate.
>> It adds new symbols:
>> - "nd_rs" stands for Router Solicitation message,
>> - "nd_ra" stands for Router Advertisement message,
>> - "nd_rd" stands for Redirect message,
>>
>> v1 -> v2
>> rebased.
>> ---
>
> Add "signed-off-by:" and move version information.

Thanks, I just know all my patches have problem with "signed-off-by:"
and version information.

>
>> +expr_symtab_add_predicate(symtab, "nd_rd",
>> +  "icmp6.type == 137 && icmp6.code == 0 && ip.ttl == 255");
>
> Are you using redirects anywhere?  They require a different message type, 
> which I don't think has been introduced.  I'd just drop it if you're not 
> introducing full support.
>

Not in this patch, but I was planning it. You're right, I shouldn't
get it in for now.

> --Justin
>
>

Thanks for your review. :)
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 4/8] flow: update is_nd for all ND msg types

[Zong Kai Li]

On Thu, Sep 8, 2016 at 9:39 AM, Justin Pettit  wrote:
>
>> On Sep 2, 2016, at 10:09 PM, Zongkai LI  wrote:
>>
>> This patch updates method is_nd to let type ND_ROUTER_SOLICIT,
>> ND_ROUTER_ADVERT, ND_REDIRECT can be recoginzed.
>> And introduces method is_nd_neighbor for inherit current is_nd behavior.
>>
>> v1 -> v2
>> rebased, introduces method is_nd_neighbor.
>
> I won't mention it for the rest of this series, but please add 
> "signed-off-by:" and move version information to comment.

Sure, I know it now.

>
>> ---
>> lib/flow.h   | 11 +--
>> lib/nx-match.c   |  2 +-
>> lib/odp-util.c   |  4 ++--
>> ovn/controller/pinctrl.c | 10 ++
>> 4 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/flow.h b/lib/flow.h
>> index 5b83695..64200a5 100644
>> --- a/lib/flow.h
>> +++ b/lib/flow.h
>> @@ -936,12 +936,19 @@ static inline bool is_nd(const struct flow *flow,
>> if (wc) {
>> memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
>> }
>> -return (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
>> -flow->tp_src == htons(ND_NEIGHBOR_ADVERT));
>> +return (flow->tp_src >= htons(ND_ROUTER_SOLICIT)
>> +&& flow->tp_src <= htons(ND_REDIRECT));
>
> You seem to be doing math on network byte order.  It probably doesn't affect 
> the outcome since they're 8-bit values in a 16-bit field, but it doesn't look 
> right.
>
> Once again, I don't think we can do anything meaningful with redirect, so it 
> should probably be dropped.
>

Sure, I will revert this, and for redirect, since this patch is not
for it, I will drop it.

>> }
>> return false;
>> }
>>
>> +static inline bool is_nd_neighbor(const struct flow *flow,
>> +  struct flow_wildcards *wc)
>> +{
>> +return is_nd(flow, wc) && (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT)
>> +   || flow->tp_src == 
>> htons(ND_NEIGHBOR_ADVERT));
>> +}
>
> The "is_nd" and "is_nd_neighbor" commands have similar names.  I think it 
> would be worth adding comments to both to help people distinguish the 
> difference.
>

Good idea.

>> +
>> static inline bool is_igmp(const struct flow *flow, struct flow_wildcards 
>> *wc)
>> {
>> if (flow->dl_type == htons(ETH_TYPE_IP)) {
>> diff --git a/lib/nx-match.c b/lib/nx-match.c
>> index b03ccf2..806da3e 100644
>> --- a/lib/nx-match.c
>> +++ b/lib/nx-match.c
>> @@ -879,7 +879,7 @@ nxm_put_ip(struct ofpbuf *b, const struct match *match, 
>> enum ofp_version oxm)
>> nxm_put_8(b, MFF_ICMPV6_CODE, oxm,
>>   ntohs(flow->tp_dst));
>> }
>> -if (is_nd(flow, NULL)) {
>> +if (is_nd_neighbor(flow, NULL)) {
>> nxm_put_ipv6(b, MFF_ND_TARGET, oxm,
>>  &flow->nd_target, &match->wc.masks.nd_target);
>> if (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT)) {
>> diff --git a/lib/odp-util.c b/lib/odp-util.c
>> index 6d29b67..0d7bede 100644
>> --- a/lib/odp-util.c
>> +++ b/lib/odp-util.c
>> @@ -4428,7 +4428,7 @@ odp_flow_key_from_flow__(const struct 
>> odp_flow_key_parms *parms,
>> icmpv6_key->icmpv6_type = ntohs(data->tp_src);
>> icmpv6_key->icmpv6_code = ntohs(data->tp_dst);
>>
>> -if (is_nd(flow, NULL)
>> +if (is_nd_neighbor(flow, NULL)
>> /* Even though 'tp_src' and 'tp_dst' are 16 bits wide, ICMP
>>  * type and code are 8 bits wide.  Therefore, an exact match
>>  * looks like htons(0xff), not htons(0x).  See
>> @@ -4963,7 +4963,7 @@ parse_l2_5_onward(const struct nlattr 
>> *attrs[OVS_KEY_ATTR_MAX + 1],
>> flow->tp_src = htons(icmpv6_key->icmpv6_type);
>> flow->tp_dst = htons(icmpv6_key->icmpv6_code);
>> expected_bit = OVS_KEY_ATTR_ICMPV6;
>> -if (is_nd(src_flow, NULL)) {
>> +if (is_nd_neighbor(src_flow, NULL)) {
>> if (!is_mask) {
>> expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
>> }
>
> The previous set of changes introduced support for generating and matching 
> router advertisements, but I don't think you've done any of the work to make 
> OVS parse the messages.  Don't we need that?  These changes just seem to 
> side-step that by only continuing to parse NS and NA messages.
>
> --Justin
>
>

To be honest, I didn't bring this changing at the beginning.
I wanted to put handling for RS in pinctrl_handle_nd_na (and rename
pinctrl_handle_nd_na to pinctrl_handle_nd) , while
pinctrl_handle_nd_na uses is_nd, so I changed is_nd.
For other places calling is_nd, I'm not sure whether should I update
them for RS/RA, so I added is_nd_neighbor to inherit original is_nd in
behavior.
After I completed my patches, I found things work as I expected, VIF
can get RA packet after sending RS packet, so I didn't investigate
whether 

Re: [ovs-dev] [PATCH RFC v3] ovn-northd: add default_dhcpvx_options for Logical_Switch

[Zong Kai Li]

On Thu, Sep 15, 2016 at 7:48 AM, Ben Pfaff  wrote:
> On Thu, Sep 01, 2016 at 09:56:30AM +, Zongkai LI wrote:
>> This patch adds default_dhcpv4_options and default_dhcpv6_options columns for
>> Logical_Switch, which should help CMS not to calculate and set dhcpv4_options
>> and dhcpv6_options columns for lswitch ports on lswitchs one by one, when
>> most of lswitch ports on the same lswitch are using the DHCPv4_Options and
>> DHCPv6_Options. Default DHCP(v4 and v6) Options should benefit in case
>> scalling up and DB synchronization between CMS and OVN NB.
>>
>> v1 -> v2
>> add ACL lflows support for lswitch ports using default_dhcpvx_options from
>> lswitch.
>>
>> v2 -> v3
>> update ovn dhcpv4 and dhcpv6 tests for lswitch ports using specific dhcp
>> options than defualt ones in lswitch.
>
> The CMS can point the multiple dhcpvx_options column in as many rows as
> it likes to the same row in the DHCP_Options table, so I don't know why
> this offers an advantage.

Sorry for my poor english, I couldn't get your points well.
"The CMS can point the multiple dhcpvx_options column in as many rows
as it likes to the same row in the DHCP_Options table", do you mean
the CMS can point multiple options in a DHCP_Options row option column
? This patch won't help CMS on that.

The deep reason I submitted this patch is, we locate a DHCP_Options
row for Logical_Switch_Port.dhcpvx_options to refer by scanning
DHCP_Options table to find a row matching some rules we defined. Like
"subnet_id" and "port_id" defined in external_ids column, the
OpenStack integrator, networking-ovn uses them to locate a certain
DHCP_Options row for a port to set its dhcpvx_options.

It works, but not necessary for case [1] most ports from the same
logical-switch using the same dhcpvx_options, since we will locate the
same uuid duplicately.
It's possible for CMS to use some cache to store mappings between
networks/subnets and  DHCP_Options rows uuids, but I don't think this
is a CMS-friendly approach.

By adding default dhcpvx options in Logical_Switch, CMS will skip the
scanning step for ports using the default dhcp options cases, such as
creating a new port, logical switch port lost with/without dhcp
options lost(or become stale) when DB sync, and logical switch port
exist but dhcp options lost when DB sync.

This is a option way let CMS to handle dhcp options, it won't change
current approach how DHCP_Options and
Logical_Switch_Port.dhcpvx_options works. It won't benefit too much
scenarios, but at least for case [1] it would.

Thanks,
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2, 1/1] ovn: Extend Address_Set to Set to support define port name sets

[Zong Kai Li]

On Wed, Sep 28, 2016 at 11:47 PM, Ben Pfaff  wrote:
> On Wed, Sep 28, 2016 at 03:17:04AM +, Zongkai LI wrote:
>> From: Zongkai LI 
>>
>> This patch aims to extend table Address_Set to Set, make it more common to
>> accept variable set, not only address. And by that, we can skinny down ACLs,
>> if we use Set to defines port name sets for ACL to use, since lots of ACL
>> entries are similar but only "inport" and "outport" fields are different.
>
> This will break systems on upgrade by deleting a table.  Since we've now
> declared OVN "non-experimental", meaning that we're doing our best to
> commit to smooth upgrades, it's probably better to retain the table and
> column names, even if it means that the names are now imperfect
> descriptions of their function.

OK, I think I get the prerequisite you mean to bring changing in this patch.
I will try it, without modifying the table, and also update doc to
tell user this usage.

Thanks,
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev