Re: [ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.

2019-11-12 Thread Dumitru Ceara
On Tue, Nov 12, 2019 at 6:02 PM Han Zhou  wrote:
>
>
>
> On Tue, Nov 12, 2019 at 2:28 AM Dumitru Ceara  wrote:
> >
> > Function get_router_load_balancer_ips() was incorrectly returning a
> > single address_family even though the IP set could contain a mix of
> > IPv4 and IPv6 addresses.
> >
> > Signed-off-by: Dumitru Ceara 
> > ---
> >  northd/ovn-northd.c |  126 
> > +--
> >  1 file changed, 72 insertions(+), 54 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 2f0f501..32f3200 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -2184,7 +2184,7 @@ ip_address_and_port_from_lb_key(const char *key, char 
> > **ip_address,
> >
> >  static void
> >  get_router_load_balancer_ips(const struct ovn_datapath *od,
> > - struct sset *all_ips, int *addr_family)
> > + struct sset *all_ips_v4, struct sset 
> > *all_ips_v6)
> >  {
> >  if (!od->nbr) {
> >  return;
> > @@ -2199,13 +2199,21 @@ get_router_load_balancer_ips(const struct 
> > ovn_datapath *od,
> >  /* node->key contains IP:port or just IP. */
> >  char *ip_address = NULL;
> >  uint16_t port;
> > +int addr_family;
> >
> >  ip_address_and_port_from_lb_key(node->key, _address, ,
> > -addr_family);
> > +_family);
> >  if (!ip_address) {
> >  continue;
> >  }
> >
> > +struct sset *all_ips;
> > +if (addr_family == AF_INET) {
> > +all_ips = all_ips_v4;
> > +} else {
> > +all_ips = all_ips_v6;
> > +}
> > +
> >  if (!sset_contains(all_ips, ip_address)) {
> >  sset_add(all_ips, ip_address);
> >  }
> > @@ -2299,17 +2307,22 @@ get_nat_addresses(const struct ovn_port *op, size_t 
> > *n)
> >  }
> >  }
> >
> > -/* A set to hold all load-balancer vips. */
> > -struct sset all_ips = SSET_INITIALIZER(_ips);
> > -int addr_family;
> > -get_router_load_balancer_ips(op->od, _ips, _family);
> > +/* Two sets to hold all load-balancer vips. */
> > +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> > +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> > +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> >  const char *ip_address;
> > -SSET_FOR_EACH (ip_address, _ips) {
> > +SSET_FOR_EACH (ip_address, _ips_v4) {
> >  ds_put_format(_addresses, " %s", ip_address);
> >  central_ip_address = true;
> >  }
> > -sset_destroy(_ips);
> > +SSET_FOR_EACH (ip_address, _ips_v6) {
> > +ds_put_format(_addresses, " %s", ip_address);
> > +central_ip_address = true;
> > +}
> > +sset_destroy(_ips_v4);
> > +sset_destroy(_ips_v6);
> >
> >  if (central_ip_address) {
> >  /* Gratuitous ARP for centralized NAT rules on distributed gateway
> > @@ -6911,61 +6924,66 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> > hmap *ports,
> >  }
> >
> >  /* A set to hold all load-balancer vips that need ARP responses. */
> > -struct sset all_ips = SSET_INITIALIZER(_ips);
> > -int addr_family;
> > -get_router_load_balancer_ips(op->od, _ips, _family);
> > +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> > +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> > +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> >  const char *ip_address;
> > -SSET_FOR_EACH(ip_address, _ips) {
> > +SSET_FOR_EACH (ip_address, _ips_v4) {
> >  ds_clear();
> > -if (addr_family == AF_INET) {
> > -ds_put_format(,
> > -  "inport == %s && arp.tpa == %s && arp.op == 
> > 1",
> > -  op->json_key, ip_address);
> > -} else {
> > -ds_put_format(,
> > -  "inport == %s && nd_ns && nd.target == %s",
> > -  op->json_key, ip_address);
> > -}
> > +ds_put_format(,
> > +  "inport == %s && arp.tpa == %s && arp.op == 1",
> > +  op->json_key, ip_address);
> >
> >  ds_clear();
> > -if (addr_family == AF_INET) {
> > -ds_put_format(,
> > -"eth.dst = eth.src; "
> > -"eth.src = %s; "
> > -"arp.op = 2; /* ARP reply */ "
> > -"arp.tha = arp.sha; "
> > -"arp.sha = %s; "
> > -"arp.tpa = arp.spa; "
> > -"arp.spa = %s; "
> > -"outport = %s; "
> > -"flags.loopback = 1; "
> > -"output;",
> > -

Re: [ovs-dev] [PATCH net-next] openvswitch: add TTL decrement action

2019-11-12 Thread Simon Horman
On Tue, Nov 12, 2019 at 04:46:12PM +0100, Matteo Croce wrote:
> On Tue, Nov 12, 2019 at 4:00 PM Simon Horman  
> wrote:
> >
> > On Tue, Nov 12, 2019 at 11:25:18AM +0100, Matteo Croce wrote:
> > > New action to decrement TTL instead of setting it to a fixed value.
> > > This action will decrement the TTL and, in case of expired TTL, send the
> > > packet to userspace via output_userspace() to take care of it.
> > >
> > > Supports both IPv4 and IPv6 via the ttl and hop_limit fields, 
> > > respectively.
> > >
> >
> > Usually OVS achieves this behaviour by matching on the TTL and
> > setting it to the desired value, pre-calculated as TTL -1.
> > With that in mind could you explain the motivation for this
> > change?
> >
> 
> Hi,
> 
> the problem is that OVS creates a flow for each ttl it see. I can let
> vswitchd create 255 flows with like this:
> 
> $ for i in {2..255}; do ping 192.168.0.2 -t $i -c1 -w1 &>/dev/null & done
> $ ovs-dpctl dump-flows |fgrep -c 'set(ipv4(ttl'
> 255

Hi,

so the motivation is to reduce the number of megaflows in the case
where flows otherwise match but the TTL differs. I think this makes sense
and the absence of this feature may date back to designs made before
megaflow support was added - just guessing.

I think this is a reasonable feature but I think it would be good
to explain the motivation in the changelog.

> > > @@ -1174,6 +1174,43 @@ static int execute_check_pkt_len(struct datapath 
> > > *dp, struct sk_buff *skb,
> > >nla_len(actions), last, clone_flow_key);
> > >  }
> > >
> > > +static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key)
> > > +{
> > > + int err;
> > > +
> > > + if (skb->protocol == htons(ETH_P_IPV6)) {
> > > + struct ipv6hdr *nh = ipv6_hdr(skb);
> > > +
> > > + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > > +   sizeof(*nh));
> > > + if (unlikely(err))
> > > + return err;
> > > +
> > > + if (nh->hop_limit <= 1)
> > > + return -EHOSTUNREACH;
> > > +
> > > + key->ip.ttl = --nh->hop_limit;
> > > + } else {
> > > + struct iphdr *nh = ip_hdr(skb);
> > > + u8 old_ttl;
> > > +
> > > + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > > +   sizeof(*nh));
> > > + if (unlikely(err))
> > > + return err;
> > > +
> > > + if (nh->ttl <= 1)
> > > + return -EHOSTUNREACH;
> > > +
> > > + old_ttl = nh->ttl--;
> > > + csum_replace2(>check, htons(old_ttl << 8),
> > > +   htons(nh->ttl << 8));
> > > + key->ip.ttl = nh->ttl;
> > > + }
> >
> > The above may send packets with TTL = 0, is that desired?
> >
> 
> If TTL is 1 or 0, execute_dec_ttl() returns -EHOSTUNREACH, and the
> caller will just send the packet to userspace and then free it.
> I think this is enough, am I missing something?

No, you are not. I was missing something.
I now think this logic is fine.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next v3] net: openvswitch: add hash info to upcall

2019-11-12 Thread Tonghao Zhang
On Wed, Nov 13, 2019 at 12:54 PM Pravin Shelar  wrote:
>
> On Tue, Nov 12, 2019 at 7:09 AM  wrote:
> >
> > From: Tonghao Zhang 
> >
> > When using the kernel datapath, the upcall don't
> > include skb hash info relatived. That will introduce
> > some problem, because the hash of skb is important
> > in kernel stack. For example, VXLAN module uses
> > it to select UDP src port. The tx queue selection
> > may also use the hash in stack.
> >
> > Hash is computed in different ways. Hash is random
> > for a TCP socket, and hash may be computed in hardware,
> > or software stack. Recalculation hash is not easy.
> >
> > Hash of TCP socket is computed:
> >   tcp_v4_connect
> > -> sk_set_txhash (is random)
> >
> >   __tcp_transmit_skb
> > -> skb_set_hash_from_sk
> >
> > There will be one upcall, without information of skb
> > hash, to ovs-vswitchd, for the first packet of a TCP
> > session. The rest packets will be processed in Open vSwitch
> > modules, hash kept. If this tcp session is forward to
> > VXLAN module, then the UDP src port of first tcp packet
> > is different from rest packets.
> >
> > TCP packets may come from the host or dockers, to Open vSwitch.
> > To fix it, we store the hash info to upcall, and restore hash
> > when packets sent back.
> >
> > +---+  +-+
> > |   Docker/VMs  |  | ovs-vswitchd|
> > ++--+  +-++--+
> >  |   ^|
> >  |   ||
> >  |   |  upcallv restore packet hash 
> > (not recalculate)
> >  | +-++--+
> >  |  tap netdev | |   vxlan module
> >  +---> +-->  Open vSwitch ko +-->
> >or internal type| |
> >+-+
> >
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364062.html
> > Signed-off-by: Tonghao Zhang 
> > ---
> > v3:
> > * add enum ovs_pkt_hash_types
> > * avoid duplicate call the skb_get_hash_raw.
> > * explain why we should fix this problem.
> > ---
> >  include/uapi/linux/openvswitch.h |  2 ++
> >  net/openvswitch/datapath.c   | 30 +-
> >  net/openvswitch/datapath.h   | 12 
> >  3 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/openvswitch.h 
> > b/include/uapi/linux/openvswitch.h
> > index 1887a451c388..e65407c1f366 100644
> > --- a/include/uapi/linux/openvswitch.h
> > +++ b/include/uapi/linux/openvswitch.h
> > @@ -170,6 +170,7 @@ enum ovs_packet_cmd {
> >   * output port is actually a tunnel port. Contains the output tunnel key
> >   * extracted from the packet as nested %OVS_TUNNEL_KEY_ATTR_* attributes.
> >   * @OVS_PACKET_ATTR_MRU: Present for an %OVS_PACKET_CMD_ACTION and
> > + * @OVS_PACKET_ATTR_HASH: Packet hash info (e.g. hash, sw_hash and l4_hash 
> > in skb).
> >   * @OVS_PACKET_ATTR_LEN: Packet size before truncation.
> >   * %OVS_PACKET_ATTR_USERSPACE action specify the Maximum received fragment
> >   * size.
> > @@ -190,6 +191,7 @@ enum ovs_packet_attr {
> > OVS_PACKET_ATTR_PROBE,  /* Packet operation is a feature probe,
> >error logging should be suppressed. 
> > */
> > OVS_PACKET_ATTR_MRU,/* Maximum received IP fragment size. */
> > +   OVS_PACKET_ATTR_HASH,   /* Packet hash. */
> > OVS_PACKET_ATTR_LEN,/* Packet size before truncation. */
> > __OVS_PACKET_ATTR_MAX
> >  };
> I agree with Greg, value of existing enums can not be changed in UAPI.
>
> > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> > index 2088619c03f0..b556cf62b77c 100644
> > --- a/net/openvswitch/datapath.c
> > +++ b/net/openvswitch/datapath.c
> > @@ -350,7 +350,8 @@ static size_t upcall_msg_size(const struct 
> > dp_upcall_info *upcall_info,
> > size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
> > + nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
> > + nla_total_size(ovs_key_attr_size()) /* 
> > OVS_PACKET_ATTR_KEY */
> > -   + nla_total_size(sizeof(unsigned int)); /* 
> > OVS_PACKET_ATTR_LEN */
> > +   + nla_total_size(sizeof(unsigned int)) /* 
> > OVS_PACKET_ATTR_LEN */
> > +   + nla_total_size(sizeof(u64)); /* OVS_PACKET_ATTR_HASH */
> >
> > /* OVS_PACKET_ATTR_USERDATA */
> > if (upcall_info->userdata)
> > @@ -393,6 +394,7 @@ static int queue_userspace_packet(struct datapath *dp, 
> > struct sk_buff *skb,
> > size_t len;
> > unsigned int hlen;
> > int err, dp_ifindex;
> > +   u64 hash;
> >
> > dp_ifindex = get_dpifindex(dp);
> > if (!dp_ifindex)
> > @@ -504,6 +506,23 @@ static int 

Re: [ovs-dev] [PATCH net-next] openvswitch: add TTL decrement action

2019-11-12 Thread Pravin Shelar
On Tue, Nov 12, 2019 at 2:25 AM Matteo Croce  wrote:
>
> New action to decrement TTL instead of setting it to a fixed value.
> This action will decrement the TTL and, in case of expired TTL, send the
> packet to userspace via output_userspace() to take care of it.
>
> Supports both IPv4 and IPv6 via the ttl and hop_limit fields, respectively.
>
> Tested with a corresponding change in the userspace:
>
> # ovs-dpctl dump-flows
> in_port(2),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, 
> actions:dec_ttl,1
> in_port(1),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, 
> actions:dec_ttl,2
> in_port(1),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, 
> actions:2
> in_port(2),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, 
> actions:1
>
> # ping -c1 192.168.0.2 -t 42
> IP (tos 0x0, ttl 41, id 61647, offset 0, flags [DF], proto ICMP (1), 
> length 84)
> 192.168.0.1 > 192.168.0.2: ICMP echo request, id 386, seq 1, length 64
> # ping -c1 192.168.0.2 -t 120
> IP (tos 0x0, ttl 119, id 62070, offset 0, flags [DF], proto ICMP (1), 
> length 84)
> 192.168.0.1 > 192.168.0.2: ICMP echo request, id 388, seq 1, length 64
> # ping -c1 192.168.0.2 -t 1
> #
>
> Co-authored-by: Bindiya Kurle 
> Signed-off-by: Bindiya Kurle 
> Signed-off-by: Matteo Croce 
> ---
>  include/uapi/linux/openvswitch.h |  2 ++
>  net/openvswitch/actions.c| 46 
>  net/openvswitch/flow_netlink.c   |  6 +
>  3 files changed, 54 insertions(+)
>
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index 1887a451c388..a3bdb1ecd1e7 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -890,6 +890,7 @@ struct check_pkt_len_arg {
>   * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
>   * of actions if greater than the specified packet length, else execute
>   * another set of actions.
> + * @OVS_ACTION_ATTR_DEC_TTL: Decrement the IP TTL.
>   *
>   * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not 
> all
>   * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
> @@ -925,6 +926,7 @@ enum ovs_action_attr {
> OVS_ACTION_ATTR_METER,/* u32 meter ID. */
> OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*.  */
> OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
> +   OVS_ACTION_ATTR_DEC_TTL,  /* Decrement ttl action */
>
> __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>* from userspace. */
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 12936c151cc0..077b7f309c93 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -1174,6 +1174,43 @@ static int execute_check_pkt_len(struct datapath *dp, 
> struct sk_buff *skb,
>  nla_len(actions), last, clone_flow_key);
>  }
>
> +static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> +   int err;
> +
> +   if (skb->protocol == htons(ETH_P_IPV6)) {
> +   struct ipv6hdr *nh = ipv6_hdr(skb);
> +
> +   err = skb_ensure_writable(skb, skb_network_offset(skb) +
> + sizeof(*nh));
> +   if (unlikely(err))
> +   return err;
> +
> +   if (nh->hop_limit <= 1)
> +   return -EHOSTUNREACH;
> +
> +   key->ip.ttl = --nh->hop_limit;
> +   } else {
> +   struct iphdr *nh = ip_hdr(skb);
> +   u8 old_ttl;
> +
> +   err = skb_ensure_writable(skb, skb_network_offset(skb) +
> + sizeof(*nh));
> +   if (unlikely(err))
> +   return err;
> +
> +   if (nh->ttl <= 1)
> +   return -EHOSTUNREACH;
> +
> +   old_ttl = nh->ttl--;
> +   csum_replace2(>check, htons(old_ttl << 8),
> + htons(nh->ttl << 8));
> +   key->ip.ttl = nh->ttl;
> +   }
> +
> +   return 0;
> +}
> +
>  /* Execute a list of actions against 'skb'. */
>  static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>   struct sw_flow_key *key,
> @@ -1345,6 +1382,15 @@ static int do_execute_actions(struct datapath *dp, 
> struct sk_buff *skb,
>
> break;
> }
> +
> +   case OVS_ACTION_ATTR_DEC_TTL:
> +   err = execute_dec_ttl(skb, key);
> +   if (err == -EHOSTUNREACH) {
> +   output_userspace(dp, skb, key, a, attr,
> +len, OVS_CB(skb)->cutlen);
> +   OVS_CB(skb)->cutlen = 0;
> +

Re: [ovs-dev] [PATCH net-next v3] net: openvswitch: add hash info to upcall

2019-11-12 Thread Pravin Shelar
On Tue, Nov 12, 2019 at 7:09 AM  wrote:
>
> From: Tonghao Zhang 
>
> When using the kernel datapath, the upcall don't
> include skb hash info relatived. That will introduce
> some problem, because the hash of skb is important
> in kernel stack. For example, VXLAN module uses
> it to select UDP src port. The tx queue selection
> may also use the hash in stack.
>
> Hash is computed in different ways. Hash is random
> for a TCP socket, and hash may be computed in hardware,
> or software stack. Recalculation hash is not easy.
>
> Hash of TCP socket is computed:
>   tcp_v4_connect
> -> sk_set_txhash (is random)
>
>   __tcp_transmit_skb
> -> skb_set_hash_from_sk
>
> There will be one upcall, without information of skb
> hash, to ovs-vswitchd, for the first packet of a TCP
> session. The rest packets will be processed in Open vSwitch
> modules, hash kept. If this tcp session is forward to
> VXLAN module, then the UDP src port of first tcp packet
> is different from rest packets.
>
> TCP packets may come from the host or dockers, to Open vSwitch.
> To fix it, we store the hash info to upcall, and restore hash
> when packets sent back.
>
> +---+  +-+
> |   Docker/VMs  |  | ovs-vswitchd|
> ++--+  +-++--+
>  |   ^|
>  |   ||
>  |   |  upcallv restore packet hash (not 
> recalculate)
>  | +-++--+
>  |  tap netdev | |   vxlan module
>  +---> +-->  Open vSwitch ko +-->
>or internal type| |
>+-+
>
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364062.html
> Signed-off-by: Tonghao Zhang 
> ---
> v3:
> * add enum ovs_pkt_hash_types
> * avoid duplicate call the skb_get_hash_raw.
> * explain why we should fix this problem.
> ---
>  include/uapi/linux/openvswitch.h |  2 ++
>  net/openvswitch/datapath.c   | 30 +-
>  net/openvswitch/datapath.h   | 12 
>  3 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index 1887a451c388..e65407c1f366 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -170,6 +170,7 @@ enum ovs_packet_cmd {
>   * output port is actually a tunnel port. Contains the output tunnel key
>   * extracted from the packet as nested %OVS_TUNNEL_KEY_ATTR_* attributes.
>   * @OVS_PACKET_ATTR_MRU: Present for an %OVS_PACKET_CMD_ACTION and
> + * @OVS_PACKET_ATTR_HASH: Packet hash info (e.g. hash, sw_hash and l4_hash 
> in skb).
>   * @OVS_PACKET_ATTR_LEN: Packet size before truncation.
>   * %OVS_PACKET_ATTR_USERSPACE action specify the Maximum received fragment
>   * size.
> @@ -190,6 +191,7 @@ enum ovs_packet_attr {
> OVS_PACKET_ATTR_PROBE,  /* Packet operation is a feature probe,
>error logging should be suppressed. */
> OVS_PACKET_ATTR_MRU,/* Maximum received IP fragment size. */
> +   OVS_PACKET_ATTR_HASH,   /* Packet hash. */
> OVS_PACKET_ATTR_LEN,/* Packet size before truncation. */
> __OVS_PACKET_ATTR_MAX
>  };
I agree with Greg, value of existing enums can not be changed in UAPI.

> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 2088619c03f0..b556cf62b77c 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -350,7 +350,8 @@ static size_t upcall_msg_size(const struct dp_upcall_info 
> *upcall_info,
> size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
> + nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
> + nla_total_size(ovs_key_attr_size()) /* OVS_PACKET_ATTR_KEY 
> */
> -   + nla_total_size(sizeof(unsigned int)); /* 
> OVS_PACKET_ATTR_LEN */
> +   + nla_total_size(sizeof(unsigned int)) /* OVS_PACKET_ATTR_LEN 
> */
> +   + nla_total_size(sizeof(u64)); /* OVS_PACKET_ATTR_HASH */
>
> /* OVS_PACKET_ATTR_USERDATA */
> if (upcall_info->userdata)
> @@ -393,6 +394,7 @@ static int queue_userspace_packet(struct datapath *dp, 
> struct sk_buff *skb,
> size_t len;
> unsigned int hlen;
> int err, dp_ifindex;
> +   u64 hash;
>
> dp_ifindex = get_dpifindex(dp);
> if (!dp_ifindex)
> @@ -504,6 +506,23 @@ static int queue_userspace_packet(struct datapath *dp, 
> struct sk_buff *skb,
> pad_packet(dp, user_skb);
> }
>
> +   hash = skb_get_hash_raw(skb);
> +   if (hash) {
Zero hash is valid hash of skb. due to this check packets with zero
hash would not get same vxlan source 

[ovs-dev] [PATCH AUTOSEL 4.4 01/48] net: ovs: fix return type of ndo_start_xmit function

2019-11-12 Thread Sasha Levin
From: YueHaibing 

[ Upstream commit eddf11e18dff0e8671e06ce54e64cfc843303ab9 ]

The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.

Found by coccinelle.

Signed-off-by: YueHaibing 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 net/openvswitch/vport-internal_dev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/vport-internal_dev.c 
b/net/openvswitch/vport-internal_dev.c
index ec76398a792fb..12ec61b259b9f 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -44,7 +44,8 @@ static struct internal_dev *internal_dev_priv(struct 
net_device *netdev)
 }
 
 /* Called with rcu_read_lock_bh. */
-static int internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
+static netdev_tx_t
+internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
 {
int len, err;
 
@@ -63,7 +64,7 @@ static int internal_dev_xmit(struct sk_buff *skb, struct 
net_device *netdev)
} else {
netdev->stats.tx_errors++;
}
-   return 0;
+   return NETDEV_TX_OK;
 }
 
 static int internal_dev_open(struct net_device *netdev)
-- 
2.20.1

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


[ovs-dev] [PATCH AUTOSEL 4.9 01/68] net: ovs: fix return type of ndo_start_xmit function

2019-11-12 Thread Sasha Levin
From: YueHaibing 

[ Upstream commit eddf11e18dff0e8671e06ce54e64cfc843303ab9 ]

The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.

Found by coccinelle.

Signed-off-by: YueHaibing 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 net/openvswitch/vport-internal_dev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/vport-internal_dev.c 
b/net/openvswitch/vport-internal_dev.c
index e7da29021b38b..c233924825801 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -44,7 +44,8 @@ static struct internal_dev *internal_dev_priv(struct 
net_device *netdev)
 }
 
 /* Called with rcu_read_lock_bh. */
-static int internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
+static netdev_tx_t
+internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
 {
int len, err;
 
@@ -63,7 +64,7 @@ static int internal_dev_xmit(struct sk_buff *skb, struct 
net_device *netdev)
} else {
netdev->stats.tx_errors++;
}
-   return 0;
+   return NETDEV_TX_OK;
 }
 
 static int internal_dev_open(struct net_device *netdev)
-- 
2.20.1

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


[ovs-dev] [PATCH AUTOSEL 4.14 001/115] net: ovs: fix return type of ndo_start_xmit function

2019-11-12 Thread Sasha Levin
From: YueHaibing 

[ Upstream commit eddf11e18dff0e8671e06ce54e64cfc843303ab9 ]

The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.

Found by coccinelle.

Signed-off-by: YueHaibing 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 net/openvswitch/vport-internal_dev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/vport-internal_dev.c 
b/net/openvswitch/vport-internal_dev.c
index b9377afeaba45..1025bed255b9b 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -44,7 +44,8 @@ static struct internal_dev *internal_dev_priv(struct 
net_device *netdev)
 }
 
 /* Called with rcu_read_lock_bh. */
-static int internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
+static netdev_tx_t
+internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
 {
int len, err;
 
@@ -63,7 +64,7 @@ static int internal_dev_xmit(struct sk_buff *skb, struct 
net_device *netdev)
} else {
netdev->stats.tx_errors++;
}
-   return 0;
+   return NETDEV_TX_OK;
 }
 
 static int internal_dev_open(struct net_device *netdev)
-- 
2.20.1

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


[ovs-dev] [PATCH AUTOSEL 4.19 011/209] openvswitch: Use correct reply values in datapath and vport ops

2019-11-12 Thread Sasha Levin
From: Yifeng Sun 

[ Upstream commit 804fe108fc92e591ddfe9447e7fb4691ed16daee ]

This patch fixes the bug that all datapath and vport ops are returning
wrong values (OVS_FLOW_CMD_NEW or OVS_DP_CMD_NEW) in their replies.

Signed-off-by: Yifeng Sun 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 net/openvswitch/datapath.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 8e396c7c83894..66c726595a95d 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1182,14 +1182,14 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct 
genl_info *info)
   ovs_header->dp_ifindex,
   reply, info->snd_portid,
   info->snd_seq, 0,
-  OVS_FLOW_CMD_NEW,
+  OVS_FLOW_CMD_SET,
   ufid_flags);
BUG_ON(error < 0);
}
} else {
/* Could not alloc without acts before locking. */
reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
-   info, OVS_FLOW_CMD_NEW, false,
+   info, OVS_FLOW_CMD_SET, false,
ufid_flags);
 
if (IS_ERR(reply)) {
@@ -1265,7 +1265,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct 
genl_info *info)
}
 
reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex, info,
-   OVS_FLOW_CMD_NEW, true, ufid_flags);
+   OVS_FLOW_CMD_GET, true, ufid_flags);
if (IS_ERR(reply)) {
err = PTR_ERR(reply);
goto unlock;
@@ -1389,7 +1389,7 @@ static int ovs_flow_cmd_dump(struct sk_buff *skb, struct 
netlink_callback *cb)
if (ovs_flow_cmd_fill_info(flow, ovs_header->dp_ifindex, skb,
   NETLINK_CB(cb->skb).portid,
   cb->nlh->nlmsg_seq, NLM_F_MULTI,
-  OVS_FLOW_CMD_NEW, ufid_flags) < 0)
+  OVS_FLOW_CMD_GET, ufid_flags) < 0)
break;
 
cb->args[0] = bucket;
@@ -1730,7 +1730,7 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct 
genl_info *info)
ovs_dp_change(dp, info->attrs);
 
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
-  info->snd_seq, 0, OVS_DP_CMD_NEW);
+  info->snd_seq, 0, OVS_DP_CMD_SET);
BUG_ON(err < 0);
 
ovs_unlock();
@@ -1761,7 +1761,7 @@ static int ovs_dp_cmd_get(struct sk_buff *skb, struct 
genl_info *info)
goto err_unlock_free;
}
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
-  info->snd_seq, 0, OVS_DP_CMD_NEW);
+  info->snd_seq, 0, OVS_DP_CMD_GET);
BUG_ON(err < 0);
ovs_unlock();
 
@@ -1785,7 +1785,7 @@ static int ovs_dp_cmd_dump(struct sk_buff *skb, struct 
netlink_callback *cb)
if (i >= skip &&
ovs_dp_cmd_fill_info(dp, skb, NETLINK_CB(cb->skb).portid,
 cb->nlh->nlmsg_seq, NLM_F_MULTI,
-OVS_DP_CMD_NEW) < 0)
+OVS_DP_CMD_GET) < 0)
break;
i++;
}
@@ -2101,7 +2101,7 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct 
genl_info *info)
 
err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
  info->snd_portid, info->snd_seq, 0,
- OVS_VPORT_CMD_NEW);
+ OVS_VPORT_CMD_SET);
BUG_ON(err < 0);
 
ovs_unlock();
@@ -2182,7 +2182,7 @@ static int ovs_vport_cmd_get(struct sk_buff *skb, struct 
genl_info *info)
goto exit_unlock_free;
err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
  info->snd_portid, info->snd_seq, 0,
- OVS_VPORT_CMD_NEW);
+ OVS_VPORT_CMD_GET);
BUG_ON(err < 0);
rcu_read_unlock();
 
@@ -2218,7 +2218,7 @@ static int ovs_vport_cmd_dump(struct sk_buff *skb, struct 
netlink_callback *cb)
NETLINK_CB(cb->skb).portid,

[ovs-dev] [PATCH AUTOSEL 4.19 001/209] net: ovs: fix return type of ndo_start_xmit function

2019-11-12 Thread Sasha Levin
From: YueHaibing 

[ Upstream commit eddf11e18dff0e8671e06ce54e64cfc843303ab9 ]

The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.

Found by coccinelle.

Signed-off-by: YueHaibing 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 net/openvswitch/vport-internal_dev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/vport-internal_dev.c 
b/net/openvswitch/vport-internal_dev.c
index 5a304cfc84233..5993405c25c12 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -43,7 +43,8 @@ static struct internal_dev *internal_dev_priv(struct 
net_device *netdev)
 }
 
 /* Called with rcu_read_lock_bh. */
-static int internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
+static netdev_tx_t
+internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
 {
int len, err;
 
@@ -62,7 +63,7 @@ static int internal_dev_xmit(struct sk_buff *skb, struct 
net_device *netdev)
} else {
netdev->stats.tx_errors++;
}
-   return 0;
+   return NETDEV_TX_OK;
 }
 
 static int internal_dev_open(struct net_device *netdev)
-- 
2.20.1

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


Re: [ovs-dev] [PATCH v3] travis: support ppc64le builds

2019-11-12 Thread Lance Yang (Arm Technology China)
Hi,

For pip missing issue, we have a workaround.  In .travis/linux-prepare.sh. You 
can install pip or something else with package management commands.

For arm64 on travis, we check the environment variables in the container and 
found a variable called "TRAVIS_ARCH". Thus we can use this variable to install 
specific software for a specific CPU architecture.

You can find a successful build here: 
https://travis-ci.org/yzyuestc/ovs/jobs/610716477

-Original Message-
From: ovs-dev-boun...@openvswitch.org  On 
Behalf Of dwilder
Sent: Wednesday, November 13, 2019 8:29 AM
To: Ilya Maximets 
Cc: ovs-dev@openvswitch.org; wil...@us.ibm.com
Subject: Re: [ovs-dev] [PATCH v3] travis: support ppc64le builds

On 2019-11-12 11:30, Ilya Maximets wrote:
> On 12.11.2019 18:57, dwilder wrote:
>> On 2019-11-08 14:52, Ilya Maximets wrote:
>>> On 06.11.2019 20:20, David Wilder wrote:
 Add support for travis-ci ppc64le builds.

 - Updated matrix in .travis.yml to include an arch: ppc64le build.
 - Move package install needed for 32bit builds to
 .travis/linux-prepare.sh.

 To keep the total build time at an acceptable level only a single
 build job is included in the matrix for ppc64le.

 A build report example can be found here [1] [0]
 https://urldefense.proofpoint.com/v2/url?u=http-3A__travis-2Dci.org
 _=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQ
 beAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=r0fB
 Os-21CKcV4kyZGnzh3fcjrpR8caYSl8K2i1St54=
 [1]
 https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.or
 g_djlwilder_ovs_builds_607851729=DwICaQ=jf_iaSHvJObTbx-siA1ZOg&
 r=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfg
 EiyaEIT3j9gPEIgmBatCEqCo=7t2rzVasH7Xq_R7jWkWZO9rkgm4KHMH-WavBzCRb
 F74=
 Signed-off-by: David Wilder 
 ---
 Addressed review comments:
 - Cleaned up linux-prepare.sh (v2)
 - Switch from os: linux-ppc64le to arch: ppc64le (v3)
