Re: [ovs-dev] [ovn] no tunnel from GW to compute

2020-10-27 Thread Tony Liu
Checked OF flows for working and non-working FIPs, can't find
any difference. DF flow is installed by vswitchd based on OF flow
when processing the first packet. I enabled debug logging, no log
for working FIP, a few logs for non-working FIP. Does that mean
something wrong about non-working FIP?
==
2020-10-28T05:15:22.058Z|01203|dpif(handler8)|DBG|system@ovs-system: miss 
upcall:
recirc_id(0),dp_hash(0),skb_priority(0),in_port(4),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),eth(src=e8:1c:ba:9f:b7:c6,dst=fa:16:3e:45:da:61),eth_type(0x0800),ipv4(src=172.16.160.1,dst=10.59.53.8,proto=1,tos=0,ttl=125,frag=no),icmp(type=8,code=0)
icmp,vlan_tci=0x,dl_src=e8:1c:ba:9f:b7:c6,dl_dst=fa:16:3e:45:da:61,nw_src=172.16.160.1,nw_dst=10.59.53.8,nw_tos=0,nw_ecn=0,nw_ttl=125,icmp_type=8,icmp_code=0
 icmp_csum:6013
..
2020-10-28T05:15:22.059Z|00808|vconn|DBG|unix#1: sent (Success): NXT_PACKET_IN2 
(OF1.3) (xid=0x0): table_id=24 cookie=0x9173f925 total_len=74 
ct_state=new|trk|dnat,ct_zone=507,ct_nw_src=172.16.160.1,ct_nw_dst=10.59.53.8,ct_nw_proto=1,ct_tp_src=8,ct_tp_dst=0,ip,reg0=0xa0a000a,reg1=0xa0a0001,reg9=0x8,reg10=0x1,reg11=0x1fb,reg12=0x1fa,reg14=0x1,reg15=0x7,metadata=0x121,in_port=1
 (via action) data_len=74 (unbuffered)
 
userdata=00.00.00.00.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.40.00.00.00.01.de.10.00.00.20.04.ff.ff.00.18.00.00.23.20.00.06.00.20.00.60.00.00.00.01.de.10.00.00.22.04.00.19.00.10.80.00.2a.02.00.01.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.20.00.00.00
icmp,vlan_tci=0x,dl_src=fa:16:3e:93:f4:1e,dl_dst=00:00:00:00:00:00,nw_src=172.16.160.1,nw_dst=10.10.0.10,nw_tos=0,nw_ecn=0,nw_ttl=124,icmp_type=8,icmp_code=0
 icmp_csum:6013

..
2020-10-28T05:15:27.060Z|01102|dpif(handler10)|DBG|system@ovs-system: action 
upcall:
recirc_id(0x3ad25),dp_hash(0),skb_priority(0),in_port(4),skb_mark(0),ct_state(0xa1),ct_zone(0x1fb),ct_mark(0),ct_label(0),ct_tuple4(src=172.16.160.1,dst=10.59.53.8,proto=1,tp_src=8,tp_dst=0),eth(src=fa:16:3e:93:f4:1e,dst=00:00:00:00:00:00),eth_type(0x0800),ipv4(src=172.16.160.1,dst=10.10.0.10,proto=1,tos=0,ttl=124,frag=no),icmp(type=8,code=0)
icmp,vlan_tci=0x,dl_src=fa:16:3e:93:f4:1e,dl_dst=00:00:00:00:00:00,nw_src=172.16.160.1,nw_dst=10.10.0.10,nw_tos=0,nw_ecn=0,nw_ttl=124,icmp_type=8,icmp_code=0
 icmp_csum:6012
..
2020-10-28T05:15:27.061Z|00834|vconn|DBG|unix#1: sent (Success): NXT_PACKET_IN2 
(OF1.3) (xid=0x0): table_id=24 cookie=0x9173f925 total_len=74 
ct_state=new|trk|dnat,ct_zone=507,ct_nw_src=172.16.160.1,ct_nw_dst=10.59.53.8,ct_nw_proto=1,ct_tp_src=8,ct_tp_dst=0,ip,reg0=0xa0a000a,reg1=0xa0a0001,reg9=0x8,reg10=0x1,reg11=0x1fb,reg12=0x1fa,reg14=0x1,reg15=0x7,metadata=0x121,in_port=1
 (via action) data_len=74 (unbuffered)
 
userdata=00.00.00.00.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.40.00.00.00.01.de.10.00.00.20.04.ff.ff.00.18.00.00.23.20.00.06.00.20.00.60.00.00.00.01.de.10.00.00.22.04.00.19.00.10.80.00.2a.02.00.01.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.20.00.00.00
icmp,vlan_tci=0x,dl_src=fa:16:3e:93:f4:1e,dl_dst=00:00:00:00:00:00,nw_src=172.16.160.1,nw_dst=10.10.0.10,nw_tos=0,nw_ecn=0,nw_ttl=124,icmp_type=8,icmp_code=0
 icmp_csum:6012
..
2020-10-28T05:15:32.059Z|01359|dpif(handler8)|DBG|system@ovs-system: action 
upcall:
recirc_id(0x3ad25),dp_hash(0),skb_priority(0),in_port(4),skb_mark(0),ct_state(0xa1),ct_zone(0x1fb),ct_mark(0),ct_label(0),ct_tuple4(src=172.16.160.1,dst=10.59.53.8,proto=1,tp_src=8,tp_dst=0),eth(src=fa:16:3e:93:f4:1e,dst=00:00:00:00:00:00),eth_type(0x0800),ipv4(src=172.16.160.1,dst=10.10.0.10,proto=1,tos=0,ttl=124,frag=no),icmp(type=8,code=0)
icmp,vlan_tci=0x,dl_src=fa:16:3e:93:f4:1e,dl_dst=00:00:00:00:00:00,nw_src=172.16.160.1,nw_dst=10.10.0.10,nw_tos=0,nw_ecn=0,nw_ttl=124,icmp_type=8,icmp_code=0
 icmp_csum:6011
..
2020-10-28T05:15:32.060Z|00856|vconn|DBG|unix#1: sent (Success): NXT_PACKET_IN2 
(OF1.3) (xid=0x0): table_id=24 cookie=0x9173f925 total_len=74 
ct_state=new|trk|dnat,ct_zone=507,ct_nw_src=172.16.160.1,ct_nw_dst=10.59.53.8,ct_nw_proto=1,ct_tp_src=8,ct_tp_dst=0,ip,reg0=0xa0a000a,reg1=0xa0a0001,reg9=0x8,reg10=0x1,reg11=0x1fb,reg12=0x1fa,reg14=0x1,reg15=0x7,metadata=0x121,in_port=1
 (via action) data_len=74 (unbuffered)
 
userdata=00.00.00.00.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.40.00.00.00.01.de.10.00.00.20.04.ff.ff.00.18.00.00.23.20.00.06.00.20.00.60.00.00.00.01.de.10.00.00.22.04.00.19.00.10.80.00.2a.02.00.01.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.20.00.00.00
icmp,vlan_tci=0x,dl_src=fa:16:3e:93:f4:1e,dl_dst=00:00:00:00:00:00,nw_src=172.16.160.1,nw_dst=10.10.0.10,nw_tos=0,nw_ecn=0,nw_ttl=124,icmp_type=8,icmp_code=0
 icmp_csum:6011
==

Thanks!
Tony
> -Original Message-
> From: dev  On B

Re: [ovs-dev] [PATCH v3 2/2] netdev-dpdk: Add option to configure VF MAC address.

2020-10-27 Thread Gaëtan Rivet
Hi Ilya, Kevin,

Thanks for the remarks, I will fix them.
However I have two questions inline.

On 27/10/20 15:03 +0100, Ilya Maximets wrote:
> On 10/20/20 1:15 PM, Kevin Traynor wrote:
> > On 16/09/2020 16:17, Gaetan Rivet wrote:
> >> In some cloud topologies, using DPDK VF representors in guest requires
> >> configuring a VF before it is assigned to the guest.
> >>
> >> A first basic option for such configuration is setting the VF MAC
> >> address. Add a key 'dpdk-vf-mac' to the 'options' column of the Interface
> >> table.
> >>
> >> This option can be used as such:
> >>
> >>$ ovs-vsctl add-port br0 dpdk-rep0 -- set Interface dpdk-rep0 type=dpdk 
> >> \
> >>   options:dpdk-vf-mac=00:11:22:33:44:55
> >>
> > 
> > If I got it right, it doesn't seem like it solves any issues by limiting
> > to representor, but is just an attempt to limit some of the edge cases
> > around bifurcated devices to the devices where the requested
> > functionality is really needed now.
> > 
> > This limitation has some costs...
> > 
> > We will now have, mac, mac_in_use, dpdk-vf-mac, configured_hwaddr,
> > requested_hwaddr for the user to understand. I think it can be confusing.
> 
> To be clear, 'configured_hwaddr/requested_hwaddr' type of report should not
> exist at least in the output of 'get_config()' function.  Same applies
> to all existing '{configured,requested}_something' reports.
> 'get_config()' intended to report things that could be copied and passed 
> directly
> to database.  OTOH, 'get_status()' should report what is an actual device
> status, i.e. 'dpdk-vf-mac' should be in 'get_config()' and 'configured_hwaddr'
> should be in 'get_status()', but it should be named differently, e.g. just
> 'dpdk-vf-mac' and the callback it placed in already says that it's actually
> 'configured'.  In short:
>   - 'dpdk-vf-mac' from the 'get_config()' should report dev->requested_hwaddr.
>   - 'dpdk-vf-mac' from the 'get_status()' should report dev->hwaddr.
> Of course, all this should be reported only if device is a representor.
> 
> 

Indeed I had followed the other '{configured|requested}_' fields.

This is not related to this patchset, however I'd like to know how to
proceed to fix those other fields? They were exposed to users possibly,
should there be a deprecation process to warn of the change, or should
they just be fixed directly as the get_config() API was not respected?

> > 
> > 'ovs-vsctl show' will show the dpdk-vf-mac, which is the main one we
> > document, but it's not clear if it was successful or not. (yes there is
> > a log entry, but it is a separate place).
> > 
> > As it is specific for representors, if we ever want to allow setting of
> > mac on any non representor DPDK ports, we have an awkward user
> > interface. We would need to make another 'dpdk-mac' type option, or we
> > decide then to allow use mac in interface table but that it doesn't work
> > for representors, or there are two ways to set for representors etc.
> > None of this seems great.
> > 
> > My feeling is that it is making things complicated for the user with
> > this many knobs, where we could just have mac and mac_in_use, live with
> > the edge cases (as Gaetan pointed out, we already do for MTU) and we
> > know as well if we ever need to extend further, the user interface will
> > stay simple. Just my 2C, maybe there's a good argument why we can't do
> > it like this.
> 
> Setting mac for physical device has lots of split-brain issues and unclear
> ways how and when undo changes as we do not know what was original mac or
> what to do if there was no mac at all at the start.  Moreover, some devices
> will not allow setting mac to it's original value (e.g. all zeroes).
> One more point is that vf and representor itself are different devices and
> these devices could have different mac addresses.  For now we agreed that
> DPDK api works for representors in a way that it always changes parameters
> of a virtual function, not representors', but that is not a very clear thing
> for all users (e.g. for DPDK developers), so 'vf' in 'dpdk-vf-mac' 
> specifically
> points to that difference.
> 
> After a long discussion we decided to make this option as specific as 
> possible,
> so users will not try to use it to configure anything else beside mac of a
> virtual function.  All restrictions and downsides of configuration of vf mac
> was/should be documented.
> 
> 
> > 
> > Few comments on the code as it is now below.
> > 
> > 
> >> Signed-off-by: Gaetan Rivet 
> >> Suggested-by: Ilya Maximets 
> >> Acked-by: Eli Britstein 
> >> ---
> >>  Documentation/topics/dpdk/phy.rst | 22 
> >>  NEWS  |  2 ++
> >>  lib/netdev-dpdk.c | 60 +++
> >>  vswitchd/vswitch.xml  | 13 +++
> >>  4 files changed, 97 insertions(+)
> >>
> >> diff --git a/Documentation/topics/dpdk/phy.rst 
> >> b/Documentation/topics/dpdk/phy.rst
> >> index 38e52c8de..73dcb7c28 100644
>

[ovs-dev] 答复: [PATCH V3 2/4] Add GSO support for DPDK data path

2020-10-27 Thread 杨燚
-邮件原件-
发件人: dev [mailto:ovs-dev-boun...@openvswitch.org] 代表 Ilya Maximets
发送时间: 2020年10月27日 21:03
收件人: yang_y...@163.com; ovs-dev@openvswitch.org
抄送: f...@sysclose.org; i.maxim...@ovn.org
主题: Re: [ovs-dev] [PATCH V3 2/4] Add GSO support for DPDK data path