>>>
>>> What a wonderful world of undocumented features. :)
>>>
>>> Anyway, I just tried this patch and it fails for me because of
>>> missing pip:
>>>
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org
>>> _igsilya_ovs_jobs_609402867=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndx
>>> yKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT
>>> 3j9gPEIgmBatCEqCo=PF1oO_KkZFd_RRKToj6UBN2t2YhvTVE5XnVD1GF9u60=
>>>
>>> pip install --disable-pip-version-check --user six flake8 hacking
>>> ./.travis/linux-prepare.sh: line 15: pip: command not found
>>>
>>> Restarting the job doesn't help.
>>>
>>> I'm wondering what is the base image and who controls preinstalled
>>> software?
>>> Maybe it makes sense to hold on until travis will start supporting
>>> ppc64le officially?
>>>
>>> Best regards, Ilya Maximets.
>>
>> Unfortunately it not just ppc the arm64 image has the same issue :) I
>> added a arm64 build on this attempt.
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_
>> djlwilder_ovs_builds_610517731=DwIDaQ=jf_iaSHvJObTbx-siA1ZOg=7n
>> dxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=rVTSRTh9jExG_mX8boqA-OQcZ
>> kWmbO2g7TjgtUe6jws=sCvTr8MwXrqa7AEOs60tuqnquBqbRKlp_7-WacGzJWc=
>>
>> I will attempt to adjust the package list.
>
> I think, you could just report this to TravisCI support and see what
> they will answer.  It'll be much better if they will just update their
> images.
>
> And the good news that ppc64le is officially supported now:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__blog.travis-2Dci.
> com_2019-2D11-2D12-2Dmulti-2Dcpu-2Darchitecture-2Dibm-2Dpower-2Dibm-2D
> z=DwIDaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeA
> yz8i_vwCCaI=rVTSRTh9jExG_mX8boqA-OQcZkWmbO2g7TjgtUe6jws=CRYw9o6At3
> rFT_gd_5r1Uw07mv_SEPvQ6LgBLfMyqgg=

That is great news!
I reported the issue to travis:
https://travis-ci.community/t/pip-is-not-installed-in-ppc64le-and-arm64-images/5902?u=djlwilder

I added a workaround if we need it. This is my latest travis run.
https://www.travis-ci.org/djlwilder/ovs/builds/611123734
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3] travis: support ppc64le builds

2019-11-12 Thread dwilder

On 2019-11-12 11:30, Ilya Maximets wrote:

On 12.11.2019 18:57, dwilder wrote:

On 2019-11-08 14:52, Ilya Maximets wrote:

On 06.11.2019 20:20, David Wilder wrote:

Add support for travis-ci ppc64le builds.

- Updated matrix in .travis.yml to include an arch: ppc64le build.
- Move package install needed for 32bit builds to 
.travis/linux-prepare.sh.


To keep the total build time at an acceptable level only a single 
build

job is included in the matrix for ppc64le.

A build report example can be found here [1]
[0] 
https://urldefense.proofpoint.com/v2/url?u=http-3A__travis-2Dci.org_=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=r0fBOs-21CKcV4kyZGnzh3fcjrpR8caYSl8K2i1St54= 
[1] 
https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_djlwilder_ovs_builds_607851729=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=7t2rzVasH7Xq_R7jWkWZO9rkgm4KHMH-WavBzCRbF74= 
Signed-off-by: David Wilder 

---
Addressed review comments:
- Cleaned up linux-prepare.sh (v2)
- Switch from os: linux-ppc64le to arch: ppc64le (v3)


What a wonderful world of undocumented features. :)

Anyway, I just tried this patch and it fails for me because of 
missing pip:


https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_igsilya_ovs_jobs_609402867=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=PF1oO_KkZFd_RRKToj6UBN2t2YhvTVE5XnVD1GF9u60=

pip install --disable-pip-version-check --user six flake8 hacking
./.travis/linux-prepare.sh: line 15: pip: command not found

Restarting the job doesn't help.

I'm wondering what is the base image and who controls preinstalled 
software?
Maybe it makes sense to hold on until travis will start supporting 
ppc64le

officially?

Best regards, Ilya Maximets.


Unfortunately it not just ppc the arm64 image has the same issue :)  
I added a arm64 build on this attempt.


https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_djlwilder_ovs_builds_610517731=DwIDaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=rVTSRTh9jExG_mX8boqA-OQcZkWmbO2g7TjgtUe6jws=sCvTr8MwXrqa7AEOs60tuqnquBqbRKlp_7-WacGzJWc=

I will attempt to adjust the package list.


I think, you could just report this to TravisCI support and see what 
they
will answer.  It'll be much better if they will just update their 
images.


And the good news that ppc64le is officially supported now:
https://urldefense.proofpoint.com/v2/url?u=https-3A__blog.travis-2Dci.com_2019-2D11-2D12-2Dmulti-2Dcpu-2Darchitecture-2Dibm-2Dpower-2Dibm-2Dz=DwIDaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=rVTSRTh9jExG_mX8boqA-OQcZkWmbO2g7TjgtUe6jws=CRYw9o6At3rFT_gd_5r1Uw07mv_SEPvQ6LgBLfMyqgg=


That is great news!
I reported the issue to travis:
https://travis-ci.community/t/pip-is-not-installed-in-ppc64le-and-arm64-images/5902?u=djlwilder

I added a workaround if we need it. This is my latest travis run.
https://www.travis-ci.org/djlwilder/ovs/builds/611123734
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] netdev-afxdp: Best-effort configuration of XDP mode.

2019-11-12 Thread William Tu
On Thu, Nov 7, 2019 at 3:36 AM Ilya Maximets  wrote:
>
> Until now there was only two options for XDP mode in OVS: SKB or DRV.
> i.e. 'generic XDP' or 'native XDP with zero-copy enabled'.
>
> Devices like 'veth' interfaces in Linux supports native XDP, but
> doesn't support zero-copy mode.  This case can not be covered by
> existing API and we have to use slower generic XDP for such devices.
> There are few more issues, e.g. TCP is not supported in generic XDP
> mode for veth interfaces due to kernel limitations, however it is
> supported in native mode.
>
> This change introduces ability to use native XDP without zero-copy
> along with best-effort configuration option that enabled by default.
> In best-effort case OVS will sequentially try different modes starting
> from the fastest one and will choose the first acceptable for current
> interface.  This will guarantee the best possible performance.
>
> If user will want to choose specific mode, it's still possible by
> setting the 'options:xdp-mode'.
>
> This change additionally changes the API by renaming the configuration
> knob from 'xdpmode' to 'xdp-mode' and also renaming the modes
> themselves to be more user-friendly.
>
> The full list of currently supported modes:
>   * native-with-zerocopy - former DRV
>   * native   - new one, DRV without zero-copy
>   * generic  - former SKB
>   * best-effort  - new one, chooses the best available from
>3 above modes
>
> Since 'best-effort' is a default mode, users will not need to
> explicitely set 'xdp-mode' in most cases.
>
> TCP related tests enabled back in system afxdp testsuite, because
> 'best-effort' will choose 'native' mode for veth interfaces
> and this mode has no issues with TCP.
>
> Signed-off-by: Ilya Maximets 
> ---

I'm thinking about adding some tests case and patches depends on
this API change. If there is no more comment, I will apply to master.

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


Re: [ovs-dev] [PATCH v3 ovn 0/3] Improve ovn-detrace support for parsing OpenFlow cookies.

2019-11-12 Thread Han Zhou
On Tue, Nov 12, 2019 at 8:47 AM Dumitru Ceara  wrote:
>
> Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow
translations.")
> added support for more types of cookies (partial SB uuids) that are stored
> in OpenFlow entries created by ovn-controller.
>
> The last patch in this series implements support for parsing all these
> different cookies in ovn-detrace too.
>
> The first patch is a bug fix required as ovn-detrace was broken after
> moving OVN to its own separate rundir.
>
> The second patch fixes line parsing in ovn-detrace.
>
> CC: Han Zhou 
> Signed-off-by: Dumitru Ceara 
>
> Dumitru Ceara (3):
>   ovn-detrace: Fix rundir.
>   ovn-detrace: Fix line parsing.
>   ovn-detrace: Add support for other types of SB cookies.
>
>
>  utilities/ovn-detrace.in |  215
+++---
>  1 file changed, 146 insertions(+), 69 deletions(-)
>
>
> ---
> v3:
> - Remove stray "%s".
> - Rename pprint to print_p to avoid name clashes with the library
function.
> v2:
> - Address Mark's comments:
>   - properly handle potential collisions between cookie -> UUID mappings.
>   - when looking up SB records by cookie, match on the first part of the
> UUID.
> - Further refactor ovn-detrace to simplify prefixing outputs.
> - Change print statements to print() calls.
>

Thanks Dumitru for improving the tool!
Patch 1 and 3 look good to me. I posted comments for patch 2.

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


Re: [ovs-dev] [PATCH v3 ovn 2/3] ovn-detrace: Fix line parsing.

2019-11-12 Thread Han Zhou
Hi Dumitru,

Thanks for improving the tool.

On Tue, Nov 12, 2019 at 8:47 AM Dumitru Ceara  wrote:
>
> The script was never parsing the first cookie in the input. Also, add a
> check to make sure that the cookie refers to a Logical_Flow before
> trying to print the record.
>
> Signed-off-by: Dumitru Ceara 
> ---
>  utilities/ovn-detrace.in |   24 ++--
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
> index 9471e37..34b6b0e 100755
> --- a/utilities/ovn-detrace.in
> +++ b/utilities/ovn-detrace.in
> @@ -188,22 +188,26 @@ def main():
>  cookie = None
>  while True:
>  line = sys.stdin.readline()
> +
> +if line == '':
> +break
> +
> +line = line.strip()
> +
>  if cookie:
>  # print lflow info when the current flow block ends
> -if regex_table_id.match(line) or line.strip() == '':
> +if regex_table_id.match(line):
>  lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie)
> -print_lflow(lflow, "  * ")
> -print_lflow_nb_hint(lflow, "* ", ovsdb_ovnnb)
> -cookie = None
> +if lflow:
> +print_lflow(lflow, "  * ")
> +print_lflow_nb_hint(lflow, "* ", ovsdb_ovnnb)
> +cookie = None
>
> -print line.strip()
> -if line == "":
> -break
> +print line
>
>  m = regex_cookie.match(line)
> -if not m:
> -continue
> -cookie = m.group(1)
> +if m:
> +cookie = m.group(1)
>
>
>  if __name__ == "__main__":
>

Maybe I missed something here, but the original logic seems to be correct
to me. It always parses the current line - if there is a cookie, it is
parsed. And then it prints the previous logical flow information using the
last cookie that was parsed when a *new* flow or line break is encountered.
And finally, it ensures the last logical flow information is printed
because the "break" is AFTER the "if cookie" block.
Now with the change, it seems the last logical flow (related to the last
cookie) may not be printed, if it happens to be the last line. (although it
is not a problem if the last line doesn't has cookie)

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


Re: [ovs-dev] [PATCH v3 ovn 0/3] Improve ovn-detrace support for parsing OpenFlow cookies.

2019-11-12 Thread Mark Michelson

For the series:

Acked-by: Mark Michelson 

On 11/12/19 11:47 AM, Dumitru Ceara wrote:

Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow translations.")
added support for more types of cookies (partial SB uuids) that are stored
in OpenFlow entries created by ovn-controller.

The last patch in this series implements support for parsing all these
different cookies in ovn-detrace too.

The first patch is a bug fix required as ovn-detrace was broken after
moving OVN to its own separate rundir.

The second patch fixes line parsing in ovn-detrace.

CC: Han Zhou 
Signed-off-by: Dumitru Ceara 

Dumitru Ceara (3):
   ovn-detrace: Fix rundir.
   ovn-detrace: Fix line parsing.
   ovn-detrace: Add support for other types of SB cookies.


  utilities/ovn-detrace.in |  215 +++---
  1 file changed, 146 insertions(+), 69 deletions(-)


---
v3:
- Remove stray "%s".
- Rename pprint to print_p to avoid name clashes with the library function.
v2:
- Address Mark's comments:
   - properly handle potential collisions between cookie -> UUID mappings.
   - when looking up SB records by cookie, match on the first part of the
 UUID.
- Further refactor ovn-detrace to simplify prefixing outputs.
- Change print statements to print() calls.



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


Re: [ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.

2019-11-12 Thread Han Zhou
On Tue, Nov 12, 2019 at 9:02 AM Han Zhou  wrote:
>
>
>
> On Tue, Nov 12, 2019 at 2:28 AM Dumitru Ceara  wrote:
> >
> > Function get_router_load_balancer_ips() was incorrectly returning a
> > single address_family even though the IP set could contain a mix of
> > IPv4 and IPv6 addresses.
> >
> > Signed-off-by: Dumitru Ceara 
> > ---
> >  northd/ovn-northd.c |  126
+--
> >  1 file changed, 72 insertions(+), 54 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 2f0f501..32f3200 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -2184,7 +2184,7 @@ ip_address_and_port_from_lb_key(const char *key,
char **ip_address,
> >
> >  static void
> >  get_router_load_balancer_ips(const struct ovn_datapath *od,
> > - struct sset *all_ips, int *addr_family)
> > + struct sset *all_ips_v4, struct sset
*all_ips_v6)
> >  {
> >  if (!od->nbr) {
> >  return;
> > @@ -2199,13 +2199,21 @@ get_router_load_balancer_ips(const struct
ovn_datapath *od,
> >  /* node->key contains IP:port or just IP. */
> >  char *ip_address = NULL;
> >  uint16_t port;
> > +int addr_family;
> >
> >  ip_address_and_port_from_lb_key(node->key, _address,
,
> > -addr_family);
> > +_family);
> >  if (!ip_address) {
> >  continue;
> >  }
> >
> > +struct sset *all_ips;
> > +if (addr_family == AF_INET) {
> > +all_ips = all_ips_v4;
> > +} else {
> > +all_ips = all_ips_v6;
> > +}
> > +
> >  if (!sset_contains(all_ips, ip_address)) {
> >  sset_add(all_ips, ip_address);
> >  }
> > @@ -2299,17 +2307,22 @@ get_nat_addresses(const struct ovn_port *op,
size_t *n)
> >  }
> >  }
> >
> > -/* A set to hold all load-balancer vips. */
> > -struct sset all_ips = SSET_INITIALIZER(_ips);
> > -int addr_family;
> > -get_router_load_balancer_ips(op->od, _ips, _family);
> > +/* Two sets to hold all load-balancer vips. */
> > +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> > +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> > +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> >  const char *ip_address;
> > -SSET_FOR_EACH (ip_address, _ips) {
> > +SSET_FOR_EACH (ip_address, _ips_v4) {
> >  ds_put_format(_addresses, " %s", ip_address);
> >  central_ip_address = true;
> >  }
> > -sset_destroy(_ips);
> > +SSET_FOR_EACH (ip_address, _ips_v6) {
> > +ds_put_format(_addresses, " %s", ip_address);
> > +central_ip_address = true;
> > +}
> > +sset_destroy(_ips_v4);
> > +sset_destroy(_ips_v6);
> >
> >  if (central_ip_address) {
> >  /* Gratuitous ARP for centralized NAT rules on distributed
gateway
> > @@ -6911,61 +6924,66 @@ build_lrouter_flows(struct hmap *datapaths,
struct hmap *ports,
> >  }
> >
> >  /* A set to hold all load-balancer vips that need ARP
responses. */
> > -struct sset all_ips = SSET_INITIALIZER(_ips);
> > -int addr_family;
> > -get_router_load_balancer_ips(op->od, _ips, _family);
> > +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> > +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> > +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> >  const char *ip_address;
> > -SSET_FOR_EACH(ip_address, _ips) {
> > +SSET_FOR_EACH (ip_address, _ips_v4) {
> >  ds_clear();
> > -if (addr_family == AF_INET) {
> > -ds_put_format(,
> > -  "inport == %s && arp.tpa == %s && arp.op
== 1",
> > -  op->json_key, ip_address);
> > -} else {
> > -ds_put_format(,
> > -  "inport == %s && nd_ns && nd.target ==
%s",
> > -  op->json_key, ip_address);
> > -}
> > +ds_put_format(,
> > +  "inport == %s && arp.tpa == %s && arp.op ==
1",
> > +  op->json_key, ip_address);
> >
> >  ds_clear();
> > -if (addr_family == AF_INET) {
> > -ds_put_format(,
> > -"eth.dst = eth.src; "
> > -"eth.src = %s; "
> > -"arp.op = 2; /* ARP reply */ "
> > -"arp.tha = arp.sha; "
> > -"arp.sha = %s; "
> > -"arp.tpa = arp.spa; "
> > -"arp.spa = %s; "
> > -"outport = %s; "
> > -"flags.loopback = 1; "
> > -"output;",
> > -op->lrp_networks.ea_s,
> > -   

Re: [ovs-dev] [PATCH v3] dpdk: Migrate to the new pdump API.

2019-11-12 Thread David Marchand
On Tue, Nov 12, 2019 at 8:41 PM Ilya Maximets  wrote:
>
> On 12.11.2019 19:51, Stokes, Ian wrote:
> >
> >
> > On 11/12/2019 5:15 PM, David Marchand wrote:
> >> On Tue, Nov 12, 2019 at 6:07 PM Stokes, Ian  wrote:
> >>> On 11/11/2019 3:01 PM, Ilya Maximets wrote:
>  DPDK commit 660098d61f57 ("pdump: use generic multi-process channel")
>  switched pdump to use generic DPDK IPC instead of sockets.
>  Old API was deprecated and removed.  Updating OVS code accordingly.
> 
>  Signed-off-by: Ilya Maximets 
> >>>
> >>> Thanks for the patch Ilya.
> >>>
> >>> I see compilation passing now on dpdk-latest with this applied.
> >>>
> >>> https://travis-ci.org/istokes/ovs/builds/610915636
> >>>
> >>> I still had issues with running PDUMP, but those issues are specific to
> >>> PDUMP setup in my environment. A separate issue we can discuss further
> >>> on the deprecation thread as it seems unrelated to this patch.
> >>>
> >>> @David, are you happy to ack the patch (I see some of the changes are
> >>> from your side).
> >>
> >> I did not work on the crash I saw, but it was most likely a problem on my 
> >> side.
> >> This looks good to me.
> >
> > From a some further testing on my side I'm also seeing a crash, 
> > specifically OVS crashes out once packets are received. PDUMP is still 
> > running but complains of being unable to communicate with the primary 
> > process and then exits. Is this similar to what you saw?
> >
> > @Ilya, by chance did you see anything like this?
>
>
> Honestly, I never tried to use pdump, and I don't really want to try
> preparing the setup for it (building ASLR disabled kernel and stuff).
>
>
> > I'll investigate further myself tomorrow. I was going to hold off on the 
> > merge in the meantime. Thoughts?
>
> There are 2 options here:
> 1. Apply this patch and hope that DPDK will be fixed someday.
>+ Optionally apply deprecation patch.

This is pure speculation, but when I saw the crash before, I thought
that the problem was in the way ovs creates its thread without the
dpdk being aware of it.
dpdk pdump component expects that it's running on a EAL thread, with a
known lcore, and *boom* when it dereferences some uninitialized
structures/resources.

I did not really investigate, I just fear we have this class of
issues, since dpdk (and its sub components) is not instructed by ovs
how it placed its threads.
ovs has been doing this for some time, without people hitting bugs, so
I might just be paranoid.


> 2. Completely remove pdump support now without prior deprecation
>because it just doesn't work.

That is an alternative too.



--
David Marchand

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


Re: [ovs-dev] [PATCH v7 ovn 2/2] ovn-northd: Limit ARP/ND broadcast domain whenever possible.

2019-11-12 Thread Han Zhou
On Tue, Nov 12, 2019 at 10:10 AM Dumitru Ceara  wrote:
>
> On Tue, Nov 12, 2019 at 6:17 PM Han Zhou  wrote:
> >
> >
> >
> > On Tue, Nov 12, 2019 at 2:29 AM Dumitru Ceara  wrote:
> > >
> > > ARP request and ND NS packets for router owned IPs were being
> > > flooded in the complete L2 domain (using the MC_FLOOD multicast
group).
> > > However this creates a scaling issue in scenarios where aggregation
> > > logical switches are connected to more logical routers (~350). The
> > > logical pipelines of all routers would have to be executed before the
> > > packet is finally replied to by a single router, the owner of the IP
> > > address.
> > >
> > > This commit limits the broadcast domain by bypassing the L2 Lookup
stage
> > > for ARP requests that will be replied by a single router. The packets
> > > are forwarded only to the router port that owns the target IP address.
> > >
> > > IPs that are owned by the routers and for which this fix applies are:
> > > - IP addresses configured on the router ports.
> > > - VIPs.
> > > - NAT IPs.
> > >
> > > Reported-at: https://bugzilla.redhat.com/1756945
> > > Reported-by: Anil Venkata 
> > > Signed-off-by: Dumitru Ceara 
> > >
> > > ---
> > > v7:
> > > - Address Han's comments:
> > > - Remove flooding for all ARPs received on VLAN networks. To avoid
> > >   that we now identify self originated (G)ARPs by matching on
source
> > >   MAC address too.
> > > - Rename REGBIT_NOT_VXLAN to FLAGBIT_NOT_VXLAN.
> > > - Fix ovn-sb manpage.
> > > - Split patch in a series of 2:
> > > - patch1: fixes the get_router_load_balancer_ips() function.
> > > - patch2: limits the ARP/ND broadcast domain.
> > > v6:
> > > - Address Han's comments:
> > > - remove flooding of ARPs targeting OVN owned IP addresses.
> > > - update ovn-architecture documentation.
> > > - rename ARP handling functions.
> > > - Adapt "ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR" autotest to take
into
> > > account the new way of forwarding ARPs.
> > > - Also, properly deal with ARP packets on VLAN-backed networks.
> > > v5: Address Numan's comments: update comments & make autotest more
> > > robust.
> > > v4: Rebase.
> > > v3: Properly deal with VXLAN traffic. Address review comments from
> > > Numan (add autotests). Fix function get_router_load_balancer_ips.
> > > Rebase -> deal with IPv6 NAT too.
> > > v2: Move ARP broadcast domain limiting to table S_SWITCH_IN_L2_LKUP to
> > > address localnet ports too.
> > > ---
> > >  northd/ovn-northd.8.xml |   14 ++
> > >  northd/ovn-northd.c |  230 +++
> > >  ovn-architecture.7.xml  |   19 +++
> > >  tests/ovn.at|  307
+--
> > >  4 files changed, 530 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > > index 0a33dcd..344cc0d 100644
> > > --- a/northd/ovn-northd.8.xml
> > > +++ b/northd/ovn-northd.8.xml
> > > @@ -1005,6 +1005,20 @@ output;
> > >
> > >
> > >
> > > +Priority-80 flows for each port connected to a logical router
> > > +matching self originated GARP/ARP request/ND packets. These
packets
> > > +are flooded to the MC_FLOOD which contains all
logical
> > > +ports.
> > > +  
> > > +
> > > +  
> > > +Priority-75 flows for each IP address/VIP/NAT address owned
by a
> > > +router port connected to the switch. These flows match ARP
requests
> > > +and ND packets for the specific IP addresses.  Matched
packets are
> > > +forwarded only to the router that owns the IP address.
> > > +  
> > > +
> > > +  
> > >  A priority-70 flow that outputs all packets with an Ethernet
broadcast
> > >  or multicast eth.dst to the
MC_FLOOD
> > >  multicast group.
> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > > index 32f3200..d6beb97 100644
> > > --- a/northd/ovn-northd.c
> > > +++ b/northd/ovn-northd.c
> > > @@ -210,6 +210,8 @@ enum ovn_stage {
> > >  #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[4]"
> > >  #define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[5]"
> > >
> > > +#define FLAGBIT_NOT_VXLAN "flags[1] == 0"
> > > +
> > >  /* Returns an "enum ovn_stage" built from the arguments. */
> > >  static enum ovn_stage
> > >  ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline
pipeline,
> > > @@ -1202,6 +1204,34 @@ ovn_port_allocate_key(struct ovn_datapath *od)
> > >1, (1u << 15) - 1, >port_key_hint);
> > >  }
> > >
> > > +/* Returns true if the logical switch port 'enabled' column is empty
or
> > > + * set to true.  Otherwise, returns false. */
> > > +static bool
> > > +lsp_is_enabled(const struct nbrec_logical_switch_port *lsp)
> > > +{
> > > +return !lsp->n_enabled || *lsp->enabled;
> > > +}
> > > +
> > > +/* Returns true only if the logical switch port 'up' column is set
to true.
> > > + * Otherwise, if the column is 

Re: [ovs-dev] [PATCH v3] dpdk: Migrate to the new pdump API.

2019-11-12 Thread Ilya Maximets
On 12.11.2019 19:51, Stokes, Ian wrote:
> 
> 
> On 11/12/2019 5:15 PM, David Marchand wrote:
>> On Tue, Nov 12, 2019 at 6:07 PM Stokes, Ian  wrote:
>>> On 11/11/2019 3:01 PM, Ilya Maximets wrote:
 DPDK commit 660098d61f57 ("pdump: use generic multi-process channel")
 switched pdump to use generic DPDK IPC instead of sockets.
 Old API was deprecated and removed.  Updating OVS code accordingly.

 Signed-off-by: Ilya Maximets 
>>>
>>> Thanks for the patch Ilya.
>>>
>>> I see compilation passing now on dpdk-latest with this applied.
>>>
>>> https://travis-ci.org/istokes/ovs/builds/610915636
>>>
>>> I still had issues with running PDUMP, but those issues are specific to
>>> PDUMP setup in my environment. A separate issue we can discuss further
>>> on the deprecation thread as it seems unrelated to this patch.
>>>
>>> @David, are you happy to ack the patch (I see some of the changes are
>>> from your side).
>>
>> I did not work on the crash I saw, but it was most likely a problem on my 
>> side.
>> This looks good to me.
> 
> From a some further testing on my side I'm also seeing a crash, specifically 
> OVS crashes out once packets are received. PDUMP is still running but 
> complains of being unable to communicate with the primary process and then 
> exits. Is this similar to what you saw?
> 
> @Ilya, by chance did you see anything like this?


Honestly, I never tried to use pdump, and I don't really want to try
preparing the setup for it (building ASLR disabled kernel and stuff).


> I'll investigate further myself tomorrow. I was going to hold off on the 
> merge in the meantime. Thoughts?

There are 2 options here:
1. Apply this patch and hope that DPDK will be fixed someday.
   + Optionally apply deprecation patch.
2. Completely remove pdump support now without prior deprecation
   because it just doesn't work.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3] travis: support ppc64le builds

2019-11-12 Thread Ilya Maximets
On 12.11.2019 18:57, dwilder wrote:
> On 2019-11-08 14:52, Ilya Maximets wrote:
>> On 06.11.2019 20:20, David Wilder wrote:
>>> Add support for travis-ci ppc64le builds.
>>>
>>> - Updated matrix in .travis.yml to include an arch: ppc64le build.
>>> - Move package install needed for 32bit builds to .travis/linux-prepare.sh.
>>>
>>> To keep the total build time at an acceptable level only a single build
>>> job is included in the matrix for ppc64le.
>>>
>>> A build report example can be found here [1]
>>> [0] 
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__travis-2Dci.org_=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=r0fBOs-21CKcV4kyZGnzh3fcjrpR8caYSl8K2i1St54=
>>>  [1] 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_djlwilder_ovs_builds_607851729=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=7t2rzVasH7Xq_R7jWkWZO9rkgm4KHMH-WavBzCRbF74=
>>>  Signed-off-by: David Wilder 
>>> ---
>>> Addressed review comments:
>>> - Cleaned up linux-prepare.sh (v2)
>>> - Switch from os: linux-ppc64le to arch: ppc64le (v3)
>>
>> What a wonderful world of undocumented features. :)
>>
>> Anyway, I just tried this patch and it fails for me because of missing pip:
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_igsilya_ovs_jobs_609402867=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=PF1oO_KkZFd_RRKToj6UBN2t2YhvTVE5XnVD1GF9u60=
>>
>> pip install --disable-pip-version-check --user six flake8 hacking
>> ./.travis/linux-prepare.sh: line 15: pip: command not found
>>
>> Restarting the job doesn't help.
>>
>> I'm wondering what is the base image and who controls preinstalled software?
>> Maybe it makes sense to hold on until travis will start supporting ppc64le
>> officially?
>>
>> Best regards, Ilya Maximets.
> 
> Unfortunately it not just ppc the arm64 image has the same issue :)  I added 
> a arm64 build on this attempt.
> 
> https://travis-ci.org/djlwilder/ovs/builds/610517731
> 
> I will attempt to adjust the package list.

I think, you could just report this to TravisCI support and see what they
will answer.  It'll be much better if they will just update their images.

And the good news that ppc64le is officially supported now:
https://blog.travis-ci.com/2019-11-12-multi-cpu-architecture-ibm-power-ibm-z

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3] dpdk: Migrate to the new pdump API.

2019-11-12 Thread Stokes, Ian




On 11/12/2019 5:15 PM, David Marchand wrote:

On Tue, Nov 12, 2019 at 6:07 PM Stokes, Ian  wrote:

On 11/11/2019 3:01 PM, Ilya Maximets wrote:

DPDK commit 660098d61f57 ("pdump: use generic multi-process channel")
switched pdump to use generic DPDK IPC instead of sockets.
Old API was deprecated and removed.  Updating OVS code accordingly.

Signed-off-by: Ilya Maximets 


Thanks for the patch Ilya.

I see compilation passing now on dpdk-latest with this applied.

https://travis-ci.org/istokes/ovs/builds/610915636

I still had issues with running PDUMP, but those issues are specific to
PDUMP setup in my environment. A separate issue we can discuss further
on the deprecation thread as it seems unrelated to this patch.

@David, are you happy to ack the patch (I see some of the changes are
from your side).


I did not work on the crash I saw, but it was most likely a problem on my side.
This looks good to me.


From a some further testing on my side I'm also seeing a crash, 
specifically OVS crashes out once packets are received. PDUMP is still 
running but complains of being unable to communicate with the primary 
process and then exits. Is this similar to what you saw?


@Ilya, by chance did you see anything like this? I'll investigate 
further myself tomorrow. I was going to hold off on the merge in the 
meantime. Thoughts?


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


Re: [ovs-dev] [PATCH net-next v3] net: openvswitch: add hash info to upcall

2019-11-12 Thread Tonghao Zhang
On Wed, Nov 13, 2019 at 2:06 AM Gregory Rose  wrote:
>
>
> On 11/12/2019 7:08 AM, xiangxia.m@gmail.com wrote:
> > From: Tonghao Zhang 
> >
> > When using the kernel datapath, the upcall don't
> > include skb hash info relatived. That will introduce
> > some problem, because the hash of skb is important
> > in kernel stack. For example, VXLAN module uses
> > it to select UDP src port. The tx queue selection
> > may also use the hash in stack.
> >
> > Hash is computed in different ways. Hash is random
> > for a TCP socket, and hash may be computed in hardware,
> > or software stack. Recalculation hash is not easy.
> >
> > Hash of TCP socket is computed:
> >tcp_v4_connect
> >  -> sk_set_txhash (is random)
> >
> >__tcp_transmit_skb
> >  -> skb_set_hash_from_sk
> >
> > There will be one upcall, without information of skb
> > hash, to ovs-vswitchd, for the first packet of a TCP
> > session. The rest packets will be processed in Open vSwitch
> > modules, hash kept. If this tcp session is forward to
> > VXLAN module, then the UDP src port of first tcp packet
> > is different from rest packets.
> >
> > TCP packets may come from the host or dockers, to Open vSwitch.
> > To fix it, we store the hash info to upcall, and restore hash
> > when packets sent back.
> >
> > +---+  +-+
> > |   Docker/VMs  |  | ovs-vswitchd|
> > ++--+  +-++--+
> >   |   ^|
> >   |   ||
> >   |   |  upcallv restore packet hash 
> > (not recalculate)
> >   | +-++--+
> >   |  tap netdev | |   vxlan module
> >   +---> +-->  Open vSwitch ko +-->
> > or internal type| |
> > +-+
> >
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364062.html
> > Signed-off-by: Tonghao Zhang 
> > ---
> > v3:
> > * add enum ovs_pkt_hash_types
> > * avoid duplicate call the skb_get_hash_raw.
> > * explain why we should fix this problem.
> > ---
> >   include/uapi/linux/openvswitch.h |  2 ++
> >   net/openvswitch/datapath.c   | 30 +-
> >   net/openvswitch/datapath.h   | 12 
> >   3 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/openvswitch.h 
> > b/include/uapi/linux/openvswitch.h
> > index 1887a451c388..e65407c1f366 100644
> > --- a/include/uapi/linux/openvswitch.h
> > +++ b/include/uapi/linux/openvswitch.h
> > @@ -170,6 +170,7 @@ enum ovs_packet_cmd {
> >* output port is actually a tunnel port. Contains the output tunnel key
> >* extracted from the packet as nested %OVS_TUNNEL_KEY_ATTR_* attributes.
> >* @OVS_PACKET_ATTR_MRU: Present for an %OVS_PACKET_CMD_ACTION and
> > + * @OVS_PACKET_ATTR_HASH: Packet hash info (e.g. hash, sw_hash and l4_hash 
> > in skb).
> >* @OVS_PACKET_ATTR_LEN: Packet size before truncation.
> >* %OVS_PACKET_ATTR_USERSPACE action specify the Maximum received fragment
> >* size.
> > @@ -190,6 +191,7 @@ enum ovs_packet_attr {
> >   OVS_PACKET_ATTR_PROBE,  /* Packet operation is a feature probe,
> >  error logging should be suppressed. */
> >   OVS_PACKET_ATTR_MRU,/* Maximum received IP fragment size. */
> > + OVS_PACKET_ATTR_HASH,   /* Packet hash. */
> >   OVS_PACKET_ATTR_LEN,/* Packet size before truncation. */
> >   __OVS_PACKET_ATTR_MAX
> >   };
>
> Why do you add the new enum before the last entry OVS_PACKET_ATTR_LEN
> instead of
> just adding it to the end of the list?
Should be at end of the list, but I run the "make check" without patch
for ovs-vswitchd,
There is not error. There is not a OVS_PACKET_ATTR_LEN test case ?
I will change the order of enum in next version, thanks.
> Just curious.
>
> - Greg
>
> > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> > index 2088619c03f0..b556cf62b77c 100644
> > --- a/net/openvswitch/datapath.c
> > +++ b/net/openvswitch/datapath.c
> > @@ -350,7 +350,8 @@ static size_t upcall_msg_size(const struct 
> > dp_upcall_info *upcall_info,
> >   size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
> >   + nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
> >   + nla_total_size(ovs_key_attr_size()) /* OVS_PACKET_ATTR_KEY 
> > */
> > - + nla_total_size(sizeof(unsigned int)); /* 
> > OVS_PACKET_ATTR_LEN */
> > + + nla_total_size(sizeof(unsigned int)) /* OVS_PACKET_ATTR_LEN 
> > */
> > + + nla_total_size(sizeof(u64)); /* OVS_PACKET_ATTR_HASH */
> >
> >   /* OVS_PACKET_ATTR_USERDATA */
> >   if (upcall_info->userdata)
> > @@ -393,6 +394,7 @@ static int 

Re: [ovs-dev] [PATCH v7 ovn 2/2] ovn-northd: Limit ARP/ND broadcast domain whenever possible.

2019-11-12 Thread Dumitru Ceara
On Tue, Nov 12, 2019 at 6:17 PM Han Zhou  wrote:
>
>
>
> On Tue, Nov 12, 2019 at 2:29 AM Dumitru Ceara  wrote:
> >
> > ARP request and ND NS packets for router owned IPs were being
> > flooded in the complete L2 domain (using the MC_FLOOD multicast group).
> > However this creates a scaling issue in scenarios where aggregation
> > logical switches are connected to more logical routers (~350). The
> > logical pipelines of all routers would have to be executed before the
> > packet is finally replied to by a single router, the owner of the IP
> > address.
> >
> > This commit limits the broadcast domain by bypassing the L2 Lookup stage
> > for ARP requests that will be replied by a single router. The packets
> > are forwarded only to the router port that owns the target IP address.
> >
> > IPs that are owned by the routers and for which this fix applies are:
> > - IP addresses configured on the router ports.
> > - VIPs.
> > - NAT IPs.
> >
> > Reported-at: https://bugzilla.redhat.com/1756945
> > Reported-by: Anil Venkata 
> > Signed-off-by: Dumitru Ceara 
> >
> > ---
> > v7:
> > - Address Han's comments:
> > - Remove flooding for all ARPs received on VLAN networks. To avoid
> >   that we now identify self originated (G)ARPs by matching on source
> >   MAC address too.
> > - Rename REGBIT_NOT_VXLAN to FLAGBIT_NOT_VXLAN.
> > - Fix ovn-sb manpage.
> > - Split patch in a series of 2:
> > - patch1: fixes the get_router_load_balancer_ips() function.
> > - patch2: limits the ARP/ND broadcast domain.
> > v6:
> > - Address Han's comments:
> > - remove flooding of ARPs targeting OVN owned IP addresses.
> > - update ovn-architecture documentation.
> > - rename ARP handling functions.
> > - Adapt "ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR" autotest to take into
> > account the new way of forwarding ARPs.
> > - Also, properly deal with ARP packets on VLAN-backed networks.
> > v5: Address Numan's comments: update comments & make autotest more
> > robust.
> > v4: Rebase.
> > v3: Properly deal with VXLAN traffic. Address review comments from
> > Numan (add autotests). Fix function get_router_load_balancer_ips.
> > Rebase -> deal with IPv6 NAT too.
> > v2: Move ARP broadcast domain limiting to table S_SWITCH_IN_L2_LKUP to
> > address localnet ports too.
> > ---
> >  northd/ovn-northd.8.xml |   14 ++
> >  northd/ovn-northd.c |  230 +++
> >  ovn-architecture.7.xml  |   19 +++
> >  tests/ovn.at|  307 
> > +--
> >  4 files changed, 530 insertions(+), 40 deletions(-)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index 0a33dcd..344cc0d 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1005,6 +1005,20 @@ output;
> >
> >
> >
> > +Priority-80 flows for each port connected to a logical router
> > +matching self originated GARP/ARP request/ND packets. These packets
> > +are flooded to the MC_FLOOD which contains all logical
> > +ports.
> > +  
> > +
> > +  
> > +Priority-75 flows for each IP address/VIP/NAT address owned by a
> > +router port connected to the switch. These flows match ARP requests
> > +and ND packets for the specific IP addresses.  Matched packets are
> > +forwarded only to the router that owns the IP address.
> > +  
> > +
> > +  
> >  A priority-70 flow that outputs all packets with an Ethernet 
> > broadcast
> >  or multicast eth.dst to the MC_FLOOD
> >  multicast group.
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 32f3200..d6beb97 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -210,6 +210,8 @@ enum ovn_stage {
> >  #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[4]"
> >  #define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[5]"
> >
> > +#define FLAGBIT_NOT_VXLAN "flags[1] == 0"
> > +
> >  /* Returns an "enum ovn_stage" built from the arguments. */
> >  static enum ovn_stage
> >  ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline pipeline,
> > @@ -1202,6 +1204,34 @@ ovn_port_allocate_key(struct ovn_datapath *od)
> >1, (1u << 15) - 1, >port_key_hint);
> >  }
> >
> > +/* Returns true if the logical switch port 'enabled' column is empty or
> > + * set to true.  Otherwise, returns false. */
> > +static bool
> > +lsp_is_enabled(const struct nbrec_logical_switch_port *lsp)
> > +{
> > +return !lsp->n_enabled || *lsp->enabled;
> > +}
> > +
> > +/* Returns true only if the logical switch port 'up' column is set to true.
> > + * Otherwise, if the column is not set or set to false, returns false. */
> > +static bool
> > +lsp_is_up(const struct nbrec_logical_switch_port *lsp)
> > +{
> > +return lsp->n_up && *lsp->up;
> > +}
> > +
> > +static bool
> > +lsp_is_external(const struct nbrec_logical_switch_port *nbsp)
> > +{
> > + 

Re: [ovs-dev] [PATCH net-next v3] net: openvswitch: add hash info to upcall

2019-11-12 Thread Gregory Rose



On 11/12/2019 7:08 AM, xiangxia.m@gmail.com wrote:

From: Tonghao Zhang 

When using the kernel datapath, the upcall don't
include skb hash info relatived. That will introduce
some problem, because the hash of skb is important
in kernel stack. For example, VXLAN module uses
it to select UDP src port. The tx queue selection
may also use the hash in stack.

Hash is computed in different ways. Hash is random
for a TCP socket, and hash may be computed in hardware,
or software stack. Recalculation hash is not easy.

Hash of TCP socket is computed:
   tcp_v4_connect
 -> sk_set_txhash (is random)

   __tcp_transmit_skb
 -> skb_set_hash_from_sk

There will be one upcall, without information of skb
hash, to ovs-vswitchd, for the first packet of a TCP
session. The rest packets will be processed in Open vSwitch
modules, hash kept. If this tcp session is forward to
VXLAN module, then the UDP src port of first tcp packet
is different from rest packets.

TCP packets may come from the host or dockers, to Open vSwitch.
To fix it, we store the hash info to upcall, and restore hash
when packets sent back.

+---+  +-+
|   Docker/VMs  |  | ovs-vswitchd|
++--+  +-++--+
  |   ^|
  |   ||
  |   |  upcallv restore packet hash (not 
recalculate)
  | +-++--+
  |  tap netdev | |   vxlan module
  +---> +-->  Open vSwitch ko +-->
or internal type| |
+-+

Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364062.html
Signed-off-by: Tonghao Zhang 
---
v3:
* add enum ovs_pkt_hash_types
* avoid duplicate call the skb_get_hash_raw.
* explain why we should fix this problem.
---
  include/uapi/linux/openvswitch.h |  2 ++
  net/openvswitch/datapath.c   | 30 +-
  net/openvswitch/datapath.h   | 12 
  3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 1887a451c388..e65407c1f366 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -170,6 +170,7 @@ enum ovs_packet_cmd {
   * output port is actually a tunnel port. Contains the output tunnel key
   * extracted from the packet as nested %OVS_TUNNEL_KEY_ATTR_* attributes.
   * @OVS_PACKET_ATTR_MRU: Present for an %OVS_PACKET_CMD_ACTION and
+ * @OVS_PACKET_ATTR_HASH: Packet hash info (e.g. hash, sw_hash and l4_hash in 
skb).
   * @OVS_PACKET_ATTR_LEN: Packet size before truncation.
   * %OVS_PACKET_ATTR_USERSPACE action specify the Maximum received fragment
   * size.
@@ -190,6 +191,7 @@ enum ovs_packet_attr {
OVS_PACKET_ATTR_PROBE,  /* Packet operation is a feature probe,
   error logging should be suppressed. */
OVS_PACKET_ATTR_MRU,/* Maximum received IP fragment size. */
+   OVS_PACKET_ATTR_HASH,   /* Packet hash. */
OVS_PACKET_ATTR_LEN,/* Packet size before truncation. */
__OVS_PACKET_ATTR_MAX
  };


Why do you add the new enum before the last entry OVS_PACKET_ATTR_LEN 
instead of

just adding it to the end of the list?

Just curious.

- Greg


diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 2088619c03f0..b556cf62b77c 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -350,7 +350,8 @@ static size_t upcall_msg_size(const struct dp_upcall_info 
*upcall_info,
size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
+ nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
+ nla_total_size(ovs_key_attr_size()) /* OVS_PACKET_ATTR_KEY */
-   + nla_total_size(sizeof(unsigned int)); /* OVS_PACKET_ATTR_LEN 
*/
+   + nla_total_size(sizeof(unsigned int)) /* OVS_PACKET_ATTR_LEN */
+   + nla_total_size(sizeof(u64)); /* OVS_PACKET_ATTR_HASH */
  
  	/* OVS_PACKET_ATTR_USERDATA */

if (upcall_info->userdata)
@@ -393,6 +394,7 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
size_t len;
unsigned int hlen;
int err, dp_ifindex;
+   u64 hash;
  
  	dp_ifindex = get_dpifindex(dp);

if (!dp_ifindex)
@@ -504,6 +506,23 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
pad_packet(dp, user_skb);
}
  
+	hash = skb_get_hash_raw(skb);

+   if (hash) {
+   if (skb->sw_hash)
+   hash |= OVS_PACKET_HASH_SW_BIT;
+
+   if (skb->l4_hash)
+   hash |= OVS_PACKET_HASH_L4_BIT;
+
+   if 

Re: [ovs-dev] [PATCH v2.12] ovn: Prevent erroneous duplicate IP address messages.

2019-11-12 Thread 0-day Robot
Bleep bloop.  Greetings Mark Michelson, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 ovn: Prevent erroneous duplicate IP address messages.
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3] travis: support ppc64le builds

2019-11-12 Thread dwilder

On 2019-11-08 14:52, Ilya Maximets wrote:

On 06.11.2019 20:20, David Wilder wrote:

Add support for travis-ci ppc64le builds.

- Updated matrix in .travis.yml to include an arch: ppc64le build.
- Move package install needed for 32bit builds to 
.travis/linux-prepare.sh.


To keep the total build time at an acceptable level only a single 
build

job is included in the matrix for ppc64le.

A build report example can be found here [1]
[0] 
https://urldefense.proofpoint.com/v2/url?u=http-3A__travis-2Dci.org_=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=r0fBOs-21CKcV4kyZGnzh3fcjrpR8caYSl8K2i1St54= 
[1] 
https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_djlwilder_ovs_builds_607851729=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=7t2rzVasH7Xq_R7jWkWZO9rkgm4KHMH-WavBzCRbF74= 
Signed-off-by: David Wilder 

---
Addressed review comments:
- Cleaned up linux-prepare.sh (v2)
- Switch from os: linux-ppc64le to arch: ppc64le (v3)


What a wonderful world of undocumented features. :)

Anyway, I just tried this patch and it fails for me because of missing 
pip:


https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.org_igsilya_ovs_jobs_609402867=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbeAyz8i_vwCCaI=_UtshYcJsj4Pt3b9hfgEiyaEIT3j9gPEIgmBatCEqCo=PF1oO_KkZFd_RRKToj6UBN2t2YhvTVE5XnVD1GF9u60=

pip install --disable-pip-version-check --user six flake8 hacking
./.travis/linux-prepare.sh: line 15: pip: command not found

Restarting the job doesn't help.

I'm wondering what is the base image and who controls preinstalled 
software?
Maybe it makes sense to hold on until travis will start supporting 
ppc64le

officially?

Best regards, Ilya Maximets.


Unfortunately it not just ppc the arm64 image has the same issue :)  I 
added a arm64 build on this attempt.


https://travis-ci.org/djlwilder/ovs/builds/610517731

I will attempt to adjust the package list.



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


Re: [ovs-dev] [PATCH ovn v2 08/13] ovn-ic: Interconnection gateway controller.

2019-11-12 Thread Numan Siddique
On Thu, Oct 31, 2019 at 2:48 AM Han Zhou  wrote:
>
> Sync local and remote gateways between SB and ISB.
>
> Signed-off-by: Han Zhou 
> ---
>  ic/ovn-ic.c | 147 
> 
>  tests/ovn-ic.at |  56 +
>  2 files changed, 203 insertions(+)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index cf9bf96..025aad5 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -246,6 +246,152 @@ ts_run(struct ic_context *ctx)
>  shash_destroy(_dps);
>  }
>
> +/* Returns true if any information in gw and chassis is different. */
> +static bool
> +is_gateway_data_changed(const struct isbrec_gateway *gw,
> +   const struct sbrec_chassis *chassis)
> +{
> +if (strcmp(gw->hostname, chassis->hostname)) {
> +return true;
> +}
> +
> +if (gw->n_encaps != chassis->n_encaps) {
> +return true;
> +}
> +
> +for (int g = 0; g < gw->n_encaps; g++) {
> +
> +bool found = false;
> +const struct isbrec_encap *gw_encap = gw->encaps[g];
> +for (int s = 0; s < chassis->n_encaps; s++) {
> +const struct sbrec_encap *chassis_encap = chassis->encaps[s];
> +if (!strcmp(gw_encap->type, chassis_encap->type) &&
> +!strcmp(gw_encap->ip, chassis_encap->ip)) {
> +found = true;
> +if (!smap_equal(_encap->options, 
> _encap->options)) {
> +return true;
> +}
> +break;
> +}
> +}
> +if (!found) {
> +return true;
> +}
> +}
> +
> +return false;
> +}
> +
> +static void
> +sync_isb_gw_to_sb(struct ic_context *ctx,
> +  const struct isbrec_gateway *gw,
> +  const struct sbrec_chassis *chassis)
> +{
> +sbrec_chassis_set_hostname(chassis, gw->hostname);
> +sbrec_chassis_set_is_remote(chassis, true);
> +
> +/* Sync encaps used by this gateway. */
> +ovs_assert(gw->n_encaps);
> +struct sbrec_encap *sb_encap;
> +struct sbrec_encap **sb_encaps =
> +xmalloc(gw->n_encaps * sizeof *sb_encaps);
> +for (int i = 0; i < gw->n_encaps; i++) {
> +sb_encap = sbrec_encap_insert(ctx->ovnsb_txn);
> +sbrec_encap_set_chassis_name(sb_encap, gw->name);
> +sbrec_encap_set_ip(sb_encap, gw->encaps[i]->ip);
> +sbrec_encap_set_type(sb_encap, gw->encaps[i]->type);
> +sbrec_encap_set_options(sb_encap, >encaps[i]->options);
> +sb_encaps[i] = sb_encap;
> +}
> +sbrec_chassis_set_encaps(chassis, sb_encaps, gw->n_encaps);
> +free(sb_encaps);
> +}
> +
> +static void
> +sync_sb_gw_to_isb(struct ic_context *ctx,
> +  const struct sbrec_chassis *chassis,
> +  const struct isbrec_gateway *gw)
> +{
> +isbrec_gateway_set_hostname(gw, chassis->hostname);
> +
> +/* Sync encaps used by this chassis. */
> +ovs_assert(chassis->n_encaps);
> +struct isbrec_encap *isb_encap;
> +struct isbrec_encap **isb_encaps =
> +xmalloc(chassis->n_encaps * sizeof *isb_encaps);
> +for (int i = 0; i < chassis->n_encaps; i++) {
> +isb_encap = isbrec_encap_insert(ctx->ovnisb_txn);
> +isbrec_encap_set_gateway_name(isb_encap,
> +  chassis->name);
> +isbrec_encap_set_ip(isb_encap, chassis->encaps[i]->ip);
> +isbrec_encap_set_type(isb_encap,
> +  chassis->encaps[i]->type);
> +isbrec_encap_set_options(isb_encap,
> + >encaps[i]->options);
> +isb_encaps[i] = isb_encap;
> +}
> +isbrec_gateway_set_encaps(gw, isb_encaps,
> +  chassis->n_encaps);
> +free(isb_encaps);
> +}
> +
> +static void
> +gateway_run(struct ic_context *ctx, const struct isbrec_availability_zone 
> *az)
> +{
> +if (!ctx->ovnisb_txn || !ctx->ovnsb_txn) {
> +return;
> +}
> +
> +struct shash local_gws = SHASH_INITIALIZER(_gws);
> +struct shash remote_gws = SHASH_INITIALIZER(_gws);
> +const struct isbrec_gateway *gw;
> +ISBREC_GATEWAY_FOR_EACH (gw, ctx->ovnisb_idl) {
> +if (gw->availability_zone == az) {
> +shash_add(_gws, gw->name, gw);
> +} else {
> +shash_add(_gws, gw->name, gw);
> +}
> +}
> +
> +const struct sbrec_chassis *chassis;
> +SBREC_CHASSIS_FOR_EACH (chassis, ctx->ovnsb_idl) {
> +if (chassis->is_interconn) {
> +gw = shash_find_and_delete(_gws, chassis->name);
> +if (!gw) {
> +gw = isbrec_gateway_insert(ctx->ovnisb_txn);
> +isbrec_gateway_set_availability_zone(gw, az);
> +isbrec_gateway_set_name(gw, chassis->name);
> +sync_sb_gw_to_isb(ctx, chassis, gw);
> +} else if (is_gateway_data_changed(gw, chassis)) {
> +sync_sb_gw_to_isb(ctx, chassis, gw);
> +}
> + 

Re: [ovs-dev] [PATCH ovn] Require Python 3 and remove support for Python 2.

2019-11-12 Thread Numan Siddique
On Tue, Nov 12, 2019 at 10:49 PM Mark Michelson  wrote:
>
> Hi Numan,
>
> A couple of things:
>
> 1) It appears that configure will now fail if python3 is not found. Does
> this mean python is no longer optional when building?
I think so. To compile OVN we need to compile OVS and OVS fails anyway if
python3 is not present.

 If so, I think
> that needs to be reflected in the commit message. Also, if it's
> required, then I don't see the point of having the HAVE_PYTHON3 variable.

Sure. I will update the commit message and remove HAVE_PYTHON3 variable.

>
> 2) I think ovn-detrace requires some further changes. For instance, it
> uses the non-function version of "print". Dumitru is working on a change
> to ovn-detrace [1] that, as a side effect, is switching to using the
> print function instead. It's possible that if we get his change in
> first, then all you would need to do is to remove his "from future
> import print_function" line and things would just work.

Ok. I will take a look at ovn-detrace.

Thanks
Numan

>
> I'm not sure if there are other python2-isms present in ovn-detrace or
> other python utilities.
>
> On 11/12/19 11:31 AM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > OVS removed the support for Python 2 in the commit [1].
> > And its time we do the same for OVN.
> >
> > This patch takes care of removing Python 2 references.
> >
> > [1] - 1ca0323e7c29("Require Python 3 and remove support for Python 2.")
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >   Makefile.am | 12 ++--
> >   automake.mk |  8 +--
> >   build-aux/dpdkstrip.py  |  2 +-
> >   build-aux/sodepends.py  |  2 +-
> >   build-aux/soexpand.py   |  2 +-
> >   configure.ac|  2 -
> >   ipsec/ovs-monitor-ipsec.in  |  2 +-
> >   m4/ovn.m4   | 89 ++---
> >   rhel/ovn-fedora.spec.in | 35 +-
> >   tests/atlocal.in| 16 +
> >   tests/checkpatch.at |  2 -
> >   tests/ovn-controller-vtep.at|  1 -
> >   tests/ovn-northd.at | 10 ---
> >   tests/ovn.at| 62 -
> >   tests/ovsdb-macros.at   |  4 --
> >   tests/system-kmod-macros.at |  1 -
> >   tests/system-userspace-macros.at|  1 -
> >   utilities/bugtool/automake.mk   |  2 -
> >   utilities/ovn-detrace.in|  2 +-
> >   utilities/ovn-docker-overlay-driver.in  |  2 +-
> >   utilities/ovn-docker-underlay-driver.in |  2 +-
> >   21 files changed, 28 insertions(+), 231 deletions(-)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 1e41e49ea..8eed7a72b 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -71,7 +71,7 @@ endif
> >   # foo/__init__.pyc will cause Python to ignore foo.py.
> >   run_python = \
> >   PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH \
> > - PYTHONDONTWRITEBYTECODE=yes $(PYTHON)
> > + PYTHONDONTWRITEBYTECODE=yes $(PYTHON3)
> >
> >   ALL_LOCAL =
> >   BUILT_SOURCES =
> > @@ -165,13 +165,13 @@ ro_shell = printf '\043 Generated automatically -- do 
> > not modify!-*- buffer-
> >
> >   SUFFIXES += .in
> >   .in:
> > - 
> > $(AM_V_GEN)PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python
> >  $(PYTHON) $(srcdir)/build-aux/soexpand.py -I$(srcdir) -I$(OVS_SRCDIR) < $< 
> > | \
> > -   $(PYTHON) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
> > + 
> > $(AM_V_GEN)PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python
> >  $(PYTHON3) $(srcdir)/build-aux/soexpand.py -I$(srcdir) -I$(OVS_SRCDIR) < 
> > $< | \
> > +   $(PYTHON3) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
> > sed \
> >   -e 's,[@]PKIDIR[@],$(PKIDIR),g' \
> >   -e 's,[@]LOGDIR[@],$(LOGDIR),g' \
> >   -e 's,[@]DBDIR[@],$(DBDIR),g' \
> > - -e 's,[@]PYTHON[@],$(PYTHON),g' \
> > + -e 's,[@]PYTHON3[@],$(PYTHON3),g' \
> >   -e 's,[@]OVN_RUNDIR[@],$(OVN_RUNDIR),g' \
> >   -e 's,[@]OVSBUILDDIR[@],$(OVSBUILDDIR),g' \
> >   -e 's,[@]VERSION[@],$(VERSION),g' \
> > @@ -197,7 +197,7 @@ SUFFIXES += .xml
> > PKIDIR='$(PKIDIR)' \
> > LOGDIR='$(LOGDIR)' \
> > DBDIR='$(DBDIR)' \
> > -   PYTHON='$(PYTHON)' \
> > +   PYTHON3='$(PYTHON3)' \
> > RUNDIR='$(RUNDIR)' \
> > OVN_RUNDIR='$(OVN_RUNDIR)' \
> > VERSION='$(VERSION)' \
> > @@ -425,7 +425,7 @@ CLEANFILES += flake8-check
> >
> >   include $(srcdir)/manpages.mk
> >   $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py 
> > $(OVS_SRCDIR)/python/build/soutil.py
> > - 
> > @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python 
> > $(PYTHON) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR) 
> > $(MAN_ROOTS) 

[ovs-dev] CDMX: Planes de carrera y sucesión

2019-11-12 Thread Impacto que tiene en nuestra empresa
06 de Diciembre | Horario de 10:00 A 13:30 Y 15:00 A 18:30 HRS  |  (hora del 
centro de México) | PRESENCIAL

- CDMX: Planes de carrera y sucesión - 

¿De qué hablaremos?


La empresa moderna se enfrenta entre otras problemáticas a la rotación de 
personal. Los nuevos
colaboradores buscan crecimiento y desarrollo de vida en una organización.
El no disponer de un estudio de planes de carrera del personal puede suponer un 
factor de
desmotivación de los trabajadores/as, que ven limitada su capacidad de ascenso 
y mejora en las
condiciones de trabajo.

¿Qué aprenderás?:

- El participante identificará los lineamientos estratégicos para establecer 
planes de vida y carrera dentro de la organización.
- El participante conocerá los principales objetivos y finalidades de los 
planes de desarrollo dentro de una empresa.
- El participante definirá técnicas de valuación diagnóstica para identificar 
competencias clave.
- El participante revisará estrategias de entrenamiento para elevar el 
potencial de sus colaboradores.

Solicita información respondiendo a este correo con la palabra Planes, junto 
con los siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:

Desarrollaremos planes de carrera y sucesión que aseguren el funcionamiento 
óptimo de una empresa.

Números de Atención: 

(045) 55 15 54 66 30 - (045) 55 85567293 - (045) 5530167085 

En caso de que haya recibido este correo sin haberlo solicitado o si desea 
dejar de recibir nuestra promoción favor de responder con la palabra baja o 
enviar un correo a bajas@ innovalearn.net


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


Re: [ovs-dev] [PATCH ovn] Require Python 3 and remove support for Python 2.

2019-11-12 Thread Mark Michelson

Hi Numan,

A couple of things:

1) It appears that configure will now fail if python3 is not found. Does 
this mean python is no longer optional when building? If so, I think 
that needs to be reflected in the commit message. Also, if it's 
required, then I don't see the point of having the HAVE_PYTHON3 variable.


2) I think ovn-detrace requires some further changes. For instance, it 
uses the non-function version of "print". Dumitru is working on a change 
to ovn-detrace [1] that, as a side effect, is switching to using the 
print function instead. It's possible that if we get his change in 
first, then all you would need to do is to remove his "from future 
import print_function" line and things would just work.


I'm not sure if there are other python2-isms present in ovn-detrace or 
other python utilities.


On 11/12/19 11:31 AM, num...@ovn.org wrote:

From: Numan Siddique 

OVS removed the support for Python 2 in the commit [1].
And its time we do the same for OVN.

This patch takes care of removing Python 2 references.

[1] - 1ca0323e7c29("Require Python 3 and remove support for Python 2.")

Signed-off-by: Numan Siddique 
---
  Makefile.am | 12 ++--
  automake.mk |  8 +--
  build-aux/dpdkstrip.py  |  2 +-
  build-aux/sodepends.py  |  2 +-
  build-aux/soexpand.py   |  2 +-
  configure.ac|  2 -
  ipsec/ovs-monitor-ipsec.in  |  2 +-
  m4/ovn.m4   | 89 ++---
  rhel/ovn-fedora.spec.in | 35 +-
  tests/atlocal.in| 16 +
  tests/checkpatch.at |  2 -
  tests/ovn-controller-vtep.at|  1 -
  tests/ovn-northd.at | 10 ---
  tests/ovn.at| 62 -
  tests/ovsdb-macros.at   |  4 --
  tests/system-kmod-macros.at |  1 -
  tests/system-userspace-macros.at|  1 -
  utilities/bugtool/automake.mk   |  2 -
  utilities/ovn-detrace.in|  2 +-
  utilities/ovn-docker-overlay-driver.in  |  2 +-
  utilities/ovn-docker-underlay-driver.in |  2 +-
  21 files changed, 28 insertions(+), 231 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1e41e49ea..8eed7a72b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -71,7 +71,7 @@ endif
  # foo/__init__.pyc will cause Python to ignore foo.py.
  run_python = \
PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH \
-   PYTHONDONTWRITEBYTECODE=yes $(PYTHON)
+   PYTHONDONTWRITEBYTECODE=yes $(PYTHON3)
  
  ALL_LOCAL =

  BUILT_SOURCES =
@@ -165,13 +165,13 @@ ro_shell = printf '\043 Generated automatically -- do not 
modify!-*- buffer-
  
  SUFFIXES += .in

  .in:
-   
$(AM_V_GEN)PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python 
$(PYTHON) $(srcdir)/build-aux/soexpand.py -I$(srcdir) -I$(OVS_SRCDIR) < $< | \
- $(PYTHON) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
+   
$(AM_V_GEN)PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python 
$(PYTHON3) $(srcdir)/build-aux/soexpand.py -I$(srcdir) -I$(OVS_SRCDIR) < $< | \
+ $(PYTHON3) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
  sed \
-e 's,[@]PKIDIR[@],$(PKIDIR),g' \
-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
-e 's,[@]DBDIR[@],$(DBDIR),g' \
-   -e 's,[@]PYTHON[@],$(PYTHON),g' \
+   -e 's,[@]PYTHON3[@],$(PYTHON3),g' \
-e 's,[@]OVN_RUNDIR[@],$(OVN_RUNDIR),g' \
-e 's,[@]OVSBUILDDIR[@],$(OVSBUILDDIR),g' \
-e 's,[@]VERSION[@],$(VERSION),g' \
@@ -197,7 +197,7 @@ SUFFIXES += .xml
  PKIDIR='$(PKIDIR)' \
  LOGDIR='$(LOGDIR)' \
  DBDIR='$(DBDIR)' \
- PYTHON='$(PYTHON)' \
+ PYTHON3='$(PYTHON3)' \
  RUNDIR='$(RUNDIR)' \
  OVN_RUNDIR='$(OVN_RUNDIR)' \
  VERSION='$(VERSION)' \
@@ -425,7 +425,7 @@ CLEANFILES += flake8-check
  
  include $(srcdir)/manpages.mk

  $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py 
$(OVS_SRCDIR)/python/build/soutil.py
-   @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python 
$(PYTHON) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR) 
$(MAN_ROOTS) >$(@F).tmp
+   @PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python 
$(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR) 
$(MAN_ROOTS) >$(@F).tmp
@if cmp -s $(@F).tmp $@; then \
  touch $@; \
  rm -f $(@F).tmp; \
diff --git a/automake.mk b/automake.mk
index ad801f1e5..591e00751 100644
--- a/automake.mk
+++ b/automake.mk
@@ -6,19 +6,17 @@ CLEANFILES += ovn-architecture.7
  #
  # If "python" or "dot" is not available, then we do not add graphical diagram
  # to the documentation.
-if HAVE_PYTHON
  if HAVE_DOT
  OVSDB_DOT = 

Re: [ovs-dev] [PATCH v7 ovn 2/2] ovn-northd: Limit ARP/ND broadcast domain whenever possible.

2019-11-12 Thread Han Zhou
On Tue, Nov 12, 2019 at 2:29 AM Dumitru Ceara  wrote:
>
> ARP request and ND NS packets for router owned IPs were being
> flooded in the complete L2 domain (using the MC_FLOOD multicast group).
> However this creates a scaling issue in scenarios where aggregation
> logical switches are connected to more logical routers (~350). The
> logical pipelines of all routers would have to be executed before the
> packet is finally replied to by a single router, the owner of the IP
> address.
>
> This commit limits the broadcast domain by bypassing the L2 Lookup stage
> for ARP requests that will be replied by a single router. The packets
> are forwarded only to the router port that owns the target IP address.
>
> IPs that are owned by the routers and for which this fix applies are:
> - IP addresses configured on the router ports.
> - VIPs.
> - NAT IPs.
>
> Reported-at: https://bugzilla.redhat.com/1756945
> Reported-by: Anil Venkata 
> Signed-off-by: Dumitru Ceara 
>
> ---
> v7:
> - Address Han's comments:
> - Remove flooding for all ARPs received on VLAN networks. To avoid
>   that we now identify self originated (G)ARPs by matching on source
>   MAC address too.
> - Rename REGBIT_NOT_VXLAN to FLAGBIT_NOT_VXLAN.
> - Fix ovn-sb manpage.
> - Split patch in a series of 2:
> - patch1: fixes the get_router_load_balancer_ips() function.
> - patch2: limits the ARP/ND broadcast domain.
> v6:
> - Address Han's comments:
> - remove flooding of ARPs targeting OVN owned IP addresses.
> - update ovn-architecture documentation.
> - rename ARP handling functions.
> - Adapt "ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR" autotest to take into
> account the new way of forwarding ARPs.
> - Also, properly deal with ARP packets on VLAN-backed networks.
> v5: Address Numan's comments: update comments & make autotest more
> robust.
> v4: Rebase.
> v3: Properly deal with VXLAN traffic. Address review comments from
> Numan (add autotests). Fix function get_router_load_balancer_ips.
> Rebase -> deal with IPv6 NAT too.
> v2: Move ARP broadcast domain limiting to table S_SWITCH_IN_L2_LKUP to
> address localnet ports too.
> ---
>  northd/ovn-northd.8.xml |   14 ++
>  northd/ovn-northd.c |  230 +++
>  ovn-architecture.7.xml  |   19 +++
>  tests/ovn.at|  307
+--
>  4 files changed, 530 insertions(+), 40 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 0a33dcd..344cc0d 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1005,6 +1005,20 @@ output;
>
>
>
> +Priority-80 flows for each port connected to a logical router
> +matching self originated GARP/ARP request/ND packets. These
packets
> +are flooded to the MC_FLOOD which contains all
logical
> +ports.
> +  
> +
> +  
> +Priority-75 flows for each IP address/VIP/NAT address owned by a
> +router port connected to the switch. These flows match ARP
requests
> +and ND packets for the specific IP addresses.  Matched packets
are
> +forwarded only to the router that owns the IP address.
> +  
> +
> +  
>  A priority-70 flow that outputs all packets with an Ethernet
broadcast
>  or multicast eth.dst to the MC_FLOOD
>  multicast group.
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 32f3200..d6beb97 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -210,6 +210,8 @@ enum ovn_stage {
>  #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[4]"
>  #define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[5]"
>
> +#define FLAGBIT_NOT_VXLAN "flags[1] == 0"
> +
>  /* Returns an "enum ovn_stage" built from the arguments. */
>  static enum ovn_stage
>  ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline
pipeline,
> @@ -1202,6 +1204,34 @@ ovn_port_allocate_key(struct ovn_datapath *od)
>1, (1u << 15) - 1, >port_key_hint);
>  }
>
> +/* Returns true if the logical switch port 'enabled' column is empty or
> + * set to true.  Otherwise, returns false. */
> +static bool
> +lsp_is_enabled(const struct nbrec_logical_switch_port *lsp)
> +{
> +return !lsp->n_enabled || *lsp->enabled;
> +}
> +
> +/* Returns true only if the logical switch port 'up' column is set to
true.
> + * Otherwise, if the column is not set or set to false, returns false. */
> +static bool
> +lsp_is_up(const struct nbrec_logical_switch_port *lsp)
> +{
> +return lsp->n_up && *lsp->up;
> +}
> +
> +static bool
> +lsp_is_external(const struct nbrec_logical_switch_port *nbsp)
> +{
> +return !strcmp(nbsp->type, "external");
> +}
> +
> +static bool
> +lrport_is_enabled(const struct nbrec_logical_router_port *lrport)
> +{
> +return !lrport->enabled || *lrport->enabled;
> +}
> +
>  static char *
>  chassis_redirect_name(const char *port_name)
>  {
> @@ -3750,28 +3780,6 @@ 

Re: [ovs-dev] [PATCH v3] dpdk: Migrate to the new pdump API.

2019-11-12 Thread David Marchand
On Tue, Nov 12, 2019 at 6:07 PM Stokes, Ian  wrote:
> On 11/11/2019 3:01 PM, Ilya Maximets wrote:
> > DPDK commit 660098d61f57 ("pdump: use generic multi-process channel")
> > switched pdump to use generic DPDK IPC instead of sockets.
> > Old API was deprecated and removed.  Updating OVS code accordingly.
> >
> > Signed-off-by: Ilya Maximets 
>
> Thanks for the patch Ilya.
>
> I see compilation passing now on dpdk-latest with this applied.
>
> https://travis-ci.org/istokes/ovs/builds/610915636
>
> I still had issues with running PDUMP, but those issues are specific to
> PDUMP setup in my environment. A separate issue we can discuss further
> on the deprecation thread as it seems unrelated to this patch.
>
> @David, are you happy to ack the patch (I see some of the changes are
> from your side).

I did not work on the crash I saw, but it was most likely a problem on my side.
This looks good to me.

Acked-by: David Marchand 


--
David Marchand

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


Re: [ovs-dev] [PATCH v3] dpdk: Migrate to the new pdump API.

2019-11-12 Thread Stokes, Ian




On 11/11/2019 3:01 PM, Ilya Maximets wrote:

DPDK commit 660098d61f57 ("pdump: use generic multi-process channel")
switched pdump to use generic DPDK IPC instead of sockets.
Old API was deprecated and removed.  Updating OVS code accordingly.

Signed-off-by: Ilya Maximets 


Thanks for the patch Ilya.

I see compilation passing now on dpdk-latest with this applied.

https://travis-ci.org/istokes/ovs/builds/610915636

I still had issues with running PDUMP, but those issues are specific to 
PDUMP setup in my environment. A separate issue we can discuss further 
on the deprecation thread as it seems unrelated to this patch.


@David, are you happy to ack the patch (I see some of the changes are 
from your side).


Thanks
Ian

---

Version 3:
   * Added note about XDG_RUNTIME_DIR.

Version 2:
   * Removed unneeded deinitialization on error.
   * Docs updated.

  Documentation/topics/dpdk/pdump.rst | 13 +++--
  lib/dpdk.c  | 11 +--
  2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/Documentation/topics/dpdk/pdump.rst 
b/Documentation/topics/dpdk/pdump.rst
index 7bd1d3e9f..affd98371 100644
--- a/Documentation/topics/dpdk/pdump.rst
+++ b/Documentation/topics/dpdk/pdump.rst
@@ -41,8 +41,7 @@ To use pdump, simply launch OVS as usual, then navigate to 
the ``app/pdump``
  directory in DPDK, ``make`` the application and run like so::
  
  $ sudo ./build/app/dpdk-pdump -- \

---pdump port=0,queue=0,rx-dev=/tmp/pkts.pcap \
---server-socket-path=/usr/local/var/run/openvswitch
+--pdump port=0,queue=0,rx-dev=/tmp/pkts.pcap
  
  The above command captures traffic received on queue 0 of port 0 and stores it

  in ``/tmp/pkts.pcap``. Other combinations of port numbers, queues numbers and
@@ -50,11 +49,13 @@ pcap locations are of course also available to use. For 
example, to capture all
  packets that traverse port 0 in a single pcap file::
  
  $ sudo ./build/app/dpdk-pdump -- \

---pdump 'port=0,queue=*,rx-dev=/tmp/pkts.pcap,tx-dev=/tmp/pkts.pcap' \
---server-socket-path=/usr/local/var/run/openvswitch
+--pdump 'port=0,queue=*,rx-dev=/tmp/pkts.pcap,tx-dev=/tmp/pkts.pcap'
  
-``server-socket-path`` must be set to the value of ``ovs_rundir()`` which

-typically resolves to ``/usr/local/var/run/openvswitch``.
+.. note::
+
+   ``XDG_RUNTIME_DIR`` environment variable might need to be adjusted to
+   OVS runtime directory (``/var/run/openvswitch`` in most cases) for
+   ``dpdk-pdump`` utility if OVS started by non-root user.
  
  Many tools are available to view the contents of the pcap file. Once example is

  tcpdump. Issue the following command to view the contents of ``pkts.pcap``::
diff --git a/lib/dpdk.c b/lib/dpdk.c
index f90cda75a..748f63d31 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -27,7 +27,6 @@
  #include 
  #include 
  #ifdef DPDK_PDUMP
-#include 
  #include 
  #endif
  
@@ -434,17 +433,9 @@ dpdk_init__(const struct smap *ovs_other_config)
  
  #ifdef DPDK_PDUMP

  VLOG_INFO("DPDK pdump packet capture enabled");
-err = rte_pdump_init(ovs_rundir());
+err = rte_pdump_init();
  if (err) {
  VLOG_INFO("Error initialising DPDK pdump");
-rte_pdump_uninit();
-} else {
-char *server_socket_path;
-
-server_socket_path = xasprintf("%s/%s", ovs_rundir(),
-   "pdump_server_socket");
-fatal_signal_add_file_to_unlink(server_socket_path);
-free(server_socket_path);
  }
  #endif
  


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


Re: [ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.

2019-11-12 Thread Han Zhou
On Tue, Nov 12, 2019 at 2:28 AM Dumitru Ceara  wrote:
>
> Function get_router_load_balancer_ips() was incorrectly returning a
> single address_family even though the IP set could contain a mix of
> IPv4 and IPv6 addresses.
>
> Signed-off-by: Dumitru Ceara 
> ---
>  northd/ovn-northd.c |  126
+--
>  1 file changed, 72 insertions(+), 54 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 2f0f501..32f3200 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -2184,7 +2184,7 @@ ip_address_and_port_from_lb_key(const char *key,
char **ip_address,
>
>  static void
>  get_router_load_balancer_ips(const struct ovn_datapath *od,
> - struct sset *all_ips, int *addr_family)
> + struct sset *all_ips_v4, struct sset
*all_ips_v6)
>  {
>  if (!od->nbr) {
>  return;
> @@ -2199,13 +2199,21 @@ get_router_load_balancer_ips(const struct
ovn_datapath *od,
>  /* node->key contains IP:port or just IP. */
>  char *ip_address = NULL;
>  uint16_t port;
> +int addr_family;
>
>  ip_address_and_port_from_lb_key(node->key, _address,
,
> -addr_family);
> +_family);
>  if (!ip_address) {
>  continue;
>  }
>
> +struct sset *all_ips;
> +if (addr_family == AF_INET) {
> +all_ips = all_ips_v4;
> +} else {
> +all_ips = all_ips_v6;
> +}
> +
>  if (!sset_contains(all_ips, ip_address)) {
>  sset_add(all_ips, ip_address);
>  }
> @@ -2299,17 +2307,22 @@ get_nat_addresses(const struct ovn_port *op,
size_t *n)
>  }
>  }
>
> -/* A set to hold all load-balancer vips. */
> -struct sset all_ips = SSET_INITIALIZER(_ips);
> -int addr_family;
> -get_router_load_balancer_ips(op->od, _ips, _family);
> +/* Two sets to hold all load-balancer vips. */
> +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
>
>  const char *ip_address;
> -SSET_FOR_EACH (ip_address, _ips) {
> +SSET_FOR_EACH (ip_address, _ips_v4) {
>  ds_put_format(_addresses, " %s", ip_address);
>  central_ip_address = true;
>  }
> -sset_destroy(_ips);
> +SSET_FOR_EACH (ip_address, _ips_v6) {
> +ds_put_format(_addresses, " %s", ip_address);
> +central_ip_address = true;
> +}
> +sset_destroy(_ips_v4);
> +sset_destroy(_ips_v6);
>
>  if (central_ip_address) {
>  /* Gratuitous ARP for centralized NAT rules on distributed
gateway
> @@ -6911,61 +6924,66 @@ build_lrouter_flows(struct hmap *datapaths,
struct hmap *ports,
>  }
>
>  /* A set to hold all load-balancer vips that need ARP responses.
*/
> -struct sset all_ips = SSET_INITIALIZER(_ips);
> -int addr_family;
> -get_router_load_balancer_ips(op->od, _ips, _family);
> +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
>
>  const char *ip_address;
> -SSET_FOR_EACH(ip_address, _ips) {
> +SSET_FOR_EACH (ip_address, _ips_v4) {
>  ds_clear();
> -if (addr_family == AF_INET) {
> -ds_put_format(,
> -  "inport == %s && arp.tpa == %s && arp.op
== 1",
> -  op->json_key, ip_address);
> -} else {
> -ds_put_format(,
> -  "inport == %s && nd_ns && nd.target == %s",
> -  op->json_key, ip_address);
> -}
> +ds_put_format(,
> +  "inport == %s && arp.tpa == %s && arp.op == 1",
> +  op->json_key, ip_address);
>
>  ds_clear();
> -if (addr_family == AF_INET) {
> -ds_put_format(,
> -"eth.dst = eth.src; "
> -"eth.src = %s; "
> -"arp.op = 2; /* ARP reply */ "
> -"arp.tha = arp.sha; "
> -"arp.sha = %s; "
> -"arp.tpa = arp.spa; "
> -"arp.spa = %s; "
> -"outport = %s; "
> -"flags.loopback = 1; "
> -"output;",
> -op->lrp_networks.ea_s,
> -op->lrp_networks.ea_s,
> -ip_address,
> -op->json_key);
> -} else {
> -ds_put_format(,
> -"nd_na { "
> -"eth.src = %s; "
> -"ip6.src = %s; "
> -"nd.target = 

Re: [ovs-dev] [PATCH v2.12] ovn: Prevent erroneous duplicate IP address messages.

2019-11-12 Thread Mark Michelson
In addition to applying this to 2.12, I am requesting a backport to 2.11 
as well. Thank you.


On 11/12/19 11:59 AM, Mark Michelson wrote:

This is a backport to OVS 2.12 of OVN master commit 21c29d5b0c.

When using dynamic address assignment for logical switches, OVN reserves
the first address in the subnet for the attached router port to use.

In commit 488d153ee87841c042af05bc0eb8b5481aaa98cf, the IPAM code was
modified to add assigned router port addresses to IPAM. The use case for
this was when a switch was joined to multiple routers, and all router
addresses were dynamically assigned.

However, that commit also made it so that when a router rightly claimed
the first address in the subnet, ovn-northd would issue a warning about
a duplicate IP address being set. This change fixes the issue by adding
a special case so that we don't add the router's IP address to IPAM if
it is the first address in the subnet. This prevents the warning message
from appearing.

Signed-off-by: Mark Michelson 
Acked-by: Numan Siddique 
Acked-by: Han ZHou 
---
  ovn/northd/ovn-northd.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 6c6de2afd..1c9164924 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1194,7 +1194,14 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct 
ovn_port *op)
  
  for (size_t i = 0; i < lrp_networks.n_ipv4_addrs; i++) {

  uint32_t ip = ntohl(lrp_networks.ipv4_addrs[i].addr);
-ipam_insert_ip(op->peer->od, ip);
+/* If the router has the first IP address of the subnet, don't add
+ * it to IPAM. We already added this when we initialized IPAM for
+ * the datapath. This will just result in an erroneous message
+ * about a duplicate IP address.
+ */
+if (ip != op->peer->od->ipam_info.start_ipv4) {
+ipam_insert_ip(op->peer->od, ip);
+}
  }
  
  destroy_lport_addresses(_networks);




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


[ovs-dev] [PATCH v2.12] ovn: Prevent erroneous duplicate IP address messages.

2019-11-12 Thread Mark Michelson
This is a backport to OVS 2.12 of OVN master commit 21c29d5b0c.

When using dynamic address assignment for logical switches, OVN reserves
the first address in the subnet for the attached router port to use.

In commit 488d153ee87841c042af05bc0eb8b5481aaa98cf, the IPAM code was
modified to add assigned router port addresses to IPAM. The use case for
this was when a switch was joined to multiple routers, and all router
addresses were dynamically assigned.

However, that commit also made it so that when a router rightly claimed
the first address in the subnet, ovn-northd would issue a warning about
a duplicate IP address being set. This change fixes the issue by adding
a special case so that we don't add the router's IP address to IPAM if
it is the first address in the subnet. This prevents the warning message
from appearing.

Signed-off-by: Mark Michelson 
Acked-by: Numan Siddique 
Acked-by: Han ZHou 
---
 ovn/northd/ovn-northd.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 6c6de2afd..1c9164924 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1194,7 +1194,14 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct 
ovn_port *op)
 
 for (size_t i = 0; i < lrp_networks.n_ipv4_addrs; i++) {
 uint32_t ip = ntohl(lrp_networks.ipv4_addrs[i].addr);
-ipam_insert_ip(op->peer->od, ip);
+/* If the router has the first IP address of the subnet, don't add
+ * it to IPAM. We already added this when we initialized IPAM for
+ * the datapath. This will just result in an erroneous message
+ * about a duplicate IP address.
+ */
+if (ip != op->peer->od->ipam_info.start_ipv4) {
+ipam_insert_ip(op->peer->od, ip);
+}
 }
 
 destroy_lport_addresses(_networks);
-- 
2.14.5

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


Re: [ovs-dev] [PATCH ovn 3/3] ovn-detrace: Add support for other types of SB cookies.

2019-11-12 Thread Dumitru Ceara
On Tue, Nov 12, 2019 at 5:21 PM Dumitru Ceara  wrote:
>
> On Mon, Nov 11, 2019 at 8:02 PM Mark Michelson  wrote:
> >
> > Hi Dumitru. Everything you've done looks good to me.
> >
> > However, when reviewing the patch, I educated myself on how ovn-detrace
> > is implemented and I found a couple of problems. These problems aren't
> > introduced by you. However, I think that since you have broadened the
> > scope of the southbound database search area, they should probably be
> > addressed.
>
> Hi Mark,
>
> Thanks for looking into this.
>
> I addressed your remarks and sent a v2:
> https://patchwork.ozlabs.org/project/openvswitch/list/?series=142383
>
> I also refactored a bit more the printing so we don't have to
> explicitly specify the 'prefix' every time.

Sorry for the noise.. I had forgotten a stray "%s" in v2. Fixed in v3:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=142396

Thanks,
Dumitru

>
> Thanks,
> Dumitru
>
> >
> > On 11/8/19 6:15 AM, Dumitru Ceara wrote:
> > > Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow
> > > translations.") added cookies for Port_Binding, Mac_Binding,
> > > Multicast_Group, Chassis records to the OpenfFlow entries they
> > > generate.
> > >
> > > Add support for parsing such cookies in ovn-detrace too.
> > > Also, refactor ovn-detrace to allow a more generic way of defining
> > > cookie handlers.
> > >
> > > Signed-off-by: Dumitru Ceara 
> > > ---
> > >   utilities/ovn-detrace.in |  166 
> > > --
> > >   1 file changed, 117 insertions(+), 49 deletions(-)
> > >
> > > diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
> > > index 34b6b0e..79c6d3b 100755
> > > --- a/utilities/ovn-detrace.in
> > > +++ b/utilities/ovn-detrace.in
> > > @@ -98,48 +98,112 @@ class OVSDB(object):
> > >   def find_row_by_partial_uuid(self, table_name, value):
> > >   return self._find_row(table_name, lambda row: value in 
> > > str(row.uuid))
> >
> > (Note: this problem existed prior to the patch series)
> >
> > Since the cookie corresponds to the beginning of the southbound record's
> > UUID, this lambda should probably be altered to:
> >
> > lambda row: str(row.uuid).startswith(value)
> >
> > The existing lambda could match the cookie with a later part of the
> > UUID, returning an invalid match.
> >
> > >
> > > -
> > > -def get_lflow_from_cookie(ovnsb_db, cookie):
> > > -return ovnsb_db.find_row_by_partial_uuid('Logical_Flow', cookie)
> > > -
> > > -
> > > -def print_lflow(lflow, prefix):
> > > -ldp_uuid = lflow.logical_datapath.uuid
> > > -ldp_name = str(lflow.logical_datapath.external_ids.get('name'))
> > > -
> > > -print '%sLogical datapath: "%s" (%s) [%s]' % (prefix,
> > > -  ldp_name,
> > > -  ldp_uuid,
> > > -  lflow.pipeline)
> > > -print "%sLogical flow: table=%s (%s), priority=%s, " \
> > > -  "match=(%s), actions=(%s)" % (prefix,
> > > -lflow.table_id,
> > > -
> > > lflow.external_ids.get('stage-name'),
> > > -lflow.priority,
> > > -str(lflow.match).strip('"'),
> > > -str(lflow.actions).strip('"'))
> > > -
> > > -
> > > -def print_lflow_nb_hint(lflow, prefix, ovnnb_db):
> > > -external_ids = lflow.external_ids
> > > -if external_ids.get('stage-name') in ['ls_in_acl',
> > > -  'ls_out_acl']:
> > > -acl_hint = external_ids.get('stage-hint')
> > > -if not acl_hint:
> > > -return
> > > -acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint)
> > > -if not acl:
> > > +class CookieHandler(object):
> > > +def __init__(self, db, table):
> > > +self._db = db
> > > +self._table = table
> > > +
> > > +def get_record(self, cookie):
> > > +return self._db.find_row_by_partial_uuid(self._table, cookie)
> > > +
> > > +def print_record(self, record, prefix):
> > > +pass
> > > +
> > > +def print_hint(self, record, prefix, db):
> > > +pass
> > > +
> > > +def datapath_str(datapath):
> > > +return '"%s" (%s)' % (str(datapath.external_ids.get('name')),
> > > +  datapath.uuid)
> > > +
> > > +def chassis_str(chassis):
> > > +if len(chassis) == 0:
> > > +return ''
> > > +ch = chassis[0]
> > > +return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname)
> > > +
> > > +class LogicalFlowHandler(CookieHandler):
> > > +def __init__(self, ovnsb_db):
> > > +super(LogicalFlowHandler, self).__init__(ovnsb_db, 
> > > 'Logical_Flow')
> > > +
> > > +def print_record(self, lflow, prefix):
> > > +print('%sLogical datapath: %s [%s]' 

[ovs-dev] [PATCH v3 ovn 3/3] ovn-detrace: Add support for other types of SB cookies.

2019-11-12 Thread Dumitru Ceara
Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow
translations.") added cookies for Port_Binding, Mac_Binding,
Multicast_Group, Chassis records to the OpenfFlow entries they
generate.

Add support for parsing such cookies in ovn-detrace too.
Also:
- refactor ovn-detrace to allow a more generic way of defining
  cookie handlers.
- properly handle potential collisions between cookie -> UUID
  mappings.
- change print statements to print() calls.
- add custom printing functions to simplify prefixing outputs.

Signed-off-by: Dumitru Ceara 
---
 utilities/ovn-detrace.in |  197 --
 1 file changed, 135 insertions(+), 62 deletions(-)

diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
index 34b6b0e..5a43d03 100755
--- a/utilities/ovn-detrace.in
+++ b/utilities/ovn-detrace.in
@@ -14,6 +14,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from __future__ import print_function
+
+import functools
 import getopt
 import os
 import re
@@ -37,7 +40,7 @@ argv0 = sys.argv[0]
 
 
 def usage():
-print """\
+print("""\
 %(argv0)s:
 usage: %(argv0)s < FILE
 where FILE is output from ovs-appctl ofproto/trace.
@@ -47,9 +50,21 @@ The following options are also available:
   -V, --version   display version information
   --ovnsb=DATABASEuse DATABASE as southbound DB
   --ovnnb=DATABASEuse DATABASE as northbound DB\
-""" % {'argv0': argv0}
+""" % {'argv0': argv0})
 sys.exit(0)
 
+print_p = functools.partial(print, '  * ')
+print_h = functools.partial(print, '   * ')
+
+def datapath_str(datapath):
+return '"%s" (%s)' % (str(datapath.external_ids.get('name')),
+  datapath.uuid)
+
+def chassis_str(chassis):
+if len(chassis) == 0:
+return ''
+ch = chassis[0]
+return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname)
 
 class OVSDB(object):
 @staticmethod
@@ -87,59 +102,112 @@ class OVSDB(object):
 def get_table(self, table_name):
 return self._idl_conn.tables[table_name]
 
-def _find_row(self, table_name, find):
-return next(
-(row for row in self.get_table(table_name).rows.values()
- if find(row)), None)
+def _find_row(self, table_name, find_fn):
+return filter(find_fn, self.get_table(table_name).rows.values())
 
-def _find_row_by_name(self, table_name, value):
+def _find_rows_by_name(self, table_name, value):
 return self._find_row(table_name, lambda row: row.name == value)
 
-def find_row_by_partial_uuid(self, table_name, value):
-return self._find_row(table_name, lambda row: value in str(row.uuid))
-
-
-def get_lflow_from_cookie(ovnsb_db, cookie):
-return ovnsb_db.find_row_by_partial_uuid('Logical_Flow', cookie)
-
-
-def print_lflow(lflow, prefix):
-ldp_uuid = lflow.logical_datapath.uuid
-ldp_name = str(lflow.logical_datapath.external_ids.get('name'))
-
-print '%sLogical datapath: "%s" (%s) [%s]' % (prefix,
-  ldp_name,
-  ldp_uuid,
-  lflow.pipeline)
-print "%sLogical flow: table=%s (%s), priority=%s, " \
-  "match=(%s), actions=(%s)" % (prefix,
-lflow.table_id,
-lflow.external_ids.get('stage-name'),
-lflow.priority,
-str(lflow.match).strip('"'),
-str(lflow.actions).strip('"'))
-
-
-def print_lflow_nb_hint(lflow, prefix, ovnnb_db):
-external_ids = lflow.external_ids
-if external_ids.get('stage-name') in ['ls_in_acl',
-  'ls_out_acl']:
-acl_hint = external_ids.get('stage-hint')
-if not acl_hint:
-return
-acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint)
-if not acl:
-return
-output = "%sACL: %s, priority=%s, " \
- "match=(%s), %s" % (prefix,
- acl.direction,
- acl.priority,
- acl.match.strip('"'),
- acl.action)
-if acl.log:
-output += ' (log)'
-print output
-
+def find_rows_by_partial_uuid(self, table_name, value):
+return self._find_row(table_name,
+  lambda row: str(row.uuid).startswith(value))
+
+class CookieHandler(object):
+def __init__(self, db, table):
+self._db = db
+self._table = table
+
+def get_records(self, cookie):
+# Adjust cookie to include leading zeroes if needed.
+cookie = cookie.zfill(8)
+return 

[ovs-dev] [PATCH v3 ovn 2/3] ovn-detrace: Fix line parsing.

2019-11-12 Thread Dumitru Ceara
The script was never parsing the first cookie in the input. Also, add a
check to make sure that the cookie refers to a Logical_Flow before
trying to print the record.

Signed-off-by: Dumitru Ceara 
---
 utilities/ovn-detrace.in |   24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
index 9471e37..34b6b0e 100755
--- a/utilities/ovn-detrace.in
+++ b/utilities/ovn-detrace.in
@@ -188,22 +188,26 @@ def main():
 cookie = None
 while True:
 line = sys.stdin.readline()
+
+if line == '':
+break
+
+line = line.strip()
+
 if cookie:
 # print lflow info when the current flow block ends
-if regex_table_id.match(line) or line.strip() == '':
+if regex_table_id.match(line):
 lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie)
-print_lflow(lflow, "  * ")
-print_lflow_nb_hint(lflow, "* ", ovsdb_ovnnb)
-cookie = None
+if lflow:
+print_lflow(lflow, "  * ")
+print_lflow_nb_hint(lflow, "* ", ovsdb_ovnnb)
+cookie = None
 
-print line.strip()
-if line == "":
-break
+print line
 
 m = regex_cookie.match(line)
-if not m:
-continue
-cookie = m.group(1)
+if m:
+cookie = m.group(1)
 
 
 if __name__ == "__main__":

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


[ovs-dev] [PATCH v3 ovn 1/3] ovn-detrace: Fix rundir.

2019-11-12 Thread Dumitru Ceara
After the separation of OVS and OVN rundirs, the ovn-detrace script
hasn't been updated to use the OVN rundir instead of the OVS one.

Signed-off-by: Dumitru Ceara 
---
 utilities/ovn-detrace.in |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
index c842adc..9471e37 100755
--- a/utilities/ovn-detrace.in
+++ b/utilities/ovn-detrace.in
@@ -169,16 +169,16 @@ def main():
  "(use --help for help)\n" % argv0)
 sys.exit(1)
 
-ovs_rundir = os.getenv('OVS_RUNDIR', '@RUNDIR@')
+ovn_rundir = os.getenv('OVN_RUNDIR', '@OVN_RUNDIR@')
 if not ovnsb_db:
 ovnsb_db = os.getenv('OVN_SB_DB')
 if not ovnsb_db:
-ovnsb_db = 'unix:%s/ovnsb_db.sock' % ovs_rundir
+ovnsb_db = 'unix:%s/ovnsb_db.sock' % ovn_rundir
 
 if not ovnnb_db:
 ovnnb_db = os.getenv('OVN_NB_DB')
 if not ovnnb_db:
-ovnnb_db = 'unix:%s/ovnnb_db.sock' % ovs_rundir
+ovnnb_db = 'unix:%s/ovnnb_db.sock' % ovn_rundir
 
 ovsdb_ovnsb = OVSDB(ovnsb_db, 'OVN_Southbound')
 ovsdb_ovnnb = OVSDB(ovnnb_db, 'OVN_Northbound')

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


[ovs-dev] [PATCH v3 ovn 0/3] Improve ovn-detrace support for parsing OpenFlow cookies.

2019-11-12 Thread Dumitru Ceara
Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow translations.")
added support for more types of cookies (partial SB uuids) that are stored
in OpenFlow entries created by ovn-controller.

The last patch in this series implements support for parsing all these
different cookies in ovn-detrace too.

The first patch is a bug fix required as ovn-detrace was broken after
moving OVN to its own separate rundir.

The second patch fixes line parsing in ovn-detrace.

CC: Han Zhou 
Signed-off-by: Dumitru Ceara 

Dumitru Ceara (3):
  ovn-detrace: Fix rundir.
  ovn-detrace: Fix line parsing.
  ovn-detrace: Add support for other types of SB cookies.


 utilities/ovn-detrace.in |  215 +++---
 1 file changed, 146 insertions(+), 69 deletions(-)


---
v3:
- Remove stray "%s".
- Rename pprint to print_p to avoid name clashes with the library function.
v2:
- Address Mark's comments:
  - properly handle potential collisions between cookie -> UUID mappings.
  - when looking up SB records by cookie, match on the first part of the
UUID.
- Further refactor ovn-detrace to simplify prefixing outputs.
- Change print statements to print() calls.

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


[ovs-dev] [PATCH ovn] Require Python 3 and remove support for Python 2.

2019-11-12 Thread numans
From: Numan Siddique 

OVS removed the support for Python 2 in the commit [1].
And its time we do the same for OVN.

This patch takes care of removing Python 2 references.

[1] - 1ca0323e7c29("Require Python 3 and remove support for Python 2.")

Signed-off-by: Numan Siddique 
---
 Makefile.am | 12 ++--
 automake.mk |  8 +--
 build-aux/dpdkstrip.py  |  2 +-
 build-aux/sodepends.py  |  2 +-
 build-aux/soexpand.py   |  2 +-
 configure.ac|  2 -
 ipsec/ovs-monitor-ipsec.in  |  2 +-
 m4/ovn.m4   | 89 ++---
 rhel/ovn-fedora.spec.in | 35 +-
 tests/atlocal.in| 16 +
 tests/checkpatch.at |  2 -
 tests/ovn-controller-vtep.at|  1 -
 tests/ovn-northd.at | 10 ---
 tests/ovn.at| 62 -
 tests/ovsdb-macros.at   |  4 --
 tests/system-kmod-macros.at |  1 -
 tests/system-userspace-macros.at|  1 -
 utilities/bugtool/automake.mk   |  2 -
 utilities/ovn-detrace.in|  2 +-
 utilities/ovn-docker-overlay-driver.in  |  2 +-
 utilities/ovn-docker-underlay-driver.in |  2 +-
 21 files changed, 28 insertions(+), 231 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1e41e49ea..8eed7a72b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -71,7 +71,7 @@ endif
 # foo/__init__.pyc will cause Python to ignore foo.py.
 run_python = \
PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH \
-   PYTHONDONTWRITEBYTECODE=yes $(PYTHON)
+   PYTHONDONTWRITEBYTECODE=yes $(PYTHON3)
 
 ALL_LOCAL =
 BUILT_SOURCES =
@@ -165,13 +165,13 @@ ro_shell = printf '\043 Generated automatically -- do not 
modify!-*- buffer-
 
 SUFFIXES += .in
 .in:
-   
$(AM_V_GEN)PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python
 $(PYTHON) $(srcdir)/build-aux/soexpand.py -I$(srcdir) -I$(OVS_SRCDIR) < $< | \
- $(PYTHON) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
+   
$(AM_V_GEN)PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python
 $(PYTHON3) $(srcdir)/build-aux/soexpand.py -I$(srcdir) -I$(OVS_SRCDIR) < $< | \
+ $(PYTHON3) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
  sed \
-e 's,[@]PKIDIR[@],$(PKIDIR),g' \
-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
-e 's,[@]DBDIR[@],$(DBDIR),g' \
-   -e 's,[@]PYTHON[@],$(PYTHON),g' \
+   -e 's,[@]PYTHON3[@],$(PYTHON3),g' \
-e 's,[@]OVN_RUNDIR[@],$(OVN_RUNDIR),g' \
-e 's,[@]OVSBUILDDIR[@],$(OVSBUILDDIR),g' \
-e 's,[@]VERSION[@],$(VERSION),g' \
@@ -197,7 +197,7 @@ SUFFIXES += .xml
  PKIDIR='$(PKIDIR)' \
  LOGDIR='$(LOGDIR)' \
  DBDIR='$(DBDIR)' \
- PYTHON='$(PYTHON)' \
+ PYTHON3='$(PYTHON3)' \
  RUNDIR='$(RUNDIR)' \
  OVN_RUNDIR='$(OVN_RUNDIR)' \
  VERSION='$(VERSION)' \
@@ -425,7 +425,7 @@ CLEANFILES += flake8-check
 
 include $(srcdir)/manpages.mk
 $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py 
$(OVS_SRCDIR)/python/build/soutil.py
-   
@PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python 
$(PYTHON) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR) 
$(MAN_ROOTS) >$(@F).tmp
+   
@PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python 
$(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR) 
$(MAN_ROOTS) >$(@F).tmp
@if cmp -s $(@F).tmp $@; then \
  touch $@; \
  rm -f $(@F).tmp; \