On 8/7/20 12:56 PM, yang_y...@163.com wrote:
> From: Yi Yang 
> 
> GSO(Generic Segment Offload) can segment large UDP  and TCP packet to 
> small packets per MTU of destination , especially for the case that 
> physical NIC can't do hardware offload VXLAN TSO and VXLAN UFO, GSO 
> can make sure userspace TSO can still work but not drop.
> 
> In addition, GSO can help improve UDP performane when UFO is enabled 
> in VM.
> 
> GSO can support TCP, UDP, VXLAN TCP, VXLAN UDP, it is done in Tx 
> function of physical NIC.
> 
> Signed-off-by: Yi Yang 
> ---
>  lib/dp-packet.h|  21 +++-
>  lib/netdev-dpdk.c  | 358 
> +
>  lib/netdev-linux.c |  17 ++-
>  lib/netdev.c   |  67 +++---
>  4 files changed, 417 insertions(+), 46 deletions(-)
> 
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h index 79895f2..c33868d 
> 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -83,6 +83,8 @@ enum dp_packet_offload_mask {
>  DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CKSUM, PKT_TX_SCTP_CKSUM, 0x800),
>  /* VXLAN TCP Segmentation Offload. */
>  DEF_OL_FLAG(DP_PACKET_OL_TX_TUNNEL_VXLAN, PKT_TX_TUNNEL_VXLAN, 
> 0x1000),
> +/* UDP Segmentation Offload. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_UDP_SEG, PKT_TX_UDP_SEG, 0x2000),
>  /* Adding new field requires adding to 
> DP_PACKET_OL_SUPPORTED_MASK. */  };
>  
> @@ -97,7 +99,8 @@ enum dp_packet_offload_mask {
>   DP_PACKET_OL_TX_IPV6  | \
>   DP_PACKET_OL_TX_TCP_CKSUM | \
>   DP_PACKET_OL_TX_UDP_CKSUM | \
> - DP_PACKET_OL_TX_SCTP_CKSUM)
> + DP_PACKET_OL_TX_SCTP_CKSUM| \
> + DP_PACKET_OL_TX_UDP_SEG)
>  
>  #define DP_PACKET_OL_TX_L4_MASK (DP_PACKET_OL_TX_TCP_CKSUM | \
>   DP_PACKET_OL_TX_UDP_CKSUM | \ @@ 
> -956,6 +959,13 @@ dp_packet_hwol_is_tso(const struct dp_packet *b)
>  return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_TCP_SEG);  
> }
>  
> +/* Returns 'true' if packet 'b' is marked for UDP segmentation 
> +offloading. */ static inline bool dp_packet_hwol_is_uso(const struct 
> +dp_packet *b) {
> +return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_UDP_SEG); 
> +}
> +
>  /* Returns 'true' if packet 'b' is marked for IPv4 checksum 
> offloading. */  static inline bool  dp_packet_hwol_is_ipv4(const 
> struct dp_packet *b) @@ -1034,6 +1044,15 @@ 
> dp_packet_hwol_set_tcp_seg(struct dp_packet *b)
>  *dp_packet_ol_flags_ptr(b) |= DP_PACKET_OL_TX_TCP_SEG;  }
>  
> +/* Mark packet 'b' for UDP segmentation offloading.  It implies that
> + * either the packet 'b' is marked for IPv4 or IPv6 checksum 
> +offloading
> + * and also for UDP checksum offloading. */ static inline void 
> +dp_packet_hwol_set_udp_seg(struct dp_packet *b) {
> +*dp_packet_ol_flags_ptr(b) |= DP_PACKET_OL_TX_UDP_SEG; }
> +
>  #ifdef DPDK_NETDEV
>  /* Mark packet 'b' for VXLAN TCP segmentation offloading. */  static 
> inline void diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 
> 30493ed..888a45e 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -38,13 +38,15 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  
>  #include "cmap.h"
>  #include "coverage.h"
> @@ -162,6 +164,7 @@ typedef uint16_t dpdk_port_t;
> | DEV_TX_OFFLOAD_UDP_CKSUM\
> | DEV_TX_OFFLOAD_IPV4_CKSUM)
>  
> +#define MAX_GSO_MBUFS 64
>  
>  static const struct rte_eth_conf port_conf = {
>  .rxmode = {
> @@ -2171,6 +2174,16 @@ is_local_to_local(uint16_t src_port_id, struct 
> netdev_dpdk *dev)
>  return ret;
>  }
>  
> +static uint16_t
> +get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype) {
> +if (ethertype == htons(RTE_ETHER_TYPE_IPV4)) {
> +return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> +} else { /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
> +return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> +}
> +}
> +
>  /* Prepare the packet for HWOL.
>   * Return True if the packet is OK to continue. */  static bool @@ 
> -2203,10 +2216,9 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
> struct rte_mbuf *mbuf)
>   * also can't be handled. So PKT_TX_TUNNEL_VXLAN must be cleared
>   * outer_l2_len and outer_l3_len must be zeroed.
>   */
> -if (!(dev->up.ol_flags & NETDEV_TX_OFFLOAD_VXLAN_TSO)
> -

[ovs-dev] 答复: [PATCH V3 3/4] Add VXLAN TCP and UDP GRO support for DPDK data path

2020-10-27 Thread 杨燚
-邮件原件-
发件人: dev [mailto:ovs-dev-boun...@openvswitch.org] 代表 Ilya Maximets
发送时间: 2020年10月27日 21:12
收件人: yang_y...@163.com; ovs-dev@openvswitch.org
抄送: f...@sysclose.org; i.maxim...@ovn.org
主题: Re: [ovs-dev] [PATCH V3 3/4] Add VXLAN TCP and UDP GRO support for DPDK 
data path

On 8/7/20 12:56 PM, yang_y...@163.com wrote:
> From: Yi Yang 
> 
> GRO(Generic Receive Offload) can help improve performance when TSO 
> (TCP Segment Offload) or VXLAN TSO is enabled on transmit side, this 
> can avoid overhead of ovs DPDK data path and enqueue vhost for VM by 
> merging many small packets to large packets (65535 bytes at most) once 
> it receives packets from physical NIC.

IIUC, this patch allows multi-segment mbufs to float across different parts of 
OVS.  This will definitely crash it somewhere.  Much more changes all over the 
OVS required to make it safely work with such mbufs.  There were few attempts 
to introduce this support, but all of them ended up being rejected.  As it is 
this patch is not acceptable as it doesn't cover almost anything beside simple 
cases inside netdev implementation.

Here is the latest attempt with multi-segment mbufs:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=130193&state=*

Best regards, Ilya Maximets.

[Yi Yang] We have to support this because we have supported TSO for TCP, it 
can't handle big UDP, this is why we must introduce GSO, the prerequisite of 
GSO is multi-segment  must be enabled because GSOed mbufs are multi-segmented, 
but it is just last  step before dpdk Tx, so I don't think it is an issue, per 
my test in our openstack environment, I didn't encounter any crash, this just 
enabled DPDK PMD driver to handle GSOed mbuf. For GRO, reassembling also use 
chained multi-segment mbuf to avoid copy, per long time test, it also didn't 
lead to any crash. We can fix some corner cases if they aren't covered. 


> 
> It can work for both VXLAN and vlan case.
> 
> Signed-off-by: Yi Yang 
> ---
>  lib/dp-packet.h|  37 -
>  lib/netdev-dpdk.c  | 227 
> -
>  lib/netdev-linux.c | 112 --
>  3 files changed, 365 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h index c33868d..18307c0 
> 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -580,7 +580,16 @@ dp_packet_set_size(struct dp_packet *b, uint32_t v)
>   * (and thus 'v') will always be <= UINT16_MAX; this means that there is 
> no
>   * loss of accuracy in assigning 'v' to 'data_len'.
>   */
> -b->mbuf.data_len = (uint16_t)v;  /* Current seg length. */
> +if (b->mbuf.nb_segs <= 1) {
> +b->mbuf.data_len = (uint16_t)v;  /* Current seg length. */
> +} else {
> +/* For multi-seg packet, if it is resize, data_len should be
> + * adjusted by offset, this will happend in case of push or pop.
> + */
> +if (b->mbuf.pkt_len != 0) {
> +b->mbuf.data_len += v - b->mbuf.pkt_len;
> +}
> +}
>  b->mbuf.pkt_len = v; /* Total length of all segments linked 
> to
>* this segment. */  } @@ 
> -1092,6 +1101,20 @@ dp_packet_hwol_set_l4_len(struct dp_packet *b, int 
> l4_len)  {
>  b->mbuf.l4_len = l4_len;
>  }
> +
> +/* Set outer_l2_len for the packet 'b' */ static inline void 
> +dp_packet_hwol_set_outer_l2_len(struct dp_packet *b, int 
> +outer_l2_len) {
> +b->mbuf.outer_l2_len = outer_l2_len; }
> +
> +/* Set outer_l3_len for the packet 'b' */ static inline void 
> +dp_packet_hwol_set_outer_l3_len(struct dp_packet *b, int 
> +outer_l3_len) {
> +b->mbuf.outer_l3_len = outer_l3_len; }
>  #else
>  /* Mark packet 'b' for VXLAN TCP segmentation offloading. */  static 
> inline void @@ -1125,6 +1148,18 @@ dp_packet_hwol_set_l4_len(struct 
> dp_packet *b OVS_UNUSED,
>int l4_len OVS_UNUSED)  {  }
> +
> +/* Set outer_l2_len for the packet 'b' */ static inline void 
> +dp_packet_hwol_set_outer_l2_len(struct dp_packet *b, int 
> +outer_l2_len) { }
> +
> +/* Set outer_l3_len for the packet 'b' */ static inline void 
> +dp_packet_hwol_set_outer_l3_len(struct dp_packet *b, int 
> +outer_l3_len) { }
>  #endif /* DPDK_NETDEV */
>  
>  static inline bool
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 888a45e..b6c57a6 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /* Include rte_compat.h first to allow experimental API's needed for the
>   * rte_meter.h rfc4115 functions. Once they are no longer marked as
> @@ -47,6 +48,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "cmap.h"
>  #include "coverage.h"
> @@ -2184,6 +2186,8 @@ get_udptcp_checksum(void *l3_hdr, void *l4_hdr, 
> uint16_t ethertype)
>  }
>  }
>  
> +#define UDP_VXLAN_ETH_HDR_SIZE 30
> +
>  /* Prepare the packet for HWOL.
>   * Return True 

Re: [ovs-dev] [PATCH v10 ovn] Allow to run multiple controllers on the same machine

2020-10-27 Thread Ihar Hrachyshka
On Wed, Sep 23, 2020 at 10:28 AM Numan Siddique  wrote:
>
>
>
> On Wed, Sep 23, 2020 at 6:13 PM Numan Siddique  wrote:
>>
>>
>>
>> On Wed, Sep 23, 2020 at 1:00 PM Han Zhou  wrote:
>>>
>>> On Tue, Sep 22, 2020 at 12:46 PM Ihar Hrachyshka 
>>> wrote:
>>> >
>>> > User stories:
>>> > 1) NFV: an admin wants to run two separate instances of OVN controller
>>> >using the same database but configuring ports on different bridges.
>>> >Some of these bridges may use DPDK while others may not.
>>> >
>>> > 2) Parallel OVN instances: an admin wants to run two separate
>>> >instances of OVN controller using different databases. The
>>> >instances are completely independent and serve different consumers.
>>> >For example, the same machine runs both OpenStack and OpenShift
>>> >stacks, each running its own separate OVN stack.
>>> >
>>> > To serve these use cases, several features should be added to
>>> > ovn-controller:
>>> >
>>> > - use different database configuration for multiple controllers;
>>> > - customize chassis name used by controller.
>>> >
>>> > =
>>> >
>>> > For each of the following database configuration options, their
>>> > extended chassis specific counterparts are introduced:
>>> >
>>> > external_ids:hostname
>>> > external_ids:ovn-bridge
>>> > external_ids:ovn-bridge-datapath-type
>>> > external_ids:ovn-bridge-mappings
>>> > external_ids:ovn-chassis-mac-mappings
>>> > external_ids:ovn-cms-options
>>> > external_ids:ovn-encap-csum
>>> > external_ids:ovn-encap-ip
>>> > external_ids:ovn-encap-type
>>> > external_ids:ovn-is-interconn
>>> > external_ids:ovn-monitor-all
>>> > external_ids:ovn-openflow-probe-interval
>>> > external_ids:ovn-remote
>>> > external_ids:ovn-remote-probe-interval
>>> >
>>> > For example,
>>> >
>>> > external_ids:ovn-bridge -> external_ids:ovn-bridge-=
>>> > external_ids:ovn-encap-ip -> external_ids:ovn-encap-ip-=
>>> > external_ids:ovn-remote -> external_ids:ovn-remote-=
>>> >
>>> > Priority wise,  specific options take precedence.
>>> >
>>> > =
>>> >
>>> > For system-id,
>>> >
>>> > You can now pass intended chassis name via CLI argument:
>>> >
>>> >   $ ovn-controller ... -n 
>>> >
>>> > Alternatively, you can configure a chassis name by putting it into the
>>> > ${ovn_sysconfdir}/system-id-override file before running the
>>> > controller.
>>> >
>>> > The latter option may be more useful in container environment where
>>> > the same image may be reused for multiple controller instances, where
>>> > ovs_sysconfigdir/ovn/system-id-override is a volume mounted into this
>>> > generic image. The override file is read once on startup. If you want
>>> > to apply a new chassis name to a controller instance, restart it to
>>> > reread the file.
>>> >
>>> > Priority wise, this is the order in which different means to configure
>>> > the chassis name are used:
>>> >
>>> > - ovn-controller ... -n  CLI argument.
>>> > - ${ovs_sysconfdir}/ovn/system-id-override file;
>>> > - external_ids:system-id= ovsdb option;
>>> >
>>> > =
>>> >
>>> > Concurrent chassis running on the same host may inadvertantly remove
>>> > patch ports that belong to their peer chassis. To avoid that, patch
>>> > ports are now tagged in external-ids:ovn-chassis-id with the
>>> > appropriate chassis name, and only patch ports that belong to the
>>> > chassis are touched when cleaning up. Also, now only tunnels on the
>>> > active integration bridge are being cleaned up.
>>> >
>>> > Note that external-ids:ovn-chassis-id key is already used for tunnel
>>> > ports to identify the remote tunnel endpoint. We can reuse the same
>>> > key for patch ports because the key usage is not overlapping.
>>> >
>>> > Alternatively, we could introduce a new key with a similar but
>>> > different name. This would simplify code changes needed but would
>>> > arguably introduce even more confusion. Since the key name is not
>>> > entirely self-descriptive for tunnel ports (a better name would be
>>> > e.g. ovn-remote-chassis or ovn-peer-chassis), the ideal scenario would
>>> > be to rename the key for tunnel endpoints but reuse it for patch
>>> > ports. This would involve additional migration steps and is probably
>>> > not worth the hassle.
>>> >
>>> Hi Ihar,
>>>
>>> Thanks for your patience on this. From my perspective, even if similar key
>>> names can be confusing, using exactly the same name is definitely *more*
>>> confusing. Since we already know there is a conflict, why not just picking
>>> a different name for the new one? Whatever key we use, documentation is
>>> still needed and will be helpful to avoid confusion. What do you think?
>>>
>>> Thanks,
>>> Han
>>>
>>
>> Hi Ihar,
>>
>> Thanks for the patch and for the patience. I did not review the code. But I 
>> did some testing
>> with this patch and below are the comments.
>>
>>
>>  1. The patch needs a rebase as it doesn't apply cleanly. I was able to 
>> resolve it locally though.
>>
>>  2. The below system test fails with this patch
>>
>> ***

[ovs-dev] 答复: [PATCH] netdev-dpdk: fix incorrect shinfo initialization

2020-10-27 Thread 杨燚
-邮件原件-
发件人: dev [mailto:ovs-dev-boun...@openvswitch.org] 代表 Flavio Leitner
发送时间: 2020年10月27日 21:08
收件人: Ilya Maximets 
抄送: yang_y...@163.com; ovs-dev@openvswitch.org; olivier.m...@6wind.com
主题: Re: [ovs-dev] [PATCH] netdev-dpdk: fix incorrect shinfo initialization

On Tue, Oct 27, 2020 at 01:47:22PM +0100, Ilya Maximets wrote:
> On 10/27/20 12:34 PM, Flavio Leitner wrote:
> > On Wed, Oct 14, 2020 at 03:22:48PM +0800, yang_y...@163.com wrote:
> >> From: Yi Yang 
> >>
> >> shinfo is used to store reference counter and free callback of an 
> >> external buffer, but it is stored in mbuf if the mbuf has tailroom 
> >> for it.
> >>
> >> This is wrong because the mbuf (and its data) can be freed before 
> >> the external buffer, for example:
> >>
> >>   pkt2 = rte_pktmbuf_alloc(mp);
> >>   rte_pktmbuf_attach(pkt2, pkt);
> >>   rte_pktmbuf_free(pkt);
> 
> How is that possible with OVS?  Right now OVS doesn't support 
> multi-segement mbufs and will, likely, not support them in a near 
> future because it requires changes all other the codebase.
> 
> Is there any other scenario that could lead to issues with current OVS 
> implementation?

This is copying packets. The shinfo is allocated in the mbuf of the first 
packet which could be deleted without any references to the external buffer 
still using it.

Fbl

[Yi Yang]  Yes, this is not related with multi-segment mbuf, dpdk interfaces to 
system interfaces communication will use it if the packet size is greater than 
mtu size, i.e. TSO case from veth/tap to dpdk/vhost and backward will use it, 
this is a wrong use of shinfo, the same fix (which is used by virtio/vhost 
driver)has been merged into dpdk branch.
 
[Yi Yang] 



> 
> >>
> >> After this, pkt is freed, but it still contains shinfo, which is 
> >> referenced by pkt2.
> >>
> >> Fix this by always storing shinfo at the tail of external buffer.
> >>
> >> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload 
> >> support")
> >> Co-authored-by: Olivier Matz 
> >> Signed-off-by: Olivier Matz 
> >> Signed-off-by: Yi Yang 
> >> ---
> > 
> > Acked-by: Flavio Leitner 
> > 
> > Thanks Yi,
> > fbl
> > 
> 

--
fbl
___
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] Marco jurídico y desarrollo de casos prácticos

2020-10-27 Thread Finiquitos y liquidaciones
Webinar en Vivo: 
Finiquitos y liquidaciones.
Miércoles 11 de Noviembre- Horario de 10:00 a 17:00 Hrs.

Desarrollar de una forma dinámica y fácil los aspectos a considerar para el 
cálculo de los finiquitos y liquidaciones, mediante el
 estudio del marco jurídico y el desarrollo de casos prácticos.

Objetivos Específicos:

- Estudiará del marco legal de los finiquitos y liquidaciones. 
- Proporcionará una guía básica para el cálculo de los finiquitos y las 
liquidaciones. 
- Desarrollará de casos prácticos que ejemplifiquen los diferentes escenarios 
en el calculo de finiquitos y liquidaciones. 

Este curso va dirigido a los Contadores Públicos, auxiliares contables y 
cualquier persona que quiera conocer los puntos clave del cálculo 
de liquidaciones e indemnizaciones.

Para mayor información, responder sobre este correo con la palabra Finiquitos + 
los siguientes datos:

NOMBRE:
TELÉFONO:
EMPRESA:
CORREO ALTERNO: 

Para información inmediata llamar al:
(+52) 55 15 54 66 30 - (+52) 55 30 16 70 85
O puede enviarnos un mensaje vía whatsapp 

Innova Learn México - innovalearn. mx - Mérida, Yucatán, México


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


Re: [ovs-dev] [PATCH v2 5/6] lldp: correctly increase discarded count

2020-10-27 Thread 0-day Robot
Bleep bloop.  Greetings Fabrizio D'Angelo, 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.


checkpatch:
ERROR: Author Vincent Bernat  needs to sign off.
Lines checked: 45, Warnings: 0, Errors: 1


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 v2 4/6] lldp: increase statsTLVsUnrecognizedTotal on unknown TLV

2020-10-27 Thread 0-day Robot
Bleep bloop.  Greetings Fabrizio D'Angelo, 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.


checkpatch:
ERROR: Author Vincent Bernat  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Fabrizio D'Angelo 
Lines checked: 34, Warnings: 1, Errors: 1


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 v2 3/6] lldp: fix a buffer overflow when handling management address TLV

2020-10-27 Thread 0-day Robot
Bleep bloop.  Greetings Fabrizio D'Angelo, 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.


checkpatch:
ERROR: Author Vincent Bernat  needs to sign off.
Lines checked: 52, Warnings: 0, Errors: 1


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 v2 2/6] lldp: Fix size of PEEK_DISCARD_UINT32()

2020-10-27 Thread 0-day Robot
Bleep bloop.  Greetings Fabrizio D'Angelo, 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.


checkpatch:
ERROR: Author Jonas Johansson  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Fabrizio D'Angelo 
Lines checked: 35, Warnings: 1, Errors: 1


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


[ovs-dev] Benachrichtigung der Gewinner !!!.

2020-10-27 Thread max
Benachrichtigung der Gewinner !!!.

Ihre E-Mail-ID hat Ђ 150.000,00 Euro (ein hundertfьnfzigtausend Euro) in 
LottoMax International Charity program.Ref Well Sp gewonnen
/179/0-39/44/4-07/ES.Lucky No.9 / 44/15/27 / 49.For Weitere Informationen und 
Antragsverfahren , benutzen Sie bitte unser Agent unten in Verbindung;

National Post-Code Agency.S.L
Mr.Jaime Sanchez
E-mail: infocod...@aol.com


Mit Ihren vollstдndigen Namen, Adresse, Alter, Beruf, Telefonnummern Senden Sie 
Ihre Antwort auf diese E-Mail:  infocod...@aol.com
Hinweis: Dies ist eine internationale Lotterie-Programm. Dieser Eintrag wurde 
automatisch aus dem Englischen ins Deutschland ьbersetzt.

Herzlichen Glьckwunsch !
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn 2/2] pinctrl: Fix memory leak when handling empty lb backends.

2020-10-27 Thread Dumitru Ceara
Fixes: 7c3523c71932 ("OVN: introduce trigger_event() action")
Signed-off-by: Dumitru Ceara 
---
 controller/pinctrl.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index bc482c0..08adc10 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -5744,6 +5744,9 @@ pinctrl_handle_empty_lb_backends_opts(struct ofpbuf 
*userdata)
 if (!vip || !protocol || !load_balancer) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 VLOG_WARN_RL(&rl, "missing lb parameters in userdata");
+free(vip);
+free(protocol);
+free(load_balancer);
 return false;
 }
 

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


[ovs-dev] [PATCH ovn 0/2] Fix memory leaks due to controller events.

2020-10-27 Thread Dumitru Ceara
Fixes: be1eeb09d373 ("OVN: introduce Controller_Event table")
Fixes: 7c3523c71932 ("OVN: introduce trigger_event() action")
Signed-off-by: Dumitru Ceara 

Dumitru Ceara (2):
  pinctrl: Fix memory leak in controller_event_run().
  pinctrl: Fix memory leak when handling empty lb backends.


 controller/pinctrl.c |4 
 1 file changed, 4 insertions(+)

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


[ovs-dev] [PATCH v2 3/6] lldp: fix a buffer overflow when handling management address TLV

2020-10-27 Thread Fabrizio D'Angelo
From: Vincent Bernat 

Upstream commit:
commit a8d8006c06d9ac16ebcf33295cbd625c0847ca9b
Author: Vincent Bernat 
Date: Sun, 4 Oct 2015 01:50:38 +0200

lldp: fix a buffer overflow when handling management address TLV

When a remote device was advertising a too large management address
while still respecting TLV boundaries, lldpd would crash due to a buffer
overflow. However, the buffer being a static one, this buffer overflow
is not exploitable if hardening was not disabled. This bug exists since
version 0.5.6.

Co-authored-by: Fabrizio D'Angelo 
Signed-off-by: Fabrizio D'Angelo 
---
 lib/lldp/lldp.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
index 593c5e1c34..172bccdc71 100644
--- a/lib/lldp/lldp.c
+++ b/lib/lldp/lldp.c
@@ -530,6 +530,11 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int 
s,
 case LLDP_TLV_MGMT_ADDR:
 CHECK_TLV_SIZE(1, "Management address");
 addr_str_length = PEEK_UINT8;
+if (addr_str_length > sizeof(addr_str_buffer)) {
+VLOG_WARN("too large management address on %s",
+  hardware->h_ifname);
+goto malformed;
+}
 CHECK_TLV_SIZE(1 + addr_str_length, "Management address");
 PEEK_BYTES(addr_str_buffer, addr_str_length);
 addr_length = addr_str_length - 1;
@@ -554,7 +559,7 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int 
s,
 break;
 
 case LLDP_TLV_ORG:
-CHECK_TLV_SIZE(4, "Organisational");
+CHECK_TLV_SIZE(1 + (int)sizeof(orgid), "Organisational");
 PEEK_BYTES(orgid, sizeof orgid);
 tlv_subtype = PEEK_UINT8;
 if (memcmp(dot1, orgid, sizeof orgid) == 0) {
-- 
2.26.2

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


[ovs-dev] [PATCH v2 1/6] lldp: validate a bit more received LLDP frames

2020-10-27 Thread Fabrizio D'Angelo
From: Aaron Conole 

From: Vincent Bernat 

Upstream commit:
commit 3aeae72b97716fddac290634fad02b952d981f17
Author: Vincent Bernat 
Date:   Tue, 1 Oct 2019 21:42:42 +0200

lldp: validate a bit more received LLDP frames

Notably, we ensure the order and unicity of Chassis ID, Port ID and
TTL TLV. For Chassis ID and Port ID, we also ensure the maximum size
does not exceed 256.

Fix https://github.com/vincentbernat/lldpd/issues/351

Signed-off-by: Aaron Conole 
Co-authored-by: Aaron Conole 
Signed-off-by: Vincent Bernat 
---
 lib/lldp/lldp.c | 53 +++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
index 74f747fcdc..e61ce67746 100644
--- a/lib/lldp/lldp.c
+++ b/lib/lldp/lldp.c
@@ -341,6 +341,12 @@ lldp_send(struct lldpd *global OVS_UNUSED,
 
 return dp_packet_size(p);
 }
+#define CHECK_TLV_MAX_SIZE(x, name) \
+do { if (tlv_size > (x)) {  \
+VLOG_WARN(name " TLV too large received on %s", \
+  hardware->h_ifname);  \
+goto malformed; \
+} } while (0)
 
 int
 lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int s,
@@ -359,7 +365,7 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int 
s,
 int length, af;
 bool gotend = false;
 bool ttl_received = false;
-int tlv_size, tlv_type, tlv_subtype;
+int tlv_size, tlv_type, tlv_subtype, tlv_count = 0;
 u_int8_t *pos, *tlv;
 void *b;
 struct lldpd_aa_isid_vlan_maps_tlv *isid_vlan_map = NULL;
@@ -411,6 +417,31 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int 
s,
   hardware->h_ifname);
 goto malformed;
 }
+/* Check order for mandatory TLVs */
+tlv_count++;
+switch (tlv_type) {
+case LLDP_TLV_CHASSIS_ID:
+if (tlv_count != 1) {
+VLOG_WARN("first TLV should be a chassis ID on %s, not %d",
+  hardware->h_ifname, tlv_type);
+goto malformed;
+}
+break;
+case LLDP_TLV_PORT_ID:
+if (tlv_count != 2) {
+VLOG_WARN("second TLV should be a port ID on %s, not %d",
+  hardware->h_ifname, tlv_type);
+goto malformed;
+}
+break;
+case LLDP_TLV_TTL:
+if (tlv_count != 3) {
+VLOG_WARN("third TLV should be a TTL on %s, not %d",
+  hardware->h_ifname, tlv_type);
+goto malformed;
+}
+break;
+}
 
 switch (tlv_type) {
 case LLDP_TLV_END:
@@ -428,7 +459,8 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int 
s,
 
 case LLDP_TLV_CHASSIS_ID:
 case LLDP_TLV_PORT_ID:
-CHECK_TLV_SIZE(2, "Port Id");
+CHECK_TLV_SIZE(2, "Port/Chassis Id");
+CHECK_TLV_MAX_SIZE(256, "Port/Chassis Id");
 tlv_subtype = PEEK_UINT8;
 if (tlv_subtype == 0 || tlv_subtype > 7) {
 VLOG_WARN("unknown subtype for tlv id received on %s",
@@ -438,10 +470,22 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, 
int s,
 b = xzalloc(tlv_size - 1);
 PEEK_BYTES(b, tlv_size - 1);
 if (tlv_type == LLDP_TLV_PORT_ID) {
+if (port->p_id != NULL) {
+VLOG_WARN("Port ID TLV received twice on %s",
+  hardware->h_ifname);
+free(b);
+goto malformed;
+}
 port->p_id_subtype = tlv_subtype;
 port->p_id = b;
 port->p_id_len = tlv_size - 1;
 } else {
+if (chassis->c_id != NULL) {
+VLOG_WARN("Chassis ID TLV received twice on %s",
+  hardware->h_ifname);
+free(b);
+goto malformed;
+}
 chassis->c_id_subtype = tlv_subtype;
 chassis->c_id = b;
 chassis->c_id_len = tlv_size - 1;
@@ -449,6 +493,11 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int 
s,
 break;
 
 case LLDP_TLV_TTL:
+if (ttl_received) {
+VLOG_WARN("TTL TLV received twice on %s",
+  hardware->h_ifname);
+goto malformed;
+}
 CHECK_TLV_SIZE(2, "TTL");
 chassis->c_ttl = PEEK_UINT16;
 ttl_received = true;
-- 
2.26.2

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


[ovs-dev] [PATCH v2 0/6] Incorporate fixes from lldpd upstream

2020-10-27 Thread Fabrizio D'Angelo
This patchset pulls in a number of required fixes that have been
accumulated over the years in lldpd.

https://travis-ci.org/github/fabrizio8/ovs/builds/739390567

Aaron Conole (1):
  lldp: validate a bit more received LLDP frames

Fabrizio D'Angelo (5):
  lldp: Fix size of PEEK_DISCARD_UINT32()
  lldp: fix a buffer overflow when handling management address TLV
  lldp: increase statsTLVsUnrecognizedTotal on unknown TLV
  lldp: correctly increase discarded count
  AUTHORS: Add Fabrizio D'Angelo.

 AUTHORS.rst  |  1 +
 lib/lldp/lldp.c  | 63 +---
 lib/lldp/lldpd.c |  2 ++
 3 files changed, 62 insertions(+), 4 deletions(-)

-- 
2.26.2

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


[ovs-dev] [PATCH v2 6/6] AUTHORS: Add Fabrizio D'Angelo.

2020-10-27 Thread Fabrizio D'Angelo
Signed-off-by: Fabrizio D'Angelo 
---
 AUTHORS.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 9e9d210a20..31c1cf18d2 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -145,6 +145,7 @@ Eric Sesterhenn
eric.sesterh...@lsexperts.de
 Ethan J. Jackson   e...@eecs.berkeley.edu
 Ethan Rahn er...@arista.com
 Eziz Durdyyev  ezizdu...@gmail.com
+Fabrizio D'Angelo  fdang...@redhat.com
 Flavio Fernandes   fla...@flaviof.com
 Flavio Leitner f...@redhat.com
 Francesco Fuscoffu...@redhat.com
-- 
2.26.2

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


[ovs-dev] [PATCH v2 4/6] lldp: increase statsTLVsUnrecognizedTotal on unknown TLV

2020-10-27 Thread Fabrizio D'Angelo
From: Vincent Bernat 

Upstream commit:
commit 109bcd423cd560545ec7940d73a50c5584aebb0c
Author: Vincent Bernat 
Date: Sat, 6 Apr 2019 21:17:25 +0200

This was done for organization TLVs, but not for other TLVs.

Fix https://github.com/vincentbernat/lldpd/issues/323

Signed-off-by: Fabrizio D'Angelo 
---
 lib/lldp/lldp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
index 172bccdc71..3314f18189 100644
--- a/lib/lldp/lldp.c
+++ b/lib/lldp/lldp.c
@@ -679,6 +679,7 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int 
s,
 VLOG_WARN("unknown tlv (%d) received on %s",
   tlv_type,
   hardware->h_ifname);
+hardware->h_rx_unrecognized_cnt++;
 goto malformed;
 }
 if (pos > tlv + tlv_size) {
-- 
2.26.2

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


[ovs-dev] [PATCH v2 5/6] lldp: correctly increase discarded count

2020-10-27 Thread Fabrizio D'Angelo
From: Vincent Bernat 

Upstream commit:
commit 32f0deeebc9172c3f5f4a4d02aab32e6904947f6
Date: Sat, 18 Feb 2017 20:11:47 +0100

lldpd: correctly increase discarded count

When a frame cannot be decoded but has been guessed, increase the
discarded count.

Fix https://github.com/vincentbernat/lldpd/issues/223

Co-authored-by: Fabrizio D'Angelo 
Signed-off-by: Fabrizio D'Angelo 
---
 lib/lldp/lldpd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/lldp/lldpd.c b/lib/lldp/lldpd.c
index 19e9305266..34738535db 100644
--- a/lib/lldp/lldpd.c
+++ b/lib/lldp/lldpd.c
@@ -244,6 +244,7 @@ lldpd_decode(struct lldpd *cfg, char *frame, int s,
 
 if (s < sizeof(struct eth_header) + 4) {
 /* Too short, just discard it */
+hw->h_rx_discarded_cnt++;
 return;
 }
 
@@ -284,6 +285,7 @@ lldpd_decode(struct lldpd *cfg, char *frame, int s,
 VLOG_DBG("function for %s protocol did not "
  "decode this frame",
  cfg->g_protocols[i].name);
+hw->h_rx_discarded_cnt++;
 return;
 }
 chassis->c_protocol = port->p_protocol = cfg->g_protocols[i].mode;
-- 
2.26.2

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


[ovs-dev] [PATCH v2 2/6] lldp: Fix size of PEEK_DISCARD_UINT32()

2020-10-27 Thread Fabrizio D'Angelo
From: Jonas Johansson 

Upstream commit:
commit a8d8006c06d9ac16ebcf33295cbd625c0847ca9b
Author: Jonas Johansson 
Date:   Thu, 21 Apr 2016 11:50:06 +0200

Fix size of PEEK_DISCARD_UINT32()

Signed-off-by: Jonas Johansson 

Signed-off-by: Fabrizio D'Angelo 
---
 lib/lldp/lldp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
index e61ce67746..593c5e1c34 100644
--- a/lib/lldp/lldp.c
+++ b/lib/lldp/lldp.c
@@ -59,7 +59,7 @@ VLOG_DEFINE_THIS_MODULE(lldp);
 } while (0)
 #define PEEK_DISCARD_UINT8 PEEK_DISCARD(1)
 #define PEEK_DISCARD_UINT16 PEEK_DISCARD(2)
-#define PEEK_DISCARD_UINT32 PEEK_DISCARD(3)
+#define PEEK_DISCARD_UINT32 PEEK_DISCARD(4)
 #define PEEK_CMP(value, bytes) \
  (length -= (bytes),   \
  pos += (bytes),   \
-- 
2.26.2

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


[ovs-dev] [PATCH ovn 1/2] pinctrl: Fix memory leak in controller_event_run().

2020-10-27 Thread Dumitru Ceara
Valgrind report:
==4689== 639 (96 direct, 543 indirect) bytes in 3 blocks are definitely lost in 
loss record 141 of 146
==4689==at 0x4C29E63: malloc (vg_replace_malloc.c:309)
==4689==by 0x51EEB3: xmalloc (util.c:138)
==4689==by 0x485034: resize (hmap.c:100)
==4689==by 0x4852BF: hmap_expand_at (hmap.c:175)
==4689==by 0x511A8E: hmap_insert_at (hmap.h:283)
==4689==by 0x512824: smap_add__ (smap.c:408)
==4689==by 0x511D20: smap_add (smap.c:55)
==4689==by 0x41B51E: controller_event_run (pinctrl.c:459)
==4689==by 0x422CCD: pinctrl_run (pinctrl.c:3155)
==4689==by 0x433511: main (ovn-controller.c:2588)

Fixes: be1eeb09d373 ("OVN: introduce Controller_Event table")
Signed-off-by: Dumitru Ceara 
---
 controller/pinctrl.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index f15afc5..bc482c0 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -465,6 +465,7 @@ controller_event_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 sbrec_controller_event_set_seq_num(event, ++event_seq_num);
 sbrec_controller_event_set_event_info(event, &event_info);
 sbrec_controller_event_set_chassis(event, chassis);
+smap_destroy(&event_info);
 }
 }
 

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


Re: [ovs-dev] [PATCH ovn v2 1/7] Add new table Load_Balancer in Southbound database.

2020-10-27 Thread Dumitru Ceara
On 10/27/20 6:16 PM, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> This patch adds a new table 'Load_Balancer' in SB DB and syncs the 
> Load_Balancer table rows
> from NB DB to SB DB. An upcoming patch will make use of this table for 
> handling the
> load balancer hairpin traffic.
> 
> Signed-off-by: Numan Siddique 
> ---
>  northd/ovn-northd.c   | 141 ++
>  ovn-sb.ovsschema  |  27 +++-
>  ovn-sb.xml|  45 ++
>  tests/ovn-northd.at   |  81 
>  utilities/ovn-sbctl.c |   3 +
>  5 files changed, 295 insertions(+), 2 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index c06139d75b..d11888d203 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11860,6 +11860,136 @@ sync_dns_entries(struct northd_context *ctx, struct 
> hmap *datapaths)
>  }
>  hmap_destroy(&dns_map);
>  }
> +
> +/*
> + * struct 'sync_lb_info' is used to sync the load balancer records between
> + * OVN Northbound db and Southbound db.
> + */
> +struct sync_lb_info {
> +struct hmap_node hmap_node;
> +const struct nbrec_load_balancer *nlb; /* LB record in the NB db. */
> +const struct sbrec_load_balancer *slb; /* LB record in the SB db. */
> +
> +/* Datapaths to which the LB entry is associated with it. */
> +const struct sbrec_datapath_binding **sbs;
> +size_t n_sbs;
> +};
> +

Hi Numan,

Why do we need the sync_lb_info intermediate data structure?  There's a 1:1
relation between NB Load_Balancer and SB Load_Balancer records (for logical
switches).  Why don't we just store the SB record in "struct ovn_lb"?  I think
this might make the code a bit simpler.

> +static inline struct sync_lb_info *
> +get_sync_lb_info_from_hmap(struct hmap *sync_lb_map, struct uuid *uuid)

Static functions in .c files should not be inline:

https://github.com/ovn-org/ovn/blob/master/Documentation/internals/contributing/coding-style.rst#functions

Also, to be more in sync with the naming scheme in ovn-northd (e.g.,
ovn_datapath_find(), ovn_lb_find()) this should probably be:

s/get_sync_lb_info_from_hmap/sync_lb_info_find

Thanks,
Dumitru

> +{
> +struct sync_lb_info *lb_info;
> +size_t hash = uuid_hash(uuid);
> +HMAP_FOR_EACH_WITH_HASH (lb_info, hmap_node, hash, sync_lb_map) {
> +if (uuid_equals(&lb_info->nlb->header_.uuid, uuid)) {
> +return lb_info;
> +}
> +}
> +
> +return NULL;
> +}
> +
> +static void
> +sync_lb_entries(struct northd_context *ctx, struct hmap *datapaths)
> +{
> +struct hmap lb_map = HMAP_INITIALIZER(&lb_map);
> +struct ovn_datapath *od;
> +
> +HMAP_FOR_EACH (od, key_node, datapaths) {
> +if (!od->nbs || !od->nbs->n_load_balancer) {
> +continue;
> +}
> +
> +for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
> +struct sync_lb_info *lb_info =
> +get_sync_lb_info_from_hmap(
> +&lb_map, &od->nbs->load_balancer[i]->header_.uuid);
> +
> +if (!lb_info) {
> +size_t hash = uuid_hash(
> +&od->nbs->load_balancer[i]->header_.uuid);
> +lb_info = xzalloc(sizeof *lb_info);;
> +lb_info->nlb = od->nbs->load_balancer[i];
> +hmap_insert(&lb_map, &lb_info->hmap_node, hash);
> +}
> +
> +lb_info->n_sbs++;
> +lb_info->sbs = xrealloc(lb_info->sbs,
> +lb_info->n_sbs * sizeof *lb_info->sbs);
> +lb_info->sbs[lb_info->n_sbs - 1] = od->sb;
> +}
> +}
> +
> +const struct sbrec_load_balancer *sbrec_lb, *next;
> +SBREC_LOAD_BALANCER_FOR_EACH_SAFE (sbrec_lb, next, ctx->ovnsb_idl) {
> +const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids, "lb_id");
> +struct uuid lb_uuid;
> +if (!nb_lb_uuid || !uuid_from_string(&lb_uuid, nb_lb_uuid)) {
> +sbrec_load_balancer_delete(sbrec_lb);
> +continue;
> +}
> +
> +struct sync_lb_info *lb_info =
> +get_sync_lb_info_from_hmap(&lb_map, &lb_uuid);
> +if (lb_info) {
> +lb_info->slb = sbrec_lb;
> +} else {
> +sbrec_load_balancer_delete(sbrec_lb);
> +}
> +}
> +
> +struct sync_lb_info *lb_info;
> +HMAP_FOR_EACH (lb_info, hmap_node, &lb_map) {
> +if (!lb_info->slb) {
> +sbrec_lb = sbrec_load_balancer_insert(ctx->ovnsb_txn);
> +lb_info->slb = sbrec_lb;
> +char *lb_id = xasprintf(
> +UUID_FMT, UUID_ARGS(&lb_info->nlb->header_.uuid));
> +const struct smap external_ids =
> +SMAP_CONST1(&external_ids, "lb_id", lb_id);
> +sbrec_load_balancer_set_external_ids(sbrec_lb, &external_ids);
> +free(lb_id);
> +}
> +
> +/* Set the datapaths and other columns.  If nothing has changed, then

Re: [ovs-dev] OVN-OVS build compatibility, take 2

2020-10-27 Thread Brian Haley



On 10/23/20 2:56 PM, Mark Michelson wrote:

On 10/23/20 8:42 AM, Brian Haley wrote:

On 10/22/20 3:31 PM, Mark Michelson wrote:

Hi,

In today's OVN meeting [1], Numan brought up that he had proposed an OVN
patch [2] that deals with a compilation error that occurred after
updating to the latest OVS master. This sparked a discussion about the
process behind OVN/OVS build compatibility.

After OVN was split from OVS last year, the attitude with regard to
build compatibility was that

1) Only devs are likely to be building OVN, so building against the
latest and greatest OVS should be acceptable.
2) Since OVN links to OVS's libraries statically, it's fine if the
version of OVS used to build OVS is different from the version of OVS
that OVN runs against.

After nearly a year of having OVN separated, we've come to the
realization that this may not be the best way to do things. Reasons why
include

1) The "latest and greatest" OVS could actually be a very unstable
mid-version build of OVS. Since OVN is released more often than OVS,
this necessitates OVN releases being built against an unstable version
of OVS.
2) Debugging OVN problems that are rooted in OVSDB or OVS libraries can
be tremendously difficult. Bisecting OVN commits likely requires
changing the OVS commit to build against. This effectively gives two
moving targets for tracking the issue.
3) OVN includes "non-public" headers in OVS.

Based on the meeting today, proposed ideas for fixing this are

1) Move headers from ovs's lib/ folder to include/ since they are
consumed by OVN, an external program.
2) Anchor OVN builds on a specific release of OVS rather than just using
the latest OVS release.


This is what we do in Openstack Neutron.  For example, we have two gate
jobs - one using released code, the other using the tip of the master
branch.  It can take either a branch, tag, or commit, but it's currently:

  OVN_BRANCH: v20.06.1
  # TODO(jlibosva): v2.13.1 is incompatible with kernel 4.15.0-118,
sticking to commit hash until new v2.13 tag is created
  OVS_BRANCH: 0047ca3a0290f1ef954f2c76b31477cf4b9755f5

and master:

  OVN_BRANCH: master
  OVS_BRANCH: master

Doing something like this would let you pick a point in time, and allow
the user to override if they wish, like if the OS/kernel in question had
an issue (as shown above).  It also keeps OVN out of keeping a copy if
the OVS tree.

My $.02

-Brian


Thanks Brian,

During our meeting we discussed something similar. If I understand your 
proposal correctly, the problem is that we won't have as much control 
over which changes in OVS we consume. For instance, if we start by 
anchoring OVN development to OVS at commit A, we may find a month later 
that there is a bug we need to fix. In the meantime, commits B, C, D, 
and E have gone into OVS. So when we merge our bug fix, we are doing so 
as commit F. If we then point OVN to commit F of OVS, then we are also 
consuming commits B, C, D, and E, which we don't want. By cherry-picking 
commit F to a separate branch, we know that we are only getting commit F 
on top of commit A.


Hi Mark,

Understood.  I guess my thought was that there are two scenarios - 
master and stable branches.  Moving forward from A -> F on a stable 
branch should be Ok as it typically wouldn't do something to break any 
APIs, right?  Master is more volatile and going to break things eventually.


-Brian


The problem with (2) of course is that there may be a bug in the OVS
version we select. Or we could require a feature be merged in OVS in
order for OVN to function properly. To deal with that, there were a
couple of ideas mentioned in the meeting

1) Clone the version of OVS that we rely on into a repo on ovn-org, and
then include that as a submodule. If we need a specific bugfix or new
feature, we can backport the fixes to this clone after first getting
them pushed to OVS.

2) If possible, we can backport the fix or feature into a local file in
OVN and use that version of the function/feature rather than what's 
in OVS.


What are people's thoughts on the matter? Any other suggestions for how
to tackle this problem?

Thanks,
Mark Michelson

[1]
http://eavesdrop.openstack.org/meetings//ovn_community_development_discussion/2020/ovn_community_development_discussion.2020-10-22-17.16.log.html 



[2]
https://patchwork.ozlabs.org/project/ovn/patch/20201022155339.300989-1-num...@ovn.org/ 




___
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] [ovn] no tunnel from GW to compute

2020-10-27 Thread Tony Liu
Saw the same problem again. Recreate network, attach to router and
launch VM, problem is gone. Probably some glitch happened during
the early deployment. Any hints how to look into it?

Thanks!
Tony
> -Original Message-
> From: discuss  On Behalf Of Tony
> Liu
> Sent: Friday, October 16, 2020 6:48 PM
> To: ovs-discuss 
> Subject: [ovs-discuss] [ovn] no tunnel from GW to compute
> 
> Hi,
> 
> I am seeing an interesting issue today.
> When ping a FIP from external, request arrives on GW, but no tunnel from
> GW to compute.
> When ping from VM to external, egress works fine, request goes through
> tunnel from compute to GW, then to external.
> Reply arrives at GW, no tunnel from GW back to compute.
> 
> I checked DP flows on GW and compared working vs. non-working.
> 
> non-working, no tunnel
> 
> recirc_id(0),in_port(3),ct_state(-new-est-rel-rpl-inv-
> trk),ct_label(0/0x1),eth(src=e8:1c:ba:9f:b7:c6,dst=fa:16:3e:67:5c:d9),et
> h_type(0x0800),ipv4(src=128.0.0.0/192.0.0.0,dst=10.59.53.18,proto=1,ttl=
> 63,frag=no),icmp(type=8/0xf8), packets:8, bytes:784, used:0.992s,
> actions:ct_clear,ct(zone=20,nat),recirc(0x6e1)
> 
> recirc_id(0x6e1),in_port(3),ct_state(+new-est-rel-rpl-
> inv+trk),ct_label(0/0x1),eth(),eth_type(0x0800),ipv4(dst=10.59.53.18,fra
> g=no), packets:29, bytes:2842, used:0.992s,
> actions:ct(commit,zone=21,nat(dst=192.168.1.8)),recirc(0x6e2)
> 
> recirc_id(0x6e2),in_port(3),ct_state(+new-est-rel-rpl-
> inv+trk),ct_label(0/0x1),eth(src=e8:1c:ba:9f:b7:c6,dst=fa:16:3e:67:5c:d9
> ),eth_type(0x0800),ipv4(dst=192.168.1.8,proto=1,ttl=63,frag=no),icmp(typ
> e=8/0xf8), packets:8, bytes:784, used:0.992s, actions:ct_clear
> 
> 
> working, with tunnel
> 
> recirc_id(0),in_port(3),ct_state(-new-est-rel-rpl-inv-
> trk),ct_label(0/0x1),eth(src=e8:1c:ba:9f:b7:c6,dst=fa:16:3e:67:5c:d9),et
> h_type(0x0800),ipv4(src=128.0.0.0/192.0.0.0,dst=10.59.53.14,proto=1,ttl=
> 63,frag=no),icmp(type=8/0xf8), packets:2, bytes:196, used:3.427s,
> actions:ct_clear,ct(zone=20,nat),recirc(0x716)
> 
> recirc_id(0x716),in_port(3),ct_state(+new-est-rel-rpl-
> inv+trk),ct_label(0/0x1),eth(),eth_type(0x0800),ipv4(dst=10.59.53.14,fra
> g=no), packets:2, bytes:196, used:3.428s,
> actions:ct(commit,zone=21,nat(dst=192.168.1.5)),recirc(0x717)
> 
> recirc_id(0x717),in_port(3),ct_state(+new-est-rel-rpl-
> inv+trk),ct_label(0/0x1),eth(src=e8:1c:ba:9f:b7:c6,dst=fa:16:3e:67:5c:d9
> ),eth_type(0x0800),ipv4(dst=192.168.1.5,proto=1,tos=0/0x3,ttl=63,frag=no
> ),icmp(type=8/0xf8), packets:0, bytes:0, used:never,
> actions:ct_clear,set(tunnel(tun_id=0x139,dst=10.6.30.63,ttl=64,tp_dst=60
> 81,geneve({class=0x102,type=0x80,len=4,0x2000a}),flags(df|csum|key))),se
> t(eth(src=fa:16:3e:aa:2a:5d,dst=fa:16:3e:a6:79:6f)),set(ipv4(ttl=62)),1
> 
> 
> The difference is on the third flow (0x6e2 and 0x717).
> In non-working case, "set(tunnel..." is missing.
> Note, the working VM and non-working VM are on the same compute.
> I want to trace the root cause. Any hints or comments where and how I
> should look into it?
> 
> 
> Thanks!
> Tony
> 
> ___
> discuss mailing list
> disc...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v9] dpif-netlink: distribute polling to discreet handlers

2020-10-27 Thread Flavio Leitner
On Fri, Sep 18, 2020 at 02:48:17PM -0400, Mark Gray wrote:
> From: Aaron Conole 
> 
> Currently, the channel handlers are polled globally.  On some
> systems, this causes a thundering herd issue where multiple
> handler threads become active, only to do no work and immediately
> sleep.
> 
> The approach here is to push the netlink socket channels to discreet
> handler threads to process, rather than polling on every thread.
> This will eliminate the need to wake multiple threads.
> 
> To check:
> 
>   ip netns add left
>   ip netns add right
>   ip link add center-left type veth peer name left0 netns left
>   ip link add center-right type veth peer name right0 netns right
>   ip link set center-left up
>   ip link set center-right up
>   ip -n left ip link set left0 up
>   ip -n left ip addr add 172.31.110.10/24 dev left0
>   ip -n right ip link set right0 up
>   ip -n right ip addr add 172.31.110.11/24 dev right0
> 
>   ovs-vsctl add-br br0
>   ovs-vsctl add-port br0 center-right
>   ovs-vsctl add-port br0 center-left
> 
>   # in one terminal
>   perf record -e sched:sched_wakeup,irq:softirq_entry -ag
> 
>   # in a separate terminal
>   ip netns exec left arping -I left0 -c 1 172.31.110.11
> 
>   # in the perf terminal after exiting
>   perf script
> 
> Look for the number of 'handler' threads which were made active.
> 
> Suggested-by: Ben Pfaff 
> Suggested-by: Flavio Leitner 
> Co-authored-by: Mark Gray 
> Reported-by: David Ahern 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-December/365857.html
> Cc: Matteo Croce 
> Fixes: 69c51582f ("dpif-netlink: don't allocate per thread netlink sockets")
> Signed-off-by: Aaron Conole 
> Signed-off-by: Mark Gray 
> ---
> 
> v2: Oops - forgot to commit my whitespace cleanups.
> v3: one port latency results as per Matteo's comments
> 
> Stock:
>   min/avg/max/mdev = 21.5/36.5/96.5/1.0 us
> With Patch:
>   min/avg/max/mdev = 5.3/9.7/98.4/0.5 us 
> v4: Oops - first email did not send
> v5:   min/avg/max/mdev = 9.3/10.4/33.6/2.2 us
> v6: Split out the AUTHORS patch and added a cover letter as
> per Ilya's suggestion.
> Fixed 0-day issues.
> v7: Merged patches as per Flavio's suggestion. This is
> no longer a series. Fixed some other small issues.
> v9: Udated based on feedback from Ilya. Also implemented
> another suggestion by Flavio
>   min/avg/max/mdev = 5.5/11.0/83.2/0.8 us
> 
>  lib/dpif-netlink.c | 309 ++---
>  1 file changed, 149 insertions(+), 160 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 7da4fb54d..7e4823ea0 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -160,8 +160,15 @@ static void dpif_netlink_flow_to_dpif_flow(struct 
> dpif_flow *,
>  
>  /* One of the dpif channels between the kernel and userspace. */
>  struct dpif_channel {
> -struct nl_sock *sock;   /* Netlink socket. */
> -long long int last_poll;/* Last time this channel was polled. */
> +struct hmap_node handler_node; /* Used by 'struct dpif_handler' to track
> +  all channels associated with a
> +  handler. */
> +struct hmap_node dpif_node;/* Used by 'struct dpif' to track all dpif
> +  channels and their associated port. */
> +struct dpif_handler *handler;  /* Pointer to owning 'struct 
> dpif_handler'*/
> +odp_port_t port_no;/* Datapath port number of this channel. 
> */
> +struct nl_sock *sock;  /* Netlink socket. */
> +long long int last_poll;   /* Last time this channel was polled. */
>  };
>  
>  #ifdef _WIN32
> @@ -183,6 +190,9 @@ struct dpif_handler {
>  int n_events; /* Num events returned by epoll_wait(). */
>  int event_offset; /* Offset into 'epoll_events'. */
>  
> +struct hmap channels; /* Map of channels for each port associated
> + with this handler. */
> +
>  #ifdef _WIN32
>  /* Pool of sockets. */
>  struct dpif_windows_vport_sock *vport_sock_pool;
> @@ -201,9 +211,7 @@ struct dpif_netlink {
>  struct fat_rwlock upcall_lock;
>  struct dpif_handler *handlers;
>  uint32_t n_handlers;   /* Num of upcall handlers. */
> -struct dpif_channel *channels; /* Array of channels for each port. */
> -int uc_array_size; /* Size of 'handler->channels' and */
> -   /* 'handler->epoll_events'. */
> +struct hmap channels;  /* Map of all channels. */
>  
>  /* Change notification. */
>  struct nl_sock *port_notifier; /* vport multicast group subscriber. */
> @@ -287,6 +295,44 @@ close_nl_sock(struct nl_sock *sock)
>  #endif
>  }
>  
> +static size_t
> +dpif_handler_channels_count(const struct dpif_handler *handler)
> +{
> +return hmap_count(&handler->channels);
> +}
> +
> +static struct d

[ovs-dev] Noticia

2020-10-27 Thread ABERTO MORALES




 

ALBERTO MORALES  LEGAL CONSULTANTS

AV/DE GRAN VIA NO.38, 28008 MADRID, SPAIN  

TEL: 0034 631 272 276

FAX: 0034 917 857 722

E-MAIL: albertomorales2...@gmail.com

REF NO: WASUR/ TE/57-876/ZL20   
  

 




    DATE: 27/10/2020



   

Attention: Beneficiary, 
   

 

 

Final notification for the payment of the unclaimed prize money:   REF NO: 
WASUR/ TE/57-876/ZL20   
 

 

We write to inform you that the office of Unclaimed fund of the Security and 
Exchange Commission here in Spain, has decided to verify and confirm the true 
identity of  the owner of the fund which was left unclaimed and later got 
invested in the capital market and managed by an investment company since over 
years. The total amount currently is €9,000,000.00 EUROS

The capital together with the profit from investment as accrued till date is 
€9,000,000.00 Euros. The original sum of €8,650,810.00 was awarded to you after 
a coupon which your name was attached to emerged with the winning number, the 
coupons were bought by an investment company, who in order to enhance their 
chance of winning collect and bet on names from around the world. Soon after 
your names emerged as a winner, a letter of notification was sent to you, but 
you did not complete the process of claiming the fund. The money was therefore 
released to the investment company which invested it in profitable stocks for 
an initial period of years.

The money was to be released to you after the initial period of investment 
expired, but because there were some complications with the address of the 
winners, some notification letters were probably sent to the wrong addresses. 
This resulted in some false applications for the claiming of the money by some 
unscrupulous individuals, in response to this problem, the security and 
Exchange Commission ordered the money retained with the office of unclaimed 
funds pending the conclusion of an investigation to identify the true 
owner/owners of the fund.

To make sure that the legal procedure is followed in the payment of the money 
to the real beneficiary, the investment company has subsequently hired our firm 
to act as legal consultants in the processing of the unclaimed fund. For 
advice, on how to begin your claim, we request you contact immediately, our 
senior partner, BARRISTER ALBERTO MORALES, on the telephone numbers TEL: 0034 
631 272 276. FAX: 0034 917 857 722. 
 E-Mail: albertomorales2...@gmail.com Furthermore, your interest for 
the processing and payment of this money to you must be made before the 25th of 
November, 2020. The money will be reinvested automatically after this date, 
please do let us know if you would like the money to be reinvested. If after 
verification, you are confirmed as the true owner of the money, our 
responsibility as Attorney is to ensure that due process is followed, and your 
interest protected in the processing and payment of the money to you, we will 
commence the procedure as soon as we receive your instructions.

 

We look forward to hearing from you while we assure you of our legal 
assistance. 

 

Best Regards



    

BARRISTER ALBERTO MORALES  

 

 

Please provide the following information as required below and fax it back to 
my office or you can scan it to my email immediately for us to be able to 
complete the legalization process of your prize money and the money will be 
paid to you by the SANTANDER Int bank. All process verification through our law 
firm is free for you because our charges will be paid by the investment company 
at the end of the process when you receive your money. If you have not given 
the required information before the time, the lawyer's office would not be held 
liable when your money reinvested again.

A confirmation letter will be faxed or posted to you immediately after we have 
complete the verification of information you provided to us. I will contact the 
investment bank immediately about the information you provides before they will 
contact you for the paymen

[ovs-dev] [PATCH branch-2.11] python: Add 'six' to list of install requirements

2020-10-27 Thread Thomas Neuman

From ba3b444d8c182ab7ff2e007c8960e30706089faa Mon Sep 17 00:00:00 2001
From: Thomas Neuman 
Date: Tue, 27 Oct 2020 17:39:27 +
Subject: [PATCH branch-2.11] python: Add 'six' to list of install requirements

Fixes: 73eb682edb
Signed-off-by: Thomas Neuman 
---
 python/setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/setup.py b/python/setup.py
index b52657df3..2cff74eec 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -82,7 +82,7 @@ setup_args = dict(
 ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
   libraries=['openvswitch'])],
 cmdclass={'build_ext': try_build_ext},
-install_requires=['sortedcontainers'],
+install_requires=['six', 'sortedcontainers'],
 )
 
 try:

--
2.22.3


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


[ovs-dev] Beneficios de la documentación de los procesos

2020-10-27 Thread Mapeo de procesos
Jueves 05 de Noviembre | Horario de 10:00 a 17:00 hrs.  |  (hora del centro de 
México) 

- Mapeo de procesos - 

¿De qué hablaremos?

Este taller está dirigido al personal de las áreas de calidad, procesos, 
proyectos de mejora y profesionales dedicados a
la documentación de procesos en general y les permitirá conocer los pasos a 
seguir para el mapeo de los procesos
de su organización ya sea para una documentación posterior de procedimientos o 
para la implementación de un
sistema de gestión basado en el mapa de procesos.

¿Qué aprenderás?:

- Conocerá los términos generales que se utilizan para la documentación de 
procesos.
- Aplicará una secuencia lógica para la documentación de los procesos e 
identificar los procedimientos que se pueden derivar.
- Aprovechará los beneficios de la documentación de los procesos para los 
diferentes negocios.

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

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

Para información inmediata llamar al:
(+52) 55 15 54 66 30 - (+52) 55 30 16 70 85
O puede enviarnos un mensaje vía Whatsapp 

Innova Learn México - innovalearn. mx - Mérida, Yucatán, México


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


Re: [ovs-dev] [PATCH ovn v2 1/7] Add new table Load_Balancer in Southbound database.