diff --git a/automake.mk b/automake.mk
index ad801f1e5..591e00751 100644
--- a/automake.mk
+++ b/automake.mk
@@ -6,19 +6,17 @@ CLEANFILES += ovn-architecture.7
 #
 # If "python" or "dot" is not available, then we do not add graphical diagram
 # to the documentation.
-if HAVE_PYTHON
 if HAVE_DOT
 OVSDB_DOT = $(run_python) ${OVSDIR}/ovsdb/ovsdb-dot.in
 ovn-nb.gv: ${OVSDIR}/ovsdb/ovsdb-dot.in $(srcdir)/ovn-nb.ovsschema
$(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn-nb.ovsschema > $@
 ovn-nb.pic: ovn-nb.gv ${OVSDIR}/ovsdb/dot2pic
-   $(AM_V_GEN)(dot -T plain < ovn-nb.gv | $(PYTHON) 
${OVSDIR}/ovsdb/dot2pic -f 3) > $@.tmp && \
+   $(AM_V_GEN)(dot -T plain < ovn-nb.gv | $(PYTHON3) 
${OVSDIR}/ovsdb/dot2pic -f 3) > $@.tmp && \
mv $@.tmp $@
 OVN_NB_PIC = ovn-nb.pic
 OVN_NB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_NB_PIC)
 CLEANFILES += ovn-nb.gv ovn-nb.pic
 endif
-endif
 
 # OVN northbound schema documentation
 EXTRA_DIST += ovn-nb.xml
@@ -39,18 +37,16 @@ ovn-nb.5: \
 #
 # If "python" or "dot" is not available, then we do not add graphical diagram
 # to the documentation.
-if HAVE_PYTHON
 if HAVE_DOT
 ovn-sb.gv: ${OVSDIR}/ovsdb/ovsdb-dot.in $(srcdir)/ovn-sb.ovsschema

Re: [ovs-dev] [PATCH ovn v2 04/13] ovn-ic: Interconnection controller with AZ registeration.

2019-11-12 Thread Numan Siddique
On Tue, Nov 12, 2019 at 9:12 PM Han Zhou  wrote:
>
> On Tue, Nov 12, 2019 at 4:29 AM Numan Siddique  wrote:
> >
> > On Thu, Oct 31, 2019 at 2:47 AM Han Zhou  wrote:
> > >
> > > This patch introduces interconnection controller, ovn-ic, and
> > > implements the basic AZ registration feature: taking the AZ
> > > name from NB DB and create an Availability_Zone entry in
> > > IC-SB DB.
> > >
> > > Signed-off-by: Han Zhou 
> >
> > This patch doesn' apply on top of the patch 3 in this series.
> >
> > I am using your branch https://github.com/hzhou8/ovn/tree/ic2 for now.
> >
>
> I need a rebase again since ovn-nb.ovsschema is updated, but yes you can
> use the ic2 branch for now.
>
> >
> >
> > > ---
> > >  Makefile.am  |   1 +
> > >  ic/.gitignore|   2 +
> > >  ic/automake.mk   |  10 ++
> > >  ic/ovn-ic.8.xml  | 111 ++
> > >  ic/ovn-ic.c  | 421
> +++
> > >  ovn-nb.ovsschema |   5 +-
> > >  ovn-nb.xml   |   7 +
> > >  tests/automake.mk|   4 +-
> > >  tests/ovn-ic.at  |  29 
> > >  tests/ovn-macros.at  | 161 
> > >  tests/testsuite.at   |   1 +
> > >  tutorial/ovs-sandbox |  78 +-
> > >  12 files changed, 800 insertions(+), 30 deletions(-)
> > >  create mode 100644 ic/.gitignore
> > >  create mode 100644 ic/automake.mk
> > >  create mode 100644 ic/ovn-ic.8.xml
> > >  create mode 100644 ic/ovn-ic.c
> > >  create mode 100644 tests/ovn-ic.at
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 33c18c5..d22a220 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -500,4 +500,5 @@ include selinux/automake.mk
> > >  include controller/automake.mk
> > >  include controller-vtep/automake.mk
> > >  include northd/automake.mk
> > > +include ic/automake.mk
> > >  include build-aux/automake.mk
> > > diff --git a/ic/.gitignore b/ic/.gitignore
> > > new file mode 100644
> > > index 000..1b73eb4
> > > --- /dev/null
> > > +++ b/ic/.gitignore
> > > @@ -0,0 +1,2 @@
> > > +/ovn-ic
> > > +/ovn-ic.8
> > > diff --git a/ic/automake.mk b/ic/automake.mk
> > > new file mode 100644
> > > index 000..8e71bc3
> > > --- /dev/null
> > > +++ b/ic/automake.mk
> > > @@ -0,0 +1,10 @@
> > > +# ovn-ic
> > > +bin_PROGRAMS += ic/ovn-ic
> > > +ic_ovn_ic_SOURCES = ic/ovn-ic.c
> > > +ic_ovn_ic_LDADD = \
> > > +   lib/libovn.la \
> > > +   $(OVSDB_LIBDIR)/libovsdb.la \
> > > +   $(OVS_LIBDIR)/libopenvswitch.la
> > > +man_MANS += ic/ovn-ic.8
> > > +EXTRA_DIST += ic/ovn-ic.8.xml
> > > +CLEANFILES += ic/ovn-ic.8
> > > diff --git a/ic/ovn-ic.8.xml b/ic/ovn-ic.8.xml
> > > new file mode 100644
> > > index 000..00f33aa
> > > --- /dev/null
> > > +++ b/ic/ovn-ic.8.xml
> > > @@ -0,0 +1,111 @@
> > > +
> > > +
> > > +Name
> > > +ovn-ic -- Open Virtual Network interconnection controller
> > > +
> > > +Synopsis
> > > +ovn-ic [options]
> > > +
> > > +Description
> > > +
> > > +  ovn-ic, OVN interconnection controller, is a
> centralized
> > > +  daemon which communicates with global interconnection databases
> INB/ISB
> > > +  to configure and exchange data with local NB/SB for
> interconnecting
> > > +  with other OVN deployments.
> > > +
> > > +
> > > +Options
> > > +
> > > +  --ovnnb-db=database
> > > +  
> > > +The OVSDB database containing the OVN Northbound Database.  If
> the
> > > +OVN_NB_DB environment variable is set, its value is
> used
> > > +as the default.  Otherwise, the default is
> > > +unix:@RUNDIR@/ovnnb_db.sock.
> > > +  
> > > +  --ovnsb-db=database
> > > +  
> > > +The OVSDB database containing the OVN Southbound Database.  If
> the
> > > +OVN_SB_DB environment variable is set, its value is
> used
> > > +as the default.  Otherwise, the default is
> > > +unix:@RUNDIR@/ovnsb_db.sock.
> > > +  
> > > +  --ovninb-db=database
> > > +  
> > > +The OVSDB database containing the OVN Interconnection
> Northbound
> > > +Database.  If the OVN_INB_DB environment variable
> is set,
> > > +its value is used as the default.  Otherwise, the default is
> > > +unix:@RUNDIR@/ovninb_db.sock.
> > > +  
> > > +  --ovnisb-db=database
> > > +  
> > > +The OVSDB database containing the OVN Interconnection
> Southbound
> > > +Database.  If the OVN_ISB_DB environment variable
> is set,
> > > +its value is used as the default.  Otherwise, the default is
> > > +unix:@RUNDIR@/ovnisb_db.sock.
> > > +  
> > > +
> > > +
> > > +  database in the above options must be an OVSDB active
> or
> > > +  passive connection method, as described in ovsdb(7).
> > > +
> > > +
> > > +Daemon Options
> > > +http://www.w3.org/2003/XInclude"/>
> > > +
> > > +Logging Options
> > > +http://www.w3.org/2003/XInclude"/>
> > > +
> > > +PKI Options
> > > +

Re: [ovs-dev] [PATCH ovn 3/3] ovn-detrace: Add support for other types of SB cookies.

2019-11-12 Thread Dumitru Ceara
On Mon, Nov 11, 2019 at 8:02 PM Mark Michelson  wrote:
>
> Hi Dumitru. Everything you've done looks good to me.
>
> However, when reviewing the patch, I educated myself on how ovn-detrace
> is implemented and I found a couple of problems. These problems aren't
> introduced by you. However, I think that since you have broadened the
> scope of the southbound database search area, they should probably be
> addressed.

Hi Mark,

Thanks for looking into this.

I addressed your remarks and sent a v2:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=142383

I also refactored a bit more the printing so we don't have to
explicitly specify the 'prefix' every time.

Thanks,
Dumitru

>
> On 11/8/19 6:15 AM, Dumitru Ceara wrote:
> > Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow
> > translations.") added cookies for Port_Binding, Mac_Binding,
> > Multicast_Group, Chassis records to the OpenfFlow entries they
> > generate.
> >
> > Add support for parsing such cookies in ovn-detrace too.
> > Also, refactor ovn-detrace to allow a more generic way of defining
> > cookie handlers.
> >
> > Signed-off-by: Dumitru Ceara 
> > ---
> >   utilities/ovn-detrace.in |  166 
> > --
> >   1 file changed, 117 insertions(+), 49 deletions(-)
> >
> > diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
> > index 34b6b0e..79c6d3b 100755
> > --- a/utilities/ovn-detrace.in
> > +++ b/utilities/ovn-detrace.in
> > @@ -98,48 +98,112 @@ class OVSDB(object):
> >   def find_row_by_partial_uuid(self, table_name, value):
> >   return self._find_row(table_name, lambda row: value in 
> > str(row.uuid))
>
> (Note: this problem existed prior to the patch series)
>
> Since the cookie corresponds to the beginning of the southbound record's
> UUID, this lambda should probably be altered to:
>
> lambda row: str(row.uuid).startswith(value)
>
> The existing lambda could match the cookie with a later part of the
> UUID, returning an invalid match.
>
> >
> > -
> > -def get_lflow_from_cookie(ovnsb_db, cookie):
> > -return ovnsb_db.find_row_by_partial_uuid('Logical_Flow', cookie)
> > -
> > -
> > -def print_lflow(lflow, prefix):
> > -ldp_uuid = lflow.logical_datapath.uuid
> > -ldp_name = str(lflow.logical_datapath.external_ids.get('name'))
> > -
> > -print '%sLogical datapath: "%s" (%s) [%s]' % (prefix,
> > -  ldp_name,
> > -  ldp_uuid,
> > -  lflow.pipeline)
> > -print "%sLogical flow: table=%s (%s), priority=%s, " \
> > -  "match=(%s), actions=(%s)" % (prefix,
> > -lflow.table_id,
> > -
> > lflow.external_ids.get('stage-name'),
> > -lflow.priority,
> > -str(lflow.match).strip('"'),
> > -str(lflow.actions).strip('"'))
> > -
> > -
> > -def print_lflow_nb_hint(lflow, prefix, ovnnb_db):
> > -external_ids = lflow.external_ids
> > -if external_ids.get('stage-name') in ['ls_in_acl',
> > -  'ls_out_acl']:
> > -acl_hint = external_ids.get('stage-hint')
> > -if not acl_hint:
> > -return
> > -acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint)
> > -if not acl:
> > +class CookieHandler(object):
> > +def __init__(self, db, table):
> > +self._db = db
> > +self._table = table
> > +
> > +def get_record(self, cookie):
> > +return self._db.find_row_by_partial_uuid(self._table, cookie)
> > +
> > +def print_record(self, record, prefix):
> > +pass
> > +
> > +def print_hint(self, record, prefix, db):
> > +pass
> > +
> > +def datapath_str(datapath):
> > +return '"%s" (%s)' % (str(datapath.external_ids.get('name')),
> > +  datapath.uuid)
> > +
> > +def chassis_str(chassis):
> > +if len(chassis) == 0:
> > +return ''
> > +ch = chassis[0]
> > +return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname)
> > +
> > +class LogicalFlowHandler(CookieHandler):
> > +def __init__(self, ovnsb_db):
> > +super(LogicalFlowHandler, self).__init__(ovnsb_db, 'Logical_Flow')
> > +
> > +def print_record(self, lflow, prefix):
> > +print('%sLogical datapath: %s [%s]' %
> > +(prefix, datapath_str(lflow.logical_datapath), lflow.pipeline))
> > +print('%sLogical flow: table=%s (%s), priority=%s, '
> > +  'match=(%s), actions=(%s)' %
> > +(prefix, lflow.table_id, 
> > lflow.external_ids.get('stage-name'),
> > + lflow.priority,
> > + str(lflow.match).strip('"'),
> > + str(lflow.actions).strip('"')))
> > +
> > +def 

[ovs-dev] [PATCH v2 ovn 3/3] ovn-detrace: Add support for other types of SB cookies.

2019-11-12 Thread Dumitru Ceara
Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow
translations.") added cookies for Port_Binding, Mac_Binding,
Multicast_Group, Chassis records to the OpenfFlow entries they
generate.

Add support for parsing such cookies in ovn-detrace too.
Also:
- refactor ovn-detrace to allow a more generic way of defining
  cookie handlers.
- properly handle potential collisions between cookie -> UUID
  mappings.
- change print statements to print() calls.
- add custom printing functions to simplify prefixing outputs.

Signed-off-by: Dumitru Ceara 
---
 utilities/ovn-detrace.in |  197 --
 1 file changed, 135 insertions(+), 62 deletions(-)

diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
index 34b6b0e..00d162f 100755
--- a/utilities/ovn-detrace.in
+++ b/utilities/ovn-detrace.in
@@ -14,6 +14,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from __future__ import print_function
+
+import functools
 import getopt
 import os
 import re
@@ -37,7 +40,7 @@ argv0 = sys.argv[0]
 
 
 def usage():
-print """\
+print("""\
 %(argv0)s:
 usage: %(argv0)s < FILE
 where FILE is output from ovs-appctl ofproto/trace.
@@ -47,9 +50,21 @@ The following options are also available:
   -V, --version   display version information
   --ovnsb=DATABASEuse DATABASE as southbound DB
   --ovnnb=DATABASEuse DATABASE as northbound DB\
-""" % {'argv0': argv0}
+""" % {'argv0': argv0})
 sys.exit(0)
 
+pprint = functools.partial(print, '  * ')
+hprint = functools.partial(print, '   * ')
+
+def datapath_str(datapath):
+return '"%s" (%s)' % (str(datapath.external_ids.get('name')),
+  datapath.uuid)
+
+def chassis_str(chassis):
+if len(chassis) == 0:
+return ''
+ch = chassis[0]
+return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname)
 
 class OVSDB(object):
 @staticmethod
@@ -87,59 +102,112 @@ class OVSDB(object):
 def get_table(self, table_name):
 return self._idl_conn.tables[table_name]
 
-def _find_row(self, table_name, find):
-return next(
-(row for row in self.get_table(table_name).rows.values()
- if find(row)), None)
+def _find_row(self, table_name, find_fn):
+return filter(find_fn, self.get_table(table_name).rows.values())
 
-def _find_row_by_name(self, table_name, value):
+def _find_rows_by_name(self, table_name, value):
 return self._find_row(table_name, lambda row: row.name == value)
 
-def find_row_by_partial_uuid(self, table_name, value):
-return self._find_row(table_name, lambda row: value in str(row.uuid))
-
-
-def get_lflow_from_cookie(ovnsb_db, cookie):
-return ovnsb_db.find_row_by_partial_uuid('Logical_Flow', cookie)
-
-
-def print_lflow(lflow, prefix):
-ldp_uuid = lflow.logical_datapath.uuid
-ldp_name = str(lflow.logical_datapath.external_ids.get('name'))
-
-print '%sLogical datapath: "%s" (%s) [%s]' % (prefix,
-  ldp_name,
-  ldp_uuid,
-  lflow.pipeline)
-print "%sLogical flow: table=%s (%s), priority=%s, " \
-  "match=(%s), actions=(%s)" % (prefix,
-lflow.table_id,
-lflow.external_ids.get('stage-name'),
-lflow.priority,
-str(lflow.match).strip('"'),
-str(lflow.actions).strip('"'))
-
-
-def print_lflow_nb_hint(lflow, prefix, ovnnb_db):
-external_ids = lflow.external_ids
-if external_ids.get('stage-name') in ['ls_in_acl',
-  'ls_out_acl']:
-acl_hint = external_ids.get('stage-hint')
-if not acl_hint:
-return
-acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint)
-if not acl:
-return
-output = "%sACL: %s, priority=%s, " \
- "match=(%s), %s" % (prefix,
- acl.direction,
- acl.priority,
- acl.match.strip('"'),
- acl.action)
-if acl.log:
-output += ' (log)'
-print output
-
+def find_rows_by_partial_uuid(self, table_name, value):
+return self._find_row(table_name,
+  lambda row: str(row.uuid).startswith(value))
+
+class CookieHandler(object):
+def __init__(self, db, table):
+self._db = db
+self._table = table
+
+def get_records(self, cookie):
+# Adjust cookie to include leading zeroes if needed.
+cookie = cookie.zfill(8)
+return 

[ovs-dev] [PATCH v2 ovn 2/3] ovn-detrace: Fix line parsing.

2019-11-12 Thread Dumitru Ceara
The script was never parsing the first cookie in the input. Also, add a
check to make sure that the cookie refers to a Logical_Flow before
trying to print the record.

Signed-off-by: Dumitru Ceara 
---
 utilities/ovn-detrace.in |   24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
index 9471e37..34b6b0e 100755
--- a/utilities/ovn-detrace.in
+++ b/utilities/ovn-detrace.in
@@ -188,22 +188,26 @@ def main():
 cookie = None
 while True:
 line = sys.stdin.readline()
+
+if line == '':
+break
+
+line = line.strip()
+
 if cookie:
 # print lflow info when the current flow block ends
-if regex_table_id.match(line) or line.strip() == '':
+if regex_table_id.match(line):
 lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie)
-print_lflow(lflow, "  * ")
-print_lflow_nb_hint(lflow, "* ", ovsdb_ovnnb)
-cookie = None
+if lflow:
+print_lflow(lflow, "  * ")
+print_lflow_nb_hint(lflow, "* ", ovsdb_ovnnb)
+cookie = None
 
-print line.strip()
-if line == "":
-break
+print line
 
 m = regex_cookie.match(line)
-if not m:
-continue
-cookie = m.group(1)
+if m:
+cookie = m.group(1)
 
 
 if __name__ == "__main__":

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


[ovs-dev] [PATCH v2 ovn 1/3] ovn-detrace: Fix rundir.

2019-11-12 Thread Dumitru Ceara
After the separation of OVS and OVN rundirs, the ovn-detrace script
hasn't been updated to use the OVN rundir instead of the OVS one.

Signed-off-by: Dumitru Ceara 
---
 utilities/ovn-detrace.in |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
index c842adc..9471e37 100755
--- a/utilities/ovn-detrace.in
+++ b/utilities/ovn-detrace.in
@@ -169,16 +169,16 @@ def main():
  "(use --help for help)\n" % argv0)
 sys.exit(1)
 
-ovs_rundir = os.getenv('OVS_RUNDIR', '@RUNDIR@')
+ovn_rundir = os.getenv('OVN_RUNDIR', '@OVN_RUNDIR@')
 if not ovnsb_db:
 ovnsb_db = os.getenv('OVN_SB_DB')
 if not ovnsb_db:
-ovnsb_db = 'unix:%s/ovnsb_db.sock' % ovs_rundir
+ovnsb_db = 'unix:%s/ovnsb_db.sock' % ovn_rundir
 
 if not ovnnb_db:
 ovnnb_db = os.getenv('OVN_NB_DB')
 if not ovnnb_db:
-ovnnb_db = 'unix:%s/ovnnb_db.sock' % ovs_rundir
+ovnnb_db = 'unix:%s/ovnnb_db.sock' % ovn_rundir
 
 ovsdb_ovnsb = OVSDB(ovnsb_db, 'OVN_Southbound')
 ovsdb_ovnnb = OVSDB(ovnnb_db, 'OVN_Northbound')

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


[ovs-dev] [PATCH v2 ovn 0/3] Improve ovn-detrace support for parsing OpenFlow cookies.

2019-11-12 Thread Dumitru Ceara
Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow translations.")
added support for more types of cookies (partial SB uuids) that are stored
in OpenFlow entries created by ovn-controller.

The last patch in this series implements support for parsing all these
different cookies in ovn-detrace too.

The first patch is a bug fix required as ovn-detrace was broken after
moving OVN to its own separate rundir.

The second patch fixes line parsing in ovn-detrace.

CC: Han Zhou 
Signed-off-by: Dumitru Ceara 

Dumitru Ceara (3):
  ovn-detrace: Fix rundir.
  ovn-detrace: Fix line parsing.
  ovn-detrace: Add support for other types of SB cookies.


 utilities/ovn-detrace.in |  215 +++---
 1 file changed, 146 insertions(+), 69 deletions(-)


---
v2:
- Address Mark's comments:
  - properly handle potential collisions between cookie -> UUID mappings.
  - when looking up SB records by cookie, match on the first part of the
UUID.
- Further refactor ovn-detrace to simplify prefixing outputs.
- Change print statements to print() calls.

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


Re: [ovs-dev] [PATCH net-next] openvswitch: add TTL decrement action

2019-11-12 Thread Matteo Croce
On Tue, Nov 12, 2019 at 4:00 PM Simon Horman  wrote:
>
> On Tue, Nov 12, 2019 at 11:25:18AM +0100, Matteo Croce wrote:
> > New action to decrement TTL instead of setting it to a fixed value.
> > This action will decrement the TTL and, in case of expired TTL, send the
> > packet to userspace via output_userspace() to take care of it.
> >
> > Supports both IPv4 and IPv6 via the ttl and hop_limit fields, respectively.
> >
>
> Usually OVS achieves this behaviour by matching on the TTL and
> setting it to the desired value, pre-calculated as TTL -1.
> With that in mind could you explain the motivation for this
> change?
>

Hi,

the problem is that OVS creates a flow for each ttl it see. I can let
vswitchd create 255 flows with like this:

$ for i in {2..255}; do ping 192.168.0.2 -t $i -c1 -w1 &>/dev/null & done
$ ovs-dpctl dump-flows |fgrep -c 'set(ipv4(ttl'
255


> > @@ -1174,6 +1174,43 @@ static int execute_check_pkt_len(struct datapath 
> > *dp, struct sk_buff *skb,
> >nla_len(actions), last, clone_flow_key);
> >  }
> >
> > +static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key)
> > +{
> > + int err;
> > +
> > + if (skb->protocol == htons(ETH_P_IPV6)) {
> > + struct ipv6hdr *nh = ipv6_hdr(skb);
> > +
> > + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > +   sizeof(*nh));
> > + if (unlikely(err))
> > + return err;
> > +
> > + if (nh->hop_limit <= 1)
> > + return -EHOSTUNREACH;
> > +
> > + key->ip.ttl = --nh->hop_limit;
> > + } else {
> > + struct iphdr *nh = ip_hdr(skb);
> > + u8 old_ttl;
> > +
> > + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > +   sizeof(*nh));
> > + if (unlikely(err))
> > + return err;
> > +
> > + if (nh->ttl <= 1)
> > + return -EHOSTUNREACH;
> > +
> > + old_ttl = nh->ttl--;
> > + csum_replace2(>check, htons(old_ttl << 8),
> > +   htons(nh->ttl << 8));
> > + key->ip.ttl = nh->ttl;
> > + }
>
> The above may send packets with TTL = 0, is that desired?
>

If TTL is 1 or 0, execute_dec_ttl() returns -EHOSTUNREACH, and the
caller will just send the packet to userspace and then free it.
I think this is enough, am I missing something?

Bye,
-- 
Matteo Croce
per aspera ad upstream

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


Re: [ovs-dev] [PATCH ovn v2 01/13] ovn-architecture: Add documentation for OVN interconnection feature.

2019-11-12 Thread Han Zhou
Thanks Numan!

On Tue, Nov 12, 2019 at 4:23 AM Numan Siddique  wrote:
>
> Hi Han,
>
> I have started with reviewing the patches, but with skimming them
> faster so that I get the complete
> picture. So you might see some review comments now :)
>
>
> Thanks
> Numan
>
>
> On Thu, Oct 31, 2019 at 2:44 AM Han Zhou  wrote:
> >
> > Signed-off-by: Han Zhou 
> > ---
> >  ovn-architecture.7.xml | 107
-
> >  1 file changed, 106 insertions(+), 1 deletion(-)
> >
> > diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml
> > index 7966b65..56b2167 100644
> > --- a/ovn-architecture.7.xml
> > +++ b/ovn-architecture.7.xml
> > @@ -1246,7 +1246,14 @@
> >
> >  Distributed gateway ports are logical router patch ports
> >  that directly connect distributed logical routers to logical
> > -switches with localnet ports.
> > +switches with external connection.
> > +  
> > +
> > +  
> > +There are two types of external connections.  Firstly, connection
to
> > +physical network through a localnet port.  Secondly, connection to
> > +another OVN deployment, which will be introduced in section "OVN
> > +Deployments Interconnection".
> >
> >
> >
> > @@ -1801,6 +1808,104 @@
> >  
> >
> >
> > +  OVN Deployments Interconnection (TODO)
> > +
> > +  
> > +It is not uncommon for an operator to deploy multiple OVN
clusters, for
> > +two main reasons.  Firstly, an operator may prefer to deploy one
OVN
> > +cluster for each availability zone, e.g. in different physical
regions,
> > +to avoid single point of failure.  Secondly, there is always an
upper limit
> > +for a single OVN control plane to scale.
> > +  
> > +
> > +  
> > +Although the control planes of the different availability zone
(AZ)s are
> > +independent from each other, the workloads from different AZs may
need
> > +to communicate across the zones.  The OVN interconnection feature
provides
> > +a native way to interconnect different AZs by L3 routing through
transit
> > +overlay networks between logical routers of different AZs.
> > +  
> > +
> > +  
> > +A global OVN Interconnection Northbound database is introduced for
the
> > +operator (probably through CMS systems) to configure transit
logical
> > +switches that connect logical routers from different AZs.  A
transit
> > +switch is similar as a
> s/as a/to a
>
>
> regular logical switch, but it is used for
> > +interconnection purpose only.  Typically, each transit switch can
be used
> > +to connect all logical routers that belong to same tenant across
all AZs.
> > +  
> > +
> > +  
> > +A dedicated daemon process ovn-ic, OVN interconnection
> > +controller, in each AZ will consume this data and populate
corresponding
> > +logical switches to their own northbound databases for each AZ, so
that
> > +logical routers can be connected to the transit switch by creating
> > +patch port pairs in their northbound databases.  Any router ports
> > +connected to the transit switches are considered interconnection
ports,
> > +which will be exchanged between AZs.
> > +  
> > +
> > +  
> > +Physically, when workloads in from different AZs communicate,
packets
> > +need to go through multiple hops: source chassis, source gateway,
> > +destination gateway and destination chassis.  All these hops are
connected
> > +through tunnels so that the packets never leave overlay networks.
> > +A distributed gateway port is required to connect the logical
router to a
> > +transit switch, with a gateway chassis specified, so that the
traffic can
> > +be forwarded through the gateway chassis.
> > +  
> > +
> > +  
> > +A global OVN Interconnection Southbound database is introduced for
> > +exchanging control plane information between the AZs.  The data in
> > +this database is populated and consumed by the ovn-ic,
> > +of each AZ.  The main information in this database includes:
> > +  
> > +
> > +  
> > +
> > +  Datapath bindings for transit switches, which mainly contains
the tunnel
> > +  keys generated for each transit switch.  Separate key ranges are
reserved
> > +  for transit switches so that they will never conflict with any
tunnel
> > +  keys locally assigned for datapaths within each AZ.
> > +
> > +
> > +  Availability zones, which are registerd by ovn-ic
> > +  from each AZ.
> > +
> > +
> > +  Gateways.  Each AZ specifies chassises that are supposed to work
> > +  as interconnection gateways, and the ovn-ic will
> > +  populate this information to the interconnection southbound DB.
> > +  The ovn-ic from all the other AZs will learn the
> > +  gateways and populate to their own southbound DB as a chassis.
> > +
> > +
> > +  Port bindings for logical switch ports created on the transit
switch.
> > +  Each AZ maintains their 