2020-10-27 Thread 0-day Robot
Bleep bloop.  Greetings Numan Siddique, 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:
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 Add new table Load_Balancer in Southbound database.
When you have resolved this problem, run "git am --continue".
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 ovn 3/7] northd: Improve comments.

2020-10-27 Thread Numan Siddique
On Tue, Oct 27, 2020, 11:14 PM Ben Pfaff  wrote:

> On Tue, Oct 27, 2020 at 09:00:10PM +0530, Numan Siddique wrote:
> > On Mon, Oct 26, 2020 at 11:46 PM Ben Pfaff  wrote:
> > > -bool derived; /* Indicates whether this is an additional port
> > > -   * derived from nbsp or nbrp. */
> > > +/* This is ordinarily false.  It is true iff this ovn_port is
> derived from
> >
> > nit: s/iff/if
>
> "iff" is fairly common mathematical jargon for "if and only if".  This
> isn't space-constrained so I just wrote it out in full.
>

Ok. Then please ignore my comment. I wasn't aware of it.

Thanks
Numan


> Thanks,
>
> Ben.
> ___
> 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 7/7] northd: Enhance implementation of port tunnel key requests.

2020-10-27 Thread Ben Pfaff
On Tue, Oct 27, 2020 at 10:28:00PM +0530, Numan Siddique wrote:
> On Mon, Oct 26, 2020 at 11:47 PM Ben Pfaff  wrote:
> >
> > When a tunnel key was requested, the implementation was not smart enough
> > to reassign a port that had been automatically assigned the same
> > key.  This fixes the problem and adds a test.
> >
> > Signed-off-by: Ben Pfaff 
> > +if (!ovs_list_is_empty(&sb_only)) {
> > +LIST_FOR_EACH_SAFE(op, next, list, &sb_only) {
> 
> There is a checkpatch error here.
> 
> With that addressed,
> Acked-by: Numan Siddique 

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


Re: [ovs-dev] [PATCH ovn 1/7] northd: Count mask length and priority correctly for IPv6 addresses.

2020-10-27 Thread Ben Pfaff
Thanks for the reviews.  I applied this series to ovn master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn 3/7] northd: Improve comments.

2020-10-27 Thread Ben Pfaff
On Tue, Oct 27, 2020 at 09:00:10PM +0530, Numan Siddique wrote:
> On Mon, Oct 26, 2020 at 11:46 PM Ben Pfaff  wrote:
> > -bool derived; /* Indicates whether this is an additional port
> > -   * derived from nbsp or nbrp. */
> > +/* This is ordinarily false.  It is true iff this ovn_port is derived 
> > from
> 
> nit: s/iff/if

"iff" is fairly common mathematical jargon for "if and only if".  This
isn't space-constrained so I just wrote it out in full.

Thanks,

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


[ovs-dev] [PATCH ovn v2 5/7] northd: Make use of new hairpin actions.

2020-10-27 Thread numans
From: Numan Siddique 

This patch makes use of the new hairpin OVN actions - chk_lb_hairpin, 
chk_lb_hairpin_reply
and ct_snat_to_vip.

Suppose there are 'm' load balancers associated to a logical switch and each 
load balancer
has 'n' VIPs and each VIP has 'p' backends then ovn-northd adds (m * ((n * p) + 
n))
hairpin logical flows. After this patch, ovn-northd adds just 5 hairpin logical 
flows.

With this patch number of hairpin related OF flows on a chassis are almost the 
same as before,
but in a large scale deployment, this reduces memory consumption and load on 
ovn-northd and
SB DB ovsdb-servers.

Signed-off-by: Numan Siddique 
---
 northd/ovn-northd.8.xml |  65 +++-
 northd/ovn-northd.c | 159 ++--
 tests/ovn-northd.at |  28 +++
 tests/ovn.at|  36 -
 4 files changed, 133 insertions(+), 155 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 9b96ce9a38..53ee4b58c0 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -718,24 +718,55 @@
 Ingress Table 12: Pre-Hairpin
 
   
-For all configured load balancer VIPs a priority-2 flow that
-matches on traffic that needs to be hairpinned, i.e., after load
-balancing the destination IP matches the source IP, which sets
-reg0[6] = 1  and executes ct_snat(VIP)
-to force replies to these packets to come back through OVN.
+If the logical switch has load balancer(s) configured, then a
+priorirty-100 flow is added with the match
+ip && ct.trk&& ct.dnat to check if the
+packet needs to be hairpinned ( if after load balancing the destination
+IP matches the source IP) or not by executing the action
+reg0[6] = chk_lb_hairpin(); and advances the packet to
+the next table.
+  
+
+  
+If the logical switch has load balancer(s) configured, then a
+priorirty-90 flow is added with the match ip to check if
+the packet is a reply for a hairpinned connection or not by executing
+the action reg0[6] = chk_lb_hairpin_reply(); and advances
+the packet to the next table.
   
+
   
-For all configured load balancer VIPs a priority-1 flow that
-matches on replies to hairpinned traffic, i.e., destination IP is VIP,
-source IP is the backend IP and source L4 port is backend port, which
-sets reg0[6] = 1  and executes ct_snat;.
+A priority-0 flow that simply moves traffic to the next table.
   
+
+
+Ingress Table 13: Nat-Hairpin
+
+  
+ If the logical switch has load balancer(s) configured, then a
+ priorirty-100 flow is added with the match
+ ip && (ct.new || ct.est) && ct.trk &&
+ ct.dnat && reg0[6] == 1 which hairpins the traffic by
+ NATting source IP to the load balancer VIP by executing the action
+ ct_snat_to_vip and advances the packet to the next table.
+  
+
+  
+ If the logical switch has load balancer(s) configured, then a
+ priorirty-90 flow is added with the match
+ ip && reg0[6] == 1 which matches on the replies
+ of hairpinned traffic ( i.e., destination IP is VIP,
+ source IP is the backend IP and source L4 port is backend port for L4
+ load balancers) and executes ct_snat and advances the
+ packet to the next table.
+  
+
   
 A priority-0 flow that simply moves traffic to the next table.
   
 
 
-Ingress Table 13: Hairpin
+Ingress Table 14: Hairpin
 
   
 A priority-1 flow that hairpins traffic matched by non-default
@@ -748,7 +779,7 @@
   
 
 
-Ingress Table 14: ARP/ND responder
+Ingress Table 15: ARP/ND responder
 
 
   This table implements ARP/ND responder in a logical switch for known
@@ -1038,7 +1069,7 @@ output;
   
 
 
-Ingress Table 15: DHCP option processing
+Ingress Table 16: DHCP option processing
 
 
   This table adds the DHCPv4 options to a DHCPv4 packet from the
@@ -1099,7 +1130,7 @@ next;
   
 
 
-Ingress Table 16: DHCP responses
+Ingress Table 17: DHCP responses
 
 
   This table implements DHCP responder for the DHCP replies generated by
@@ -1180,7 +1211,7 @@ output;
   
 
 
-Ingress Table 17 DNS Lookup
+Ingress Table 18 DNS Lookup
 
 
   This table looks up and resolves the DNS names to the corresponding
@@ -1209,7 +1240,7 @@ reg0[4] = dns_lookup(); next;
   
 
 
-Ingress Table 18 DNS Responses
+Ingress Table 19 DNS Responses
 
 
   This table implements DNS responder for the DNS replies generated by
@@ -1244,7 +1275,7 @@ output;
   
 
 
-Ingress table 19 External ports
+Ingress table 20 External ports
 
 
   Traffic from the external logical ports enter the ingress
@@ -1287,7 +1318,7 @@ output;
   
 

[ovs-dev] [PATCH ovn v2 7/7] sbctl: Add Load Balancer support for vflows option.

2020-10-27 Thread numans
From: Numan Siddique 

Signed-off-by: Numan Siddique 
---
 utilities/ovn-sbctl.c | 55 +++
 1 file changed, 55 insertions(+)

diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index 00c112c7e5..5b593b38cb 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -542,6 +542,11 @@ pre_get_info(struct ctl_context *ctx)
 ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_logical_port);
 ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_ip);
 ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_mac);
+
+ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_datapaths);
+ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_vips);
+ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_name);
+ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_protocol);
 }
 
 static struct cmd_show_table cmd_show_tables[] = {
@@ -1009,6 +1014,55 @@ cmd_lflow_list_chassis(struct ctl_context *ctx, struct 
vconn *vconn,
 }
 }
 
+static void
+cmd_lflow_list_load_balancers(struct ctl_context *ctx, struct vconn *vconn,
+  const struct sbrec_datapath_binding *datapath,
+  bool stats, bool print_uuid)
+{
+const struct sbrec_load_balancer *lb;
+const struct sbrec_load_balancer *lb_prev = NULL;
+SBREC_LOAD_BALANCER_FOR_EACH (lb, ctx->idl) {
+bool dp_found = false;
+if (datapath) {
+for (size_t i = 0; i < lb->n_datapaths; i++) {
+if (datapath == lb->datapaths[i]) {
+dp_found = true;
+break;
+}
+}
+if (datapath && !dp_found) {
+continue;
+}
+}
+
+if (!lb_prev) {
+printf("\nLoad Balancers:\n");
+}
+
+printf("  ");
+print_uuid_part(&lb->header_.uuid, print_uuid);
+printf("name=\"%s\", protocol=\"%s\", ", lb->name, lb->protocol);
+if (!dp_found) {
+for (size_t i = 0; i < lb->n_datapaths; i++) {
+print_vflow_datapath_name(lb->datapaths[i], true);
+}
+}
+
+printf("\n  vips:\n");
+struct smap_node *node;
+SMAP_FOR_EACH (node, &lb->vips) {
+printf("%s = %s\n", node->key, node->value);
+}
+printf("\n");
+
+if (vconn) {
+sbctl_dump_openflow(vconn, &lb->header_.uuid, stats);
+}
+
+lb_prev = lb;
+}
+}
+
 static void
 cmd_lflow_list(struct ctl_context *ctx)
 {
@@ -1118,6 +1172,7 @@ cmd_lflow_list(struct ctl_context *ctx)
 cmd_lflow_list_mac_bindings(ctx, vconn, datapath, stats, print_uuid);
 cmd_lflow_list_mc_groups(ctx, vconn, datapath, stats, print_uuid);
 cmd_lflow_list_chassis(ctx, vconn, stats, print_uuid);
+cmd_lflow_list_load_balancers(ctx, vconn, datapath, stats, print_uuid);
 }
 
 vconn_close(vconn);
-- 
2.26.2

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


[ovs-dev] [PATCH ovn v2 6/7] ovn-detrace: Add SB Load Balancer cookier handler.

2020-10-27 Thread numans
From: Numan Siddique 

Signed-off-by: Numan Siddique 
---
 utilities/ovn-detrace.in | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
index 4f8dd5f3d8..1214be6fa1 100755
--- a/utilities/ovn-detrace.in
+++ b/utilities/ovn-detrace.in
@@ -316,6 +316,14 @@ class ChassisHandler(CookieHandlerByUUUID):
 def print_record(self, chassis):
 print_p('Chassis: %s' % (chassis_str([chassis])))
 
+class SBLoadBalancerHandler(CookieHandlerByUUUID):
+def __init__(self, ovnsb_db):
+super(SBLoadBalancerHandler, self).__init__(ovnsb_db, 'Load_Balancer')
+
+def print_record(self, lb):
+print_p('Load Balancer: %s protocol %s vips %s' % (
+lb.name, lb.protocol, lb.vips))
+
 class OvsInterfaceHandler(CookieHandler):
 def __init__(self, ovs_db):
 super(OvsInterfaceHandler, self).__init__(ovs_db, 'Interface')
@@ -424,7 +432,8 @@ def main():
 PortBindingHandler(ovsdb_ovnsb),
 MacBindingHandler(ovsdb_ovnsb),
 MulticastGroupHandler(ovsdb_ovnsb),
-ChassisHandler(ovsdb_ovnsb)
+ChassisHandler(ovsdb_ovnsb),
+SBLoadBalancerHandler(ovsdb_ovnsb)
 ]
 
 regex_cookie = re.compile(r'^.*cookie 0x([0-9a-fA-F]+)')
-- 
2.26.2

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


[ovs-dev] [PATCH ovn v2 4/7] actions: Add new actions chk_lb_hairpin, chk_lb_hairpin_reply and ct_snat_to_vip.

2020-10-27 Thread numans
From: Numan Siddique 

The action - chk_lb_hairpin checks if the packet destined to a load balancer VIP
is to be hairpinned back to the same destination and if so, sets the 
destination register
bit to 1.

The action - chk_lb_hairpin_reply checks if the packet is a reply of the 
hairpinned
packet. If so, it sets the destination register bit to 1.

The action ct_snat_to_vip snats the source IP to the load balancer VIP if 
chk_lb_hairpin()
returned true.

These actions will be used in the upcoming patch by ovn-northd in the hairpin 
logical flows.
This helps in reducing lots of hairpin logical flows.

Signed-off-by: Numan Siddique 
---
 controller/lflow.c|   3 ++
 include/ovn/actions.h |  15 --
 lib/actions.c | 116 ++
 ovn-sb.xml|  37 ++
 tests/ovn.at  |  39 ++
 tests/test-ovn.c  |   3 ++
 utilities/ovn-trace.c |  65 ++-
 7 files changed, 265 insertions(+), 13 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 657482626d..588c72dc22 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -698,6 +698,9 @@ add_matches_to_flow_table(const struct sbrec_logical_flow 
*lflow,
 .output_ptable = output_ptable,
 .mac_bind_ptable = OFTABLE_MAC_BINDING,
 .mac_lookup_ptable = OFTABLE_MAC_LOOKUP,
+.lb_hairpin_ptable = OFTABLE_CHK_LB_HAIRPIN,
+.lb_hairpin_reply_ptable = OFTABLE_CHK_LB_HAIRPIN_REPLY,
+.ct_snat_vip_ptable = OFTABLE_CT_SNAT_FOR_VIP,
 };
 ovnacts_encode(ovnacts->data, ovnacts->size, &ep, &ofpacts);
 
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index b4e5acabb9..32f9c53dfc 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -83,7 +83,7 @@ struct ovn_extend_table;
 OVNACT(PUT_DHCPV4_OPTS,   ovnact_put_opts)\
 OVNACT(PUT_DHCPV6_OPTS,   ovnact_put_opts)\
 OVNACT(SET_QUEUE, ovnact_set_queue)   \
-OVNACT(DNS_LOOKUP,ovnact_dns_lookup)  \
+OVNACT(DNS_LOOKUP,ovnact_result)  \
 OVNACT(LOG,   ovnact_log) \
 OVNACT(PUT_ND_RA_OPTS,ovnact_put_opts)\
 OVNACT(ND_NS, ovnact_nest)\
@@ -97,6 +97,9 @@ struct ovn_extend_table;
 OVNACT(DHCP6_REPLY,   ovnact_null)\
 OVNACT(ICMP6_ERROR,   ovnact_nest)\
 OVNACT(REJECT,ovnact_nest)\
+OVNACT(CHK_LB_HAIRPIN,ovnact_result)  \
+OVNACT(CHK_LB_HAIRPIN_REPLY, ovnact_result)   \
+OVNACT(CT_SNAT_TO_VIP,ovnact_null)\
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -338,8 +341,8 @@ struct ovnact_set_queue {
 uint16_t queue_id;
 };
 
-/* OVNACT_DNS_LOOKUP. */
-struct ovnact_dns_lookup {
+/* OVNACT_DNS_LOOKUP, OVNACT_CHK_LB_HAIRPIN, OVNACT_CHK_LB_HAIRPIN_REPLY. */
+struct ovnact_result {
 struct ovnact ovnact;
 struct expr_field dst;  /* 1-bit destination field. */
 };
@@ -727,6 +730,12 @@ struct ovnact_encode_params {
resubmit. */
 uint8_t mac_lookup_ptable;  /* OpenFlow table for
'lookup_arp'/'lookup_nd' to resubmit. */
+uint8_t lb_hairpin_ptable;  /* OpenFlow table for
+ * 'chk_lb_hairpin' to resubmit. */
+uint8_t lb_hairpin_reply_ptable;  /* OpenFlow table for
+   * 'chk_lb_hairpin_reply' to resubmit. */
+uint8_t ct_snat_vip_ptable;  /* OpenFlow table for
+  * 'ct_snat_to_vip' to resubmit. */
 };
 
 void ovnacts_encode(const struct ovnact[], size_t ovnacts_len,
diff --git a/lib/actions.c b/lib/actions.c
index 23e54ef2a6..015bcbc4dc 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -2655,13 +2655,14 @@ ovnact_set_queue_free(struct ovnact_set_queue *a 
OVS_UNUSED)
 }
 
 static void
-parse_dns_lookup(struct action_context *ctx, const struct expr_field *dst,
- struct ovnact_dns_lookup *dl)
+parse_ovnact_result(struct action_context *ctx, const char *name,
+const char *prereq, const struct expr_field *dst,
+struct ovnact_result *res)
 {
-lexer_get(ctx->lexer); /* Skip dns_lookup. */
+lexer_get(ctx->lexer); /* Skip action name. */
 lexer_get(ctx->lexer); /* Skip '('. */
 if (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
-lexer_error(ctx->lexer, "dns_lookup doesn't take any parameters");
+lexer_error(ctx->lexer, "%s doesn't take any parameters", name);
 return;
 }
 /* Validate that the destination is a 1-bit, modifiable field. */
@@ -2671,19 +2672,29 @@ parse_dns_lookup(struct action_context *ctx, const 
struct expr_field *dst,
 free(error);
 return;
 }
-dl->dst = *dst;
-add_prerequisite(ctx, "udp");
+res->dst = *dst;
+
+if (prer

[ovs-dev] [PATCH ovn v2 3/7] controller: Add load balancer hairpin OF flows.

2020-10-27 Thread numans
From: Numan Siddique 

Presently to handle the load balancer hairpin traffic (the traffic destined to 
the
load balancer VIP is dnatted to the backend which originated the traffic), 
ovn-northd
adds a lot of logical flows to check this scenario. This patch attempts to 
reduce the
these logical flows. Each ovn-controller will read the load balancers from
the newly added southbound Load_Balancer table and adds the load balancer 
hairpin OF
flows in the table 68, 69 and 70. If suppose a below load balancer is configured

10.0.0.10:80 = 10.0.0.4:8080, 10.0.0.5:8090, then the below flows are added

table=68, ip.src = 10.0.0.4,ip.dst=10.0.0.4,tcp.dst=8080 
actions=load:1->NXM_NX_REG9[7]
table=68, ip.src = 10.0.0.5,ip.dst=10.0.0.5,tcp.dst=8090 
actions=load:1->NXM_NX_REG9[7]
table=69, ip.src = 10.0.0.4,ip.dst=10.0.0.10,tcp.src=8080 
actions=load:1->NXM_NX_REG9[7]
table=69, ip.src = 10.0.0.5,ip.dst=10.0.0.10,tcp.src=8090 
actions=load:1->NXM_NX_REG9[7]
table=70, ct.trk && ct.dnat && ct.nw_dst == 10.0.0.10. actions=ct(commit, 
zone=reg12, nat(src=10.0.0.5))

Upcoming patch will add OVN actions which does the lookup in these tables to 
handle the
hairpin traffic.

Signed-off-by: Numan Siddique 
---
 controller/lflow.c   | 253 +++
 controller/lflow.h   |   6 +-
 controller/ovn-controller.c  |  27 +-
 include/ovn/logical-fields.h |   3 +
 tests/ovn.at | 469 +++
 5 files changed, 756 insertions(+), 2 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index f631679c3f..657482626d 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -26,6 +26,7 @@
 #include "ovn-controller.h"
 #include "ovn/actions.h"
 #include "ovn/expr.h"
+#include "lib/lb.h"
 #include "lib/ovn-l7.h"
 #include "lib/ovn-sb-idl.h"
 #include "lib/extend-table.h"
@@ -1138,6 +1139,213 @@ add_neighbor_flows(struct ovsdb_idl_index 
*sbrec_port_binding_by_name,
 }
 }
 
+static void
+add_lb_vip_hairpin_flows(struct ovn_lb *lb, struct lb_vip *lb_vip,
+ struct lb_vip_backend *lb_backend,
+ uint8_t lb_proto,
+ struct ovn_desired_flow_table *flow_table)
+{
+uint64_t stub[1024 / 8];
+struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
+
+uint8_t value = 1;
+put_load(&value, sizeof value, MFF_LOG_FLAGS,
+ MLF_LOOKUP_LB_HAIRPIN_BIT, 1, &ofpacts);
+
+ovs_be32 vip4;
+struct in6_addr vip6;
+
+if (lb_vip->addr_family == AF_INET) {
+ovs_assert(ip_parse(lb_vip->vip, &vip4));
+} else {
+ovs_assert(ipv6_parse(lb_vip->vip, &vip6));
+}
+
+struct match hairpin_match = MATCH_CATCHALL_INITIALIZER;
+struct match hairpin_reply_match = MATCH_CATCHALL_INITIALIZER;
+
+if (lb_vip->addr_family == AF_INET) {
+ovs_be32 ip4;
+ovs_assert(ip_parse(lb_backend->ip, &ip4));
+
+match_set_dl_type(&hairpin_match, htons(ETH_TYPE_IP));
+match_set_nw_src(&hairpin_match, ip4);
+match_set_nw_dst(&hairpin_match, ip4);
+
+match_set_dl_type(&hairpin_reply_match,
+htons(ETH_TYPE_IP));
+match_set_nw_src(&hairpin_reply_match, ip4);
+match_set_nw_dst(&hairpin_reply_match, vip4);
+} else {
+struct in6_addr ip6;
+ovs_assert(ipv6_parse(lb_backend->ip, &ip6));
+
+match_set_dl_type(&hairpin_match, htons(ETH_TYPE_IPV6));
+match_set_ipv6_src(&hairpin_match, &ip6);
+match_set_ipv6_dst(&hairpin_match, &ip6);
+
+match_set_dl_type(&hairpin_reply_match,
+htons(ETH_TYPE_IPV6));
+match_set_ipv6_src(&hairpin_reply_match, &ip6);
+match_set_ipv6_dst(&hairpin_reply_match, &vip6);
+}
+
+if (lb_backend->port) {
+match_set_nw_proto(&hairpin_match, lb_proto);
+match_set_tp_dst(&hairpin_match, htons(lb_backend->port));
+
+match_set_nw_proto(&hairpin_reply_match, lb_proto);
+match_set_tp_src(&hairpin_reply_match,
+htons(lb_backend->port));
+}
+
+for (size_t i = 0; i < lb->slb->n_datapaths; i++) {
+match_set_metadata(&hairpin_match,
+htonll(lb->slb->datapaths[i]->tunnel_key));
+match_set_metadata(&hairpin_reply_match,
+htonll(lb->slb->datapaths[i]->tunnel_key));
+
+ofctrl_add_flow(flow_table, OFTABLE_CHK_LB_HAIRPIN, 100,
+lb->slb->header_.uuid.parts[0], &hairpin_match,
+&ofpacts, &lb->slb->header_.uuid);
+
+ofctrl_add_flow(flow_table, OFTABLE_CHK_LB_HAIRPIN_REPLY, 100,
+lb->slb->header_.uuid.parts[0],
+&hairpin_reply_match,
+&ofpacts, &lb->slb->header_.uuid);
+}
+
+ofpbuf_uninit(&ofpacts);
+}
+
+static void
+add_lb_ct_snat_vip_flows(struct ovn_lb *lb, struct lb_vip *lb_vip,
+ struct ovn_

[ovs-dev] [PATCH ovn v2 2/7] northd: Refactor load balancer vip parsing.

2020-10-27 Thread numans
From: Numan Siddique 

Parsing of the load balancer VIPs is moved to a separate file - lib/lb.c.
ovn-northd makes use of these functions. Upcoming patch will make use of these
util functions for parsing SB Load_Balancers.

Signed-off-by: Numan Siddique 
---
 lib/automake.mk |   4 +-
 lib/lb.c| 236 
 lib/lb.h|  77 
 lib/ovn-util.c  |  28 +
 lib/ovn-util.h  |   2 +
 northd/ovn-northd.c | 286 +---
 6 files changed, 378 insertions(+), 255 deletions(-)
 create mode 100644 lib/lb.c
 create mode 100644 lib/lb.h

diff --git a/lib/automake.mk b/lib/automake.mk
index f3e9c8818b..430cd11fc6 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -23,7 +23,9 @@ lib_libovn_la_SOURCES = \
lib/ovn-util.h \
lib/logical-fields.c \
lib/inc-proc-eng.c \
-   lib/inc-proc-eng.h
+   lib/inc-proc-eng.h \
+   lib/lb.c \
+   lib/lb.h
 nodist_lib_libovn_la_SOURCES = \
lib/ovn-dirs.c \
lib/ovn-nb-idl.c \
diff --git a/lib/lb.c b/lib/lb.c
new file mode 100644
index 00..db2d3d552f
--- /dev/null
+++ b/lib/lb.c
@@ -0,0 +1,236 @@
+/* Copyright (c) 2020, Red Hat, Inc.
+ *
+ * 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.
+ */
+
+#include 
+
+#include "lb.h"
+#include "lib/ovn-nb-idl.h"
+#include "lib/ovn-sb-idl.h"
+#include "lib/ovn-util.h"
+
+/* OpenvSwitch lib includes. */
+#include "openvswitch/vlog.h"
+#include "lib/smap.h"
+
+VLOG_DEFINE_THIS_MODULE(lb);
+
+static struct ovn_lb *
+ovn_lb_create(const struct smap *vips)
+{
+struct ovn_lb *lb = xzalloc(sizeof *lb);
+
+lb->n_vips = smap_count(vips);
+lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip));
+struct smap_node *node;
+size_t n_vips = 0;
+
+SMAP_FOR_EACH (node, vips) {
+char *vip;
+uint16_t port;
+int addr_family;
+
+if (!ip_address_and_port_from_key(node->key, &vip, &port,
+  &addr_family)) {
+continue;
+}
+
+lb->vips[n_vips].vip = vip;
+lb->vips[n_vips].vip_port = port;
+lb->vips[n_vips].addr_family = addr_family;
+lb->vips[n_vips].vip_port_str = xstrdup(node->key);
+lb->vips[n_vips].backend_ips = xstrdup(node->value);
+
+char *tokstr = xstrdup(node->value);
+char *save_ptr = NULL;
+char *token;
+size_t n_backends = 0;
+/* Format for a backend ips : IP1:port1,IP2:port2,...". */
+for (token = strtok_r(tokstr, ",", &save_ptr);
+token != NULL;
+token = strtok_r(NULL, ",", &save_ptr)) {
+n_backends++;
+}
+
+free(tokstr);
+tokstr = xstrdup(node->value);
+save_ptr = NULL;
+
+lb->vips[n_vips].n_backends = n_backends;
+lb->vips[n_vips].backends = xcalloc(n_backends,
+sizeof (struct lb_vip_backend));
+
+size_t i = 0;
+for (token = strtok_r(tokstr, ",", &save_ptr);
+token != NULL;
+token = strtok_r(NULL, ",", &save_ptr)) {
+char *backend_ip;
+uint16_t backend_port;
+
+if (!ip_address_and_port_from_key(token, &backend_ip,
+  &backend_port,
+  &addr_family)) {
+continue;
+}
+
+lb->vips[n_vips].backends[i].ip = backend_ip;
+lb->vips[n_vips].backends[i].port = backend_port;
+lb->vips[n_vips].backends[i].addr_family = addr_family;
+i++;
+}
+
+free(tokstr);
+n_vips++;
+}
+
+return lb;
+}
+
+struct ovn_lb *
+ovn_nb_lb_create(const struct nbrec_load_balancer *nbrec_lb,
+ struct hmap *ports, struct hmap *lbs,
+ void * (*ovn_port_find)(const struct hmap *ports,
+ const char *name))
+{
+struct ovn_lb *lb = ovn_lb_create(&nbrec_lb->vips);
+hmap_insert(lbs, &lb->hmap_node, uuid_hash(&nbrec_lb->header_.uuid));
+lb->nlb = nbrec_lb;
+lb->nb_lb = true;
+
+for (size_t i = 0; i < lb->n_vips; i++) {
+struct lb_vip *lb_vip = &lb->vips[i];
+
+struct nbrec_load_balancer_health_check *lb_health_check = NULL;
+if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) {
+if (nbrec_lb->n_health

[ovs-dev] [PATCH ovn v2 1/7] Add new table Load_Balancer in Southbound database.

2020-10-27 Thread numans
From: Numan Siddique 

This patch adds a new table 'Load_Balancer' in SB DB and syncs the 
Load_Balancer table rows
from NB DB to SB DB. An upcoming patch will make use of this table for handling 
the
load balancer hairpin traffic.

Signed-off-by: Numan Siddique 
---
 northd/ovn-northd.c   | 141 ++
 ovn-sb.ovsschema  |  27 +++-
 ovn-sb.xml|  45 ++
 tests/ovn-northd.at   |  81 
 utilities/ovn-sbctl.c |   3 +
 5 files changed, 295 insertions(+), 2 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index c06139d75b..d11888d203 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11860,6 +11860,136 @@ sync_dns_entries(struct northd_context *ctx, struct 
hmap *datapaths)
 }
 hmap_destroy(&dns_map);
 }
+
+/*
+ * struct 'sync_lb_info' is used to sync the load balancer records between
+ * OVN Northbound db and Southbound db.
+ */
+struct sync_lb_info {
+struct hmap_node hmap_node;
+const struct nbrec_load_balancer *nlb; /* LB record in the NB db. */
+const struct sbrec_load_balancer *slb; /* LB record in the SB db. */
+
+/* Datapaths to which the LB entry is associated with it. */
+const struct sbrec_datapath_binding **sbs;
+size_t n_sbs;
+};
+
+static inline struct sync_lb_info *
+get_sync_lb_info_from_hmap(struct hmap *sync_lb_map, struct uuid *uuid)
+{
+struct sync_lb_info *lb_info;
+size_t hash = uuid_hash(uuid);
+HMAP_FOR_EACH_WITH_HASH (lb_info, hmap_node, hash, sync_lb_map) {
+if (uuid_equals(&lb_info->nlb->header_.uuid, uuid)) {
+return lb_info;
+}
+}
+
+return NULL;
+}
+
+static void
+sync_lb_entries(struct northd_context *ctx, struct hmap *datapaths)
+{
+struct hmap lb_map = HMAP_INITIALIZER(&lb_map);
+struct ovn_datapath *od;
+
+HMAP_FOR_EACH (od, key_node, datapaths) {
+if (!od->nbs || !od->nbs->n_load_balancer) {
+continue;
+}
+
+for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
+struct sync_lb_info *lb_info =
+get_sync_lb_info_from_hmap(
+&lb_map, &od->nbs->load_balancer[i]->header_.uuid);
+
+if (!lb_info) {
+size_t hash = uuid_hash(
+&od->nbs->load_balancer[i]->header_.uuid);
+lb_info = xzalloc(sizeof *lb_info);;
+lb_info->nlb = od->nbs->load_balancer[i];
+hmap_insert(&lb_map, &lb_info->hmap_node, hash);
+}
+
+lb_info->n_sbs++;
+lb_info->sbs = xrealloc(lb_info->sbs,
+lb_info->n_sbs * sizeof *lb_info->sbs);
+lb_info->sbs[lb_info->n_sbs - 1] = od->sb;
+}
+}
+
+const struct sbrec_load_balancer *sbrec_lb, *next;
+SBREC_LOAD_BALANCER_FOR_EACH_SAFE (sbrec_lb, next, ctx->ovnsb_idl) {
+const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids, "lb_id");
+struct uuid lb_uuid;
+if (!nb_lb_uuid || !uuid_from_string(&lb_uuid, nb_lb_uuid)) {
+sbrec_load_balancer_delete(sbrec_lb);
+continue;
+}
+
+struct sync_lb_info *lb_info =
+get_sync_lb_info_from_hmap(&lb_map, &lb_uuid);
+if (lb_info) {
+lb_info->slb = sbrec_lb;
+} else {
+sbrec_load_balancer_delete(sbrec_lb);
+}
+}
+
+struct sync_lb_info *lb_info;
+HMAP_FOR_EACH (lb_info, hmap_node, &lb_map) {
+if (!lb_info->slb) {
+sbrec_lb = sbrec_load_balancer_insert(ctx->ovnsb_txn);
+lb_info->slb = sbrec_lb;
+char *lb_id = xasprintf(
+UUID_FMT, UUID_ARGS(&lb_info->nlb->header_.uuid));
+const struct smap external_ids =
+SMAP_CONST1(&external_ids, "lb_id", lb_id);
+sbrec_load_balancer_set_external_ids(sbrec_lb, &external_ids);
+free(lb_id);
+}
+
+/* Set the datapaths and other columns.  If nothing has changed, then
+ * this will be a no-op.
+ */
+sbrec_load_balancer_set_datapaths(
+lb_info->slb,
+(struct sbrec_datapath_binding **)lb_info->sbs,
+lb_info->n_sbs);
+
+sbrec_load_balancer_set_name(lb_info->slb, lb_info->nlb->name);
+sbrec_load_balancer_set_vips(lb_info->slb, &lb_info->nlb->vips);
+sbrec_load_balancer_set_protocol(lb_info->slb, lb_info->nlb->protocol);
+}
+
+HMAP_FOR_EACH (od, key_node, datapaths) {
+if (!od->nbs || !od->nbs->n_load_balancer) {
+continue;
+}
+
+const struct sbrec_load_balancer **lbs =
+xmalloc(od->nbs->n_load_balancer * sizeof *lbs);
+for (size_t i = 0; i < od->nbs->n_load_balancer; i++) {
+lb_info = get_sync_lb_info_from_hmap(
+&lb_map, &od->nbs->load_balancer[i]->header_.uuid);
+ovs_assert(lb_info);
+   

[ovs-dev] [PATCH ovn v2 0/7] Optimize load balancer hairpin logical flows.

2020-10-27 Thread numans
From: Numan Siddique 

This patch series optimizes the load balancer hairpin logical flows.
Presently, ovn-northd generates a lot of these logical flows.

Suppose there are 'm' load balancers associated to a logical switch and each 
load balancer
has 'n' VIPs and each VIP has 'p' backends then ovn-northd adds (m * ((n * p) + 
n))
hairpin logical flows.

With this patch series, ovn-northd adds just 5 hairpin logical flows.

To reduce the number of lflows, load balancer information is now pushed
to the Southbound database using a new 'Load_Balancer' table.
ovn-controller programs the OF flows required for handling the load
balancer hairpin traffic directly and new OVN actions are added to
abstract the hairpining from ovn-northd.

v1 -> v2
-
  * Addressed the review comments from Mark Michelson.
  * Added 2 more patches to the series to have SB Load balancer support
ovn-detrace and 'ovn-sbctl --vflows' as pointed by Dumitru Ceara.

Numan Siddique (7):
  Add new table Load_Balancer in Southbound database.
  northd: Refactor load balancer vip parsing.
  controller:  Add load balancer hairpin OF flows.
  actions: Add new actions chk_lb_hairpin, chk_lb_hairpin_reply and
ct_snat_to_vip.
  northd: Make use of new hairpin actions.
  ovn-detrace: Add SB Load Balancer cookier handler.
  sbctl: Add Load Balancer support for vflows option.

 controller/lflow.c   | 256 +++
 controller/lflow.h   |   6 +-
 controller/ovn-controller.c  |  27 +-
 include/ovn/actions.h|  15 +-
 include/ovn/logical-fields.h |   3 +
 lib/actions.c| 116 ++-
 lib/automake.mk  |   4 +-
 lib/lb.c | 236 ++
 lib/lb.h |  77 +
 lib/ovn-util.c   |  28 ++
 lib/ovn-util.h   |   2 +
 northd/ovn-northd.8.xml  |  65 +++-
 northd/ovn-northd.c  | 586 ++-
 ovn-sb.ovsschema |  27 +-
 ovn-sb.xml   |  82 +
 tests/ovn-northd.at  | 109 ++-
 tests/ovn.at | 544 ++--
 tests/test-ovn.c |   3 +
 utilities/ovn-detrace.in |  11 +-
 utilities/ovn-sbctl.c|  58 
 utilities/ovn-trace.c|  65 +++-
 21 files changed, 1892 insertions(+), 428 deletions(-)
 create mode 100644 lib/lb.c
 create mode 100644 lib/lb.h

-- 
2.26.2

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


Re: [ovs-dev] [PATCH ovn] update NEWS file with iPXE support

2020-10-27 Thread Numan Siddique
On Tue, Oct 27, 2020 at 8:51 PM Lorenzo Bianconi
 wrote:
>
> Signed-off-by: Lorenzo Bianconi 

Thanks Lorenzo. I applied this patch to master.

Numan

> ---
>  NEWS | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index 48b21fae4..ba04c6bbe 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,7 +4,8 @@ Post-v20.09.0
>   datapath can be inferred from the inport (which is required).
> - The obsolete "redirect-chassis" way to configure gateways has been
>   removed.  See ovn-nb(5) for advice on how to update your config if 
> needed.
> -
> +   - Add IPv4 iPXE support introducing "bootfile_name_alt" option to ovn dhcp
> + server
>
>  OVN v20.09.0 - 28 Sep 2020
>  --
> --
> 2.26.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn 7/7] northd: Enhance implementation of port tunnel key requests.

2020-10-27 Thread Numan Siddique
On Mon, Oct 26, 2020 at 11:47 PM Ben Pfaff  wrote:
>
> When a tunnel key was requested, the implementation was not smart enough
> to reassign a port that had been automatically assigned the same
> key.  This fixes the problem and adds a test.
>
> Signed-off-by: Ben Pfaff 
> ---
>  northd/ovn-northd.c | 142 
>  tests/ovn-northd.at |  59 +-
>  2 files changed, 136 insertions(+), 65 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 92d578c405a2..66b0a81267cc 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -1427,6 +1427,8 @@ struct ovn_port {
>
>  const struct sbrec_port_binding *sb; /* May be NULL. */
>
> +uint32_t tunnel_key;
> +
>  /* Logical switch port data. */
>  const struct nbrec_logical_switch_port *nbsp; /* May be NULL. */
>
> @@ -1465,13 +1467,6 @@ struct ovn_port {
>  struct ovs_list list;   /* In list of similar records. */
>  };
>
> -static void
> -ovn_port_set_sb(struct ovn_port *op,
> -const struct sbrec_port_binding *sb)
> -{
> -op->sb = sb;
> -}
> -
>  static void
>  ovn_port_set_nb(struct ovn_port *op,
>  const struct nbrec_logical_switch_port *nbsp,
> @@ -1495,7 +1490,7 @@ ovn_port_create(struct hmap *ports, const char *key,
>  op->json_key = ds_steal_cstr(&json_key);
>
>  op->key = xstrdup(key);
> -ovn_port_set_sb(op, sb);
> +op->sb = sb;
>  ovn_port_set_nb(op, nbsp, nbrp);
>  op->derived = false;
>  hmap_insert(ports, &op->key_node, hash_string(op->key, 0));
> @@ -1541,13 +1536,6 @@ ovn_port_find(const struct hmap *ports, const char 
> *name)
>  return NULL;
>  }
>
> -static uint32_t
> -ovn_port_allocate_key(struct ovn_datapath *od)
> -{
> -return ovn_allocate_tnlid(&od->port_tnlids, "port",
> -  1, (1u << 15) - 1, &od->port_key_hint);
> -}
> -
>  /* Returns true if the logical switch port 'enabled' column is empty or
>   * set to true.  Otherwise, returns false. */
>  static bool
> @@ -2979,15 +2967,6 @@ copy_gw_chassis_from_nbrp_to_sbpb(
>  free(sb_ha_chassis);
>  }
>
> -static int64_t
> -op_get_requested_tnl_key(const struct ovn_port *op)
> -{
> -ovs_assert(op->nbsp || op->nbrp);
> -const struct smap *op_options = op->nbsp ? &op->nbsp->options
> -: &op->nbrp->options;
> -return smap_get_int(op_options, "requested-tnl-key", 0);
> -}
> -
>  static const char*
>  op_get_name(const struct ovn_port *op)
>  {
> @@ -3304,17 +3283,8 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>  sbrec_port_binding_set_external_ids(op->sb, &ids);
>  smap_destroy(&ids);
>  }
> -int64_t tnl_key = op_get_requested_tnl_key(op);
> -if (tnl_key && tnl_key != op->sb->tunnel_key) {
> -if (!ovn_add_tnlid(&op->od->port_tnlids, tnl_key)) {
> -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> -VLOG_WARN_RL(&rl, "Cannot update port binding for "
> - "%s due to duplicate key set "
> - "in options:requested-tnl-key: %"PRId64,
> - op_get_name(op), tnl_key);
> -} else {
> -sbrec_port_binding_set_tunnel_key(op->sb, tnl_key);
> -}
> +if (op->tunnel_key != op->sb->tunnel_key) {
> +sbrec_port_binding_set_tunnel_key(op->sb, op->tunnel_key);
>  }
>  }
>
> @@ -3707,6 +3677,52 @@ destroy_ovn_lbs(struct hmap *lbs)
>  }
>  }
>
> +static bool
> +ovn_port_add_tnlid(struct ovn_port *op, uint32_t tunnel_key)
> +{
> +bool added = ovn_add_tnlid(&op->od->port_tnlids, tunnel_key);
> +if (added) {
> +op->tunnel_key = tunnel_key;
> +if (tunnel_key > op->od->port_key_hint) {
> +op->od->port_key_hint = tunnel_key;
> +}
> +}
> +return added;
> +}
> +
> +static void
> +ovn_port_assign_requested_tnl_id(struct ovn_port *op)
> +{
> +const struct smap *options = (op->nbsp
> +  ? &op->nbsp->options
> +  : &op->nbrp->options);
> +uint32_t tunnel_key = smap_get_int(options, "requested-tnl-key", 0);
> +if (tunnel_key && !ovn_port_add_tnlid(op, tunnel_key)) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> +VLOG_WARN_RL(&rl, "Logical %s port %s requests same tunnel key "
> + "%"PRIu32" as another LSP or LRP",
> + op->nbsp ? "switch" : "router",
> + op_get_name(op), tunnel_key);
> +}
> +}
> +
> +static void
> +ovn_port_allocate_key(struct hmap *ports, struct ovn_port *op)
> +{
> +if (!op->tunnel_key) {
> +op->tunnel_key = ovn_allocate_tnlid(&op->od->port_tnlids, "port",
> +1, (1u << 15) - 1,
> +&op->od->port_key_hint);
> +if (!op->tunnel_key) 

Re: [ovs-dev] [PATCH ovn 6/7] northd: Enhance implementation of datapath tunnel key requests.

2020-10-27 Thread Numan Siddique
On Mon, Oct 26, 2020 at 11:47 PM Ben Pfaff  wrote:
>
> When a tunnel key was requested, the implementation was not smart enough
> to reassign a datapath that had been automatically assigned the same
> key.  This fixes the problem and adds a test.
>
> I didn't see a reason not to make this feature available for logical
> routers so I added that feature too.
>
> Signed-off-by: Ben Pfaff 

Acked-by: Numan Siddique 

Thanks
Numan

> ---
>  lib/ovn-util.c  |  26 --
>  lib/ovn-util.h  |   3 +-
>  northd/ovn-northd.c | 124 
>  ovn-nb.xml  |   8 +++
>  tests/ovn-northd.at |  46 
>  5 files changed, 135 insertions(+), 72 deletions(-)
>
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index 1daf6650379f..321fc89e275a 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -517,24 +517,21 @@ ovn_destroy_tnlids(struct hmap *tnlids)
>  hmap_destroy(tnlids);
>  }
>
> -void
> -ovn_add_tnlid(struct hmap *set, uint32_t tnlid)
> -{
> -struct tnlid_node *node = xmalloc(sizeof *node);
> -hmap_insert(set, &node->hmap_node, hash_int(tnlid, 0));
> -node->tnlid = tnlid;
> -}
> -
>  bool
> -ovn_tnlid_in_use(const struct hmap *set, uint32_t tnlid)
> +ovn_add_tnlid(struct hmap *set, uint32_t tnlid)
>  {
> -const struct tnlid_node *node;
> -HMAP_FOR_EACH_IN_BUCKET (node, hmap_node, hash_int(tnlid, 0), set) {
> +uint32_t hash = hash_int(tnlid, 0);
> +struct tnlid_node *node;
> +HMAP_FOR_EACH_IN_BUCKET (node, hmap_node, hash, set) {
>  if (node->tnlid == tnlid) {
> -return true;
> +return false;
>  }
>  }
> -return false;
> +
> +node = xmalloc(sizeof *node);
> +hmap_insert(set, &node->hmap_node, hash);
> +node->tnlid = tnlid;
> +return true;
>  }
>
>  static uint32_t
> @@ -549,8 +546,7 @@ ovn_allocate_tnlid(struct hmap *set, const char *name, 
> uint32_t min,
>  {
>  for (uint32_t tnlid = next_tnlid(*hint, min, max); tnlid != *hint;
>   tnlid = next_tnlid(tnlid, min, max)) {
> -if (!ovn_tnlid_in_use(set, tnlid)) {
> -ovn_add_tnlid(set, tnlid);
> +if (ovn_add_tnlid(set, tnlid)) {
>  *hint = tnlid;
>  return tnlid;
>  }
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index 1d22a691f56f..6162f30225b3 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -114,8 +114,7 @@ void ovn_conn_show(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>
>  struct hmap;
>  void ovn_destroy_tnlids(struct hmap *tnlids);
> -void ovn_add_tnlid(struct hmap *set, uint32_t tnlid);
> -bool ovn_tnlid_in_use(const struct hmap *set, uint32_t tnlid);
> +bool ovn_add_tnlid(struct hmap *set, uint32_t tnlid);
>  uint32_t ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min,
>  uint32_t max, uint32_t *hint);
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index b96e0db516be..92d578c405a2 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -596,6 +596,8 @@ struct ovn_datapath {
>
>  struct ovs_list list;   /* In list of similar records. */
>
> +uint32_t tunnel_key;
> +
>  /* Logical switch data. */
>  struct ovn_port **router_ports;
>  size_t n_router_ports;
> @@ -1296,12 +1298,45 @@ get_ovn_max_dp_key_local(struct northd_context *ctx)
>  return OVN_MAX_DP_KEY - OVN_MAX_DP_GLOBAL_NUM;
>  }
>
> -static uint32_t
> -ovn_datapath_allocate_key(struct northd_context *ctx, struct hmap *dp_tnlids)
> +static void
> +ovn_datapath_allocate_key(struct northd_context *ctx,
> +  struct hmap *datapaths, struct hmap *dp_tnlids,
> +  struct ovn_datapath *od, uint32_t *hint)
> +{
> +if (!od->tunnel_key) {
> +od->tunnel_key = ovn_allocate_tnlid(dp_tnlids, "datapath",
> +OVN_MIN_DP_KEY_LOCAL,
> +get_ovn_max_dp_key_local(ctx),
> +hint);
> +if (!od->tunnel_key) {
> +if (od->sb) {
> +sbrec_datapath_binding_delete(od->sb);
> +}
> +ovs_list_remove(&od->list);
> +ovn_datapath_destroy(datapaths, od);
> +}
> +}
> +}
> +
> +static void
> +ovn_datapath_assign_requested_tnl_id(struct hmap *dp_tnlids,
> + struct ovn_datapath *od)
>  {
> -static uint32_t hint;
> -return ovn_allocate_tnlid(dp_tnlids, "datapath", OVN_MIN_DP_KEY_LOCAL,
> -  get_ovn_max_dp_key_local(ctx), &hint);
> +const struct smap *other_config = (od->nbs
> +   ? &od->nbs->other_config
> +   : &od->nbr->options);
> +uint32_t tunnel_key = smap_get_int(other_config, "requested-tnl-key", 0);
> +if (tunnel_key) {
> +if (ovn_add_tnlid(dp_tnlids, tunnel_

Re: [ovs-dev] [PATCH v3 1/2] Documentation: update IPsec tutorial for F32

2020-10-27 Thread Eric Garver
On Tue, Oct 27, 2020 at 10:45:17AM -0400, Mark Gray wrote:
> F32 requires the "python3-openvswitch" package now. Also, the
> iptables chain "IN_FedoraServer_allow" does not exist on Fedora 32.
> 
> Signed-off-by: Mark Gray 

Thanks.

Acked-by: Eric Garver 

> ---
>  Documentation/tutorials/ipsec.rst | 108 +++---
>  1 file changed, 55 insertions(+), 53 deletions(-)
> 
> diff --git a/Documentation/tutorials/ipsec.rst 
> b/Documentation/tutorials/ipsec.rst
> index b4c3235132bc..ebc0ae429c19 100644
> --- a/Documentation/tutorials/ipsec.rst
> +++ b/Documentation/tutorials/ipsec.rst
> @@ -42,7 +42,7 @@ Installing OVS and IPsec Packages
>  -
>  
>  OVS IPsec has .deb and .rpm packages. You should use the right package
> -based on your Linux distribution. This tutorial uses Ubuntu 16.04 and Fedora 
> 27
> +based on your Linux distribution. This tutorial uses Ubuntu 16.04 and Fedora 
> 32
>  as examples.
>  
>  Ubuntu
> @@ -59,8 +59,8 @@ Ubuntu
>  
>  2. Install the related packages::
>  
> -   $ apt-get install dkms strongswan
> -   $ dpkg -i libopenvswitch_*.deb openvswitch-common_*.deb \
> +   # apt-get install dkms strongswan
> +   # dpkg -i libopenvswitch_*.deb openvswitch-common_*.deb \
>   openvswitch-switch_*.deb openvswitch-datapath-dkms_*.deb \
>   python-openvswitch_*.deb openvswitch-pki_*.deb \
>   openvswitch-ipsec_*.deb
> @@ -71,23 +71,25 @@ Ubuntu
>  Fedora
>  ~~
>  
> -1. Follow :doc:`/intro/install/fedora` to build RPM packages.
> +1. Install the related packages. Fedora 32 does not require installation of
> +   the out-of-tree kernel module::
>  
> -2. Install the related packages::
> +   # dnf install python3-openvswitch libreswan \
> + openvswitch openvswitch-ipsec
> +
> +2. Install firewall rules to allow ESP and IKE traffic::
>  
> -   $ dnf install python2-openvswitch libreswan \
> - "kernel-devel-uname-r == $(uname -r)"
> -   $ rpm -i openvswitch-*.rpm openvswitch-kmod-*.rpm \
> -openvswitch-openvswitch-ipsec-*.rpm
> +   # systemctl start firewalld
> +   # firewall-cmd --add-service ipsec
>  
> -3. Install firewall rules to allow ESP and IKE traffic::
> +   Or to make permanent::
>  
> -   $ iptables -A IN_FedoraServer_allow -p esp -j ACCEPT
> -   $ iptables -A IN_FedoraServer_allow -p udp --dport 500 -j ACCEPT
> +   # systemctl enable firewalld
> +   # firewall-cmd --permanent --add-service ipsec
>  
> -4. Run the openvswitch-ipsec service::
> +3. Run the openvswitch-ipsec service::
>  
> -   $ systemctl start openvswitch-ipsec.service
> +   # systemctl start openvswitch-ipsec.service
>  
> .. note::
>  
> @@ -97,47 +99,47 @@ Fedora
>  Configuring IPsec tunnel
>  
>  
> -Suppose you want to build IPsec tunnel between two hosts. Assume `host_1`'s
> +Suppose you want to build an IPsec tunnel between two hosts. Assume 
> `host_1`'s
>  external IP is 1.1.1.1, and `host_2`'s external IP is 2.2.2.2. Make sure
>  `host_1` and `host_2` can ping each other via these external IPs.
>  
>  0. Set up some variables to make life easier.  On both hosts, set ``ip_1`` 
> and
> ``ip_2`` variables, e.g.::
>  
> - $ ip_1=1.1.1.1
> - $ ip_2=2.2.2.2
> + # ip_1=1.1.1.1
> + # ip_2=2.2.2.2
>  
>  1. Set up OVS bridges in both hosts.
>  
> In `host_1`::
>  
> -   $ ovs-vsctl add-br br-ipsec
> -   $ ip addr add 192.0.0.1/24 dev br-ipsec
> -   $ ip link set br-ipsec up
> +   # ovs-vsctl add-br br-ipsec
> +   # ip addr add 192.0.0.1/24 dev br-ipsec
> +   # ip link set br-ipsec up
>  
> In `host_2`::
>  
> -   $ ovs-vsctl add-br br-ipsec
> -   $ ip addr add 192.0.0.2/24 dev br-ipsec
> -   $ ip link set br-ipsec up
> +   # ovs-vsctl add-br br-ipsec
> +   # ip addr add 192.0.0.2/24 dev br-ipsec
> +   # ip link set br-ipsec up
>  
>  2. Set up IPsec tunnel.
>  
> -   There are three authentication methods. You can choose one to set up your
> -   IPsec tunnel.
> +   There are three authentication methods.  Choose one method to set up your
> +   IPsec tunnel and follow the steps below.
>  
> a) Using pre-shared key:
>  
>In `host_1`::
>  
> -  $ ovs-vsctl add-port br-ipsec tun -- \
> +  # ovs-vsctl add-port br-ipsec tun -- \
>set interface tun type=gre \
>  options:remote_ip=$ip_2 \
>  options:psk=swordfish
>  
>In `host_2`::
>  
> -  $ ovs-vsctl add-port br-ipsec tun -- \
> +  # ovs-vsctl add-port br-ipsec tun -- \
>set interface tun type=gre \
>  options:remote_ip=$ip_1 \
>  options:psk=swordfish
> @@ -156,15 +158,15 @@ external IP is 1.1.1.1, and `host_2`'s external IP is 

Re: [ovs-dev] [PATCH v2] Don't raise an Exception on failure to connect via SSL

2020-10-27 Thread Terry Wilson
Just bumping to make sure this doesn't fall through the cracks. 11 days
since the last ack. Let me know if there's anything else that needs
changing. Thanks!

On Fri, Oct 16, 2020 at 1:37 AM Thomas Neuman 
wrote:

> Yup, fair enough. LGTM
>
> Acked-by: Thomas Neuman 
>
> >
> > I get why it seems a little weird, but the reason I chose @staticmethod
> is
> > because @classmethod would imply that we are going to actually reference
> > the class that is passed in the first argument, and we won't. The only
> > thing this method depends on is the socket, so it seemed a staticmethod
> > that could be overridden in a subclass with a special need was the way to
> > go.
> >
>
> ___
> 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 5/7] northd: Use address set for service monitor MAC.

2020-10-27 Thread Numan Siddique
On Mon, Oct 26, 2020 at 11:47 PM Ben Pfaff  wrote:
>
> Until now, the service monitor MAC has been inlined into logical flow
> matches.  This makes it a little hard to compare flow tables from one
> run or deployment to another, since the service monitor MAC is random
> and will always differ.  This commit changes flow matches to use an
> address set $svc_monitor_mac everywhere that it can be.  This makes
> the flow matches the same in every deployment.
>
> The service monitor MAC is also used in actions to set Ethernet
> addresses.  This can't be replaced by an address set, so these flows
> will still have some differences.
>
> Signed-off-by: Ben Pfaff 

Acked-by: Numan Siddique 

Thanks
Numan


> ---
>  northd/ovn-northd.c | 43 +--
>  1 file changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index f25f5cd82f39..b96e0db516be 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5004,15 +5004,11 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
> *lflows)
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 0, "1", "next;");
>  ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;");
>
> -char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
> -ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, svc_check_match,
> -  "next;");
> -free(svc_check_match);
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
> +  "eth.dst == $svc_monitor_mac", "next;");
>
> -svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac);
> -ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, svc_check_match,
> -  "next;");
> -free(svc_check_match);
> +ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
> +  "eth.src == $svc_monitor_mac", "next;");
>
>  /* If there are any stateful ACL rules in this datapath, we must
>   * send all IP packets through the conntrack action, which handles
> @@ -5170,15 +5166,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
> *lflows,
>"next;");
>
>  /* Do not send service monitor packets to conntrack. */
> -char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> -  svc_check_match, "next;");
> -free(svc_check_match);
> -
> -svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac);
> +  "eth.dst == $svc_monitor_mac", "next;");
>  ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
> -  svc_check_match, "next;");
> -free(svc_check_match);
> +  "eth.src == $svc_monitor_mac", "next;");
>
>  /* Allow all packets to go to next tables by default. */
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
> @@ -5831,17 +5822,13 @@ build_acls(struct ovn_datapath *od, struct hmap 
> *lflows,
>
>  /* Add a 34000 priority flow to advance the service monitor reply
>   * packets to skip applying ingress ACLs. */
> -char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
> -ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 34000, svc_check_match,
> -  "next;");
> -free(svc_check_match);
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 34000,
> +  "eth.dst == $svc_monitor_mac", "next;");
>
>  /* Add a 34000 priority flow to advance the service monitor packets
>   * generated by ovn-controller to skip applying egress ACLs. */
> -svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac);
> -ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 34000, svc_check_match,
> -  "next;");
> -free(svc_check_match);
> +ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 34000,
> +  "eth.src == $svc_monitor_mac", "next;");
>  }
>
>  static void
> @@ -7172,7 +7159,6 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
> *ports,
>  }
>  }
>
> -char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
>  /* Ingress table 19: Destination lookup, broadcast and multicast handling
>   * (priority 70 - 100). */
>  HMAP_FOR_EACH (od, key_node, datapaths) {
> @@ -7180,7 +7166,8 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
> *ports,
>  continue;
>  }
>
> -ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110, svc_check_match,
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
> +  "eth.dst == $svc_monitor_mac",
>"handle_svc_check(inport);");
>
>  struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw;
> @@ -7253,7 +7240,6 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
> *ports,
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 70, "eth.mcast",
>"outport = \""MC_FLOOD"\"; output;");
>  

Re: [ovs-dev] [PATCH ovn 4/7] northd: Don't redundantly set Port_Binding options.

2020-10-27 Thread Numan Siddique
On Mon, Oct 26, 2020 at 11:47 PM Ben Pfaff  wrote:
>
> This is called twice within a few lines of code.
>
> Signed-off-by: Ben Pfaff 

Acked-by: Numan Siddique 

Thanks
Numan

> ---
>  northd/ovn-northd.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index e69845fbb219..f25f5cd82f39 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -10223,7 +10223,6 @@ build_ND_RA_flows_for_lrouter_port(
>  }
>  smap_add(&options, "ipv6_prefix_delegation",
>   prefix_delegation ? "true" : "false");
> -sbrec_port_binding_set_options(op->sb, &options);
>
>  bool ipv6_prefix = smap_get_bool(&op->nbrp->options,
>   "prefix", false);
> --
> 2.26.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn 3/7] northd: Improve comments.

2020-10-27 Thread Numan Siddique
On Mon, Oct 26, 2020 at 11:46 PM Ben Pfaff  wrote:
>
> Signed-off-by: Ben Pfaff 
> ---
>  northd/ovn-northd.c | 33 +
>  1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 4457cdc2d392..e69845fbb219 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -1386,9 +1386,29 @@ build_datapaths(struct northd_context *ctx, struct 
> hmap *datapaths,
>  }
>  }
>
> +/* A logical switch port or logical router port.
> + *
> + * In steady state, an ovn_port points to a northbound Logical_Switch_Port
> + * record (via 'nbsp') *or* a Logical_Router_Port record (via 'nbrp'), and 
> to a
> + * southbound Port_Binding record (via 'sb').  As the state of the system
> + * changes, join_logical_ports() may determine that there is a new LSP or LRP
> + * that has no corresponding Port_Binding record (in which case 
> build_ports())
> + * will create the missing Port_Binding) or that a Port_Binding record exists
> + * that has no coresponding LSP (in which case build_ports() will delete the
> + * spurious Port_Binding).  Thus, after build_ports() runs, any given 
> ovn_port
> + * will have 'sb' nonnull, and 'nbsp' xor 'nbrp' nonnull.
> + *
> + * Ordinarily there is only one ovn_port that points to a given LSP or LRP 
> (but
> + * distributed gateway ports point a "derived" ovn_port to a duplicate LRP).
> + */
>  struct ovn_port {
> +/* Port name aka key.
> + *
> + * This is ordinarily the same as nbsp->name or nbrp->name and
> + * sb->logical_port.  (A distributed gateway port creates a "derived"
> + * ovn_port with key "cr-%s" % nbrp->name.) */
>  struct hmap_node key_node;  /* Index on 'key'. */
> -char *key;  /* nbs->name, nbr->name, sb->logical_port. */
> +char *key;  /* nbsp->name, nbrp->name, sb->logical_port. 
> */
>  char *json_key; /* 'key', quoted for use in JSON. */
>
>  const struct sbrec_port_binding *sb; /* May be NULL. */
> @@ -1410,15 +1430,20 @@ struct ovn_port {
>  /* Logical port multicast data. */
>  struct mcast_port_info mcast_info;
>
> -bool derived; /* Indicates whether this is an additional port
> -   * derived from nbsp or nbrp. */
> +/* This is ordinarily false.  It is true iff this ovn_port is derived 
> from

nit: s/iff/if

Acked-by: Numan Siddique 

Thanks
Numan

> + * a chassis-redirect port. */
> +bool derived;
> +
>  bool has_unknown; /* If the addresses have 'unknown' defined. */
> +
>  /* The port's peer:
>   *
>   * - A switch port S of type "router" has a router port R as a peer,
>   *   and R in turn has S has its peer.
>   *
> - * - Two connected logical router ports have each other as peer. */
> + * - Two connected logical router ports have each other as peer.
> + *
> + * - Other kinds of ports have no peer. */
>  struct ovn_port *peer;
>
>  struct ovn_datapath *od;
> --
> 2.26.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn] update NEWS file with iPXE support

2020-10-27 Thread Lorenzo Bianconi
Signed-off-by: Lorenzo Bianconi 
---
 NEWS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 48b21fae4..ba04c6bbe 100644
--- a/NEWS
+++ b/NEWS
@@ -4,7 +4,8 @@ Post-v20.09.0
  datapath can be inferred from the inport (which is required).
- The obsolete "redirect-chassis" way to configure gateways has been
  removed.  See ovn-nb(5) for advice on how to update your config if needed.
-
+   - Add IPv4 iPXE support introducing "bootfile_name_alt" option to ovn dhcp
+ server
 
 OVN v20.09.0 - 28 Sep 2020
 --
-- 
2.26.2

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


Re: [ovs-dev] [PATCH ovn 2/7] northd: Add missing space in log message.

2020-10-27 Thread Numan Siddique
On Mon, Oct 26, 2020 at 11:46 PM Ben Pfaff  wrote:
>
> Signed-off-by: Ben Pfaff 
> Fixes: fc79d690b9e5 ("External IP based NAT: NORTHD changes to use 
> allowed/exempted external ip")

Acked-by: Numan Siddique 

Thanks
Numan

> ---
>  northd/ovn-northd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index d677f357f5d0..4457cdc2d392 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -9276,7 +9276,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
> *ports,
>
>  if (allowed_ext_ips && exempted_ext_ips) {
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 
> 1);
> -VLOG_WARN_RL(&rl, "NAT rule: "UUID_FMT" not applied, since"
> +VLOG_WARN_RL(&rl, "NAT rule: "UUID_FMT" not applied, since "
>   "both allowed and exempt external ips set",
>   UUID_ARGS(&(nat->header_.uuid)));
>  continue;
> --
> 2.26.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn 1/7] northd: Count mask length and priority correctly for IPv6 addresses.

2020-10-27 Thread Numan Siddique
On Mon, Oct 26, 2020 at 11:46 PM Ben Pfaff  wrote:
>
> Before this commit, the IPv4 calculation was used even for IPv6
> addresses, which was wrong.
>
> Signed-off-by: Ben Pfaff 

Acked-by: Numan Siddique 

Thanks
Numan

> ---
>  northd/ovn-northd.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 59b7b3d2ee3a..d677f357f5d0 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -9303,10 +9303,13 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>
>  /* Check the validity of nat->logical_ip. 'logical_ip' can
>   * be a subnet when the type is "snat". */
> +int cidr_bits;
>  if (is_v6) {
>  error = ipv6_parse_masked(nat->logical_ip, &ipv6, &mask_v6);
> +cidr_bits = ipv6_count_cidr_bits(&mask_v6);
>  } else {
>  error = ip_parse_masked(nat->logical_ip, &ip, &mask);
> +cidr_bits = ip_count_cidr_bits(mask);
>  }
>  if (!strcmp(nat->type, "snat")) {
>  if (error) {
> @@ -9612,11 +9615,11 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>   * nat->logical_ip with the longest mask gets a higher
>   * priority. */
>  ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_SNAT,
> -count_1bits(ntohl(mask)) + 1,
> +cidr_bits + 1,
>  ds_cstr(&match), 
> ds_cstr(&actions),
>  &nat->header_);
>  } else {
> -uint16_t priority = count_1bits(ntohl(mask)) + 1;
> +uint16_t priority = cidr_bits + 1;
>
>  /* Distributed router. */
>  ds_clear(&match);
> --
> 2.26.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2 1/2] Documentation: update IPsec tutorial for F32

2020-10-27 Thread Mark Gray
On 23/10/2020 20:23, Eric Garver wrote:
> On Fri, Oct 23, 2020 at 02:39:21PM -0400, Mark Gray wrote:

> 
> nit: These commands require root. So prompt should be '#'.

The whole file was incorrect. I just updated the whole thing. v3 is
posted at:

https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376642.html

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


[ovs-dev] [PATCH v3 0/2] Some fixes for OVS IPsec on Fedora

2020-10-27 Thread Mark Gray
In the process of testing OVS IPsec on Fedora, I discovered
some small issues to be resolved.

I am following the tutorial at

https://docs.openvswitch.org/en/latest/tutorials/ipsec/

Mark Gray (2):
  Documentation: update IPsec tutorial for F32
  python: set ovs.dirs variables with build system values

 Documentation/tutorials/ipsec.rst | 108 +++---
 lib/automake.mk   |   2 +-
 python/automake.mk|  13 ++--
 python/ovs/.gitignore |   1 +
 python/ovs/dirs.py|  31 -
 5 files changed, 64 insertions(+), 91 deletions(-)
 delete mode 100644 python/ovs/dirs.py

-- 

v2:
  - Updated tutorial
v3:
  - Changed bash prompt in entire tutorial to indicate root
privileges

2.26.2

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


[ovs-dev] [PATCH v3 2/2] python: set ovs.dirs variables with build system values

2020-10-27 Thread Mark Gray
ovs/dirs.py should be auto-generated using the template
ovs/dirs.py.template at build time. This will set the
ovs.dirs python variables with a value specified by the
environment or, if the environment variable is not set, from
the build system.

Signed-off-by: Mark Gray 
---
 lib/automake.mk   |  2 +-
 python/automake.mk| 13 +++--
 python/ovs/.gitignore |  1 +
 python/ovs/dirs.py| 31 ---
 4 files changed, 9 insertions(+), 38 deletions(-)
 delete mode 100644 python/ovs/dirs.py

diff --git a/lib/automake.mk b/lib/automake.mk
index 380a672287ac..8eeb6c3f676c 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -575,7 +575,7 @@ MAN_FRAGMENTS += \
 OVSIDL_BUILT += lib/vswitch-idl.c lib/vswitch-idl.h lib/vswitch-idl.ovsidl
 
 EXTRA_DIST += lib/vswitch-idl.ann
-lib/vswitch-idl.ovsidl: vswitchd/vswitch.ovsschema lib/vswitch-idl.ann
+lib/vswitch-idl.ovsidl: vswitchd/vswitch.ovsschema lib/vswitch-idl.ann 
python/ovs/dirs.py
$(AM_V_GEN)$(OVSDB_IDLC) annotate $(srcdir)/vswitchd/vswitch.ovsschema 
$(srcdir)/lib/vswitch-idl.ann > $@.tmp && mv $@.tmp $@
 
 lib/dirs.c: lib/dirs.c.in Makefile
diff --git a/python/automake.mk b/python/automake.mk
index 2f08c7701484..a88addf72326 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -107,12 +107,13 @@ ALL_LOCAL += $(srcdir)/python/ovs/dirs.py
 $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
$(AM_V_GEN)sed \
-e '/^##/d' \
--e 's,[@]pkgdatadir[@],/usr/local/share/openvswitch,g' \
--e 's,[@]RUNDIR[@],/var/run,g' \
--e 's,[@]LOGDIR[@],/usr/local/var/log,g' \
--e 's,[@]bindir[@],/usr/local/bin,g' \
--e 's,[@]sysconfdir[@],/usr/local/etc,g' \
--e 's,[@]DBDIR[@],/usr/local/etc/openvswitch,g' \
+-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
+-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
+-e 's,[@]LOGDIR[@],$(LOGDIR),g' \
+-e 's,[@]bindir[@],$(bindir),g' \
+-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
+-e 's,[@]DBDIR[@],$(sysconfdir),g' \
< $? > $@.tmp && \
mv $@.tmp $@
 EXTRA_DIST += python/ovs/dirs.py.template
+CLEANFILES += python/ovs/dirs.py
diff --git a/python/ovs/.gitignore b/python/ovs/.gitignore
index 98527864664d..51030beca437 100644
--- a/python/ovs/.gitignore
+++ b/python/ovs/.gitignore
@@ -1 +1,2 @@
 version.py
+dir.py
diff --git a/python/ovs/dirs.py b/python/ovs/dirs.py
deleted file mode 100644
index c67aecbb46da..
--- a/python/ovs/dirs.py
+++ /dev/null
@@ -1,31 +0,0 @@
-# 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.
-
-# The @variables@ in this file are replaced by default directories for
-# use in python/ovs/dirs.py in the source directory and replaced by the
-# configured directories for use in the installed python/ovs/dirs.py.
-#
-import os
-
-# Note that the use of """ is to aid in dealing with paths with quotes in them.
-PKGDATADIR = os.environ.get("OVS_PKGDATADIR", 
"""/usr/local/share/openvswitch""")
-RUNDIR = os.environ.get("OVS_RUNDIR", """/var/run""")
-LOGDIR = os.environ.get("OVS_LOGDIR", """/usr/local/var/log""")
-BINDIR = os.environ.get("OVS_BINDIR", """/usr/local/bin""")
-
-DBDIR = os.environ.get("OVS_DBDIR")
-if not DBDIR:
-sysconfdir = os.environ.get("OVS_SYSCONFDIR")
-if sysconfdir:
-DBDIR = "%s/openvswitch" % sysconfdir
-else:
-DBDIR = """/usr/local/etc/openvswitch"""
-- 
2.26.2

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


[ovs-dev] [PATCH v3 1/2] Documentation: update IPsec tutorial for F32

2020-10-27 Thread Mark Gray
F32 requires the "python3-openvswitch" package now. Also, the
iptables chain "IN_FedoraServer_allow" does not exist on Fedora 32.

Signed-off-by: Mark Gray 
---
 Documentation/tutorials/ipsec.rst | 108 +++---
 1 file changed, 55 insertions(+), 53 deletions(-)

diff --git a/Documentation/tutorials/ipsec.rst 
b/Documentation/tutorials/ipsec.rst
index b4c3235132bc..ebc0ae429c19 100644
--- a/Documentation/tutorials/ipsec.rst
+++ b/Documentation/tutorials/ipsec.rst
@@ -42,7 +42,7 @@ Installing OVS and IPsec Packages
 -
 
 OVS IPsec has .deb and .rpm packages. You should use the right package
-based on your Linux distribution. This tutorial uses Ubuntu 16.04 and Fedora 27
+based on your Linux distribution. This tutorial uses Ubuntu 16.04 and Fedora 32
 as examples.
 
 Ubuntu
@@ -59,8 +59,8 @@ Ubuntu
 
 2. Install the related packages::
 
-   $ apt-get install dkms strongswan
-   $ dpkg -i libopenvswitch_*.deb openvswitch-common_*.deb \
+   # apt-get install dkms strongswan
+   # dpkg -i libopenvswitch_*.deb openvswitch-common_*.deb \
  openvswitch-switch_*.deb openvswitch-datapath-dkms_*.deb \
  python-openvswitch_*.deb openvswitch-pki_*.deb \
  openvswitch-ipsec_*.deb
@@ -71,23 +71,25 @@ Ubuntu
 Fedora
 ~~
 
-1. Follow :doc:`/intro/install/fedora` to build RPM packages.
+1. Install the related packages. Fedora 32 does not require installation of
+   the out-of-tree kernel module::
 
-2. Install the related packages::
+   # dnf install python3-openvswitch libreswan \
+ openvswitch openvswitch-ipsec
+
+2. Install firewall rules to allow ESP and IKE traffic::
 
-   $ dnf install python2-openvswitch libreswan \
- "kernel-devel-uname-r == $(uname -r)"
-   $ rpm -i openvswitch-*.rpm openvswitch-kmod-*.rpm \
-openvswitch-openvswitch-ipsec-*.rpm
+   # systemctl start firewalld
+   # firewall-cmd --add-service ipsec
 
-3. Install firewall rules to allow ESP and IKE traffic::
+   Or to make permanent::
 
-   $ iptables -A IN_FedoraServer_allow -p esp -j ACCEPT
-   $ iptables -A IN_FedoraServer_allow -p udp --dport 500 -j ACCEPT
+   # systemctl enable firewalld
+   # firewall-cmd --permanent --add-service ipsec
 
-4. Run the openvswitch-ipsec service::
+3. Run the openvswitch-ipsec service::
 
-   $ systemctl start openvswitch-ipsec.service
+   # systemctl start openvswitch-ipsec.service
 
.. note::
 
@@ -97,47 +99,47 @@ Fedora
 Configuring IPsec tunnel
 
 
-Suppose you want to build IPsec tunnel between two hosts. Assume `host_1`'s
+Suppose you want to build an IPsec tunnel between two hosts. Assume `host_1`'s
 external IP is 1.1.1.1, and `host_2`'s external IP is 2.2.2.2. Make sure
 `host_1` and `host_2` can ping each other via these external IPs.
 
 0. Set up some variables to make life easier.  On both hosts, set ``ip_1`` and
``ip_2`` variables, e.g.::
 
- $ ip_1=1.1.1.1
- $ ip_2=2.2.2.2
+ # ip_1=1.1.1.1
+ # ip_2=2.2.2.2
 
 1. Set up OVS bridges in both hosts.
 
In `host_1`::
 
-   $ ovs-vsctl add-br br-ipsec
-   $ ip addr add 192.0.0.1/24 dev br-ipsec
-   $ ip link set br-ipsec up
+   # ovs-vsctl add-br br-ipsec
+   # ip addr add 192.0.0.1/24 dev br-ipsec
+   # ip link set br-ipsec up
 
In `host_2`::
 
-   $ ovs-vsctl add-br br-ipsec
-   $ ip addr add 192.0.0.2/24 dev br-ipsec
-   $ ip link set br-ipsec up
+   # ovs-vsctl add-br br-ipsec
+   # ip addr add 192.0.0.2/24 dev br-ipsec
+   # ip link set br-ipsec up
 
 2. Set up IPsec tunnel.
 
-   There are three authentication methods. You can choose one to set up your
-   IPsec tunnel.
+   There are three authentication methods.  Choose one method to set up your
+   IPsec tunnel and follow the steps below.
 
a) Using pre-shared key:
 
   In `host_1`::
 
-  $ ovs-vsctl add-port br-ipsec tun -- \
+  # ovs-vsctl add-port br-ipsec tun -- \
   set interface tun type=gre \
 options:remote_ip=$ip_2 \
 options:psk=swordfish
 
   In `host_2`::
 
-  $ ovs-vsctl add-port br-ipsec tun -- \
+  # ovs-vsctl add-port br-ipsec tun -- \
   set interface tun type=gre \
 options:remote_ip=$ip_1 \
 options:psk=swordfish
@@ -156,15 +158,15 @@ external IP is 1.1.1.1, and `host_2`'s external IP is 
2.2.2.2. Make sure
 
   In `host_1`::
 
-  $ ovs-pki req -u host_1
-  $ ovs-pki self-sign host_1
-  $ scp host_1-cert.pem $ip_2:/etc/keys/host_1-cert.pem
+  # ovs-pki req -u host_1
+  # ovs-pki self-sign host_1
+  # scp host_1-cert.pem $ip_2:/etc/keys/host_1-cert.pem
 
   In `host_2`::
 
-  $ ovs-pki re

Re: [ovs-dev] [PATCH v3 ovn] dhcp: add iPXE support to OVN

2020-10-27 Thread Numan Siddique
On Tue, Oct 27, 2020 at 7:40 PM Numan Siddique  wrote:
>
> On Fri, Oct 23, 2020 at 4:04 PM Lorenzo Bianconi
>  wrote:
> >
> > Add iPXE support to OVN introducing "bootfile_name_alt" dhcp option.
> > "bootfile_name_alt" dhcp userdata is encoded as option 254 since
> > it is not currently used.
> > When both "bootfile_name" and "bootfile_name_alt" are provided
> > by the CMS, "bootfile_name" will be used for option 67 if the
> > dhcp request contains etherboot option (175), otherwise
> > "bootfile_name_alt" will be used.
> > "bootfile_name" and "bootfile_name_alt" are placed just after offer_ip
> > in userdata buffer.
> >
> > Tested-by: Lucas Alvares Gomes Martins 
> > Signed-off-by: Lorenzo Bianconi 
>
> Thanks Lorenzo.
>
> I did the below changes and applied to master.
>
> **
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 6f5499f0b..f15afc54f 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -2024,16 +2024,16 @@ pinctrl_handle_put_dhcp_opts(
>   */
>  struct dhcp_opt_header *in_dhcp_opt =
>  (struct dhcp_opt_header *)reply_dhcp_opts_ptr->data;
> -if (in_dhcp_opt->code == 67) { /* DHCP_OPT_BOOTFILENAME */
> +if (in_dhcp_opt->code == DHCP_OPT_BOOTFILE_CODE) {
>  unsigned char *ptr = (unsigned char *)in_dhcp_opt;
>  int len = sizeof *in_dhcp_opt + in_dhcp_opt->len;
>  struct dhcp_opt_header *next_dhcp_opt =
>  (struct dhcp_opt_header *)(ptr + len);
>
> -if (next_dhcp_opt->code == 254) { /* DHCP_OPT_BOOTFILE_ALT */
> +if (next_dhcp_opt->code == DHCP_OPT_BOOTFILE_ALT_CODE) {
>  if (!ipxe_req) {
>  ofpbuf_pull(reply_dhcp_opts_ptr, len);
> -next_dhcp_opt->code = 67;
> +next_dhcp_opt->code = DHCP_OPT_BOOTFILE_CODE;
>  } else {
>  char *buf = xmalloc(len);
>
> @@ -2044,8 +2044,8 @@ pinctrl_handle_put_dhcp_opts(
>  free(buf);
>  }
>  }
> -} else if (in_dhcp_opt->code == 254) { /* DHCP_OPT_BOOTFILE_ALT */
> -in_dhcp_opt->code = 67;
> +} else if (in_dhcp_opt->code == DHCP_OPT_BOOTFILE_ALT_CODE) {
> +in_dhcp_opt->code = DHCP_OPT_BOOTFILE_CODE;
>  }
>
>  uint16_t new_l4_size = UDP_HEADER_LEN + DHCP_HEADER_LEN + 16;
> diff --git a/lib/actions.c b/lib/actions.c
> index b0d03bc96..23e54ef2a 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -2542,9 +2542,9 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts 
> *pdo,
>  ovs_be32 offerip = offerip_opt->value.values[0].value.ipv4;
>  ofpbuf_put(ofpacts, &offerip, sizeof offerip);
>
> -/* Encode and bootfile_name opt (67) */
> -const struct ovnact_gen_option *boot_opt = find_opt(
> -pdo->options, pdo->n_options, 67);
> +/* Encode bootfile_name opt (67) */
> +const struct ovnact_gen_option *boot_opt =
> +find_opt(pdo->options, pdo->n_options, DHCP_OPT_BOOTFILE_CODE);
>  if (boot_opt) {
>  uint8_t *opt_header = ofpbuf_put_zeros(ofpacts, 2);
>  const union expr_constant *c = boot_opt->value.values;
> @@ -2553,8 +2553,8 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts 
> *pdo,
>  ofpbuf_put(ofpacts, c->string, opt_header[1]);
>  }
>  /* Encode bootfile_name_alt opt (254) */
> -const struct ovnact_gen_option *boot_alt_opt = find_opt(
> -pdo->options, pdo->n_options, 254);
> +const struct ovnact_gen_option *boot_alt_opt =
> +find_opt(pdo->options, pdo->n_options, DHCP_OPT_BOOTFILE_ALT_CODE);
>  if (boot_alt_opt) {
>  uint8_t *opt_header = ofpbuf_put_zeros(ofpacts, 2);
>  const union expr_constant *c = boot_alt_opt->value.values;
> @@ -2565,7 +2565,7 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts 
> *pdo,
>
>  for (const struct ovnact_gen_option *o = pdo->options;
>   o < &pdo->options[pdo->n_options]; o++) {
> -if (o != offerip_opt && o != boot_alt_opt && o != boot_opt) {
> +if (o != offerip_opt && o != boot_opt && o != boot_alt_opt) {
>  encode_put_dhcpv4_option(o, ofpacts);
>  }
>  }
> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> index fb0088582..c3e8fd660 100644
> --- a/lib/ovn-l7.h
> +++ b/lib/ovn-l7.h
> @@ -93,10 +93,14 @@ struct gen_opts_map {
>  #define DHCP_OPT_DOMAIN_SEARCH_LIST \
>  DHCP_OPTION("domain_search_list", 119, "domains")
>
> -/* let's use unused 254 option for iPXE bootfile_name_alt
> - * userdata dhcp option
> +#define DHCP_OPT_BOOTFILE_CODE 67
> +
> +/* Use unused 254 option for iPXE bootfile_name_alt userdata DHCP option.
> + * This option code is replaced by 67 when sending the DHCP reply.
>   */
> -#define DHCP_OPT_BOOTFILE_ALT DHCP_OPTION("bootfile_name_alt", 254, "str")
> +#define DHCP_OPT_BOOTFILE_ALT_CODE 254
> +#define DHCP_OPT_BOOTFILE_ALT DHCP_OPTION("bootfile_name_alt", \
> +  DHCP_OPT_BOOTFILE_ALT_CODE, "str")
>
>  #define DHCP_OPT_

Re: [ovs-dev] [PATCH v3 ovn] dhcp: add iPXE support to OVN

2020-10-27 Thread Numan Siddique
On Fri, Oct 23, 2020 at 4:04 PM Lorenzo Bianconi
 wrote:
>
> Add iPXE support to OVN introducing "bootfile_name_alt" dhcp option.
> "bootfile_name_alt" dhcp userdata is encoded as option 254 since
> it is not currently used.
> When both "bootfile_name" and "bootfile_name_alt" are provided
> by the CMS, "bootfile_name" will be used for option 67 if the
> dhcp request contains etherboot option (175), otherwise
> "bootfile_name_alt" will be used.
> "bootfile_name" and "bootfile_name_alt" are placed just after offer_ip
> in userdata buffer.
>
> Tested-by: Lucas Alvares Gomes Martins 
> Signed-off-by: Lorenzo Bianconi 

Thanks Lorenzo.

I did the below changes and applied to master.

**

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 6f5499f0b..f15afc54f 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2024,16 +2024,16 @@ pinctrl_handle_put_dhcp_opts(
  */
 struct dhcp_opt_header *in_dhcp_opt =
 (struct dhcp_opt_header *)reply_dhcp_opts_ptr->data;
-if (in_dhcp_opt->code == 67) { /* DHCP_OPT_BOOTFILENAME */
+if (in_dhcp_opt->code == DHCP_OPT_BOOTFILE_CODE) {
 unsigned char *ptr = (unsigned char *)in_dhcp_opt;
 int len = sizeof *in_dhcp_opt + in_dhcp_opt->len;
 struct dhcp_opt_header *next_dhcp_opt =
 (struct dhcp_opt_header *)(ptr + len);

-if (next_dhcp_opt->code == 254) { /* DHCP_OPT_BOOTFILE_ALT */
+if (next_dhcp_opt->code == DHCP_OPT_BOOTFILE_ALT_CODE) {
 if (!ipxe_req) {
 ofpbuf_pull(reply_dhcp_opts_ptr, len);
-next_dhcp_opt->code = 67;
+next_dhcp_opt->code = DHCP_OPT_BOOTFILE_CODE;
 } else {
 char *buf = xmalloc(len);

@@ -2044,8 +2044,8 @@ pinctrl_handle_put_dhcp_opts(
 free(buf);
 }
 }
-} else if (in_dhcp_opt->code == 254) { /* DHCP_OPT_BOOTFILE_ALT */
-in_dhcp_opt->code = 67;
+} else if (in_dhcp_opt->code == DHCP_OPT_BOOTFILE_ALT_CODE) {
+in_dhcp_opt->code = DHCP_OPT_BOOTFILE_CODE;
 }

 uint16_t new_l4_size = UDP_HEADER_LEN + DHCP_HEADER_LEN + 16;
diff --git a/lib/actions.c b/lib/actions.c
index b0d03bc96..23e54ef2a 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -2542,9 +2542,9 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts *pdo,
 ovs_be32 offerip = offerip_opt->value.values[0].value.ipv4;
 ofpbuf_put(ofpacts, &offerip, sizeof offerip);

-/* Encode and bootfile_name opt (67) */
-const struct ovnact_gen_option *boot_opt = find_opt(
-pdo->options, pdo->n_options, 67);
+/* Encode bootfile_name opt (67) */
+const struct ovnact_gen_option *boot_opt =
+find_opt(pdo->options, pdo->n_options, DHCP_OPT_BOOTFILE_CODE);
 if (boot_opt) {
 uint8_t *opt_header = ofpbuf_put_zeros(ofpacts, 2);
 const union expr_constant *c = boot_opt->value.values;
@@ -2553,8 +2553,8 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts *pdo,
 ofpbuf_put(ofpacts, c->string, opt_header[1]);
 }
 /* Encode bootfile_name_alt opt (254) */
-const struct ovnact_gen_option *boot_alt_opt = find_opt(
-pdo->options, pdo->n_options, 254);
+const struct ovnact_gen_option *boot_alt_opt =
+find_opt(pdo->options, pdo->n_options, DHCP_OPT_BOOTFILE_ALT_CODE);
 if (boot_alt_opt) {
 uint8_t *opt_header = ofpbuf_put_zeros(ofpacts, 2);
 const union expr_constant *c = boot_alt_opt->value.values;
@@ -2565,7 +2565,7 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts *pdo,

 for (const struct ovnact_gen_option *o = pdo->options;
  o < &pdo->options[pdo->n_options]; o++) {
-if (o != offerip_opt && o != boot_alt_opt && o != boot_opt) {
+if (o != offerip_opt && o != boot_opt && o != boot_alt_opt) {
 encode_put_dhcpv4_option(o, ofpacts);
 }
 }
diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
index fb0088582..c3e8fd660 100644
--- a/lib/ovn-l7.h
+++ b/lib/ovn-l7.h
@@ -93,10 +93,14 @@ struct gen_opts_map {
 #define DHCP_OPT_DOMAIN_SEARCH_LIST \
 DHCP_OPTION("domain_search_list", 119, "domains")

-/* let's use unused 254 option for iPXE bootfile_name_alt
- * userdata dhcp option
+#define DHCP_OPT_BOOTFILE_CODE 67
+
+/* Use unused 254 option for iPXE bootfile_name_alt userdata DHCP option.
+ * This option code is replaced by 67 when sending the DHCP reply.
  */
-#define DHCP_OPT_BOOTFILE_ALT DHCP_OPTION("bootfile_name_alt", 254, "str")
+#define DHCP_OPT_BOOTFILE_ALT_CODE 254
+#define DHCP_OPT_BOOTFILE_ALT DHCP_OPTION("bootfile_name_alt", \
+  DHCP_OPT_BOOTFILE_ALT_CODE, "str")

 #define DHCP_OPT_ETHERBOOT 175


*

> ---
> Changes since v2:
> - encode "bootfile_name" and "bootfile_name_alt" are placed just after
>   offer_ip in userdata buffer in order to avoid loops in
>   pinctrl_handle_put_dhcp_opts()
> Changes since v1:
> - encode bootfile_name_alt option as 25

Re: [ovs-dev] [PATCH v3 2/2] netdev-dpdk: Add option to configure VF MAC address.

2020-10-27 Thread Ilya Maximets
On 10/20/20 1:15 PM, Kevin Traynor wrote:
> On 16/09/2020 16:17, Gaetan Rivet wrote:
>> In some cloud topologies, using DPDK VF representors in guest requires
>> configuring a VF before it is assigned to the guest.
>>
>> A first basic option for such configuration is setting the VF MAC
>> address. Add a key 'dpdk-vf-mac' to the 'options' column of the Interface
>> table.
>>
>> This option can be used as such:
>>
>>$ ovs-vsctl add-port br0 dpdk-rep0 -- set Interface dpdk-rep0 type=dpdk \
>>   options:dpdk-vf-mac=00:11:22:33:44:55
>>
> 
> If I got it right, it doesn't seem like it solves any issues by limiting
> to representor, but is just an attempt to limit some of the edge cases
> around bifurcated devices to the devices where the requested
> functionality is really needed now.
> 
> This limitation has some costs...
> 
> We will now have, mac, mac_in_use, dpdk-vf-mac, configured_hwaddr,
> requested_hwaddr for the user to understand. I think it can be confusing.

To be clear, 'configured_hwaddr/requested_hwaddr' type of report should not
exist at least in the output of 'get_config()' function.  Same applies
to all existing '{configured,requested}_something' reports.
'get_config()' intended to report things that could be copied and passed 
directly
to database.  OTOH, 'get_status()' should report what is an actual device
status, i.e. 'dpdk-vf-mac' should be in 'get_config()' and 'configured_hwaddr'
should be in 'get_status()', but it should be named differently, e.g. just
'dpdk-vf-mac' and the callback it placed in already says that it's actually
'configured'.  In short:
  - 'dpdk-vf-mac' from the 'get_config()' should report dev->requested_hwaddr.
  - 'dpdk-vf-mac' from the 'get_status()' should report dev->hwaddr.
Of course, all this should be reported only if device is a representor.


> 
> 'ovs-vsctl show' will show the dpdk-vf-mac, which is the main one we
> document, but it's not clear if it was successful or not. (yes there is
> a log entry, but it is a separate place).
> 
> As it is specific for representors, if we ever want to allow setting of
> mac on any non representor DPDK ports, we have an awkward user
> interface. We would need to make another 'dpdk-mac' type option, or we
> decide then to allow use mac in interface table but that it doesn't work
> for representors, or there are two ways to set for representors etc.
> None of this seems great.
> 
> My feeling is that it is making things complicated for the user with
> this many knobs, where we could just have mac and mac_in_use, live with
> the edge cases (as Gaetan pointed out, we already do for MTU) and we
> know as well if we ever need to extend further, the user interface will
> stay simple. Just my 2C, maybe there's a good argument why we can't do
> it like this.

Setting mac for physical device has lots of split-brain issues and unclear
ways how and when undo changes as we do not know what was original mac or
what to do if there was no mac at all at the start.  Moreover, some devices
will not allow setting mac to it's original value (e.g. all zeroes).
One more point is that vf and representor itself are different devices and
these devices could have different mac addresses.  For now we agreed that
DPDK api works for representors in a way that it always changes parameters
of a virtual function, not representors', but that is not a very clear thing
for all users (e.g. for DPDK developers), so 'vf' in 'dpdk-vf-mac' specifically
points to that difference.

After a long discussion we decided to make this option as specific as possible,
so users will not try to use it to configure anything else beside mac of a
virtual function.  All restrictions and downsides of configuration of vf mac
was/should be documented.


> 
> Few comments on the code as it is now below.
> 
> 
>> Signed-off-by: Gaetan Rivet 
>> Suggested-by: Ilya Maximets 
>> Acked-by: Eli Britstein 
>> ---
>>  Documentation/topics/dpdk/phy.rst | 22 
>>  NEWS  |  2 ++
>>  lib/netdev-dpdk.c | 60 +++
>>  vswitchd/vswitch.xml  | 13 +++
>>  4 files changed, 97 insertions(+)
>>
>> diff --git a/Documentation/topics/dpdk/phy.rst 
>> b/Documentation/topics/dpdk/phy.rst
>> index 38e52c8de..73dcb7c28 100644
>> --- a/Documentation/topics/dpdk/phy.rst
>> +++ b/Documentation/topics/dpdk/phy.rst
>> @@ -379,6 +379,28 @@ an eth device whose mac address is 
>> ``00:11:22:33:44:55``::
>>  $ ovs-vsctl add-port br0 dpdk-mac -- set Interface dpdk-mac type=dpdk \
>> options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
>>  
>> +Representor specific configuration
>> +~~
>> +
>> +In some topologies, a VF must be configured before being assigned to a
>> +guest (VM) machine.  This configuration is done through VF-specific fields
>> +in the ``options`` column of the Interface table.
>> +
>> +.. important::
>> +
>> +   Some DPDK port use `bifurcated drivers `__,

[ovs-dev] WiFi PTZ Camera

2020-10-27 Thread Lisa-CZTECH
Dear Customers,
Good day.hope everything is good for you.
I know all hard in this year,let's pray for all.

I know the camera industry is very competitive.do you want something new in 
market?
Yes we have the smart battery camera,the unique function is AI human detection 
and full color night version.it is very popular for outdoor security using.

For more informations,pls contact sa...@szcztech.com or mob:008618898723245
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH V3 3/4] Add VXLAN TCP and UDP GRO support for DPDK data path

2020-10-27 Thread Ilya Maximets
On 8/7/20 12:56 PM, yang_y...@163.com wrote:
> From: Yi Yang 
> 
> GRO(Generic Receive Offload) can help improve performance
> when TSO (TCP Segment Offload) or VXLAN TSO is enabled
> on transmit side, this can avoid overhead of ovs DPDK
> data path and enqueue vhost for VM by merging many small
> packets to large packets (65535 bytes at most) once it
> receives packets from physical NIC.

IIUC, this patch allows multi-segment mbufs to float across different
parts of OVS.  This will definitely crash it somewhere.  Much more changes
all over the OVS required to make it safely work with such mbufs.  There
were few attempts to introduce this support, but all of them ended up being
rejected.  As it is this patch is not acceptable as it doesn't cover almost
anything beside simple cases inside netdev implementation.

Here is the latest attempt with multi-segment mbufs:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=130193&state=*

Best regards, Ilya Maximets.

> 
> It can work for both VXLAN and vlan case.
> 
> Signed-off-by: Yi Yang 
> ---
>  lib/dp-packet.h|  37 -
>  lib/netdev-dpdk.c  | 227 
> -
>  lib/netdev-linux.c | 112 --
>  3 files changed, 365 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index c33868d..18307c0 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -580,7 +580,16 @@ dp_packet_set_size(struct dp_packet *b, uint32_t v)
>   * (and thus 'v') will always be <= UINT16_MAX; this means that there is 
> no
>   * loss of accuracy in assigning 'v' to 'data_len'.
>   */
> -b->mbuf.data_len = (uint16_t)v;  /* Current seg length. */
> +if (b->mbuf.nb_segs <= 1) {
> +b->mbuf.data_len = (uint16_t)v;  /* Current seg length. */
> +} else {
> +/* For multi-seg packet, if it is resize, data_len should be
> + * adjusted by offset, this will happend in case of push or pop.
> + */
> +if (b->mbuf.pkt_len != 0) {
> +b->mbuf.data_len += v - b->mbuf.pkt_len;
> +}
> +}
>  b->mbuf.pkt_len = v; /* Total length of all segments linked 
> to
>* this segment. */
>  }
> @@ -1092,6 +1101,20 @@ dp_packet_hwol_set_l4_len(struct dp_packet *b, int 
> l4_len)
>  {
>  b->mbuf.l4_len = l4_len;
>  }
> +
> +/* Set outer_l2_len for the packet 'b' */
> +static inline void
> +dp_packet_hwol_set_outer_l2_len(struct dp_packet *b, int outer_l2_len)
> +{
> +b->mbuf.outer_l2_len = outer_l2_len;
> +}
> +
> +/* Set outer_l3_len for the packet 'b' */
> +static inline void
> +dp_packet_hwol_set_outer_l3_len(struct dp_packet *b, int outer_l3_len)
> +{
> +b->mbuf.outer_l3_len = outer_l3_len;
> +}
>  #else
>  /* Mark packet 'b' for VXLAN TCP segmentation offloading. */
>  static inline void
> @@ -1125,6 +1148,18 @@ dp_packet_hwol_set_l4_len(struct dp_packet *b 
> OVS_UNUSED,
>int l4_len OVS_UNUSED)
>  {
>  }
> +
> +/* Set outer_l2_len for the packet 'b' */
> +static inline void
> +dp_packet_hwol_set_outer_l2_len(struct dp_packet *b, int outer_l2_len)
> +{
> +}
> +
> +/* Set outer_l3_len for the packet 'b' */
> +static inline void
> +dp_packet_hwol_set_outer_l3_len(struct dp_packet *b, int outer_l3_len)
> +{
> +}
>  #endif /* DPDK_NETDEV */
>  
>  static inline bool
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 888a45e..b6c57a6 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /* Include rte_compat.h first to allow experimental API's needed for the
>   * rte_meter.h rfc4115 functions. Once they are no longer marked as
> @@ -47,6 +48,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "cmap.h"
>  #include "coverage.h"
> @@ -2184,6 +2186,8 @@ get_udptcp_checksum(void *l3_hdr, void *l4_hdr, 
> uint16_t ethertype)
>  }
>  }
>  
> +#define UDP_VXLAN_ETH_HDR_SIZE 30
> +
>  /* Prepare the packet for HWOL.
>   * Return True if the packet is OK to continue. */
>  static bool
> @@ -2207,6 +2211,42 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
> struct rte_mbuf *mbuf)
>  return true;
>  }
>  
> +/* ol_flags is cleaned after vxlan pop, so need reset for those packets.
> + * Such packets are only for local VMs or namespaces, so need to return
> + * after ol_flags, l2_len, l3_len and tso_segsz are set.
> + */
> +if (((mbuf->ol_flags & PKT_TX_TUNNEL_VXLAN) == 0) &&
> +(mbuf->l2_len == UDP_VXLAN_ETH_HDR_SIZE) &&
> +(mbuf->pkt_len > 1464)) {
> +mbuf->ol_flags = 0;
> +mbuf->l2_len -= sizeof(struct udp_header)
> ++ sizeof(struct vxlanhdr);
> +if (mbuf->l3_len == IP_HEADER_LEN) {
> +mbuf->ol_flags |= PKT_TX_IPV4;
> +ip_hdr = (struct rte_ipv4_hdr *)(eth_hdr + 1);
> +l4_pr

Re: [ovs-dev] [PATCH] netdev-dpdk: fix incorrect shinfo initialization

2020-10-27 Thread Flavio Leitner
On Tue, Oct 27, 2020 at 01:47:22PM +0100, Ilya Maximets wrote:
> On 10/27/20 12:34 PM, Flavio Leitner wrote:
> > On Wed, Oct 14, 2020 at 03:22:48PM +0800, yang_y...@163.com wrote:
> >> From: Yi Yang 
> >>
> >> shinfo is used to store reference counter and free callback
> >> of an external buffer, but it is stored in mbuf if the mbuf
> >> has tailroom for it.
> >>
> >> This is wrong because the mbuf (and its data) can be freed
> >> before the external buffer, for example:
> >>
> >>   pkt2 = rte_pktmbuf_alloc(mp);
> >>   rte_pktmbuf_attach(pkt2, pkt);
> >>   rte_pktmbuf_free(pkt);
> 
> How is that possible with OVS?  Right now OVS doesn't support multi-segement
> mbufs and will, likely, not support them in a near future because it requires
> changes all other the codebase.
> 
> Is there any other scenario that could lead to issues with current OVS
> implementation?

This is copying packets. The shinfo is allocated in the mbuf of
the first packet which could be deleted without any references
to the external buffer still using it.

fbl


> 
> >>
> >> After this, pkt is freed, but it still contains shinfo, which
> >> is referenced by pkt2.
> >>
> >> Fix this by always storing shinfo at the tail of external buffer.
> >>
> >> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
> >> Co-authored-by: Olivier Matz 
> >> Signed-off-by: Olivier Matz 
> >> Signed-off-by: Yi Yang 
> >> ---
> > 
> > Acked-by: Flavio Leitner 
> > 
> > Thanks Yi,
> > fbl
> > 
> 

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


Re: [ovs-dev] [PATCH V3 2/4] Add GSO support for DPDK data path

2020-10-27 Thread Ilya Maximets
On 8/7/20 12:56 PM, yang_y...@163.com wrote:
> From: Yi Yang 
> 
> GSO(Generic Segment Offload) can segment large UDP
>  and TCP packet to small packets per MTU of destination
> , especially for the case that physical NIC can't
> do hardware offload VXLAN TSO and VXLAN UFO, GSO can
> make sure userspace TSO can still work but not drop.
> 
> In addition, GSO can help improve UDP performane when
> UFO is enabled in VM.
> 
> GSO can support TCP, UDP, VXLAN TCP, VXLAN UDP, it is
> done in Tx function of physical NIC.
> 
> Signed-off-by: Yi Yang 
> ---
>  lib/dp-packet.h|  21 +++-
>  lib/netdev-dpdk.c  | 358 
> +
>  lib/netdev-linux.c |  17 ++-
>  lib/netdev.c   |  67 +++---
>  4 files changed, 417 insertions(+), 46 deletions(-)
> 
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 79895f2..c33868d 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -83,6 +83,8 @@ enum dp_packet_offload_mask {
>  DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CKSUM, PKT_TX_SCTP_CKSUM, 0x800),
>  /* VXLAN TCP Segmentation Offload. */
>  DEF_OL_FLAG(DP_PACKET_OL_TX_TUNNEL_VXLAN, PKT_TX_TUNNEL_VXLAN, 0x1000),
> +/* UDP Segmentation Offload. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_UDP_SEG, PKT_TX_UDP_SEG, 0x2000),
>  /* Adding new field requires adding to DP_PACKET_OL_SUPPORTED_MASK. */
>  };
>  
> @@ -97,7 +99,8 @@ enum dp_packet_offload_mask {
>   DP_PACKET_OL_TX_IPV6  | \
>   DP_PACKET_OL_TX_TCP_CKSUM | \
>   DP_PACKET_OL_TX_UDP_CKSUM | \
> - DP_PACKET_OL_TX_SCTP_CKSUM)
> + DP_PACKET_OL_TX_SCTP_CKSUM| \
> + DP_PACKET_OL_TX_UDP_SEG)
>  
>  #define DP_PACKET_OL_TX_L4_MASK (DP_PACKET_OL_TX_TCP_CKSUM | \
>   DP_PACKET_OL_TX_UDP_CKSUM | \
> @@ -956,6 +959,13 @@ dp_packet_hwol_is_tso(const struct dp_packet *b)
>  return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_TCP_SEG);
>  }
>  
> +/* Returns 'true' if packet 'b' is marked for UDP segmentation offloading. */
> +static inline bool
> +dp_packet_hwol_is_uso(const struct dp_packet *b)
> +{
> +return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_UDP_SEG);
> +}
> +
>  /* Returns 'true' if packet 'b' is marked for IPv4 checksum offloading. */
>  static inline bool
>  dp_packet_hwol_is_ipv4(const struct dp_packet *b)
> @@ -1034,6 +1044,15 @@ dp_packet_hwol_set_tcp_seg(struct dp_packet *b)
>  *dp_packet_ol_flags_ptr(b) |= DP_PACKET_OL_TX_TCP_SEG;
>  }
>  
> +/* Mark packet 'b' for UDP segmentation offloading.  It implies that
> + * either the packet 'b' is marked for IPv4 or IPv6 checksum offloading
> + * and also for UDP checksum offloading. */
> +static inline void
> +dp_packet_hwol_set_udp_seg(struct dp_packet *b)
> +{
> +*dp_packet_ol_flags_ptr(b) |= DP_PACKET_OL_TX_UDP_SEG;
> +}
> +
>  #ifdef DPDK_NETDEV
>  /* Mark packet 'b' for VXLAN TCP segmentation offloading. */
>  static inline void
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 30493ed..888a45e 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -38,13 +38,15 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  
>  #include "cmap.h"
>  #include "coverage.h"
> @@ -162,6 +164,7 @@ typedef uint16_t dpdk_port_t;
> | DEV_TX_OFFLOAD_UDP_CKSUM\
> | DEV_TX_OFFLOAD_IPV4_CKSUM)
>  
> +#define MAX_GSO_MBUFS 64
>  
>  static const struct rte_eth_conf port_conf = {
>  .rxmode = {
> @@ -2171,6 +2174,16 @@ is_local_to_local(uint16_t src_port_id, struct 
> netdev_dpdk *dev)
>  return ret;
>  }
>  
> +static uint16_t
> +get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
> +{
> +if (ethertype == htons(RTE_ETHER_TYPE_IPV4)) {
> +return rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> +} else { /* assume ethertype == RTE_ETHER_TYPE_IPV6 */
> +return rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> +}
> +}
> +
>  /* Prepare the packet for HWOL.
>   * Return True if the packet is OK to continue. */
>  static bool
> @@ -2203,10 +2216,9 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
> struct rte_mbuf *mbuf)
>   * also can't be handled. So PKT_TX_TUNNEL_VXLAN must be cleared
>   * outer_l2_len and outer_l3_len must be zeroed.
>   */
> -if (!(dev->up.ol_flags & NETDEV_TX_OFFLOAD_VXLAN_TSO)
> -|| (!(mbuf->ol_flags & (PKT_TX_TCP_SEG | PKT_TX_UDP_SEG))
> +if (!(mbuf->ol_flags & (PKT_TX_TCP_SEG | PKT_TX_UDP_SEG))
>  && (mbuf->pkt_len <= 1450 + mbuf->outer_l2_len + 
> mbuf->outer_l3_len
> -+ mbuf->l2_len)))  {
> +

Re: [ovs-dev] [PATCH 3/6] lldp: fix a buffer overflow when handling management address TLV

2020-10-27 Thread Aaron Conole
Hi Fabrizio,

Thanks for the work on this.

Fabrizio D'Angelo  writes:

> From: Vincent Bernat 
>
> Upstream commit:
>   commit a8d8006c06d9ac16ebcf33295cbd625c0847ca9b
>   Author: Vincent Bernat 
>   Date: Sun, 4 Oct 2015 01:50:38 +0200
>
>   lldp: fix a buffer overflow when handling management address TLV
>
>   When a remote device was advertising a too large management address
>   while still respecting TLV boundaries, lldpd would crash due to a buffer
>   overflow. However, the buffer being a static one, this buffer overflow
>   is not exploitable if hardening was not disabled. This bug exists since
>   version 0.5.6.
>
> Co-authored-by: Fabrizio D'Angelo 
> Signed-off-by: Fabrizio D'Angelo 
> ---
>  lib/lldp/lldp.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
> index 593c5e1c34..aaa8ec842a 100644
> --- a/lib/lldp/lldp.c
> +++ b/lib/lldp/lldp.c
> @@ -530,6 +530,11 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, 
> int s,
>  case LLDP_TLV_MGMT_ADDR:
>  CHECK_TLV_SIZE(1, "Management address");
>  addr_str_length = PEEK_UINT8;
> +if (addr_str_length > sizeof(addr_str_buffer)) {
> +VLOG_WARN(name "too large management address on %s",
> +  hardware->h_ifname);

Two things about this change.

1. remove 'name', which seems to have been leftover from converting.
   This causes a build error.

2. Align the hardware->h_ifname argument so that it will look like:

+VLOG_WARN("too large management address on %s",
+  hardware->h_ifname);


> +goto malformed;
> +}
>  CHECK_TLV_SIZE(1 + addr_str_length, "Management address");
>  PEEK_BYTES(addr_str_buffer, addr_str_length);
>  addr_length = addr_str_length - 1;
> @@ -554,7 +559,7 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, 
> int s,
>  break;
>  
>  case LLDP_TLV_ORG:
> -CHECK_TLV_SIZE(4, "Organisational");
> +CHECK_TLV_SIZE(1 + (int)sizeof(orgid), "Organisational");
>  PEEK_BYTES(orgid, sizeof orgid);
>  tlv_subtype = PEEK_UINT8;
>  if (memcmp(dot1, orgid, sizeof orgid) == 0) {

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


Re: [ovs-dev] [PATCH] netdev-dpdk: fix incorrect shinfo initialization

2020-10-27 Thread Ilya Maximets
On 10/27/20 12:34 PM, Flavio Leitner wrote:
> On Wed, Oct 14, 2020 at 03:22:48PM +0800, yang_y...@163.com wrote:
>> From: Yi Yang 
>>
>> shinfo is used to store reference counter and free callback
>> of an external buffer, but it is stored in mbuf if the mbuf
>> has tailroom for it.
>>
>> This is wrong because the mbuf (and its data) can be freed
>> before the external buffer, for example:
>>
>>   pkt2 = rte_pktmbuf_alloc(mp);
>>   rte_pktmbuf_attach(pkt2, pkt);
>>   rte_pktmbuf_free(pkt);

How is that possible with OVS?  Right now OVS doesn't support multi-segement
mbufs and will, likely, not support them in a near future because it requires
changes all other the codebase.

Is there any other scenario that could lead to issues with current OVS
implementation?

>>
>> After this, pkt is freed, but it still contains shinfo, which
>> is referenced by pkt2.
>>
>> Fix this by always storing shinfo at the tail of external buffer.
>>
>> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
>> Co-authored-by: Olivier Matz 
>> Signed-off-by: Olivier Matz 
>> Signed-off-by: Yi Yang 
>> ---
> 
> Acked-by: Flavio Leitner 
> 
> Thanks Yi,
> fbl
> 

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


Re: [ovs-dev] [PATCH] raft: Avoid annoying debug logs if raft is connected.

2020-10-27 Thread Ilya Maximets
On 10/26/20 2:03 AM, Han Zhou wrote:
> 
> 
> On Sun, Oct 25, 2020 at 5:35 PM Ilya Maximets  > wrote:
>>
>> If debug logs enabled, "raft_is_connected: true" printed on every
>> call to raft_is_connected() which is way too frequently.
>> These messages are not very informative and only litters the log.
>>
>> Let's log only disconnected state in a rate-limited way and only
>> log positive case once at the moment cluster becomes connected.
>>
>> Fixes: 923f01cad678 ("raft.c: Set candidate_retrying if no leader elected 
>> since last election.")
>> Signed-off-by: Ilya Maximets mailto:i.maxim...@ovn.org>>
>> ---
>>  ovsdb/raft.c | 11 ++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/ovsdb/raft.c b/ovsdb/raft.c
>> index 728d60175..657eed813 100644
>> --- a/ovsdb/raft.c
>> +++ b/ovsdb/raft.c
>> @@ -1044,13 +1044,22 @@ raft_get_memory_usage(const struct raft *raft, 
>> struct simap *usage)
>>  bool
>>  raft_is_connected(const struct raft *raft)
>>  {
>> +    static bool last_state = false;
>>      bool ret = (!raft->candidate_retrying
>>              && !raft->joining
>>              && !raft->leaving
>>              && !raft->left
>>              && !raft->failed
>>              && raft->ever_had_leader);
>> -    VLOG_DBG("raft_is_connected: %s\n", ret? "true": "false");
>> +
>> +    if (!ret) {
>> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
>> +        VLOG_DBG_RL(&rl, "raft_is_connected: false");
>> +    } else if (!last_state) {
>> +        VLOG_DBG("raft_is_connected: true");
>> +    }
>> +    last_state = ret;
>> +
>>      return ret;
>>  }
>>
>> --
>> 2.25.4
>>
> Acked-by: Han Zhou mailto:hz...@ovn.org>>
> 

Thanks!

Applied to master and backported down to 2.12.

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


Re: [ovs-dev] [PATCH] raft: Fix error leak on failure while saving snapshot.

2020-10-27 Thread Ilya Maximets
On 10/26/20 2:05 AM, Han Zhou wrote:
> 
> 
> On Fri, Oct 23, 2020 at 11:30 AM Ilya Maximets  > wrote:
>>
>> Error should be destroyed before return.
>>
>> Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered 
>> databases.")
>> Signed-off-by: Ilya Maximets mailto:i.maxim...@ovn.org>>
>> ---
>>  ovsdb/raft.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ovsdb/raft.c b/ovsdb/raft.c
>> index 708b0624c..816f69e22 100644
>> --- a/ovsdb/raft.c
>> +++ b/ovsdb/raft.c
>> @@ -3986,7 +3986,7 @@ raft_handle_install_snapshot_request__(
>>      struct ovsdb_error *error = raft_save_snapshot(raft, new_log_start,
>>                                                     &new_snapshot);
>>      if (error) {
>> -        char *error_s = ovsdb_error_to_string(error);
>> +        char *error_s = ovsdb_error_to_string_free(error);
>>          VLOG_WARN("could not save snapshot: %s", error_s);
>>          free(error_s);
>>          return false;
>> --
>> 2.25.4
>>
> 
> Acked-by: Han Zhou mailto:hz...@ovn.org>>

Thanks!

Applied to master and backported down to 2.9.

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


Re: [ovs-dev] [PATCH] netdev-dpdk: fix incorrect shinfo initialization

2020-10-27 Thread Flavio Leitner
On Wed, Oct 14, 2020 at 03:22:48PM +0800, yang_y...@163.com wrote:
> From: Yi Yang 
> 
> shinfo is used to store reference counter and free callback
> of an external buffer, but it is stored in mbuf if the mbuf
> has tailroom for it.
> 
> This is wrong because the mbuf (and its data) can be freed
> before the external buffer, for example:
> 
>   pkt2 = rte_pktmbuf_alloc(mp);
>   rte_pktmbuf_attach(pkt2, pkt);
>   rte_pktmbuf_free(pkt);
> 
> After this, pkt is freed, but it still contains shinfo, which
> is referenced by pkt2.
> 
> Fix this by always storing shinfo at the tail of external buffer.
> 
> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
> Co-authored-by: Olivier Matz 
> Signed-off-by: Olivier Matz 
> Signed-off-by: Yi Yang 
> ---

Acked-by: Flavio Leitner 

Thanks Yi,
fbl

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


Re: [ovs-dev] [PATCH v2 1/2] Documentation: update IPsec tutorial for F32

2020-10-27 Thread Mark Gray
On 23/10/2020 20:23, Eric Garver wrote:
> On Fri, Oct 23, 2020 at 02:39:21PM -0400, Mark Gray wrote:

>>  
>> -   $ iptables -A IN_FedoraServer_allow -p esp -j ACCEPT
>> -   $ iptables -A IN_FedoraServer_allow -p udp --dport 500 -j ACCEPT
>> +   $ systemctl enable firewalld
>> +   $ firewall-cmd --permanent --add-service ipsec
> 
> nit: These commands require root. So prompt should be '#'.
>
Good catch, I will update

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