Re: [ovs-dev] [PATCH ovn v2 13/13] tutorial: Add tutorial for OVN Interconnection.

2019-11-12 Thread Han Zhou
On Wed, Oct 30, 2019 at 2:13 PM Han Zhou  wrote:
>
> Added tutorial, and also updated NEWS and TODO.
>
> Tested-by: Aliasgar Ginwala 
> Signed-off-by: Han Zhou 
> ---
>  Documentation/automake.mk   |   1 +
>  Documentation/tutorials/index.rst   |   1 +
>  Documentation/tutorials/ovn-interconnection.rst | 188

>  NEWS|   5 +
>  TODO.rst|  10 ++
>  5 files changed, 205 insertions(+)
>  create mode 100644 Documentation/tutorials/ovn-interconnection.rst
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 5968d69..15d261d 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -20,6 +20,7 @@ DOC_SOURCE = \
> Documentation/tutorials/ovn-sandbox.rst \
> Documentation/tutorials/ovn-ipsec.rst \
> Documentation/tutorials/ovn-rbac.rst \
> +   Documentation/tutorials/ovn-interconnection.rst \
> Documentation/topics/index.rst \
> Documentation/topics/testing.rst \
> Documentation/topics/high-availability.rst \
> diff --git a/Documentation/tutorials/index.rst
b/Documentation/tutorials/index.rst
> index 1cf083e..4ff6e16 100644
> --- a/Documentation/tutorials/index.rst
> +++ b/Documentation/tutorials/index.rst
> @@ -43,3 +43,4 @@ vSwitch.
> ovn-openstack
> ovn-rbac
> ovn-ipsec
> +   ovn-interconnection
> diff --git a/Documentation/tutorials/ovn-interconnection.rst
b/Documentation/tutorials/ovn-interconnection.rst
> new file mode 100644
> index 000..681a6d6
> --- /dev/null
> +++ b/Documentation/tutorials/ovn-interconnection.rst
> @@ -0,0 +1,188 @@
> +..
> +  Licensed under the Apache License, Version 2.0 (the "License");
you may
> +  not use this file except in compliance with the License. You may
obtain
> +  a copy of the License at
> +
> +  http://www.apache.org/licenses/LICENSE-2.0
> +
> +  Unless required by applicable law or agreed to in writing, software
> +  distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT
> +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the
> +  License for the specific language governing permissions and
limitations
> +  under the License.
> +
> +  Convention for heading levels in OVN documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +===
> +OVN Interconnection
> +===
> +
> +This document provides a guide for interconnecting multiple OVN
deployements
> +with OVN managed tunneling.  More details about the OVN Interconnectiong
design
> +can be found in ``ovn-architecture``\(7) manpage.
> +
> +This document assumes two or more OVN deployments are setup and runs
normally,
> +possibly at different data-centers, and the gateway chassises of each OVN
> +are with IP addresses that are reachable between each other.
> +
> +Setup Interconnection Databases
> +---
> +
> +To interconnect different OVNs, you need to create global OVSDB
databases that
> +store interconnection data.  The databases can be setup on any nodes
that are
> +accessible from all the central nodes of each OVN deployment.  It is
> +recommended that the global databases are setup with HA, with nodes in
> +different avaialbility zones, to avoid single point of failure.
> +
> +1. Install OVN packages on each global database node.
> +
> +2. Start OVN IC-NB and IC-SB databases.
> +
> +   On each global database node ::
> +
> +$ ovn-ctl [options] start_ic_ovsdb
> +
> +   Options depends on the HA mode you use.  To start standalone mode
with TCP
> +   connections, use ::
> +
> +$ ovn-ctl --db-inb-create-insecure-remote=yes \
> +  --db-isb-create-insecure-remote=yes start_ic_ovsdb
> +
> +   This command starts IC database servers that accept both unix socket
and
> +   TCP connections.  For other modes, see more details with ::
> +
> +$ ovn-ctl --help.
> +
> +Register OVN to Interconnection Databases
> +-
> +
> +For each OVN deployment, set an availability zone name ::
> +
> +$ ovn-nbctl set NB_Global . name=
> +
> +The name should be unique across all OVN deployments, e.g. ovn-east,
> +ovn-west, etc.
> +
> +For each OVN deployment, start the ``ovn-ic`` daemon on central nodes ::
> +
> +$ ovn-ctl --ovn-ic-inb-db= --ovn-ic-isb-db= \
> +  --ovn-northd-nb-db= --ovn-northd-sb-db= [more
options] start_ic
> +
> +An example of  is ``tcp::6645``, or for
> +clustered DB: ``tcp::6645,tcp::6645,tcp::6645``.
> + is similar, but usually with a different port number,
typically,
> +6646.
> +
> +For  and , use same connection methods as for 

Re: [ovs-dev] [PATCH ovn v2 04/13] ovn-ic: Interconnection controller with AZ registeration.

2019-11-12 Thread Han Zhou
On Tue, Nov 12, 2019 at 4:29 AM Numan Siddique  wrote:
>
> On Thu, Oct 31, 2019 at 2:47 AM Han Zhou  wrote:
> >
> > This patch introduces interconnection controller, ovn-ic, and
> > implements the basic AZ registration feature: taking the AZ
> > name from NB DB and create an Availability_Zone entry in
> > IC-SB DB.
> >
> > Signed-off-by: Han Zhou 
>
> This patch doesn' apply on top of the patch 3 in this series.
>
> I am using your branch https://github.com/hzhou8/ovn/tree/ic2 for now.
>

I need a rebase again since ovn-nb.ovsschema is updated, but yes you can
use the ic2 branch for now.

>
>
> > ---
> >  Makefile.am  |   1 +
> >  ic/.gitignore|   2 +
> >  ic/automake.mk   |  10 ++
> >  ic/ovn-ic.8.xml  | 111 ++
> >  ic/ovn-ic.c  | 421
+++
> >  ovn-nb.ovsschema |   5 +-
> >  ovn-nb.xml   |   7 +
> >  tests/automake.mk|   4 +-
> >  tests/ovn-ic.at  |  29 
> >  tests/ovn-macros.at  | 161 
> >  tests/testsuite.at   |   1 +
> >  tutorial/ovs-sandbox |  78 +-
> >  12 files changed, 800 insertions(+), 30 deletions(-)
> >  create mode 100644 ic/.gitignore
> >  create mode 100644 ic/automake.mk
> >  create mode 100644 ic/ovn-ic.8.xml
> >  create mode 100644 ic/ovn-ic.c
> >  create mode 100644 tests/ovn-ic.at
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 33c18c5..d22a220 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -500,4 +500,5 @@ include selinux/automake.mk
> >  include controller/automake.mk
> >  include controller-vtep/automake.mk
> >  include northd/automake.mk
> > +include ic/automake.mk
> >  include build-aux/automake.mk
> > diff --git a/ic/.gitignore b/ic/.gitignore
> > new file mode 100644
> > index 000..1b73eb4
> > --- /dev/null
> > +++ b/ic/.gitignore
> > @@ -0,0 +1,2 @@
> > +/ovn-ic
> > +/ovn-ic.8
> > diff --git a/ic/automake.mk b/ic/automake.mk
> > new file mode 100644
> > index 000..8e71bc3
> > --- /dev/null
> > +++ b/ic/automake.mk
> > @@ -0,0 +1,10 @@
> > +# ovn-ic
> > +bin_PROGRAMS += ic/ovn-ic
> > +ic_ovn_ic_SOURCES = ic/ovn-ic.c
> > +ic_ovn_ic_LDADD = \
> > +   lib/libovn.la \
> > +   $(OVSDB_LIBDIR)/libovsdb.la \
> > +   $(OVS_LIBDIR)/libopenvswitch.la
> > +man_MANS += ic/ovn-ic.8
> > +EXTRA_DIST += ic/ovn-ic.8.xml
> > +CLEANFILES += ic/ovn-ic.8
> > diff --git a/ic/ovn-ic.8.xml b/ic/ovn-ic.8.xml
> > new file mode 100644
> > index 000..00f33aa
> > --- /dev/null
> > +++ b/ic/ovn-ic.8.xml
> > @@ -0,0 +1,111 @@
> > +
> > +
> > +Name
> > +ovn-ic -- Open Virtual Network interconnection controller
> > +
> > +Synopsis
> > +ovn-ic [options]
> > +
> > +Description
> > +
> > +  ovn-ic, OVN interconnection controller, is a
centralized
> > +  daemon which communicates with global interconnection databases
INB/ISB
> > +  to configure and exchange data with local NB/SB for
interconnecting
> > +  with other OVN deployments.
> > +
> > +
> > +Options
> > +
> > +  --ovnnb-db=database
> > +  
> > +The OVSDB database containing the OVN Northbound Database.  If
the
> > +OVN_NB_DB environment variable is set, its value is
used
> > +as the default.  Otherwise, the default is
> > +unix:@RUNDIR@/ovnnb_db.sock.
> > +  
> > +  --ovnsb-db=database
> > +  
> > +The OVSDB database containing the OVN Southbound Database.  If
the
> > +OVN_SB_DB environment variable is set, its value is
used
> > +as the default.  Otherwise, the default is
> > +unix:@RUNDIR@/ovnsb_db.sock.
> > +  
> > +  --ovninb-db=database
> > +  
> > +The OVSDB database containing the OVN Interconnection
Northbound
> > +Database.  If the OVN_INB_DB environment variable
is set,
> > +its value is used as the default.  Otherwise, the default is
> > +unix:@RUNDIR@/ovninb_db.sock.
> > +  
> > +  --ovnisb-db=database
> > +  
> > +The OVSDB database containing the OVN Interconnection
Southbound
> > +Database.  If the OVN_ISB_DB environment variable
is set,
> > +its value is used as the default.  Otherwise, the default is
> > +unix:@RUNDIR@/ovnisb_db.sock.
> > +  
> > +
> > +
> > +  database in the above options must be an OVSDB active
or
> > +  passive connection method, as described in ovsdb(7).
> > +
> > +
> > +Daemon Options
> > +http://www.w3.org/2003/XInclude"/>
> > +
> > +Logging Options
> > +http://www.w3.org/2003/XInclude"/>
> > +
> > +PKI Options
> > +
> > +  PKI configuration is required in order to use SSL for the
connections to
> > +  the Northbound and Southbound databases.
> > +
> > +http://www.w3.org/2003/XInclude"/>
> > +
> > +Other Options
> > + > + xmlns:xi="http://www.w3.org/2003/XInclude"/>
> > +
> > + > + xmlns:xi="http://www.w3.org/2003/XInclude"/>
> 

Re: [ovs-dev] [PATCH ovn v2 02/13] ovn-inb: Interconnection northbound DB schema and CLI.

2019-11-12 Thread Han Zhou
On Tue, Nov 12, 2019 at 4:24 AM Numan Siddique  wrote:
>
> This patch has some whitespace warning when applying the patch.
>
> Applying: ovn-inb: Interconnection northbound DB schema and CLI.
> .git/rebase-apply/patch:167: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
>
It is caused by the extra line at the end of  lib/automake.mk. It seem
checkpatch can't catch such warning. I will fix in next version.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH net-next v3] net: openvswitch: add hash info to upcall

2019-11-12 Thread xiangxia . m . yue
From: Tonghao Zhang 

When using the kernel datapath, the upcall don't
include skb hash info relatived. That will introduce
some problem, because the hash of skb is important
in kernel stack. For example, VXLAN module uses
it to select UDP src port. The tx queue selection
may also use the hash in stack.

Hash is computed in different ways. Hash is random
for a TCP socket, and hash may be computed in hardware,
or software stack. Recalculation hash is not easy.

Hash of TCP socket is computed:
  tcp_v4_connect
-> sk_set_txhash (is random)

  __tcp_transmit_skb
-> skb_set_hash_from_sk

There will be one upcall, without information of skb
hash, to ovs-vswitchd, for the first packet of a TCP
session. The rest packets will be processed in Open vSwitch
modules, hash kept. If this tcp session is forward to
VXLAN module, then the UDP src port of first tcp packet
is different from rest packets.

TCP packets may come from the host or dockers, to Open vSwitch.
To fix it, we store the hash info to upcall, and restore hash
when packets sent back.

+---+  +-+
|   Docker/VMs  |  | ovs-vswitchd|
++--+  +-++--+
 |   ^|
 |   ||
 |   |  upcallv restore packet hash (not 
recalculate)
 | +-++--+
 |  tap netdev | |   vxlan module
 +---> +-->  Open vSwitch ko +-->
   or internal type| |
   +-+

Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364062.html
Signed-off-by: Tonghao Zhang 
---
v3:
* add enum ovs_pkt_hash_types
* avoid duplicate call the skb_get_hash_raw.
* explain why we should fix this problem.
---
 include/uapi/linux/openvswitch.h |  2 ++
 net/openvswitch/datapath.c   | 30 +-
 net/openvswitch/datapath.h   | 12 
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 1887a451c388..e65407c1f366 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -170,6 +170,7 @@ enum ovs_packet_cmd {
  * output port is actually a tunnel port. Contains the output tunnel key
  * extracted from the packet as nested %OVS_TUNNEL_KEY_ATTR_* attributes.
  * @OVS_PACKET_ATTR_MRU: Present for an %OVS_PACKET_CMD_ACTION and
+ * @OVS_PACKET_ATTR_HASH: Packet hash info (e.g. hash, sw_hash and l4_hash in 
skb).
  * @OVS_PACKET_ATTR_LEN: Packet size before truncation.
  * %OVS_PACKET_ATTR_USERSPACE action specify the Maximum received fragment
  * size.
@@ -190,6 +191,7 @@ enum ovs_packet_attr {
OVS_PACKET_ATTR_PROBE,  /* Packet operation is a feature probe,
   error logging should be suppressed. */
OVS_PACKET_ATTR_MRU,/* Maximum received IP fragment size. */
+   OVS_PACKET_ATTR_HASH,   /* Packet hash. */
OVS_PACKET_ATTR_LEN,/* Packet size before truncation. */
__OVS_PACKET_ATTR_MAX
 };
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 2088619c03f0..b556cf62b77c 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -350,7 +350,8 @@ static size_t upcall_msg_size(const struct dp_upcall_info 
*upcall_info,
size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
+ nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
+ nla_total_size(ovs_key_attr_size()) /* OVS_PACKET_ATTR_KEY */
-   + nla_total_size(sizeof(unsigned int)); /* OVS_PACKET_ATTR_LEN 
*/
+   + nla_total_size(sizeof(unsigned int)) /* OVS_PACKET_ATTR_LEN */
+   + nla_total_size(sizeof(u64)); /* OVS_PACKET_ATTR_HASH */
 
/* OVS_PACKET_ATTR_USERDATA */
if (upcall_info->userdata)
@@ -393,6 +394,7 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
size_t len;
unsigned int hlen;
int err, dp_ifindex;
+   u64 hash;
 
dp_ifindex = get_dpifindex(dp);
if (!dp_ifindex)
@@ -504,6 +506,23 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
pad_packet(dp, user_skb);
}
 
+   hash = skb_get_hash_raw(skb);
+   if (hash) {
+   if (skb->sw_hash)
+   hash |= OVS_PACKET_HASH_SW_BIT;
+
+   if (skb->l4_hash)
+   hash |= OVS_PACKET_HASH_L4_BIT;
+
+   if (nla_put(user_skb, OVS_PACKET_ATTR_HASH,
+   sizeof (u64), )) {
+   err = -ENOBUFS;
+   goto out;
+   }
+
+   pad_packet(dp, user_skb);
+   }
+

Re: [ovs-dev] [PATCH net-next] openvswitch: add TTL decrement action

2019-11-12 Thread Simon Horman
On Tue, Nov 12, 2019 at 11:25:18AM +0100, Matteo Croce wrote:
> New action to decrement TTL instead of setting it to a fixed value.
> This action will decrement the TTL and, in case of expired TTL, send the
> packet to userspace via output_userspace() to take care of it.
> 
> Supports both IPv4 and IPv6 via the ttl and hop_limit fields, respectively.
> 
> Tested with a corresponding change in the userspace:
> 
> # ovs-dpctl dump-flows
> in_port(2),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, 
> actions:dec_ttl,1
> in_port(1),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, 
> actions:dec_ttl,2
> in_port(1),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, 
> actions:2
> in_port(2),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, 
> actions:1
> 
> # ping -c1 192.168.0.2 -t 42
> IP (tos 0x0, ttl 41, id 61647, offset 0, flags [DF], proto ICMP (1), 
> length 84)
> 192.168.0.1 > 192.168.0.2: ICMP echo request, id 386, seq 1, length 64
> # ping -c1 192.168.0.2 -t 120
> IP (tos 0x0, ttl 119, id 62070, offset 0, flags [DF], proto ICMP (1), 
> length 84)
> 192.168.0.1 > 192.168.0.2: ICMP echo request, id 388, seq 1, length 64
> # ping -c1 192.168.0.2 -t 1
> #
> 
> Co-authored-by: Bindiya Kurle 
> Signed-off-by: Bindiya Kurle 
> Signed-off-by: Matteo Croce 

Usually OVS achieves this behaviour by matching on the TTL and
setting it to the desired value, pre-calculated as TTL -1.
With that in mind could you explain the motivation for this
change?

> ---
>  include/uapi/linux/openvswitch.h |  2 ++
>  net/openvswitch/actions.c| 46 
>  net/openvswitch/flow_netlink.c   |  6 +
>  3 files changed, 54 insertions(+)
> 
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index 1887a451c388..a3bdb1ecd1e7 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -890,6 +890,7 @@ struct check_pkt_len_arg {
>   * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
>   * of actions if greater than the specified packet length, else execute
>   * another set of actions.
> + * @OVS_ACTION_ATTR_DEC_TTL: Decrement the IP TTL.
>   *
>   * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not 
> all
>   * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
> @@ -925,6 +926,7 @@ enum ovs_action_attr {
>   OVS_ACTION_ATTR_METER,/* u32 meter ID. */
>   OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*.  */
>   OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
> + OVS_ACTION_ATTR_DEC_TTL,  /* Decrement ttl action */
>  
>   __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>  * from userspace. */
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 12936c151cc0..077b7f309c93 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -1174,6 +1174,43 @@ static int execute_check_pkt_len(struct datapath *dp, 
> struct sk_buff *skb,
>nla_len(actions), last, clone_flow_key);
>  }
>  
> +static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> + int err;
> +
> + if (skb->protocol == htons(ETH_P_IPV6)) {
> + struct ipv6hdr *nh = ipv6_hdr(skb);
> +
> + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> +   sizeof(*nh));
> + if (unlikely(err))
> + return err;
> +
> + if (nh->hop_limit <= 1)
> + return -EHOSTUNREACH;
> +
> + key->ip.ttl = --nh->hop_limit;
> + } else {
> + struct iphdr *nh = ip_hdr(skb);
> + u8 old_ttl;
> +
> + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> +   sizeof(*nh));
> + if (unlikely(err))
> + return err;
> +
> + if (nh->ttl <= 1)
> + return -EHOSTUNREACH;
> +
> + old_ttl = nh->ttl--;
> + csum_replace2(>check, htons(old_ttl << 8),
> +   htons(nh->ttl << 8));
> + key->ip.ttl = nh->ttl;
> + }

The above may send packets with TTL = 0, is that desired?

> +
> + return 0;
> +}
> +
>  /* Execute a list of actions against 'skb'. */
>  static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
> struct sw_flow_key *key,
> @@ -1345,6 +1382,15 @@ static int do_execute_actions(struct datapath *dp, 
> struct sk_buff *skb,
>  
>   break;
>   }
> +
> + case OVS_ACTION_ATTR_DEC_TTL:
> + err = execute_dec_ttl(skb, key);
> + if (err == -EHOSTUNREACH) {
> + 

[ovs-dev] Hi! ovs-dev@openvswitch.org

2019-11-12 Thread Le Huy Hoa
We propose

Sending your commercial offer through the Contact us form which can be found on 
the sites in the contact partition. Feedback forms are filled in by our program 
and the captcha is solved. The profit of this method is that messages sent 
through feedback forms are whitelisted. This method improve the odds that your 
message will be open.

Our database contains more than 35 million sites around the world to which we 
can send your message.

The cost of one million messages 49 USD

FREE TEST mailing to any country of your choice.

(We also provide other services.
1. Mailing email message to corporate addresses of any country
2. Selling the email database of any country in the world)

This message is automatically generated. Use our contacts for communication.

Contact us.
Telegram - @FeedbackMessages
Skype – Feedback_Messages
Email - feedback.messa...@mail.ru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v2 04/13] ovn-ic: Interconnection controller with AZ registeration.

2019-11-12 Thread Numan Siddique
On Thu, Oct 31, 2019 at 2:47 AM Han Zhou  wrote:
>
> This patch introduces interconnection controller, ovn-ic, and
> implements the basic AZ registration feature: taking the AZ
> name from NB DB and create an Availability_Zone entry in
> IC-SB DB.
>
> Signed-off-by: Han Zhou 

This patch doesn' apply on top of the patch 3 in this series.

I am using your branch https://github.com/hzhou8/ovn/tree/ic2 for now.


> ---
>  Makefile.am  |   1 +
>  ic/.gitignore|   2 +
>  ic/automake.mk   |  10 ++
>  ic/ovn-ic.8.xml  | 111 ++
>  ic/ovn-ic.c  | 421 
> +++
>  ovn-nb.ovsschema |   5 +-
>  ovn-nb.xml   |   7 +
>  tests/automake.mk|   4 +-
>  tests/ovn-ic.at  |  29 
>  tests/ovn-macros.at  | 161 
>  tests/testsuite.at   |   1 +
>  tutorial/ovs-sandbox |  78 +-
>  12 files changed, 800 insertions(+), 30 deletions(-)
>  create mode 100644 ic/.gitignore
>  create mode 100644 ic/automake.mk
>  create mode 100644 ic/ovn-ic.8.xml
>  create mode 100644 ic/ovn-ic.c
>  create mode 100644 tests/ovn-ic.at
>
> diff --git a/Makefile.am b/Makefile.am
> index 33c18c5..d22a220 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -500,4 +500,5 @@ include selinux/automake.mk
>  include controller/automake.mk
>  include controller-vtep/automake.mk
>  include northd/automake.mk
> +include ic/automake.mk
>  include build-aux/automake.mk
> diff --git a/ic/.gitignore b/ic/.gitignore
> new file mode 100644
> index 000..1b73eb4
> --- /dev/null
> +++ b/ic/.gitignore
> @@ -0,0 +1,2 @@
> +/ovn-ic
> +/ovn-ic.8
> diff --git a/ic/automake.mk b/ic/automake.mk
> new file mode 100644
> index 000..8e71bc3
> --- /dev/null
> +++ b/ic/automake.mk
> @@ -0,0 +1,10 @@
> +# ovn-ic
> +bin_PROGRAMS += ic/ovn-ic
> +ic_ovn_ic_SOURCES = ic/ovn-ic.c
> +ic_ovn_ic_LDADD = \
> +   lib/libovn.la \
> +   $(OVSDB_LIBDIR)/libovsdb.la \
> +   $(OVS_LIBDIR)/libopenvswitch.la
> +man_MANS += ic/ovn-ic.8
> +EXTRA_DIST += ic/ovn-ic.8.xml
> +CLEANFILES += ic/ovn-ic.8
> diff --git a/ic/ovn-ic.8.xml b/ic/ovn-ic.8.xml
> new file mode 100644
> index 000..00f33aa
> --- /dev/null
> +++ b/ic/ovn-ic.8.xml
> @@ -0,0 +1,111 @@
> +
> +
> +Name
> +ovn-ic -- Open Virtual Network interconnection controller
> +
> +Synopsis
> +ovn-ic [options]
> +
> +Description
> +
> +  ovn-ic, OVN interconnection controller, is a centralized
> +  daemon which communicates with global interconnection databases INB/ISB
> +  to configure and exchange data with local NB/SB for interconnecting
> +  with other OVN deployments.
> +
> +
> +Options
> +
> +  --ovnnb-db=database
> +  
> +The OVSDB database containing the OVN Northbound Database.  If the
> +OVN_NB_DB environment variable is set, its value is used
> +as the default.  Otherwise, the default is
> +unix:@RUNDIR@/ovnnb_db.sock.
> +  
> +  --ovnsb-db=database
> +  
> +The OVSDB database containing the OVN Southbound Database.  If the
> +OVN_SB_DB environment variable is set, its value is used
> +as the default.  Otherwise, the default is
> +unix:@RUNDIR@/ovnsb_db.sock.
> +  
> +  --ovninb-db=database
> +  
> +The OVSDB database containing the OVN Interconnection Northbound
> +Database.  If the OVN_INB_DB environment variable is set,
> +its value is used as the default.  Otherwise, the default is
> +unix:@RUNDIR@/ovninb_db.sock.
> +  
> +  --ovnisb-db=database
> +  
> +The OVSDB database containing the OVN Interconnection Southbound
> +Database.  If the OVN_ISB_DB environment variable is set,
> +its value is used as the default.  Otherwise, the default is
> +unix:@RUNDIR@/ovnisb_db.sock.
> +  
> +
> +
> +  database in the above options must be an OVSDB active or
> +  passive connection method, as described in ovsdb(7).
> +
> +
> +Daemon Options
> + xmlns:xi="http://www.w3.org/2003/XInclude"/>
> +
> +Logging Options
> + xmlns:xi="http://www.w3.org/2003/XInclude"/>
> +
> +PKI Options
> +
> +  PKI configuration is required in order to use SSL for the connections 
> to
> +  the Northbound and Southbound databases.
> +
> + xmlns:xi="http://www.w3.org/2003/XInclude"/>
> +
> +Other Options
> + + xmlns:xi="http://www.w3.org/2003/XInclude"/>
> +
> + + xmlns:xi="http://www.w3.org/2003/XInclude"/>
> +
> +Runtime Management Commands
> +
> +  ovs-appctl can send commands to a running
> +  ovn-ic process.  The currently supported commands
> +  are described below.
> +  
> +  exit
> +  
> +Causes ovn-ic to gracefully terminate.
> +  
> +
> +  pause
> +  
> +Pauses the ovn-ic operation from processing any Northbound and
> +Southbound 

Re: [ovs-dev] [PATCH ovn v2 03/13] ovn-isb: Interconnection southbound DB schema and CLI.

2019-11-12 Thread Numan Siddique
This patch has some whitespace warning when applying the patch.

Applying: ovn-isb: Interconnection southbound DB schema and CLI.
.git/rebase-apply/patch:171: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

On Thu, Oct 31, 2019 at 2:48 AM Han Zhou  wrote:
>
> This patch introduces OVN_IC_Southbound DB schema and the CLI
> ovn-isbctl that manages the DB.
>
> Signed-off-by: Han Zhou 
> ---
>  .gitignore |3 +
>  automake.mk|   38 ++
>  debian/ovn-common.install  |1 +
>  debian/ovn-common.manpages |2 +
>  lib/.gitignore |3 +
>  lib/automake.mk|   17 +-
>  lib/ovn-isb-idl.ann|9 +
>  lib/ovn-util.c |   13 +
>  lib/ovn-util.h |1 +
>  ovn-isb.ovsschema  |  129 ++
>  ovn-isb.xml|  582 +
>  tests/automake.mk  |2 +
>  tests/ovn-isbctl.at|  112 +
>  tests/testsuite.at |1 +
>  utilities/.gitignore   |2 +
>  utilities/automake.mk  |8 +
>  utilities/ovn-isbctl.8.xml |  148 +++
>  utilities/ovn-isbctl.c | 1015 
> 
>  18 files changed, 2085 insertions(+), 1 deletion(-)
>  create mode 100644 lib/ovn-isb-idl.ann
>  create mode 100644 ovn-isb.ovsschema
>  create mode 100644 ovn-isb.xml
>  create mode 100644 tests/ovn-isbctl.at
>  create mode 100644 utilities/ovn-isbctl.8.xml
>  create mode 100644 utilities/ovn-isbctl.c
>
> diff --git a/.gitignore b/.gitignore
> index 1994937..66bb101 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -70,6 +70,9 @@
>  /ovn-inb.5
>  /ovn-inb.gv
>  /ovn-inb.pic
> +/ovn-isb.5
> +/ovn-isb.gv
> +/ovn-isb.pic
>  /package.m4
>  /stamp-h1
>  /_build-gcc
> diff --git a/automake.mk b/automake.mk
> index 3bfbf57..85574fc 100644
> --- a/automake.mk
> +++ b/automake.mk
> @@ -97,6 +97,37 @@ ovn-inb.5: \
> $(srcdir)/ovn-inb.xml > $@.tmp && \
> mv $@.tmp $@
>
> +# OVN interconnection southbound E-R diagram
> +#
> +# If "python" or "dot" is not available, then we do not add graphical diagram
> +# to the documentation.
> +if HAVE_PYTHON
> +if HAVE_DOT
> +ovn-isb.gv: ${OVSDIR}/ovsdb/ovsdb-dot.in $(srcdir)/ovn-isb.ovsschema
> +   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn-isb.ovsschema > $@
> +ovn-isb.pic: ovn-isb.gv ${OVSDIR}/ovsdb/dot2pic
> +   $(AM_V_GEN)(dot -T plain < ovn-isb.gv | $(PYTHON) 
> ${OVSDIR}/ovsdb/dot2pic -f 3) > $@.tmp && \
> +   mv $@.tmp $@
> +OVN_ISB_PIC = ovn-isb.pic
> +OVN_ISB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_ISB_PIC)
> +CLEANFILES += ovn-isb.gv ovn-isb.pic
> +endif
> +endif
> +
> +# OVN interconnection southbound schema documentation
> +EXTRA_DIST += ovn-isb.xml
> +CLEANFILES += ovn-isb.5
> +man_MANS += ovn-isb.5
> +
> +ovn-isb.5: \
> +   ${OVSDIR}/ovsdb/ovsdb-doc $(srcdir)/ovn-isb.xml 
> $(srcdir)/ovn-isb.ovsschema $(OVN_ISB_PIC)
> +   $(AM_V_GEN)$(OVSDB_DOC) \
> +   $(OVN_ISB_DOT_DIAGRAM_ARG) \
> +   --version=$(VERSION) \
> +   $(srcdir)/ovn-isb.ovsschema \
> +   $(srcdir)/ovn-isb.xml > $@.tmp && \
> +   mv $@.tmp $@
> +
>  # Version checking for ovn-nb.ovsschema.
>  ALL_LOCAL += ovn-nb.ovsschema.stamp
>  ovn-nb.ovsschema.stamp: ovn-nb.ovsschema
> @@ -114,8 +145,15 @@ ovn-inb.ovsschema.stamp: ovn-inb.ovsschema
> $(srcdir)/build-aux/cksum-schema-check $? $@
>  CLEANFILES += ovn-inb.ovsschema.stamp
>
> +# Version checking for ovn-isb.ovsschema.
> +ALL_LOCAL += ovn-isb.ovsschema.stamp
> +ovn-isb.ovsschema.stamp: ovn-isb.ovsschema
> +   $(srcdir)/build-aux/cksum-schema-check $? $@
> +CLEANFILES += ovn-isb.ovsschema.stamp
> +
>  pkgdata_DATA += ovn-nb.ovsschema
>  pkgdata_DATA += ovn-sb.ovsschema
>  pkgdata_DATA += ovn-inb.ovsschema
> +pkgdata_DATA += ovn-isb.ovsschema
>
>  CLEANFILES += ovn-sb.ovsschema.stamp
> diff --git a/debian/ovn-common.install b/debian/ovn-common.install
> index 9e9bcfb..8f8ab23 100644
> --- a/debian/ovn-common.install
> +++ b/debian/ovn-common.install
> @@ -1,6 +1,7 @@
>  usr/bin/ovn-nbctl
>  usr/bin/ovn-sbctl
>  usr/bin/ovn-inbctl
> +usr/bin/ovn-isbctl
>  usr/bin/ovn-trace
>  usr/bin/ovn-detrace
>  usr/share/openvswitch/scripts/ovn-ctl
> diff --git a/debian/ovn-common.manpages b/debian/ovn-common.manpages
> index 94325dd..895da8d 100644
> --- a/debian/ovn-common.manpages
> +++ b/debian/ovn-common.manpages
> @@ -2,9 +2,11 @@ ovn/ovn-architecture.7
>  ovn/ovn-nb.5
>  ovn/ovn-sb.5
>  ovn/ovn-inb.5
> +ovn/ovn-isb.5
>  ovn/utilities/ovn-ctl.8
>  ovn/utilities/ovn-nbctl.8
>  ovn/utilities/ovn-sbctl.8
>  ovn/utilities/ovn-inbctl.8
> +ovn/utilities/ovn-isbctl.8
>  ovn/utilities/ovn-trace.8
>  ovn/utilities/ovn-detrace.1
> diff --git a/lib/.gitignore b/lib/.gitignore
> index e5d9bf3..19cc9f5 100644
> --- a/lib/.gitignore
> +++ b/lib/.gitignore
> @@ -8,4 +8,7 @@
>  /ovn-inb-idl.c
>  /ovn-inb-idl.h
>  /ovn-inb-idl.ovsidl
> +/ovn-isb-idl.c
> +/ovn-isb-idl.h
> +/ovn-isb-idl.ovsidl

Re: [ovs-dev] [PATCH ovn v2 02/13] ovn-inb: Interconnection northbound DB schema and CLI.

2019-11-12 Thread Numan Siddique
This patch has some whitespace warning when applying the patch.

Applying: ovn-inb: Interconnection northbound DB schema and CLI.
.git/rebase-apply/patch:167: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

On Thu, Oct 31, 2019 at 2:45 AM Han Zhou  wrote:
>
> This patch introduces OVN_IC_Northbound DB schema and the CLI
> ovn-inbctl that manages the DB.
>
> Signed-off-by: Han Zhou 
> ---
>  .gitignore |   3 +
>  automake.mk|  37 ++
>  debian/ovn-common.install  |   1 +
>  debian/ovn-common.manpages |   2 +
>  lib/.gitignore |   3 +
>  lib/automake.mk|  17 +-
>  lib/ovn-inb-idl.ann|   9 +
>  lib/ovn-util.c |  13 +
>  lib/ovn-util.h |   1 +
>  ovn-inb.ovsschema  |  75 
>  ovn-inb.xml| 371 ++
>  tests/automake.mk  |   2 +
>  tests/ovn-inbctl.at|  65 
>  tests/testsuite.at |   1 +
>  utilities/.gitignore   |   2 +
>  utilities/automake.mk  |   8 +
>  utilities/ovn-inbctl.8.xml | 174 +
>  utilities/ovn-inbctl.c | 948 
> +
>  18 files changed, 1731 insertions(+), 1 deletion(-)
>  create mode 100644 lib/ovn-inb-idl.ann
>  create mode 100644 ovn-inb.ovsschema
>  create mode 100644 ovn-inb.xml
>  create mode 100644 tests/ovn-inbctl.at
>  create mode 100644 utilities/ovn-inbctl.8.xml
>  create mode 100644 utilities/ovn-inbctl.c
>
> diff --git a/.gitignore b/.gitignore
> index 6fee075..1994937 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -67,6 +67,9 @@
>  /ovn-sb.5
>  /ovn-sb.gv
>  /ovn-sb.pic
> +/ovn-inb.5
> +/ovn-inb.gv
> +/ovn-inb.pic
>  /package.m4
>  /stamp-h1
>  /_build-gcc
> diff --git a/automake.mk b/automake.mk
> index ad801f1..3bfbf57 100644
> --- a/automake.mk
> +++ b/automake.mk
> @@ -66,6 +66,36 @@ ovn-sb.5: \
> $(srcdir)/ovn-sb.xml > $@.tmp && \
> mv $@.tmp $@
>
> +# OVN interconnection northbound E-R diagram
> +#
> +# If "python" or "dot" is not available, then we do not add graphical diagram
> +# to the documentation.
> +if HAVE_PYTHON
> +if HAVE_DOT
> +ovn-inb.gv: ${OVSDIR}/ovsdb/ovsdb-dot.in $(srcdir)/ovn-inb.ovsschema
> +   $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn-inb.ovsschema > $@
> +ovn-inb.pic: ovn-inb.gv ${OVSDIR}/ovsdb/dot2pic
> +   $(AM_V_GEN)(dot -T plain < ovn-inb.gv | $(PYTHON) 
> ${OVSDIR}/ovsdb/dot2pic -f 3) > $@.tmp && \
> +   mv $@.tmp $@
> +OVN_INB_PIC = ovn-inb.pic
> +OVN_INB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_INB_PIC)
> +CLEANFILES += ovn-inb.gv ovn-inb.pic
> +endif
> +endif
> +
> +# OVN interconnection northbound schema documentation
> +EXTRA_DIST += ovn-inb.xml
> +CLEANFILES += ovn-inb.5
> +man_MANS += ovn-inb.5
> +
> +ovn-inb.5: \
> +   ${OVSDIR}/ovsdb/ovsdb-doc $(srcdir)/ovn-inb.xml 
> $(srcdir)/ovn-inb.ovsschema $(OVN_INB_PIC)
> +   $(AM_V_GEN)$(OVSDB_DOC) \
> +   $(OVN_INB_DOT_DIAGRAM_ARG) \
> +   --version=$(VERSION) \
> +   $(srcdir)/ovn-inb.ovsschema \
> +   $(srcdir)/ovn-inb.xml > $@.tmp && \
> +   mv $@.tmp $@
>
>  # Version checking for ovn-nb.ovsschema.
>  ALL_LOCAL += ovn-nb.ovsschema.stamp
> @@ -78,7 +108,14 @@ ALL_LOCAL += ovn-sb.ovsschema.stamp
>  ovn-sb.ovsschema.stamp: ovn-sb.ovsschema
> $(srcdir)/build-aux/cksum-schema-check $? $@
>
> +# Version checking for ovn-inb.ovsschema.
> +ALL_LOCAL += ovn-inb.ovsschema.stamp
> +ovn-inb.ovsschema.stamp: ovn-inb.ovsschema
> +   $(srcdir)/build-aux/cksum-schema-check $? $@
> +CLEANFILES += ovn-inb.ovsschema.stamp
> +
>  pkgdata_DATA += ovn-nb.ovsschema
>  pkgdata_DATA += ovn-sb.ovsschema
> +pkgdata_DATA += ovn-inb.ovsschema
>
>  CLEANFILES += ovn-sb.ovsschema.stamp
> diff --git a/debian/ovn-common.install b/debian/ovn-common.install
> index 90484d2..9e9bcfb 100644
> --- a/debian/ovn-common.install
> +++ b/debian/ovn-common.install
> @@ -1,5 +1,6 @@
>  usr/bin/ovn-nbctl
>  usr/bin/ovn-sbctl
> +usr/bin/ovn-inbctl
>  usr/bin/ovn-trace
>  usr/bin/ovn-detrace
>  usr/share/openvswitch/scripts/ovn-ctl
> diff --git a/debian/ovn-common.manpages b/debian/ovn-common.manpages
> index 249349e..94325dd 100644
> --- a/debian/ovn-common.manpages
> +++ b/debian/ovn-common.manpages
> @@ -1,8 +1,10 @@
>  ovn/ovn-architecture.7
>  ovn/ovn-nb.5
>  ovn/ovn-sb.5
> +ovn/ovn-inb.5
>  ovn/utilities/ovn-ctl.8
>  ovn/utilities/ovn-nbctl.8
>  ovn/utilities/ovn-sbctl.8
> +ovn/utilities/ovn-inbctl.8
>  ovn/utilities/ovn-trace.8
>  ovn/utilities/ovn-detrace.1
> diff --git a/lib/.gitignore b/lib/.gitignore
> index 3eed458..e5d9bf3 100644
> --- a/lib/.gitignore
> +++ b/lib/.gitignore
> @@ -5,4 +5,7 @@
>  /ovn-sb-idl.c
>  /ovn-sb-idl.h
>  /ovn-sb-idl.ovsidl
> +/ovn-inb-idl.c
> +/ovn-inb-idl.h
> +/ovn-inb-idl.ovsidl
>  /ovn-dirs.c
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 0c8245c..83fdbcd 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -29,7 +29,9 @@ 

Re: [ovs-dev] [PATCH ovn v2 01/13] ovn-architecture: Add documentation for OVN interconnection feature.

2019-11-12 Thread Numan Siddique
Hi Han,

I have started with reviewing the patches, but with skimming them
faster so that I get the complete
picture. So you might see some review comments now :)


Thanks
Numan


On Thu, Oct 31, 2019 at 2:44 AM Han Zhou  wrote:
>
> Signed-off-by: Han Zhou 
> ---
>  ovn-architecture.7.xml | 107 
> -
>  1 file changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml
> index 7966b65..56b2167 100644
> --- a/ovn-architecture.7.xml
> +++ b/ovn-architecture.7.xml
> @@ -1246,7 +1246,14 @@
>
>  Distributed gateway ports are logical router patch ports
>  that directly connect distributed logical routers to logical
> -switches with localnet ports.
> +switches with external connection.
> +  
> +
> +  
> +There are two types of external connections.  Firstly, connection to
> +physical network through a localnet port.  Secondly, connection to
> +another OVN deployment, which will be introduced in section "OVN
> +Deployments Interconnection".
>
>
>
> @@ -1801,6 +1808,104 @@
>  
>
>
> +  OVN Deployments Interconnection (TODO)
> +
> +  
> +It is not uncommon for an operator to deploy multiple OVN clusters, for
> +two main reasons.  Firstly, an operator may prefer to deploy one OVN
> +cluster for each availability zone, e.g. in different physical regions,
> +to avoid single point of failure.  Secondly, there is always an upper 
> limit
> +for a single OVN control plane to scale.
> +  
> +
> +  
> +Although the control planes of the different availability zone (AZ)s are
> +independent from each other, the workloads from different AZs may need
> +to communicate across the zones.  The OVN interconnection feature 
> provides
> +a native way to interconnect different AZs by L3 routing through transit
> +overlay networks between logical routers of different AZs.
> +  
> +
> +  
> +A global OVN Interconnection Northbound database is introduced for the
> +operator (probably through CMS systems) to configure transit logical
> +switches that connect logical routers from different AZs.  A transit
> +switch is similar as a
s/as a/to a


regular logical switch, but it is used for
> +interconnection purpose only.  Typically, each transit switch can be used
> +to connect all logical routers that belong to same tenant across all AZs.
> +  
> +
> +  
> +A dedicated daemon process ovn-ic, OVN interconnection
> +controller, in each AZ will consume this data and populate corresponding
> +logical switches to their own northbound databases for each AZ, so that
> +logical routers can be connected to the transit switch by creating
> +patch port pairs in their northbound databases.  Any router ports
> +connected to the transit switches are considered interconnection ports,
> +which will be exchanged between AZs.
> +  
> +
> +  
> +Physically, when workloads in from different AZs communicate, packets
> +need to go through multiple hops: source chassis, source gateway,
> +destination gateway and destination chassis.  All these hops are 
> connected
> +through tunnels so that the packets never leave overlay networks.
> +A distributed gateway port is required to connect the logical router to a
> +transit switch, with a gateway chassis specified, so that the traffic can
> +be forwarded through the gateway chassis.
> +  
> +
> +  
> +A global OVN Interconnection Southbound database is introduced for
> +exchanging control plane information between the AZs.  The data in
> +this database is populated and consumed by the ovn-ic,
> +of each AZ.  The main information in this database includes:
> +  
> +
> +  
> +
> +  Datapath bindings for transit switches, which mainly contains the 
> tunnel
> +  keys generated for each transit switch.  Separate key ranges are 
> reserved
> +  for transit switches so that they will never conflict with any tunnel
> +  keys locally assigned for datapaths within each AZ.
> +
> +
> +  Availability zones, which are registerd by ovn-ic
> +  from each AZ.
> +
> +
> +  Gateways.  Each AZ specifies chassises that are supposed to work
> +  as interconnection gateways, and the ovn-ic will
> +  populate this information to the interconnection southbound DB.
> +  The ovn-ic from all the other AZs will learn the
> +  gateways and populate to their own southbound DB as a chassis.
> +
> +
> +  Port bindings for logical switch ports created on the transit switch.
> +  Each AZ maintains their logical router to transit switch connections
> +  independently, but ovn-ic automatically populates
> +  local port bindings on transit switches to the global interconnection
> +  southbound DB, and learns remote port bindings from other AZs back
> +  to its own northbound and 

Re: [ovs-dev] [PATCH ovn] Fix testsuite 85 - "ensure one gw controller restart in HA doesn't bounce the master ok".

2019-11-12 Thread Numan Siddique
On Tue, Nov 12, 2019 at 4:58 PM Dumitru Ceara  wrote:
>
> On Mon, Nov 11, 2019 at 8:15 AM  wrote:
> >
> > From: Numan Siddique 
> >
> > This testsuite is failing frequently in travis CI and locally when tests 
> > are run with "-j5".
> >
> > The test case deletes the chassis row for chassis 'gw2' and expects that 
> > this
> > doesn't cause the chassisresident port on the master ('gw1') to not bounce 
> > as 'gw1'
> > has higher priority than gw2. But since the ha chassis group has only 2 
> > chassis,
> > 'gw2' can claim the port momemtarily when 'gw2' chassis row is recreated 
> > back until
> > BFD session with 'gw1' is not established.
> >
> > This patch changes the test assertion approach and makes sure that 'gw1' is 
> > the
> > owner of the chassisresident port eventually.
> >
> > Signed-off-by: Numan Siddique 
>
> Looks good to me. I ran the autotests and everything passes. I also
> executed test 85 in a 20 iterations loop and passed every time.
>
> A small typo in the commit message: s/momemtarily/momentarily. Also,
> (I'm not sure about this but) maybe s/recreated back/recreated.
> Otherwise:
>
> Acked-by: Dumitru Ceara 


Thanks Dumitru and Lorenzo for the reviews.
I applied this patch to master correcting the typos in the commit message.

Thanks
Numan

>
> Thanks,
> Dumitru
>
> > ---
> >  tests/ovn.at | 20 +---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index cb7903db8..35ffaf331 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -11033,12 +11033,26 @@ ovn-nbctl --wait=hv --timeout=3 sync
> >  # doesn't have the same effect because "name" is conserved, and the
> >  # Chassis entry is not replaced.
> >
> > -> gw1/ovn-controller.log
> > -
> >  gw2_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw2)
> >  ovn-sbctl destroy Chassis $gw2_chassis
> >
> > -OVS_WAIT_UNTIL([test 0 = `grep -c "Releasing lport" 
> > gw1/ovn-controller.log`])
> > +# Wait for the gw2_chassis row is recreated.
> > +OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns=_uuid find Chassis 
> > name=gw2 | wc -l`])
> > +
> > +gw1_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw1)
> > +gw2_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw2)
> > +
> > +# When gw2 chassis row is destroyed, it gets recreated. There
> > +# is a small window in which gw2 may claim the cr-outside port if
> > +# it has not established bfd tunnel with gw1.
> > +# So make sure that, cr-outside is claimed by gw1 finally.
> > +OVS_WAIT_WHILE(
> > +[cr_outside_ch=`ovn-sbctl --bare --columns=chassis find Port_binding 
> > logical_port=cr-outside`
> > + test $cr_outside_ch = $gw2_chassis])
> > +
> > +OVS_WAIT_UNTIL(
> > +[cr_outside_ch=`ovn-sbctl --bare --columns=chassis find Port_binding 
> > logical_port=cr-outside`
> > + test $cr_outside_ch = $gw1_chassis])
> >
> >  OVN_CLEANUP([gw1],[gw2],[hv1])
> >
> > --
> > 2.23.0
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn] Fix testsuite 85 - "ensure one gw controller restart in HA doesn't bounce the master ok".

2019-11-12 Thread Lorenzo Bianconi
> From: Numan Siddique 
> 
> This testsuite is failing frequently in travis CI and locally when tests are 
> run with "-j5".
> 
> The test case deletes the chassis row for chassis 'gw2' and expects that this
> doesn't cause the chassisresident port on the master ('gw1') to not bounce as 
> 'gw1'
> has higher priority than gw2. But since the ha chassis group has only 2 
> chassis,
> 'gw2' can claim the port momemtarily when 'gw2' chassis row is recreated back 
> until
> BFD session with 'gw1' is not established.
> 
> This patch changes the test assertion approach and makes sure that 'gw1' is 
> the
> owner of the chassisresident port eventually.
> 

Tested-by: Lorenzo Bianconi 

> Signed-off-by: Numan Siddique 
> ---
>  tests/ovn.at | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/ovn.at b/tests/ovn.at
> index cb7903db8..35ffaf331 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -11033,12 +11033,26 @@ ovn-nbctl --wait=hv --timeout=3 sync
>  # doesn't have the same effect because "name" is conserved, and the
>  # Chassis entry is not replaced.
>  
> -> gw1/ovn-controller.log
> -
>  gw2_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw2)
>  ovn-sbctl destroy Chassis $gw2_chassis
>  
> -OVS_WAIT_UNTIL([test 0 = `grep -c "Releasing lport" gw1/ovn-controller.log`])
> +# Wait for the gw2_chassis row is recreated.
> +OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns=_uuid find Chassis 
> name=gw2 | wc -l`])
> +
> +gw1_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw1)
> +gw2_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw2)
> +
> +# When gw2 chassis row is destroyed, it gets recreated. There
> +# is a small window in which gw2 may claim the cr-outside port if
> +# it has not established bfd tunnel with gw1.
> +# So make sure that, cr-outside is claimed by gw1 finally.
> +OVS_WAIT_WHILE(
> +[cr_outside_ch=`ovn-sbctl --bare --columns=chassis find Port_binding 
> logical_port=cr-outside`
> + test $cr_outside_ch = $gw2_chassis])
> +
> +OVS_WAIT_UNTIL(
> +[cr_outside_ch=`ovn-sbctl --bare --columns=chassis find Port_binding 
> logical_port=cr-outside`
> + test $cr_outside_ch = $gw1_chassis])
>  
>  OVN_CLEANUP([gw1],[gw2],[hv1])
>  
> -- 
> 2.23.0
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH 1/1] ovsdb-server: fix memory leak while deleting zone

2019-11-12 Thread Damijan Skvarc
memory leak was detected by valgrind during execution
of "database commands -- positive checks" test.

leaked memory was allocated in ovsdb_execute_mutate() function
while parsing mutations from the apparent json entity:

==19563==at 0x4C2DB8F: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19563==by 0x4652D0: xmalloc (util.c:138)
==19563==by 0x46539E: xmemdup0 (util.c:168)
==19563==by 0x4653F7: xstrdup (util.c:177)
==19563==by 0x450379: ovsdb_base_type_clone (ovsdb-types.c:208)
==19563==by 0x450F8D: ovsdb_type_clone (ovsdb-types.c:550)
==19563==by 0x428C3F: ovsdb_mutation_from_json (mutation.c:108)
==19563==by 0x428F6B: ovsdb_mutation_set_from_json (mutation.c:187)
==19563==by 0x42578D: ovsdb_execute_mutate (execution.c:573)
==19563==by 0x4246B0: ovsdb_execute_compose (execution.c:171)
==19563==by 0x41CDE5: ovsdb_trigger_try (trigger.c:204)
==19563==by 0x41C8DF: ovsdb_trigger_init (trigger.c:61)
==19563==by 0x40E93C: ovsdb_jsonrpc_trigger_create (jsonrpc-server.c:1135)
==19563==by 0x40E20C: ovsdb_jsonrpc_session_got_request 
(jsonrpc-server.c:1002)
==19563==by 0x40D1C2: ovsdb_jsonrpc_session_run (jsonrpc-server.c:561)
==19563==by 0x40D31E: ovsdb_jsonrpc_session_run_all (jsonrpc-server.c:591)
==19563==by 0x40CD6E: ovsdb_jsonrpc_server_run (jsonrpc-server.c:406)
==19563==by 0x40627E: main_loop (ovsdb-server.c:209)
==19563==by 0x406E66: main (ovsdb-server.c:460)

This memory is usually freed at the end of ovsdb_execute_mutate()
however in the aforementioned test case this does not happen. Namely
in case of delete mutator and in case of error while calling 
ovsdb_datum_from_json()
apparent mutation was marked as invalid, what prevents freeing problematic 
memory.

Memory leak can be reproduced quickly with the following command sequence:
ovs-vsctl --no-wait -vreconnect:emer add-zone-tp netdev zone=1 icmp_first=1 
icmp_reply=2
ovs-vsctl --no-wait -vreconnect:emer del-zone-tp netdev zone=1

Signed-off-by: Damijan Skvarc 
---
 ovsdb/mutation.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ovsdb/mutation.c b/ovsdb/mutation.c
index cd20bdb..56edc5f 100644
--- a/ovsdb/mutation.c
+++ b/ovsdb/mutation.c
@@ -147,6 +147,8 @@ ovsdb_mutation_from_json(const struct ovsdb_table_schema 
*ts,
 if (error && ovsdb_type_is_map(>type)
 && m->mutator == OVSDB_M_DELETE) {
 ovsdb_error_destroy(error);
+ovsdb_base_type_destroy(>type.value);
+m->type.value.enum_ = NULL;
 m->type.value.type = OVSDB_TYPE_VOID;
 error = ovsdb_datum_from_json(>arg, >type, array->elems[2],
   symtab);
-- 
2.7.4

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


Re: [ovs-dev] [PATCH ovn] Fix testsuite 85 - "ensure one gw controller restart in HA doesn't bounce the master ok".

2019-11-12 Thread Dumitru Ceara
On Mon, Nov 11, 2019 at 8:15 AM  wrote:
>
> From: Numan Siddique 
>
> This testsuite is failing frequently in travis CI and locally when tests are 
> run with "-j5".
>
> The test case deletes the chassis row for chassis 'gw2' and expects that this
> doesn't cause the chassisresident port on the master ('gw1') to not bounce as 
> 'gw1'
> has higher priority than gw2. But since the ha chassis group has only 2 
> chassis,
> 'gw2' can claim the port momemtarily when 'gw2' chassis row is recreated back 
> until
> BFD session with 'gw1' is not established.
>
> This patch changes the test assertion approach and makes sure that 'gw1' is 
> the
> owner of the chassisresident port eventually.
>
> Signed-off-by: Numan Siddique 

Looks good to me. I ran the autotests and everything passes. I also
executed test 85 in a 20 iterations loop and passed every time.

A small typo in the commit message: s/momemtarily/momentarily. Also,
(I'm not sure about this but) maybe s/recreated back/recreated.
Otherwise:

Acked-by: Dumitru Ceara 

Thanks,
Dumitru

> ---
>  tests/ovn.at | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index cb7903db8..35ffaf331 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -11033,12 +11033,26 @@ ovn-nbctl --wait=hv --timeout=3 sync
>  # doesn't have the same effect because "name" is conserved, and the
>  # Chassis entry is not replaced.
>
> -> gw1/ovn-controller.log
> -
>  gw2_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw2)
>  ovn-sbctl destroy Chassis $gw2_chassis
>
> -OVS_WAIT_UNTIL([test 0 = `grep -c "Releasing lport" gw1/ovn-controller.log`])
> +# Wait for the gw2_chassis row is recreated.
> +OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns=_uuid find Chassis 
> name=gw2 | wc -l`])
> +
> +gw1_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw1)
> +gw2_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw2)
> +
> +# When gw2 chassis row is destroyed, it gets recreated. There
> +# is a small window in which gw2 may claim the cr-outside port if
> +# it has not established bfd tunnel with gw1.
> +# So make sure that, cr-outside is claimed by gw1 finally.
> +OVS_WAIT_WHILE(
> +[cr_outside_ch=`ovn-sbctl --bare --columns=chassis find Port_binding 
> logical_port=cr-outside`
> + test $cr_outside_ch = $gw2_chassis])
> +
> +OVS_WAIT_UNTIL(
> +[cr_outside_ch=`ovn-sbctl --bare --columns=chassis find Port_binding 
> logical_port=cr-outside`
> + test $cr_outside_ch = $gw1_chassis])
>
>  OVN_CLEANUP([gw1],[gw2],[hv1])
>
> --
> 2.23.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

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


[ovs-dev] [PATCH v7 ovn 2/2] ovn-northd: Limit ARP/ND broadcast domain whenever possible.

2019-11-12 Thread Dumitru Ceara
ARP request and ND NS packets for router owned IPs were being
flooded in the complete L2 domain (using the MC_FLOOD multicast group).
However this creates a scaling issue in scenarios where aggregation
logical switches are connected to more logical routers (~350). The
logical pipelines of all routers would have to be executed before the
packet is finally replied to by a single router, the owner of the IP
address.

This commit limits the broadcast domain by bypassing the L2 Lookup stage
for ARP requests that will be replied by a single router. The packets
are forwarded only to the router port that owns the target IP address.

IPs that are owned by the routers and for which this fix applies are:
- IP addresses configured on the router ports.
- VIPs.
- NAT IPs.

Reported-at: https://bugzilla.redhat.com/1756945
Reported-by: Anil Venkata 
Signed-off-by: Dumitru Ceara 

---
v7:
- Address Han's comments:
- Remove flooding for all ARPs received on VLAN networks. To avoid
  that we now identify self originated (G)ARPs by matching on source
  MAC address too.
- Rename REGBIT_NOT_VXLAN to FLAGBIT_NOT_VXLAN.
- Fix ovn-sb manpage.
- Split patch in a series of 2:
- patch1: fixes the get_router_load_balancer_ips() function.
- patch2: limits the ARP/ND broadcast domain.
v6:
- Address Han's comments:
- remove flooding of ARPs targeting OVN owned IP addresses.
- update ovn-architecture documentation.
- rename ARP handling functions.
- Adapt "ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR" autotest to take into
account the new way of forwarding ARPs.
- Also, properly deal with ARP packets on VLAN-backed networks.
v5: Address Numan's comments: update comments & make autotest more
robust.
v4: Rebase.
v3: Properly deal with VXLAN traffic. Address review comments from
Numan (add autotests). Fix function get_router_load_balancer_ips.
Rebase -> deal with IPv6 NAT too.
v2: Move ARP broadcast domain limiting to table S_SWITCH_IN_L2_LKUP to
address localnet ports too.
---
 northd/ovn-northd.8.xml |   14 ++
 northd/ovn-northd.c |  230 +++
 ovn-architecture.7.xml  |   19 +++
 tests/ovn.at|  307 +--
 4 files changed, 530 insertions(+), 40 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 0a33dcd..344cc0d 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1005,6 +1005,20 @@ output;
   
 
   
+Priority-80 flows for each port connected to a logical router
+matching self originated GARP/ARP request/ND packets. These packets
+are flooded to the MC_FLOOD which contains all logical
+ports.
+  
+
+  
+Priority-75 flows for each IP address/VIP/NAT address owned by a
+router port connected to the switch. These flows match ARP requests
+and ND packets for the specific IP addresses.  Matched packets are
+forwarded only to the router that owns the IP address.
+  
+
+  
 A priority-70 flow that outputs all packets with an Ethernet broadcast
 or multicast eth.dst to the MC_FLOOD
 multicast group.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 32f3200..d6beb97 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -210,6 +210,8 @@ enum ovn_stage {
 #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[4]"
 #define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[5]"
 
+#define FLAGBIT_NOT_VXLAN "flags[1] == 0"
+
 /* Returns an "enum ovn_stage" built from the arguments. */
 static enum ovn_stage
 ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline pipeline,
@@ -1202,6 +1204,34 @@ ovn_port_allocate_key(struct ovn_datapath *od)
   1, (1u << 15) - 1, >port_key_hint);
 }
 
+/* Returns true if the logical switch port 'enabled' column is empty or
+ * set to true.  Otherwise, returns false. */
+static bool
+lsp_is_enabled(const struct nbrec_logical_switch_port *lsp)
+{
+return !lsp->n_enabled || *lsp->enabled;
+}
+
+/* Returns true only if the logical switch port 'up' column is set to true.
+ * Otherwise, if the column is not set or set to false, returns false. */
+static bool
+lsp_is_up(const struct nbrec_logical_switch_port *lsp)
+{
+return lsp->n_up && *lsp->up;
+}
+
+static bool
+lsp_is_external(const struct nbrec_logical_switch_port *nbsp)
+{
+return !strcmp(nbsp->type, "external");
+}
+
+static bool
+lrport_is_enabled(const struct nbrec_logical_router_port *lrport)
+{
+return !lrport->enabled || *lrport->enabled;
+}
+
 static char *
 chassis_redirect_name(const char *port_name)
 {
@@ -3750,28 +3780,6 @@ build_port_security_ip(enum ovn_pipeline pipeline, 
struct ovn_port *op,
 
 }
 
-/* Returns true if the logical switch port 'enabled' column is empty or
- * set to true.  Otherwise, returns false. */
-static bool
-lsp_is_enabled(const struct nbrec_logical_switch_port *lsp)
-{
-return !lsp->n_enabled || 

[ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.

2019-11-12 Thread Dumitru Ceara
Function get_router_load_balancer_ips() was incorrectly returning a
single address_family even though the IP set could contain a mix of
IPv4 and IPv6 addresses.

Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.c |  126 +--
 1 file changed, 72 insertions(+), 54 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 2f0f501..32f3200 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2184,7 +2184,7 @@ ip_address_and_port_from_lb_key(const char *key, char 
**ip_address,
 
 static void
 get_router_load_balancer_ips(const struct ovn_datapath *od,
- struct sset *all_ips, int *addr_family)
+ struct sset *all_ips_v4, struct sset *all_ips_v6)
 {
 if (!od->nbr) {
 return;
@@ -2199,13 +2199,21 @@ get_router_load_balancer_ips(const struct ovn_datapath 
*od,
 /* node->key contains IP:port or just IP. */
 char *ip_address = NULL;
 uint16_t port;
+int addr_family;
 
 ip_address_and_port_from_lb_key(node->key, _address, ,
-addr_family);
+_family);
 if (!ip_address) {
 continue;
 }
 
+struct sset *all_ips;
+if (addr_family == AF_INET) {
+all_ips = all_ips_v4;
+} else {
+all_ips = all_ips_v6;
+}
+
 if (!sset_contains(all_ips, ip_address)) {
 sset_add(all_ips, ip_address);
 }
@@ -2299,17 +2307,22 @@ get_nat_addresses(const struct ovn_port *op, size_t *n)
 }
 }
 
-/* A set to hold all load-balancer vips. */
-struct sset all_ips = SSET_INITIALIZER(_ips);
-int addr_family;
-get_router_load_balancer_ips(op->od, _ips, _family);
+/* Two sets to hold all load-balancer vips. */
+struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
+struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
+get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
 
 const char *ip_address;
-SSET_FOR_EACH (ip_address, _ips) {
+SSET_FOR_EACH (ip_address, _ips_v4) {
 ds_put_format(_addresses, " %s", ip_address);
 central_ip_address = true;
 }
-sset_destroy(_ips);
+SSET_FOR_EACH (ip_address, _ips_v6) {
+ds_put_format(_addresses, " %s", ip_address);
+central_ip_address = true;
+}
+sset_destroy(_ips_v4);
+sset_destroy(_ips_v6);
 
 if (central_ip_address) {
 /* Gratuitous ARP for centralized NAT rules on distributed gateway
@@ -6911,61 +6924,66 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 }
 
 /* A set to hold all load-balancer vips that need ARP responses. */
-struct sset all_ips = SSET_INITIALIZER(_ips);
-int addr_family;
-get_router_load_balancer_ips(op->od, _ips, _family);
+struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
+struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
+get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
 
 const char *ip_address;
-SSET_FOR_EACH(ip_address, _ips) {
+SSET_FOR_EACH (ip_address, _ips_v4) {
 ds_clear();
-if (addr_family == AF_INET) {
-ds_put_format(,
-  "inport == %s && arp.tpa == %s && arp.op == 1",
-  op->json_key, ip_address);
-} else {
-ds_put_format(,
-  "inport == %s && nd_ns && nd.target == %s",
-  op->json_key, ip_address);
-}
+ds_put_format(,
+  "inport == %s && arp.tpa == %s && arp.op == 1",
+  op->json_key, ip_address);
 
 ds_clear();
-if (addr_family == AF_INET) {
-ds_put_format(,
-"eth.dst = eth.src; "
-"eth.src = %s; "
-"arp.op = 2; /* ARP reply */ "
-"arp.tha = arp.sha; "
-"arp.sha = %s; "
-"arp.tpa = arp.spa; "
-"arp.spa = %s; "
-"outport = %s; "
-"flags.loopback = 1; "
-"output;",
-op->lrp_networks.ea_s,
-op->lrp_networks.ea_s,
-ip_address,
-op->json_key);
-} else {
-ds_put_format(,
-"nd_na { "
-"eth.src = %s; "
-"ip6.src = %s; "
-"nd.target = %s; "
-"nd.tll = %s; "
-"outport = inport; "
-"flags.loopback = 1; "
-"output; "
-"};",
-op->lrp_networks.ea_s,
-ip_address,
-ip_address,
-

Re: [ovs-dev] [PATCHv4] netdev-afxdp: Enable loading XDP program.

2019-11-12 Thread Eelco Chaudron
See one remark below, however when I did a quick test with a program 
that would not load it goes into some re-try loop:


2019-11-12T10:13:21.658Z|01609|netdev_afxdp|INFO|eno1: Removing xdp 
program.
2019-11-12T10:13:21.658Z|01610|netdev_afxdp|INFO|Removed program ID: 0, 
fd: 0
2019-11-12T10:13:21.658Z|01611|netdev_afxdp|INFO|eno1: Setting XDP mode 
to DRV.
2019-11-12T10:13:22.224Z|01612|netdev_afxdp|INFO|eno1: Load custom XDP 
program at /root/xdp_pass_kern.o.
2019-11-12T10:13:22.274Z|01613|netdev_afxdp|ERR|xsk_socket__create 
failed (Resource temporarily unavailable) mode: DRV use-need-wakeup: 
true qid: 0
2019-11-12T10:13:22.300Z|01614|netdev_afxdp|ERR|Failed to create AF_XDP 
socket on queue 0.
2019-11-12T10:13:22.300Z|01615|netdev_afxdp|INFO|eno1: Removing xdp 
program.
2019-11-12T10:13:22.658Z|00047|ovs_rcu(urcu2)|WARN|blocked 1000 ms 
waiting for main to quiesce
2019-11-12T10:13:22.735Z|01616|netdev_afxdp|INFO|Removed program ID: 
320, fd: 0
2019-11-12T10:13:22.735Z|01617|netdev_afxdp|ERR|AF_XDP device eno1 
reconfig failed.
2019-11-12T10:13:22.735Z|01618|dpif_netdev|ERR|Failed to set interface 
eno1 new configuration
2019-11-12T10:13:22.735Z|01619|netdev_afxdp|INFO|FIXME: Device tapVM 
always use numa id 0.
2019-11-12T10:13:22.735Z|01620|dpif_netdev|INFO|Core 1 on numa node 0 
assigned port 'tapVM' rx queue 0 (measured processing cycles 0).
2019-11-12T10:13:22.735Z|01621|dpif|WARN|netdev@ovs-netdev: failed to 
add eno1 as port: Invalid argument
2019-11-12T10:13:22.735Z|01622|netdev_afxdp|INFO|eno1: Removing xdp 
program.
2019-11-12T10:13:22.736Z|01623|netdev_afxdp|INFO|Removed program ID: 0, 
fd: 0
2019-11-12T10:13:22.736Z|01624|timeval|WARN|Unreasonably long 1079ms 
poll interval (7ms user, 80ms system)

2019-11-12T10:13:22.736Z|01625|timeval|WARN|faults: 16387 minor, 0 major
2019-11-12T10:13:22.736Z|01626|timeval|WARN|context switches: 327 
voluntary, 337 involuntary
2019-11-12T10:13:22.738Z|01627|netdev_afxdp|INFO|eno1: Removing xdp 
program.
2019-11-12T10:13:22.738Z|01628|netdev_afxdp|INFO|Removed program ID: 0, 
fd: 0
2019-11-12T10:13:22.739Z|01629|netdev_afxdp|INFO|eno1: Setting XDP mode 
to DRV.
2019-11-12T10:13:23.312Z|01630|netdev_afxdp|INFO|eno1: Load custom XDP 
program at /root/xdp_pass_kern.o.
2019-11-12T10:13:23.363Z|01631|netdev_afxdp|ERR|xsk_socket__create 
failed (Resource temporarily unavailable) mode: DRV use-need-wakeup: 
true qid: 0
2019-11-12T10:13:23.392Z|01632|netdev_afxdp|ERR|Failed to create AF_XDP 
socket on queue 0.
2019-11-12T10:13:23.392Z|01633|netdev_afxdp|INFO|eno1: Removing xdp 
program.
2019-11-12T10:13:23.738Z|00048|ovs_rcu(urcu2)|WARN|blocked 1000 ms 
waiting for main to quiesce
2019-11-12T10:13:23.823Z|01634|netdev_afxdp|INFO|Removed program ID: 
324, fd: 0
2019-11-12T10:13:23.823Z|01635|netdev_afxdp|ERR|AF_XDP device eno1 
reconfig failed.
2019-11-12T10:13:23.823Z|01636|dpif_netdev|ERR|Failed to set interface 
eno1 new configuration
2019-11-12T10:13:23.823Z|01637|netdev_afxdp|INFO|FIXME: Device tapVM 
always use numa id 0.
2019-11-12T10:13:23.823Z|01638|dpif_netdev|INFO|Core 1 on numa node 0 
assigned port 'tapVM' rx queue 0 (measured processing cycles 0).
2019-11-12T10:13:23.823Z|01639|dpif|WARN|netdev@ovs-netdev: failed to 
add eno1 as port: Invalid argument


But in addition during this loop it’s not freeing it’s resources:

$  bpftool prog list && bpftool map
4: xdp  tag 80b55d8a76303785
loaded_at 2019-11-12T05:11:15-0500  uid 0
xlated 136B  jited 96B  memlock 4096B  map_ids 4
12: xdp  name xdp_prog_simple  tag 3b185187f1855c4c  gpl
loaded_at 2019-11-12T05:11:58-0500  uid 0
xlated 16B  jited 35B  memlock 4096B
16: xdp  name xdp_prog_simple  tag 3b185187f1855c4c  gpl
loaded_at 2019-11-12T05:11:59-0500  uid 0
xlated 16B  jited 35B  memlock 4096B
20: xdp  name xdp_prog_simple  tag 3b185187f1855c4c  gpl
loaded_at 2019-11-12T05:12:00-0500  uid 0
xlated 16B  jited 35B  memlock 4096B
…
…
120: (null)  name xsks_map  flags 0x0
key 4B  value 4B  max_entries 32  memlock 4096B
122: (null)  name xsks_map  flags 0x0
key 4B  value 4B  max_entries 32  memlock 4096B
124: (null)  name xsks_map  flags 0x0
key 4B  value 4B  max_entries 32  memlock 4096B
126: (null)  name xsks_map  flags 0x0
key 4B  value 4B  max_entries 32  memlock 4096B
128: (null)  name xsks_map  flags 0x0
key 4B  value 4B  max_entries 32  memlock 4096B
130: (null)  name xsks_map  flags 0x0
key 4B  value 4B  max_entries 32  memlock 4096B
132: (null)  name xsks_map  flags 0x0
key 4B  value 4B  max_entries 32  memlock 4096B


On 8 Nov 2019, at 15:29, William Tu wrote:


Now netdev-afxdp always forwards all packets to userspace because
it is using libbpf's default XDP program, see 'xsk_load_xdp_prog'.
There are some cases when users want to keep packets in kernel instead
of sending to userspace, for example, management traffic such as SSH
should be processed in kernel.

The patch enables 

[ovs-dev] [PATCH net-next] openvswitch: add TTL decrement action

2019-11-12 Thread Matteo Croce
New action to decrement TTL instead of setting it to a fixed value.
This action will decrement the TTL and, in case of expired TTL, send the
packet to userspace via output_userspace() to take care of it.

Supports both IPv4 and IPv6 via the ttl and hop_limit fields, respectively.

Tested with a corresponding change in the userspace:

# ovs-dpctl dump-flows
in_port(2),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, 
actions:dec_ttl,1
in_port(1),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, 
actions:dec_ttl,2
in_port(1),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, actions:2
in_port(2),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, actions:1

# ping -c1 192.168.0.2 -t 42
IP (tos 0x0, ttl 41, id 61647, offset 0, flags [DF], proto ICMP (1), length 
84)
192.168.0.1 > 192.168.0.2: ICMP echo request, id 386, seq 1, length 64
# ping -c1 192.168.0.2 -t 120
IP (tos 0x0, ttl 119, id 62070, offset 0, flags [DF], proto ICMP (1), 
length 84)
192.168.0.1 > 192.168.0.2: ICMP echo request, id 388, seq 1, length 64
# ping -c1 192.168.0.2 -t 1
#

Co-authored-by: Bindiya Kurle 
Signed-off-by: Bindiya Kurle 
Signed-off-by: Matteo Croce 
---
 include/uapi/linux/openvswitch.h |  2 ++
 net/openvswitch/actions.c| 46 
 net/openvswitch/flow_netlink.c   |  6 +
 3 files changed, 54 insertions(+)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 1887a451c388..a3bdb1ecd1e7 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -890,6 +890,7 @@ struct check_pkt_len_arg {
  * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
  * of actions if greater than the specified packet length, else execute
  * another set of actions.
+ * @OVS_ACTION_ATTR_DEC_TTL: Decrement the IP TTL.
  *
  * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not all
  * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
@@ -925,6 +926,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_METER,/* u32 meter ID. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*.  */
OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
+   OVS_ACTION_ATTR_DEC_TTL,  /* Decrement ttl action */
 
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 12936c151cc0..077b7f309c93 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1174,6 +1174,43 @@ static int execute_check_pkt_len(struct datapath *dp, 
struct sk_buff *skb,
 nla_len(actions), last, clone_flow_key);
 }
 
+static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key)
+{
+   int err;
+
+   if (skb->protocol == htons(ETH_P_IPV6)) {
+   struct ipv6hdr *nh = ipv6_hdr(skb);
+
+   err = skb_ensure_writable(skb, skb_network_offset(skb) +
+ sizeof(*nh));
+   if (unlikely(err))
+   return err;
+
+   if (nh->hop_limit <= 1)
+   return -EHOSTUNREACH;
+
+   key->ip.ttl = --nh->hop_limit;
+   } else {
+   struct iphdr *nh = ip_hdr(skb);
+   u8 old_ttl;
+
+   err = skb_ensure_writable(skb, skb_network_offset(skb) +
+ sizeof(*nh));
+   if (unlikely(err))
+   return err;
+
+   if (nh->ttl <= 1)
+   return -EHOSTUNREACH;
+
+   old_ttl = nh->ttl--;
+   csum_replace2(>check, htons(old_ttl << 8),
+ htons(nh->ttl << 8));
+   key->ip.ttl = nh->ttl;
+   }
+
+   return 0;
+}
+
 /* Execute a list of actions against 'skb'. */
 static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
  struct sw_flow_key *key,
@@ -1345,6 +1382,15 @@ static int do_execute_actions(struct datapath *dp, 
struct sk_buff *skb,
 
break;
}
+
+   case OVS_ACTION_ATTR_DEC_TTL:
+   err = execute_dec_ttl(skb, key);
+   if (err == -EHOSTUNREACH) {
+   output_userspace(dp, skb, key, a, attr,
+len, OVS_CB(skb)->cutlen);
+   OVS_CB(skb)->cutlen = 0;
+   }
+   break;
}
 
if (unlikely(err)) {
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 65c2e3458ff5..d17f2d4b420f 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ 

[ovs-dev] [PATCH v2] netdev-afxdp: add afxdp specific maximum MTU check

2019-11-12 Thread Eelco Chaudron
Drivers natively supporting AF_XDP will check that a configured MTU size
will not exceed the allowed size for AF_XDP. However, when the skb
compatibility mode is used there is no check and any value is accepted.
This, for example, is the case when using the TAP interface.

This fix adds a check to make sure only AF_XDP valid values are excepted.

Signed-off-by: Eelco Chaudron 
---

v1 -> v2:
  Included (subtracted) ethernet and vlan header from the MTU size
  to match kernel/driver MTU behavior.

  Use netdev class compare rather than based on name string

 lib/netdev-afxdp.c |   17 +
 lib/netdev-afxdp.h |1 +
 lib/netdev-linux.c |9 +
 3 files changed, 27 insertions(+)

diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index af654d498..86422955c 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -1084,6 +1084,23 @@ netdev_afxdp_destruct(struct netdev *netdev)
 ovs_mutex_destroy(>mutex);
 }
 
+int
+netdev_afxdp_verify_mtu_size(const struct netdev *netdev OVS_UNUSED, int mtu)
+{
+/*
+ * If a device is used in xdpmode skb, no driver-specific MTU size is
+ * checked and any value is allowed resulting in packet drops.
+ * This check will verify the maximum supported value based on the
+ * buffer size allocated and the additional headroom required.
+ */
+if (mtu > (FRAME_SIZE - OVS_XDP_HEADROOM -
+   XDP_PACKET_HEADROOM - VLAN_ETH_HEADER_LEN)) {
+return EINVAL;
+}
+
+return 0;
+}
+
 int
 netdev_afxdp_get_custom_stats(const struct netdev *netdev,
   struct netdev_custom_stats *custom_stats)
diff --git a/lib/netdev-afxdp.h b/lib/netdev-afxdp.h
index e2f400b72..ee6939fce 100644
--- a/lib/netdev-afxdp.h
+++ b/lib/netdev-afxdp.h
@@ -39,6 +39,7 @@ int netdev_afxdp_rxq_construct(struct netdev_rxq *rxq_);
 void netdev_afxdp_rxq_destruct(struct netdev_rxq *rxq_);
 int netdev_afxdp_construct(struct netdev *netdev_);
 void netdev_afxdp_destruct(struct netdev *netdev_);
+int netdev_afxdp_verify_mtu_size(const struct netdev *netdev, int mtu);
 
 int netdev_afxdp_rxq_recv(struct netdev_rxq *rxq_,
   struct dp_packet_batch *batch,
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index f48192373..0a32cf9bc 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1593,6 +1593,15 @@ netdev_linux_set_mtu(struct netdev *netdev_, int mtu)
 goto exit;
 }
 
+#ifdef HAVE_AF_XDP
+if (netdev_get_class(netdev_) == _afxdp_class) {
+error = netdev_afxdp_verify_mtu_size(netdev_, mtu);
+if (error) {
+goto exit;
+}
+}
+#endif
+
 if (netdev->cache_valid & VALID_MTU) {
 error = netdev->netdev_mtu_error;
 if (error || netdev->mtu == mtu) {

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


Re: [ovs-dev] [PATCH v2] compat: Add compat fix for old kernels

2019-11-12 Thread Simon Horman
On Tue, Nov 12, 2019 at 08:33:02AM +, Roi Dayan wrote:
> 
> 
> On 2019-11-12 10:28 AM, Roi Dayan wrote:
> > 
> > 
> > On 2019-11-12 10:12 AM, Simon Horman wrote:
> >> On Mon, Nov 11, 2019 at 04:49:18PM +, Roi Dayan wrote:
> >>>
> >>>
> >>> On 2019-11-11 4:08 PM, Roi Dayan wrote:
> 
> 
>  On 2019-11-11 3:39 PM, Simon Horman wrote:
> > On Mon, Nov 04, 2019 at 11:49:44AM +0200, Roi Dayan wrote:
> >> In kernels older than 4.8, struct tcf_t didn't have the firstuse.
> >> If openvswitch is compiled with the compat pkt_cls.h then there is
> >> a struct size mismatch between openvswitch and the kernel which cause
> >> parsing netlink actions to fail.
> >> After this commit parsing the netlink actions pass even if compiled 
> >> with
> >> the compat pkt_cls.h.
> >>
> >> Signed-off-by: Roi Dayan 
> >> ---
> >>
> >> v1->v2:
> >> - fix mix of tabs and spaces in acinclude.m4
> >
> > Thanks, applied to master and branch-2.8 ... branch-2.12
> 
>  thanks!
> 
> >>>
> >>> Hi Simon,
> >>>
> >>> sorry for the trouble but there is also v3 for this patch.
> >>> it uses ac_check_member instead of ac_compile.
> >>> is it possible to take v3 which is a lot cleaner?
> >>
> >> Sorry for the mix up,
> >>
> >> would it be possible to provide an incremental patch
> >> to upgrade master to v3?
> >>
> > 
> > sure. i'll do that.
> > 
> 
> I see the actual merged patch is v3 so nothing to do. thanks again.

Thanks, sorry again for the mix-up.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net 1/2] openvswitch: support asymmetric conntrack

2019-11-12 Thread Nicolas Dichtel
Le 08/11/2019 à 22:07, Aaron Conole a écrit :
> The openvswitch module shares a common conntrack and NAT infrastructure
> exposed via netfilter.  It's possible that a packet needs both SNAT and
> DNAT manipulation, due to e.g. tuple collision.  Netfilter can support
> this because it runs through the NAT table twice - once on ingress and
> again after egress.  The openvswitch module doesn't have such capability.
> 
> Like netfilter hook infrastructure, we should run through NAT twice to
> keep the symmetry.
> 
> Fixes: 05752523e565 ("openvswitch: Interface with NAT.")
> Signed-off-by: Aaron Conole 
In this case, ovs_ct_find_existing() won't be able to find the conntrack, right?
Inverting the tuple to find the conntrack doesn't work anymore with double NAT.
Am I wrong?


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


[ovs-dev] [PATCH v1] ovs container build: Make kernel module configurable

2019-11-12 Thread amginwal
From: Aliasgar Ginwala 

--with-linux can be made configurable while building containers
for leveraging kernel modules installed on host.
KERNEL_VERSION=host should be used in env variable for the same.

Signed-off-by: Aliasgar Ginwala 
---
 utilities/docker/Makefile   |  2 +-
 utilities/docker/debian/build-kernel-modules.sh | 16 +---
 utilities/docker/start-ovs  |  9 -
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
index 8c2f7810e..d8b08a3c9 100644
--- a/utilities/docker/Makefile
+++ b/utilities/docker/Makefile
@@ -10,7 +10,7 @@
 #   make push
 
 REPO = ${DOCKER_REPO}
-tag = ${OVS_VERSION}_${KERNEL_VERSION}
+tag = ${OVS_VERSION}_${DISTRO}_${KERNEL_VERSION}
 
 build: ;docker build -t ${REPO}:${tag} --build-arg DISTRO=${DISTRO} \
 --build-arg OVS_BRANCH=${OVS_BRANCH} \
diff --git a/utilities/docker/debian/build-kernel-modules.sh 
b/utilities/docker/debian/build-kernel-modules.sh
index 18ac35764..17a67bfcb 100755
--- a/utilities/docker/debian/build-kernel-modules.sh
+++ b/utilities/docker/debian/build-kernel-modules.sh
@@ -17,13 +17,17 @@ OVS_BRANCH=$2
 GITHUB_SRC=$3
 
 # Install deps
-linux="linux-image-$KERNEL_VERSION linux-headers-$KERNEL_VERSION"
 build_deps="apt-utils libelf-dev build-essential libssl-dev python3 \
 python3-six wget gdb autoconf libtool git automake bzip2 debhelper \
 dh-autoreconf openssl"
 
 apt-get update
-apt-get install -y ${linux} ${build_deps}
+
+if [ $KERNEL_VERSION != "host" ]; then
+linux="linux-image-$KERNEL_VERSION linux-headers-$KERNEL_VERSION"
+apt-get install -y ${linux}
+fi
+apt-get install -y ${build_deps}
 
 # get the source
 mkdir /build; cd /build
@@ -32,8 +36,14 @@ cd ovs
 
 # build and install
 ./boot.sh
-./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
+if [ $KERNEL_VERSION == "host" ]; then
+   ./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
+--enable-ssl
+else
+./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
 --with-linux=/lib/modules/$KERNEL_VERSION/build --enable-ssl
+fi
+
 make -j8; make install; make modules_install
 
 # remove deps to make the container light weight.
diff --git a/utilities/docker/start-ovs b/utilities/docker/start-ovs
index 4a1a16cd1..c99380519 100755
--- a/utilities/docker/start-ovs
+++ b/utilities/docker/start-ovs
@@ -38,5 +38,12 @@ case $1 in
 -vfile:info --mlockall --no-chdir \
 --log-file=/var/log/openvswitch/ovs-vswitchd.log
 ;;
-*) echo "$0 [ovsdb-server|ovs-vswitchd]"
+"ovs-vswitchd-host") /usr/share/openvswitch/scripts/ovs-ctl \
+ --no-ovsdb-server start
+ /usr/share/openvswitch/scripts/ovs-ctl stop
+ ovs-vswitchd --pidfile -vconsole:emer \
+ -vsyslog:err -vfile:info --mlockall --no-chdir \
+ --log-file=/var/log/openvswitch/ovs-vswitchd.log
+;;
+*) echo "$0 [ovsdb-server|ovs-vswitchd|ovs-vswitchd-host]"
 esac
-- 
2.20.1 (Apple Git-117)

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


Re: [ovs-dev] [PATCH v2] compat: Add compat fix for old kernels

2019-11-12 Thread Roi Dayan



On 2019-11-12 10:28 AM, Roi Dayan wrote:
> 
> 
> On 2019-11-12 10:12 AM, Simon Horman wrote:
>> On Mon, Nov 11, 2019 at 04:49:18PM +, Roi Dayan wrote:
>>>
>>>
>>> On 2019-11-11 4:08 PM, Roi Dayan wrote:


 On 2019-11-11 3:39 PM, Simon Horman wrote:
> On Mon, Nov 04, 2019 at 11:49:44AM +0200, Roi Dayan wrote:
>> In kernels older than 4.8, struct tcf_t didn't have the firstuse.
>> If openvswitch is compiled with the compat pkt_cls.h then there is
>> a struct size mismatch between openvswitch and the kernel which cause
>> parsing netlink actions to fail.
>> After this commit parsing the netlink actions pass even if compiled with
>> the compat pkt_cls.h.
>>
>> Signed-off-by: Roi Dayan 
>> ---
>>
>> v1->v2:
>> - fix mix of tabs and spaces in acinclude.m4
>
> Thanks, applied to master and branch-2.8 ... branch-2.12

 thanks!

>>>
>>> Hi Simon,
>>>
>>> sorry for the trouble but there is also v3 for this patch.
>>> it uses ac_check_member instead of ac_compile.
>>> is it possible to take v3 which is a lot cleaner?
>>
>> Sorry for the mix up,
>>
>> would it be possible to provide an incremental patch
>> to upgrade master to v3?
>>
> 
> sure. i'll do that.
> 

I see the actual merged patch is v3 so nothing to do. thanks again.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] compat: Add compat fix for old kernels

2019-11-12 Thread Roi Dayan



On 2019-11-12 10:12 AM, Simon Horman wrote:
> On Mon, Nov 11, 2019 at 04:49:18PM +, Roi Dayan wrote:
>>
>>
>> On 2019-11-11 4:08 PM, Roi Dayan wrote:
>>>
>>>
>>> On 2019-11-11 3:39 PM, Simon Horman wrote:
 On Mon, Nov 04, 2019 at 11:49:44AM +0200, Roi Dayan wrote:
> In kernels older than 4.8, struct tcf_t didn't have the firstuse.
> If openvswitch is compiled with the compat pkt_cls.h then there is
> a struct size mismatch between openvswitch and the kernel which cause
> parsing netlink actions to fail.
> After this commit parsing the netlink actions pass even if compiled with
> the compat pkt_cls.h.
>
> Signed-off-by: Roi Dayan 
> ---
>
> v1->v2:
> - fix mix of tabs and spaces in acinclude.m4

 Thanks, applied to master and branch-2.8 ... branch-2.12
>>>
>>> thanks!
>>>
>>
>> Hi Simon,
>>
>> sorry for the trouble but there is also v3 for this patch.
>> it uses ac_check_member instead of ac_compile.
>> is it possible to take v3 which is a lot cleaner?
> 
> Sorry for the mix up,
> 
> would it be possible to provide an incremental patch
> to upgrade master to v3?
> 

sure. i'll do that.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] lacp: Add support to recognize LACP Marker RX PDUs.

2019-11-12 Thread Nitin katiyar via dev
OVS does not support the LACP Marker protocol. Typically, ToR switches
send a LACP Marker PDU when restarting LACP negotiation following a link
flap or LACP timeout.

When a LACP Marker PDU is received, OVS logs the following warning and
drops the packet:
“lacp(pmdXXX)|WARN|bond-prv: received an unparsable LACP PDU.”

As the above message is logged around the same time the link flap or
LACP down events are logged, it gives a misleading impression that the
reception of an unparsable LACP PDU is the reason for the LACP down
event.

The proposed patch does not add support for the LACP Marker protocol.
It simply recognizes LACP Marker packets, drops them and logs a clear
message indicating that a Marker packet was a received. A counter to
track the number of such packets received is also added.

Signed-off-by: Nitin katiyar 
---
 lib/lacp.c | 49 +
 1 file changed, 37 insertions(+), 12 deletions(-)

diff --git a/lib/lacp.c b/lib/lacp.c
index 16c823b..705d88f 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -86,6 +86,12 @@ BUILD_ASSERT_DECL(LACP_PDU_LEN == sizeof(struct lacp_pdu));
 
 /* Implementation. */
 
+enum pdu_subtype {
+SUBTYPE_UNUSED = 0,
+SUBTYPE_LACP,   /* Link Aggregation Control Protocol. */
+SUBTYPE_MARKER, /* Link Aggregation Marker Protocol. */
+};
+
 enum slave_status {
 LACP_CURRENT,   /* Current State.  Partner up to date. */
 LACP_EXPIRED,   /* Expired State.  Partner out of date. */
@@ -130,6 +136,7 @@ struct slave {
 
 uint32_t count_rx_pdus; /* dot3adAggPortStatsLACPDUsRx */
 uint32_t count_rx_pdus_bad; /* dot3adAggPortStatsIllegalRx */
+uint32_t count_rx_pdus_marker;  /* dot3adAggPortStatsMarkerPDUsRx */
 uint32_t count_tx_pdus; /* dot3adAggPortStatsLACPDUsTx */
 uint32_t count_link_expired;/* Num of times link expired */
 uint32_t count_link_defaulted;  /* Num of times link defaulted */
@@ -183,12 +190,13 @@ compose_lacp_pdu(const struct lacp_info *actor,
 pdu->collector_delay = htons(0);
 }
 
-/* Parses 'b' which represents a packet containing a LACP PDU.  This function
- * returns NULL if 'b' is malformed, or does not represent a LACP PDU format
+/* Parses 'p' which represents a packet containing a LACP PDU. This function
+ * returns NULL if 'p' is malformed, or does not represent a LACP PDU format
  * supported by OVS.  Otherwise, it returns a pointer to the lacp_pdu contained
- * within 'b'. */
+ * within 'p'. It also returns the subtype of PDU.*/
+
 static const struct lacp_pdu *
-parse_lacp_packet(const struct dp_packet *p)
+parse_lacp_packet(const struct dp_packet *p, enum pdu_subtype *subtype)
 {
 const struct lacp_pdu *pdu;
 
@@ -198,8 +206,13 @@ parse_lacp_packet(const struct dp_packet *p)
 if (pdu && pdu->subtype == 1
 && pdu->actor_type == 1 && pdu->actor_len == 20
 && pdu->partner_type == 2 && pdu->partner_len == 20) {
+*subtype = SUBTYPE_LACP;
 return pdu;
-} else {
+} else if (pdu && pdu->subtype == SUBTYPE_MARKER) {
+*subtype = SUBTYPE_MARKER;
+return NULL;
+} else{
+*subtype = SUBTYPE_UNUSED;
 return NULL;
 }
 }
@@ -336,6 +349,7 @@ lacp_process_packet(struct lacp *lacp, const void *slave_,
 long long int tx_rate;
 struct slave *slave;
 bool lacp_may_enable = false;
+enum pdu_subtype subtype;
 
 lacp_lock();
 slave = slave_lookup(lacp, slave_);
@@ -344,11 +358,20 @@ lacp_process_packet(struct lacp *lacp, const void *slave_,
 }
 slave->count_rx_pdus++;
 
-pdu = parse_lacp_packet(packet);
-if (!pdu) {
-slave->count_rx_pdus_bad++;
-VLOG_WARN_RL(, "%s: received an unparsable LACP PDU.", lacp->name);
-goto out;
+pdu = parse_lacp_packet(packet, );
+switch (subtype) {
+case SUBTYPE_LACP:
+break;
+case SUBTYPE_MARKER:
+slave->count_rx_pdus_marker++;
+VLOG_DBG("%s: received a LACP marker PDU.", lacp->name);
+goto out;
+case SUBTYPE_UNUSED:
+default:
+slave->count_rx_pdus_bad++;
+VLOG_WARN_RL(, "%s: received an unparsable LACP PDU.",
+ lacp->name);
+goto out;
 }
 
 /* On some NICs L1 state reporting is slow. In case LACP packets are
@@ -367,7 +390,7 @@ lacp_process_packet(struct lacp *lacp, const void *slave_,
 
 slave->ntt_actor = pdu->partner;
 
-/* Update our information about our partner if it's out of date.  This may
+/* Update our information about our partner if it's out of date. This may
  * cause priorities to change so re-calculate attached status of all
  * slaves.  */
 if (memcmp(>partner, >actor, sizeof pdu->actor)) {
@@ -1054,9 +1077,11 @@ lacp_print_stats(struct ds *ds, struct lacp *lacp) 
OVS_REQUIRES(mutex)
 for (i = 0; i < shash_count(_shash); i++) {
 slave = sorted_slaves[i]->data;
 

Re: [ovs-dev] [PATCH v2] compat: Add compat fix for old kernels

2019-11-12 Thread Simon Horman
On Mon, Nov 11, 2019 at 04:49:18PM +, Roi Dayan wrote:
> 
> 
> On 2019-11-11 4:08 PM, Roi Dayan wrote:
> > 
> > 
> > On 2019-11-11 3:39 PM, Simon Horman wrote:
> >> On Mon, Nov 04, 2019 at 11:49:44AM +0200, Roi Dayan wrote:
> >>> In kernels older than 4.8, struct tcf_t didn't have the firstuse.
> >>> If openvswitch is compiled with the compat pkt_cls.h then there is
> >>> a struct size mismatch between openvswitch and the kernel which cause
> >>> parsing netlink actions to fail.
> >>> After this commit parsing the netlink actions pass even if compiled with
> >>> the compat pkt_cls.h.
> >>>
> >>> Signed-off-by: Roi Dayan 
> >>> ---
> >>>
> >>> v1->v2:
> >>> - fix mix of tabs and spaces in acinclude.m4
> >>
> >> Thanks, applied to master and branch-2.8 ... branch-2.12
> > 
> > thanks!
> > 
> 
> Hi Simon,
> 
> sorry for the trouble but there is also v3 for this patch.
> it uses ac_check_member instead of ac_compile.
> is it possible to take v3 which is a lot cleaner?

Sorry for the mix up,

would it be possible to provide an incremental patch
to upgrade master to v3?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev