Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Friday, May 19, 2017 4:28 PM
To: Ben Pfaff
Cc: ovs dev; Guru Shetty
Subject: Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc


> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Saturday, May 20, 2017 2:25 AM
> To: Alin Serdean 
> Cc: Guru Shetty ; ovs dev 
> Subject: Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc
>
> OK, I backported all the way to branch-2.5, dropping the wmi changes in
> 2.6 and 2.5.
>
[Alin Serdean] Thanks!

Thanks Ben!
___
dev mailing list
d...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=6OuVHk-mnufSWzkKa74UkQ=DSRLafbCyMsR22NckE4P-Vq0bEplB1nkNc4kK_tzC08=Pjzxd7hYiN3_gR6Tymyjf_X9OabkpxwuxD1pNMYBJjg=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc

[Alin Serdean]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Saturday, May 20, 2017 2:25 AM
> To: Alin Serdean 
> Cc: Guru Shetty ; ovs dev 
> Subject: Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc
> 
> OK, I backported all the way to branch-2.5, dropping the wmi changes in
> 2.6 and 2.5.
> 
[Alin Serdean] Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc

[Ben Pfaff]

OK, I backported all the way to branch-2.5, dropping the wmi changes in
2.6 and 2.5.

On Fri, May 19, 2017 at 10:56:33PM +, Alin Serdean wrote:
> Branch-2.7 at least please. Branch-2.6 and branch-2.5 does not have the wmi.c 
> file
> 
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Ben Pfaff
> > Sent: Saturday, May 20, 2017 1:52 AM
> > To: Guru Shetty 
> > Cc: ovs dev 
> > Subject: Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc
> > 
> > On Fri, May 19, 2017 at 03:20:41PM -0700, Guru Shetty wrote:
> > > On 19 May 2017 at 14:59, Shashank Ram  wrote:
> > >
> > > > xmalloc checks if the size is valid before allocating memory, and
> > > > also if the allocation was successful.
> > > >
> > > > Signed-off-by: Shashank Ram 
> > > >
> > > Applied, thanks!
> > 
> > Does this need any backporting?
> > ___
> > 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 3/3] build-windows: cccl fail compilation on Wimplicit-function-declaration

[Ben Pfaff]

On Fri, May 19, 2017 at 08:25:11PM +, Alin Serdean wrote:
> Gcc compiler argument -Wall contains -Wimplicit-function-declaration which
> gives warnings when a function is used before declared.
> Map VStudio compiler error C4013 to it.
> More info on C4013:
> https://msdn.microsoft.com/en-us/library/d3ct4kz9.aspx
> 
> At the moment we cannot switch to the equivalent -Werror because we need
> to solve other warnings.
> 
> As a temporary solution issue an error when this warning is triggered.
> This will help development on the Windows side.
> 
> Suggested-by: Ben Pfaff 
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  build-aux/cccl | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/build-aux/cccl b/build-aux/cccl
> index 93f9c50..e2426fb 100644
> --- a/build-aux/cccl
> +++ b/build-aux/cccl
> @@ -144,6 +144,14 @@ EOF
>  #ignore pedantic
>  ;;
>  
> +-Wall)
> +# not all warnings are implemented
> +# the following is equivalent to
> +# Wimplicit-function-declaration but we will issue a compiler
> +# error
> +clopt="$clopt ${slash}we4013"
> +;;

Looks good to me.  I don't know whether this causes errors if it is
applied before patches 1 and 2, so to be on the same side I'll defer
pushing it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/3] windows: add includes to daemon-windows

[Ben Pfaff]

On Fri, May 19, 2017 at 08:25:11PM +, Alin Serdean wrote:
> Add fatal-signal.h include since it uses: fatal_signal_atexit_handler
> and fatal_signal_add_hook
> 
> Use the defined getpid() function and also include  since
> it is defined in include/windows/unistd.h .
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---
> v2: change fprintf %d to %ld (Ben Pfaff)

I agree that this should go in once patch 1 does.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc

[Alin Serdean]

Branch-2.7 at least please. Branch-2.6 and branch-2.5 does not have the wmi.c 
file

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Saturday, May 20, 2017 1:52 AM
> To: Guru Shetty 
> Cc: ovs dev 
> Subject: Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc
> 
> On Fri, May 19, 2017 at 03:20:41PM -0700, Guru Shetty wrote:
> > On 19 May 2017 at 14:59, Shashank Ram  wrote:
> >
> > > xmalloc checks if the size is valid before allocating memory, and
> > > also if the allocation was successful.
> > >
> > > Signed-off-by: Shashank Ram 
> > >
> > Applied, thanks!
> 
> Does this need any backporting?
> ___
> 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 v5 0/7] create tunnel devices using rtnetlink interface

[Greg Rose]

On Fri, 2017-05-19 at 09:22 -0400, Eric Garver wrote:
> On Thu, May 18, 2017 at 07:48:56PM -0700, Greg Rose wrote:
> > On Thu, 2017-05-18 at 15:53 -0700, Joe Stringer wrote:
> > > On 18 May 2017 at 15:35, Greg Rose  wrote:
> > > > On Thu, 2017-05-18 at 16:10 -0400, Eric Garver wrote:
> > > >> This series adds support for the creation of tunnels using the 
> > > >> rtnetlink
> > > >> interface. This will open the possibility for new features and flags 
> > > >> on those
> > > >> vports without the need to change vport compatibility code.
> > > >>
> > > >> Support for STT and LISP have not been added because these are not 
> > > >> upstream yet,
> > > >> so we don't know how the interface will be like upstream. And there 
> > > >> are no
> > > >> features in the current drivers right now we could make use of.
> > > >>
> > > >> Note: This work originally started by Thadeu Lima de Souza Cascardo.
> > > >>
> > > >> Note: There is a known failure for GENEVE tunnels using rtnetlink on 
> > > >> newer
> > > >> kernels. This is being addressed with a separate kernel fix. The 
> > > >> fallback to
> > > >> compat will work however.
> > > >
> > > > I've applied these patches for testing and review and I get these errors
> > > > when I run make check on a 4.12-rc1 kernel:
> > > >  783: tunnel-push-pop.at:3 tunnel_push_pop - action
> > > >  784: tunnel-push-pop.at:191 tunnel_push_pop - packet_out
> > > >  785: tunnel-push-pop.at:229 tunnel_push_pop - underlay bridge match
> > > >  786: tunnel-push-pop-ipv6.at:3 tunnel_push_pop_ipv6 - action
> > > >
> > > > Are these the known failures?  Was there something in the compat that
> > > > needs fixing?  I'm sort of new to this so please excuse any obvious
> > > > ignorance showing.
> > > >
> > > > I've downloaded the 4.9.23 kernel and will try against that next.
> > > 
> > > Hmm, I've tried it on my dev box with kernel 4.9.11 and I don't see
> > > any failures.
> > > 
> > > If these reliably fail, could you post the testsuite.log?
> > 
> > It's the 4.12-rc1 kernel.  He mentions known failures on newer kernels.
> > 
> 
> The known failure is for the "check-kernel" GENEVE tests. It affects the
> rtnetlink interface, but it will fallback to the compat interface and
> therefore the test case should still pass (assuming vport-geneve.ko is
> available).
> 
> I just ran "check" on a RHEL-7 based VM with a 4.12-rc1 kernel. The
> tests you mention above passed.
> 
> Are you still seeing failures after a re-run?

Yes, but they're even occurring for user space only and don't seem
related to kernel version.  I've tried it on several kernels and they
all have the same problem.  Probably something in my own environment...

Unit testing looks good.  Feel free to add:

Tested-by: Greg Rose 

> 
> Thanks.
> Eric.



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

Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc

[Ben Pfaff]

On Fri, May 19, 2017 at 03:20:41PM -0700, Guru Shetty wrote:
> On 19 May 2017 at 14:59, Shashank Ram  wrote:
> 
> > xmalloc checks if the size is valid before allocating
> > memory, and also if the allocation was successful.
> >
> > Signed-off-by: Shashank Ram 
> >
> Applied, thanks!

Does this need any backporting?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/3] dpif-netlink-rtnl: Use OVS_NOT_REACHED in verify.

[Greg Rose]

On Fri, 2017-05-19 at 13:27 -0700, Joe Stringer wrote:
> The vport_type_to_kind() call at the top of dpif_netlink_rtnl_verify()
> ensures that these cases can never be hit, so use OVS_NOT_REACHED()
> instead of setting the err to EOPNOTSUPP.
> 
> Signed-off-by: Joe Stringer 

Acked-by: Greg Rose 

> ---
>  lib/dpif-netlink-rtnl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 76ab0fe3fdec..c57923756f42 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -256,7 +256,7 @@ dpif_netlink_rtnl_verify(const struct 
> netdev_tunnel_config *tnl_cfg,
>  case OVS_VPORT_TYPE_UNSPEC:
>  case __OVS_VPORT_TYPE_MAX:
>  default:
> -err = EOPNOTSUPP;
> +OVS_NOT_REACHED();
>  }
>  
>  ofpbuf_delete(reply);



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

Re: [ovs-dev] [PATCH 2/3] dpif-netlink-rtnl: Use getlink() in common verify path.

[Greg Rose]

On Fri, 2017-05-19 at 13:27 -0700, Joe Stringer wrote:
> The calls here were duplicated across each tunnel protocol.
> 
> Signed-off-by: Joe Stringer 
> ---
>  lib/dpif-netlink-rtnl.c | 100 
> +---
>  1 file changed, 43 insertions(+), 57 deletions(-)
> 

Acked-by: Greg Rose 

> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 0ca6529e9d82..76ab0fe3fdec 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -160,34 +160,23 @@ rtnl_policy_parse(const char *kind, struct ofpbuf 
> *reply,
>  
>  static int
>  dpif_netlink_rtnl_vxlan_verify(const struct netdev_tunnel_config *tnl_cfg,
> -   const char *name, const char *kind)
> +   const char *kind, struct ofpbuf *reply)
>  {
> -struct ofpbuf *reply;
> +struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)];
>  int err;
>  
> -err = dpif_netlink_rtnl_getlink(name, );
> -
> +err = rtnl_policy_parse(kind, reply, vxlan_policy, vxlan,
> +ARRAY_SIZE(vxlan_policy));
>  if (!err) {
> -struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)];
> -
> -err = rtnl_policy_parse(kind, reply, vxlan_policy, vxlan,
> -ARRAY_SIZE(vxlan_policy));
> -if (!err) {
> -if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING])
> -|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA])
> -|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])
> -|| (tnl_cfg->dst_port
> -!= nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT]))) {
> -err = EINVAL;
> -}
> -}
> -if (!err) {
> -if (tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)
> -&& !nl_attr_get_flag(vxlan[IFLA_VXLAN_GBP])) {
> -err = EINVAL;
> -}
> +if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING])
> +|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA])
> +|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])
> +|| (tnl_cfg->dst_port
> +!= nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT]))
> +|| (tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)
> +&& !nl_attr_get_flag(vxlan[IFLA_VXLAN_GBP]))) {
> +err = EINVAL;
>  }
> -ofpbuf_delete(reply);
>  }
>  
>  return err;
> @@ -195,24 +184,17 @@ dpif_netlink_rtnl_vxlan_verify(const struct 
> netdev_tunnel_config *tnl_cfg,
>  
>  static int
>  dpif_netlink_rtnl_gre_verify(const struct netdev_tunnel_config OVS_UNUSED 
> *tnl,
> - const char *name, const char *kind)
> + const char *kind, struct ofpbuf *reply)
>  {
> -struct ofpbuf *reply;
> +struct nlattr *gre[ARRAY_SIZE(gre_policy)];
>  int err;
>  
> -err = dpif_netlink_rtnl_getlink(name, );
> -
> +err = rtnl_policy_parse(kind, reply, gre_policy, gre,
> +ARRAY_SIZE(gre_policy));
>  if (!err) {
> -struct nlattr *gre[ARRAY_SIZE(gre_policy)];
> -
> -err = rtnl_policy_parse(kind, reply, gre_policy, gre,
> -ARRAY_SIZE(gre_policy));
> -if (!err) {
> -if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
> -err = EINVAL;
> -}
> +if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
> +err = EINVAL;
>  }
> -ofpbuf_delete(reply);
>  }
>  
>  return err;
> @@ -220,27 +202,20 @@ dpif_netlink_rtnl_gre_verify(const struct 
> netdev_tunnel_config OVS_UNUSED *tnl,
>  
>  static int
>  dpif_netlink_rtnl_geneve_verify(const struct netdev_tunnel_config *tnl_cfg,
> -const char *name, const char *kind)
> +const char *kind, struct ofpbuf *reply)
>  {
> -struct ofpbuf *reply;
> +struct nlattr *geneve[ARRAY_SIZE(geneve_policy)];
>  int err;
>  
> -err = dpif_netlink_rtnl_getlink(name, );
> -
> +err = rtnl_policy_parse(kind, reply, geneve_policy, geneve,
> +ARRAY_SIZE(geneve_policy));
>  if (!err) {
> -struct nlattr *geneve[ARRAY_SIZE(geneve_policy)];
> -
> -err = rtnl_policy_parse(kind, reply, geneve_policy, geneve,
> -ARRAY_SIZE(geneve_policy));
> -if (!err) {
> -if (!nl_attr_get_flag(geneve[IFLA_GENEVE_COLLECT_METADATA])
> -|| 1 != nl_attr_get_u8(geneve[IFLA_GENEVE_UDP_ZERO_CSUM6_RX])
> -|| (tnl_cfg->dst_port
> -!= nl_attr_get_be16(geneve[IFLA_GENEVE_PORT]))) {
> -err = EINVAL;
> -}
> +if (!nl_attr_get_flag(geneve[IFLA_GENEVE_COLLECT_METADATA])
> +|| 1 != 

Re: [ovs-dev] [PATCH 1/3] dpif-netlink-rtnl: Tidy up some code.

[Greg Rose]

On Fri, 2017-05-19 at 13:27 -0700, Joe Stringer wrote:
> Simplify and refactor a couple of bits of code for improved readability.
> 
> Signed-off-by: Joe Stringer 
> ---
>  lib/dpif-netlink-rtnl.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 7faad5248037..0ca6529e9d82 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (c) 2017 Red Hat, Inc.
> + * Copyright (c) 2017 Nicira, Inc.

???

The patch itself looks fine but changing a copyright seems strange.

- Greg

>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -369,17 +370,16 @@ dpif_netlink_rtnl_port_create(struct netdev *netdev)
>  err = dpif_netlink_rtnl_verify(tnl_cfg, type, name);
>  if (!err) {
>  return 0;
> -} else {
> -err = dpif_netlink_rtnl_destroy(name);
> -if (err) {
> -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 
> 5);
> -
> -VLOG_WARN_RL(, "RTNL device %s exists and cannot be "
> - "deleted: %s", name, ovs_strerror(err));
> -return err;
> -}
> -err = dpif_netlink_rtnl_create(tnl_cfg, name, type, kind, flags);
>  }
> +err = dpif_netlink_rtnl_destroy(name);
> +if (err) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +
> +VLOG_WARN_RL(, "RTNL device %s exists and cannot be "
> + "deleted: %s", name, ovs_strerror(err));
> +return err;
> +}
> +err = dpif_netlink_rtnl_create(tnl_cfg, name, type, kind, flags);
>  }
>  if (err) {
>  return err;



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

Re: [ovs-dev] [PATCH v2] Windows lib: Use xmalloc instead of malloc

[Guru Shetty]

On 19 May 2017 at 14:59, Shashank Ram  wrote:

> xmalloc checks if the size is valid before allocating
> memory, and also if the allocation was successful.
>
> Signed-off-by: Shashank Ram 
>
Applied, thanks!


> ---
>  lib/netdev-windows.c | 14 ++
>  lib/wmi.c| 14 ++
>  2 files changed, 4 insertions(+), 24 deletions(-)
>
> diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c
> index 375cb32..f90a439 100644
> --- a/lib/netdev-windows.c
> +++ b/lib/netdev-windows.c
> @@ -394,12 +394,7 @@ netdev_windows_arp_lookup(const struct netdev
> *netdev,
>  return ENXIO;
>  }
>
> -arp_table = (MIB_IPNETTABLE *) malloc(buffer_length);
> -
> -if (arp_table == NULL) {
> -VLOG_ERR("Could not allocate memory for all the interfaces");
> -return ENXIO;
> -}
> +arp_table = (MIB_IPNETTABLE *) xmalloc(buffer_length);
>
>  ret_val = GetIpNetTable(arp_table, _length, false);
>
> @@ -443,12 +438,7 @@ netdev_windows_get_next_hop(const struct in_addr
> *host,
>  return ENXIO;
>  }
>
> -all_addr = (IP_ADAPTER_ADDRESSES *) malloc(buffer_length);
> -
> -if (all_addr == NULL) {
> -VLOG_ERR("Could not allocate memory for all the interfaces");
> -return ENXIO;
> -}
> +all_addr = (IP_ADAPTER_ADDRESSES *) xmalloc(buffer_length);
>
>  ret_val = GetAdaptersAddresses(AF_INET,
> GAA_FLAG_INCLUDE_PREFIX |
> diff --git a/lib/wmi.c b/lib/wmi.c
> index dba8022..e6dc63c 100644
> --- a/lib/wmi.c
> +++ b/lib/wmi.c
> @@ -406,12 +406,7 @@ delete_wmi_port(char *name)
>  wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * from "
>  L"Msvm_EthernetPortAllocationSettingData  WHERE ElementName =
> \"" ;
>
> -wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
> -if (wide_name == NULL) {
> -VLOG_WARN("Could not allocate memory for wide string");
> -retval = false;
> -goto error;
> -}
> +wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
>
>  if (!tranform_wide(name, wide_name)) {
>  retval = false;
> @@ -693,12 +688,7 @@ create_wmi_port(char *name) {
>  wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * FROM "
>  L"Msvm_InternalEthernetPort WHERE ElementName = \"";
>
> -wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
> -if (wide_name == NULL) {
> -VLOG_WARN("Could not allocate memory for wide string");
> -retval = false;
> -goto error;
> -}
> +wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
>
>  if (!tranform_wide(name, wide_name)) {
>  retval = false;
> --
> 2.9.3.windows.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 v2] Windows lib: Use xmalloc instead of malloc

[Shashank Ram]

xmalloc checks if the size is valid before allocating
memory, and also if the allocation was successful.

Signed-off-by: Shashank Ram 
---
 lib/netdev-windows.c | 14 ++
 lib/wmi.c| 14 ++
 2 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c
index 375cb32..f90a439 100644
--- a/lib/netdev-windows.c
+++ b/lib/netdev-windows.c
@@ -394,12 +394,7 @@ netdev_windows_arp_lookup(const struct netdev *netdev,
 return ENXIO;
 }

-arp_table = (MIB_IPNETTABLE *) malloc(buffer_length);
-
-if (arp_table == NULL) {
-VLOG_ERR("Could not allocate memory for all the interfaces");
-return ENXIO;
-}
+arp_table = (MIB_IPNETTABLE *) xmalloc(buffer_length);

 ret_val = GetIpNetTable(arp_table, _length, false);

@@ -443,12 +438,7 @@ netdev_windows_get_next_hop(const struct in_addr *host,
 return ENXIO;
 }

-all_addr = (IP_ADAPTER_ADDRESSES *) malloc(buffer_length);
-
-if (all_addr == NULL) {
-VLOG_ERR("Could not allocate memory for all the interfaces");
-return ENXIO;
-}
+all_addr = (IP_ADAPTER_ADDRESSES *) xmalloc(buffer_length);

 ret_val = GetAdaptersAddresses(AF_INET,
GAA_FLAG_INCLUDE_PREFIX |
diff --git a/lib/wmi.c b/lib/wmi.c
index dba8022..e6dc63c 100644
--- a/lib/wmi.c
+++ b/lib/wmi.c
@@ -406,12 +406,7 @@ delete_wmi_port(char *name)
 wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * from "
 L"Msvm_EthernetPortAllocationSettingData  WHERE ElementName = \"" ;

-wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
-if (wide_name == NULL) {
-VLOG_WARN("Could not allocate memory for wide string");
-retval = false;
-goto error;
-}
+wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));

 if (!tranform_wide(name, wide_name)) {
 retval = false;
@@ -693,12 +688,7 @@ create_wmi_port(char *name) {
 wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * FROM "
 L"Msvm_InternalEthernetPort WHERE ElementName = \"";

-wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
-if (wide_name == NULL) {
-VLOG_WARN("Could not allocate memory for wide string");
-retval = false;
-goto error;
-}
+wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));

 if (!tranform_wide(name, wide_name)) {
 retval = false;
--
2.9.3.windows.2

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

Re: [ovs-dev] [PATCH] lib Windows: Use xmalloc instead of malloc

[Shashank Ram]

Sure, will send out a v2.


Thanks,

Shashank


From: Alin Serdean 
Sent: Friday, May 19, 2017 2:51:27 PM
To: Shashank Ram; d...@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] lib Windows: Use xmalloc instead of malloc

Thanks for the patch!

Xmalloc also check if the allocation was successful.

Mind also removing the checks for null afterwards?

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Shashank Ram
> Sent: Saturday, May 20, 2017 12:39 AM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH] lib Windows: Use xmalloc instead of malloc
>
> xmalloc checks if the size is valid before allocating memory.
>
> Signed-off-by: Shashank Ram 
> ---
>  lib/netdev-windows.c | 4 ++--
>  lib/wmi.c| 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c index
> 375cb32..b4b39e9 100644
> --- a/lib/netdev-windows.c
> +++ b/lib/netdev-windows.c
> @@ -394,7 +394,7 @@ netdev_windows_arp_lookup(const struct netdev
> *netdev,
>  return ENXIO;
>  }
>
> -arp_table = (MIB_IPNETTABLE *) malloc(buffer_length);
> +arp_table = (MIB_IPNETTABLE *) xmalloc(buffer_length);
>
>  if (arp_table == NULL) {
>  VLOG_ERR("Could not allocate memory for all the interfaces"); @@ -
> 443,7 +443,7 @@ netdev_windows_get_next_hop(const struct in_addr
> *host,
>  return ENXIO;
>  }
[Alin Serdean] remove if
>
> -all_addr = (IP_ADAPTER_ADDRESSES *) malloc(buffer_length);
> +all_addr = (IP_ADAPTER_ADDRESSES *) xmalloc(buffer_length);
>
>  if (all_addr == NULL) {
>  VLOG_ERR("Could not allocate memory for all the interfaces");
[Alin Serdean] remove if
 diff --git
> a/lib/wmi.c b/lib/wmi.c index dba8022..b560a7e 100644
> --- a/lib/wmi.c
> +++ b/lib/wmi.c
> @@ -406,7 +406,7 @@ delete_wmi_port(char *name)
>  wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * from "
>  L"Msvm_EthernetPortAllocationSettingData  WHERE ElementName = \""
> ;
>
> -wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
> +wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
>  if (wide_name == NULL) {
>  VLOG_WARN("Could not allocate memory for wide string");
>  retval = false;
[Alin Serdean] remove if
> @@ -693,7 +693,7 @@ create_wmi_port(char *name) {
>  wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * FROM "
>  L"Msvm_InternalEthernetPort WHERE ElementName = \"";
>
> -wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
> +wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
>  if (wide_name == NULL) {
>  VLOG_WARN("Could not allocate memory for wide string");
>  retval = false;
[Alin Serdean] remove if
> --
> 2.9.3.windows.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwIFAg=uilaK90D4TOVoH58JNXRgQ=6OuVHk-mnufSWzkKa74UkQ=mfkoaz1JMXYW8oHB1F7xAajrZhLV6Z-IlDRkryqwb5o=taYazsV3M9fsNqeoxrywyjAN0pOH7LQvcYnJV6G58_Y=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] lib Windows: Use xmalloc instead of malloc

[Alin Serdean]

Thanks for the patch!

Xmalloc also check if the allocation was successful.

Mind also removing the checks for null afterwards?

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Shashank Ram
> Sent: Saturday, May 20, 2017 12:39 AM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH] lib Windows: Use xmalloc instead of malloc
> 
> xmalloc checks if the size is valid before allocating memory.
> 
> Signed-off-by: Shashank Ram 
> ---
>  lib/netdev-windows.c | 4 ++--
>  lib/wmi.c| 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c index
> 375cb32..b4b39e9 100644
> --- a/lib/netdev-windows.c
> +++ b/lib/netdev-windows.c
> @@ -394,7 +394,7 @@ netdev_windows_arp_lookup(const struct netdev
> *netdev,
>  return ENXIO;
>  }
> 
> -arp_table = (MIB_IPNETTABLE *) malloc(buffer_length);
> +arp_table = (MIB_IPNETTABLE *) xmalloc(buffer_length);
> 
>  if (arp_table == NULL) {
>  VLOG_ERR("Could not allocate memory for all the interfaces"); @@ -
> 443,7 +443,7 @@ netdev_windows_get_next_hop(const struct in_addr
> *host,
>  return ENXIO;
>  }
[Alin Serdean] remove if
> 
> -all_addr = (IP_ADAPTER_ADDRESSES *) malloc(buffer_length);
> +all_addr = (IP_ADAPTER_ADDRESSES *) xmalloc(buffer_length);
> 
>  if (all_addr == NULL) {
>  VLOG_ERR("Could not allocate memory for all the interfaces");
[Alin Serdean] remove if
 diff --git
> a/lib/wmi.c b/lib/wmi.c index dba8022..b560a7e 100644
> --- a/lib/wmi.c
> +++ b/lib/wmi.c
> @@ -406,7 +406,7 @@ delete_wmi_port(char *name)
>  wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * from "
>  L"Msvm_EthernetPortAllocationSettingData  WHERE ElementName = \""
> ;
> 
> -wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
> +wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
>  if (wide_name == NULL) {
>  VLOG_WARN("Could not allocate memory for wide string");
>  retval = false;
[Alin Serdean] remove if
> @@ -693,7 +693,7 @@ create_wmi_port(char *name) {
>  wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * FROM "
>  L"Msvm_InternalEthernetPort WHERE ElementName = \"";
> 
> -wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
> +wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
>  if (wide_name == NULL) {
>  VLOG_WARN("Could not allocate memory for wide string");
>  retval = false;
[Alin Serdean] remove if
> --
> 2.9.3.windows.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] lib Windows: Use xmalloc instead of malloc

[Sairam Venugopal]

Acked-by: Sairam Venugopal 





On 5/19/17, 2:38 PM, "ovs-dev-boun...@openvswitch.org on behalf of Shashank 
Ram"  wrote:

>xmalloc checks if the size is valid before allocating
>memory.
>
>Signed-off-by: Shashank Ram 
>---
> lib/netdev-windows.c | 4 ++--
> lib/wmi.c| 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c
>index 375cb32..b4b39e9 100644
>--- a/lib/netdev-windows.c
>+++ b/lib/netdev-windows.c
>@@ -394,7 +394,7 @@ netdev_windows_arp_lookup(const struct netdev *netdev,
> return ENXIO;
> }
>
>-arp_table = (MIB_IPNETTABLE *) malloc(buffer_length);
>+arp_table = (MIB_IPNETTABLE *) xmalloc(buffer_length);
>
> if (arp_table == NULL) {
> VLOG_ERR("Could not allocate memory for all the interfaces");
>@@ -443,7 +443,7 @@ netdev_windows_get_next_hop(const struct in_addr *host,
> return ENXIO;
> }
>
>-all_addr = (IP_ADAPTER_ADDRESSES *) malloc(buffer_length);
>+all_addr = (IP_ADAPTER_ADDRESSES *) xmalloc(buffer_length);
>
> if (all_addr == NULL) {
> VLOG_ERR("Could not allocate memory for all the interfaces");
>diff --git a/lib/wmi.c b/lib/wmi.c
>index dba8022..b560a7e 100644
>--- a/lib/wmi.c
>+++ b/lib/wmi.c
>@@ -406,7 +406,7 @@ delete_wmi_port(char *name)
> wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * from "
> L"Msvm_EthernetPortAllocationSettingData  WHERE ElementName = \"" ;
>
>-wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
>+wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
> if (wide_name == NULL) {
> VLOG_WARN("Could not allocate memory for wide string");
> retval = false;
>@@ -693,7 +693,7 @@ create_wmi_port(char *name) {
> wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * FROM "
> L"Msvm_InternalEthernetPort WHERE ElementName = \"";
>
>-wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
>+wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
> if (wide_name == NULL) {
> VLOG_WARN("Could not allocate memory for wide string");
> retval = false;
>--
>2.9.3.windows.2
>
>___
>dev mailing list
>d...@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=A_SrOn7pygG6mPCwmqq6YGTrmTn8yCfvuSryqv25tqY=x4TwpFwUmcevmXKYYFaWo8XUR-0DNGsADZbfW6I_DU4=
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] lib Windows: Use xmalloc instead of malloc

[Shashank Ram]

xmalloc checks if the size is valid before allocating
memory.

Signed-off-by: Shashank Ram 
---
 lib/netdev-windows.c | 4 ++--
 lib/wmi.c| 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c
index 375cb32..b4b39e9 100644
--- a/lib/netdev-windows.c
+++ b/lib/netdev-windows.c
@@ -394,7 +394,7 @@ netdev_windows_arp_lookup(const struct netdev *netdev,
 return ENXIO;
 }

-arp_table = (MIB_IPNETTABLE *) malloc(buffer_length);
+arp_table = (MIB_IPNETTABLE *) xmalloc(buffer_length);

 if (arp_table == NULL) {
 VLOG_ERR("Could not allocate memory for all the interfaces");
@@ -443,7 +443,7 @@ netdev_windows_get_next_hop(const struct in_addr *host,
 return ENXIO;
 }

-all_addr = (IP_ADAPTER_ADDRESSES *) malloc(buffer_length);
+all_addr = (IP_ADAPTER_ADDRESSES *) xmalloc(buffer_length);

 if (all_addr == NULL) {
 VLOG_ERR("Could not allocate memory for all the interfaces");
diff --git a/lib/wmi.c b/lib/wmi.c
index dba8022..b560a7e 100644
--- a/lib/wmi.c
+++ b/lib/wmi.c
@@ -406,7 +406,7 @@ delete_wmi_port(char *name)
 wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * from "
 L"Msvm_EthernetPortAllocationSettingData  WHERE ElementName = \"" ;

-wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
+wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
 if (wide_name == NULL) {
 VLOG_WARN("Could not allocate memory for wide string");
 retval = false;
@@ -693,7 +693,7 @@ create_wmi_port(char *name) {
 wchar_t internal_port_query[WMI_QUERY_COUNT] = L"SELECT * FROM "
 L"Msvm_InternalEthernetPort WHERE ElementName = \"";

-wide_name = malloc((strlen(name) + 1) * sizeof(wchar_t));
+wide_name = xmalloc((strlen(name) + 1) * sizeof(wchar_t));
 if (wide_name == NULL) {
 VLOG_WARN("Could not allocate memory for wide string");
 retval = false;
--
2.9.3.windows.2

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

[ovs-dev] [PATCH] windows: Set service status when stop is issued

[Alin Serdean]

If the service manager issued a stop service, the control handler
registered by the running daemon should report that service changed
state.

Signed-off-by: Alin Gabriel Serdean 
---
 lib/daemon-windows.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/daemon-windows.c b/lib/daemon-windows.c
index 4fc97a8..bed738f 100644
--- a/lib/daemon-windows.c
+++ b/lib/daemon-windows.c
@@ -196,6 +196,7 @@ control_handler(DWORD request)
 service_status.dwCurrentState = SERVICE_STOPPED;
 service_status.dwWin32ExitCode = NO_ERROR;
 SetEvent(wevent);
+SetServiceStatus(hstatus, _status);
 break;
 
 default:
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V8 05/26] dpif: Save added ports in a port map for netdev flow api use

[Simon Horman]

On Tue, May 16, 2017 at 12:16:46PM +0300, Roi Dayan wrote:
> 
> 
> On 08/05/2017 15:49, Simon Horman wrote:
> >On Wed, May 03, 2017 at 06:07:56PM +0300, Roi Dayan wrote:
> >>From: Paul Blakey 
> >>
> >>To use netdev flow offloading api, dpifs needs to iterate over
> >>added ports. This addition inserts the added dpif ports in a hash map,
> >>The map will also be used to translate dpif ports to netdevs.
> >>
> >>Signed-off-by: Paul Blakey 
> >>Reviewed-by: Roi Dayan 
> >>Reviewed-by: Simon Horman 
> >
> >...
> >
> >>diff --git a/lib/netdev.h b/lib/netdev.h
> >>index 7435fdf..9aa7e5e 100644
> >>--- a/lib/netdev.h
> >>+++ b/lib/netdev.h
> >>@@ -181,6 +181,12 @@ int netdev_init_flow_api(struct netdev *);
> >> extern bool netdev_flow_api_enabled;
> >> void netdev_set_flow_api_enabled(const struct smap *ovs_other_config);
> >>
> >>+struct dpif_port;
> >>+int netdev_ports_insert(struct netdev *, const void *obj, struct dpif_port 
> >>*);
> >>+struct netdev *netdev_ports_get(odp_port_t port, const void *obj);
> >>+int netdev_ports_remove(odp_port_t port, const void *obj);
> >>+odp_port_t netdev_ifindex_to_odp_port(int ifindex);
> >>+
> >> /* native tunnel APIs */
> >> /* Structure to pass parameters required to build a tunnel header. */
> >> struct netdev_tnl_build_header_params {
> >
> >This patch seems to only partially address the review provided
> >by Joe Stringer for v7. In particular:
> >
> >* netdev_ports_get() -> netdev_ports_lookup()
> >* Feedback regarding 'obj' being a not particularly clear abstraction.
> >
> 
> we did refactor all functions to have prefix netdev_ports_*
> there are both functions netdev_ports_get() and netdev_ports_lookup().
> did I miss something?

Thanks, I see that now. Looks good.

> about 'obj', I mentioned this in the changelog that it's left out for now
> and could be done in followup commit. is it ok?

Yes, that is fine by me.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Fix flake8 check

[Joe Stringer]

On 19 May 2017 at 05:35, Marcin Mirecki  wrote:
> On Fri, May 19, 2017 at 2:16 PM, Numan Siddique  wrote:
>
>>
>> -- Forwarded message --
>> From: Numan Siddique 
>> Date: Fri, May 19, 2017 at 11:09 AM
>> Subject: Re: [ovs-dev] [PATCH] Fix flake8 check
>> To: Terry Wilson 
>> Cc: ovs dev 
>>
>>
>>
>>
>> On Fri, May 19, 2017 at 9:39 AM, Terry Wilson  wrote:
>>
>>> Stop occluding the previous definition of filename.
>>>
>>> Signed-off-by: Terry Wilson 
>>>
>>
>> Acked-by: Numan Siddique 
>>
>>
>>
>>> ---
>>>  Documentation/conf.py | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/conf.py b/Documentation/conf.py
>>> index d70ee6b..62531dd 100644
>>> --- a/Documentation/conf.py
>>> +++ b/Documentation/conf.py
>>> @@ -121,6 +121,6 @@ _man_pages = [
>>>
>>>  # Generate list of (path, name, description, [author, ...], section)
>>>  man_pages = [
>>> -('ref/%s' % filename, filename.split('.', 1)[0],
>>> - description, [author], filename.split('.', 1)[1])
>>> -for filename, description in _man_pages]
>>> +('ref/%s' % fname, fname.split('.', 1)[0],
>>> + description, [author], fname.split('.', 1)[1])
>>> +for fname, description in _man_pages]
>>> --
>>> 1.8.3.1
>>>
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>
>>
>>
> Tested-by: Marcin Mirecki 

Thanks all on this, looks like the equivalent patch here was merged to
the same effect:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332669.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 0/7] create tunnel devices using rtnetlink interface

[Joe Stringer]

On 19 May 2017 at 06:22, Eric Garver  wrote:
> On Thu, May 18, 2017 at 07:48:56PM -0700, Greg Rose wrote:
>> On Thu, 2017-05-18 at 15:53 -0700, Joe Stringer wrote:
>> > On 18 May 2017 at 15:35, Greg Rose  wrote:
>> > > On Thu, 2017-05-18 at 16:10 -0400, Eric Garver wrote:
>> > >> This series adds support for the creation of tunnels using the rtnetlink
>> > >> interface. This will open the possibility for new features and flags on 
>> > >> those
>> > >> vports without the need to change vport compatibility code.
>> > >>
>> > >> Support for STT and LISP have not been added because these are not 
>> > >> upstream yet,
>> > >> so we don't know how the interface will be like upstream. And there are 
>> > >> no
>> > >> features in the current drivers right now we could make use of.
>> > >>
>> > >> Note: This work originally started by Thadeu Lima de Souza Cascardo.
>> > >>
>> > >> Note: There is a known failure for GENEVE tunnels using rtnetlink on 
>> > >> newer
>> > >> kernels. This is being addressed with a separate kernel fix. The 
>> > >> fallback to
>> > >> compat will work however.
>> > >
>> > > I've applied these patches for testing and review and I get these errors
>> > > when I run make check on a 4.12-rc1 kernel:
>> > >  783: tunnel-push-pop.at:3 tunnel_push_pop - action
>> > >  784: tunnel-push-pop.at:191 tunnel_push_pop - packet_out
>> > >  785: tunnel-push-pop.at:229 tunnel_push_pop - underlay bridge match
>> > >  786: tunnel-push-pop-ipv6.at:3 tunnel_push_pop_ipv6 - action
>> > >
>> > > Are these the known failures?  Was there something in the compat that
>> > > needs fixing?  I'm sort of new to this so please excuse any obvious
>> > > ignorance showing.
>> > >
>> > > I've downloaded the 4.9.23 kernel and will try against that next.
>> >
>> > Hmm, I've tried it on my dev box with kernel 4.9.11 and I don't see
>> > any failures.
>> >
>> > If these reliably fail, could you post the testsuite.log?
>>
>> It's the 4.12-rc1 kernel.  He mentions known failures on newer kernels.
>>
>
> The known failure is for the "check-kernel" GENEVE tests. It affects the
> rtnetlink interface, but it will fallback to the compat interface and
> therefore the test case should still pass (assuming vport-geneve.ko is
> available).
>
> I just ran "check" on a RHEL-7 based VM with a 4.12-rc1 kernel. The
> tests you mention above passed.
>
> Are you still seeing failures after a re-run?

I spoke to Greg offline and it appears that the issues he's observing
are particular to his setup, so we can proceed with this patch series
separately. I don't think Travis-CI, you or I are seeing any remaining
issues in the series as is.

Thanks, I applied this series to master. Nice to have this in!

I also assembled a short cleanup series to go on top, would you mind
taking a look?
https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332675.html

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

[ovs-dev] [PATCH 2/3] dpif-netlink-rtnl: Use getlink() in common verify path.

[Joe Stringer]

The calls here were duplicated across each tunnel protocol.

Signed-off-by: Joe Stringer 
---
 lib/dpif-netlink-rtnl.c | 100 +---
 1 file changed, 43 insertions(+), 57 deletions(-)

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 0ca6529e9d82..76ab0fe3fdec 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -160,34 +160,23 @@ rtnl_policy_parse(const char *kind, struct ofpbuf *reply,
 
 static int
 dpif_netlink_rtnl_vxlan_verify(const struct netdev_tunnel_config *tnl_cfg,
-   const char *name, const char *kind)
+   const char *kind, struct ofpbuf *reply)
 {
-struct ofpbuf *reply;
+struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)];
 int err;
 
-err = dpif_netlink_rtnl_getlink(name, );
-
+err = rtnl_policy_parse(kind, reply, vxlan_policy, vxlan,
+ARRAY_SIZE(vxlan_policy));
 if (!err) {
-struct nlattr *vxlan[ARRAY_SIZE(vxlan_policy)];
-
-err = rtnl_policy_parse(kind, reply, vxlan_policy, vxlan,
-ARRAY_SIZE(vxlan_policy));
-if (!err) {
-if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING])
-|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA])
-|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])
-|| (tnl_cfg->dst_port
-!= nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT]))) {
-err = EINVAL;
-}
-}
-if (!err) {
-if (tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)
-&& !nl_attr_get_flag(vxlan[IFLA_VXLAN_GBP])) {
-err = EINVAL;
-}
+if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING])
+|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA])
+|| 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])
+|| (tnl_cfg->dst_port
+!= nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT]))
+|| (tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)
+&& !nl_attr_get_flag(vxlan[IFLA_VXLAN_GBP]))) {
+err = EINVAL;
 }
-ofpbuf_delete(reply);
 }
 
 return err;
@@ -195,24 +184,17 @@ dpif_netlink_rtnl_vxlan_verify(const struct 
netdev_tunnel_config *tnl_cfg,
 
 static int
 dpif_netlink_rtnl_gre_verify(const struct netdev_tunnel_config OVS_UNUSED *tnl,
- const char *name, const char *kind)
+ const char *kind, struct ofpbuf *reply)
 {
-struct ofpbuf *reply;
+struct nlattr *gre[ARRAY_SIZE(gre_policy)];
 int err;
 
-err = dpif_netlink_rtnl_getlink(name, );
-
+err = rtnl_policy_parse(kind, reply, gre_policy, gre,
+ARRAY_SIZE(gre_policy));
 if (!err) {
-struct nlattr *gre[ARRAY_SIZE(gre_policy)];
-
-err = rtnl_policy_parse(kind, reply, gre_policy, gre,
-ARRAY_SIZE(gre_policy));
-if (!err) {
-if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
-err = EINVAL;
-}
+if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) {
+err = EINVAL;
 }
-ofpbuf_delete(reply);
 }
 
 return err;
@@ -220,27 +202,20 @@ dpif_netlink_rtnl_gre_verify(const struct 
netdev_tunnel_config OVS_UNUSED *tnl,
 
 static int
 dpif_netlink_rtnl_geneve_verify(const struct netdev_tunnel_config *tnl_cfg,
-const char *name, const char *kind)
+const char *kind, struct ofpbuf *reply)
 {
-struct ofpbuf *reply;
+struct nlattr *geneve[ARRAY_SIZE(geneve_policy)];
 int err;
 
-err = dpif_netlink_rtnl_getlink(name, );
-
+err = rtnl_policy_parse(kind, reply, geneve_policy, geneve,
+ARRAY_SIZE(geneve_policy));
 if (!err) {
-struct nlattr *geneve[ARRAY_SIZE(geneve_policy)];
-
-err = rtnl_policy_parse(kind, reply, geneve_policy, geneve,
-ARRAY_SIZE(geneve_policy));
-if (!err) {
-if (!nl_attr_get_flag(geneve[IFLA_GENEVE_COLLECT_METADATA])
-|| 1 != nl_attr_get_u8(geneve[IFLA_GENEVE_UDP_ZERO_CSUM6_RX])
-|| (tnl_cfg->dst_port
-!= nl_attr_get_be16(geneve[IFLA_GENEVE_PORT]))) {
-err = EINVAL;
-}
+if (!nl_attr_get_flag(geneve[IFLA_GENEVE_COLLECT_METADATA])
+|| 1 != nl_attr_get_u8(geneve[IFLA_GENEVE_UDP_ZERO_CSUM6_RX])
+|| (tnl_cfg->dst_port
+!= nl_attr_get_be16(geneve[IFLA_GENEVE_PORT]))) {
+err = EINVAL;
 }
-ofpbuf_delete(reply);
 }
 
 return err;
@@ -250,20 +225,30 @@ static int
 dpif_netlink_rtnl_verify(const struct netdev_tunnel_config *tnl_cfg,
  enum 

[ovs-dev] [PATCH 3/3] dpif-netlink-rtnl: Use OVS_NOT_REACHED in verify.

[Joe Stringer]

The vport_type_to_kind() call at the top of dpif_netlink_rtnl_verify()
ensures that these cases can never be hit, so use OVS_NOT_REACHED()
instead of setting the err to EOPNOTSUPP.

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

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 76ab0fe3fdec..c57923756f42 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -256,7 +256,7 @@ dpif_netlink_rtnl_verify(const struct netdev_tunnel_config 
*tnl_cfg,
 case OVS_VPORT_TYPE_UNSPEC:
 case __OVS_VPORT_TYPE_MAX:
 default:
-err = EOPNOTSUPP;
+OVS_NOT_REACHED();
 }
 
 ofpbuf_delete(reply);
-- 
2.11.1

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

[ovs-dev] [PATCH 1/3] dpif-netlink-rtnl: Tidy up some code.

[Joe Stringer]

Simplify and refactor a couple of bits of code for improved readability.

Signed-off-by: Joe Stringer 
---
 lib/dpif-netlink-rtnl.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 7faad5248037..0ca6529e9d82 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2017 Red Hat, Inc.
+ * Copyright (c) 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -369,17 +370,16 @@ dpif_netlink_rtnl_port_create(struct netdev *netdev)
 err = dpif_netlink_rtnl_verify(tnl_cfg, type, name);
 if (!err) {
 return 0;
-} else {
-err = dpif_netlink_rtnl_destroy(name);
-if (err) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
-VLOG_WARN_RL(, "RTNL device %s exists and cannot be "
- "deleted: %s", name, ovs_strerror(err));
-return err;
-}
-err = dpif_netlink_rtnl_create(tnl_cfg, name, type, kind, flags);
 }
+err = dpif_netlink_rtnl_destroy(name);
+if (err) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+VLOG_WARN_RL(, "RTNL device %s exists and cannot be "
+ "deleted: %s", name, ovs_strerror(err));
+return err;
+}
+err = dpif_netlink_rtnl_create(tnl_cfg, name, type, kind, flags);
 }
 if (err) {
 return err;
-- 
2.11.1

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

[ovs-dev] [PATCH 0/3] Minor refactor on dpif-netlink-rtnl.

[Joe Stringer]

This is a short collection of minor style tidyups for the recently merged
rtnetlink series. We converge a bit more code into common paths.

Joe Stringer (3):
  dpif-netlink-rtnl: Tidy up some code.
  dpif-netlink-rtnl: Use getlink() in common verify path.
  dpif-netlink-rtnl: Use OVS_NOT_REACHED in verify.

 lib/dpif-netlink-rtnl.c | 120 +---
 1 file changed, 53 insertions(+), 67 deletions(-)

-- 
2.11.1

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

[ovs-dev] [PATCH v2 2/3] windows: add includes to daemon-windows

[Alin Serdean]

Add fatal-signal.h include since it uses: fatal_signal_atexit_handler
and fatal_signal_add_hook

Use the defined getpid() function and also include  since
it is defined in include/windows/unistd.h .

Signed-off-by: Alin Gabriel Serdean 
---
v2: change fprintf %d to %ld (Ben Pfaff)
---
 lib/daemon-windows.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/daemon-windows.c b/lib/daemon-windows.c
index 4fc97a8..55545f8 100644
--- a/lib/daemon-windows.c
+++ b/lib/daemon-windows.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014 Nicira, Inc.
+ * Copyright (c) 2014, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,7 +20,9 @@
 #include 
 #include 
 #include 
+#include 
 #include "dirs.h"
+#include "fatal-signal.h"
 #include "ovs-thread.h"
 #include "poll-loop.h"
 #include "openvswitch/vlog.h"
@@ -475,7 +477,7 @@ make_pidfile(void)
 
 fatal_signal_add_hook(unlink_pidfile, NULL, NULL, true);
 
-fprintf(filep_pidfile, "%d\n", _getpid());
+fprintf(filep_pidfile, "%ld\n", (long int) getpid());
 if (fflush(filep_pidfile) == EOF) {
 VLOG_FATAL("Failed to write into the pidfile %s", pidfile);
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 3/3] build-windows: cccl fail compilation on Wimplicit-function-declaration

[Alin Serdean]

Gcc compiler argument -Wall contains -Wimplicit-function-declaration which
gives warnings when a function is used before declared.
Map VStudio compiler error C4013 to it.
More info on C4013:
https://msdn.microsoft.com/en-us/library/d3ct4kz9.aspx

At the moment we cannot switch to the equivalent -Werror because we need
to solve other warnings.

As a temporary solution issue an error when this warning is triggered.
This will help development on the Windows side.

Suggested-by: Ben Pfaff 
Signed-off-by: Alin Gabriel Serdean 
---
 build-aux/cccl | 8 
 1 file changed, 8 insertions(+)

diff --git a/build-aux/cccl b/build-aux/cccl
index 93f9c50..e2426fb 100644
--- a/build-aux/cccl
+++ b/build-aux/cccl
@@ -144,6 +144,14 @@ EOF
 #ignore pedantic
 ;;
 
+-Wall)
+# not all warnings are implemented
+# the following is equivalent to
+# Wimplicit-function-declaration but we will issue a compiler
+# error
+clopt="$clopt ${slash}we4013"
+;;
+
 -W*)
 #ignore warnings
 ;;
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 1/3] windows: add definition of getpid and getcwd

[Alin Serdean]

getcwd - is used in lib/util.c. getcwd is deprecated on Windows but has
_getcwd which is defined in :
https://msdn.microsoft.com/en-us/library/sf98bd4y(v=vs.120).aspx

getpid - is used in several files (i.e. lib/vlog.c). getpid
is also and deprecated and _getpid should be used:
https://msdn.microsoft.com/en-us/library/t2y34y40(v=vs.120).aspx
The problem using _getpid is that the definition is in .
A file called process.h also exists in the lib folder. This will mess up
includes.
An option would be to use a wrapper like we use for lib/string.h(.in) but
that would mean to also add it to the automake chain.
A simple solution would be to map it to GetCurrentProcessId
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683180(v=vs.85).aspx

_getpid uses GetCurrentProcessId behind the scenes, casting the result
is not required.

Signed-off-by: Alin Gabriel Serdean 
Co-authored-by: Ben Pfaff 
---
v2: used an inline function for getpid to make it POSIX compatible as
suggested by Ben Pfaff
---
 include/windows/unistd.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/windows/unistd.h b/include/windows/unistd.h
index 8629f7e..513ecba 100644
--- a/include/windows/unistd.h
+++ b/include/windows/unistd.h
@@ -17,9 +17,12 @@
 #define _UNISTD_H   1
 
 #define WIN32_LEAN_AND_MEAN
+#include 
 #include 
+#include 
 
 #define fsync _commit
+#define getcwd _getcwd
 
 /* Standard file descriptors.  */
 #define STDIN_FILENO0   /* Standard input.  */
@@ -33,6 +36,15 @@
 #define _SC_NPROCESSORS_ONLN0x2
 #define _SC_PHYS_PAGES  0x4
 
+
+static __inline pid_t getpid(void)
+{
+/* Since _getpid: https://msdn.microsoft.com/en-us/library/t2y34y40.aspx
+ * uses GetCurrentProcessId behind the scenes it is safe to assume no
+ * casting is required */
+return GetCurrentProcessId();
+}
+
 __inline int GetNumLogicalProcessors(void)
 {
 SYSTEM_INFO info_temp;
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 2/4] datapath-windows: Add NAT module in conntrack

[Alin Serdean]

> -Original Message-
> From: Sairam Venugopal [mailto:vsai...@vmware.com]
> Sent: Wednesday, May 17, 2017 9:05 PM
> To: Yin Lin ; d...@openvswitch.org
> Cc: Alin Serdean 
> Subject: Re: [ovs-dev] [PATCH v7 2/4] datapath-windows: Add NAT module
> in conntrack
> 
> Hi Yin,
> 
> Thanks for clarifying the comments. I will ack the next version.
> 
> @Alin - will you have sometime to review the changes?
[Alin Serdean] I'll give it a shot over the weekend .
> 
> Thanks,
> Sairam
> 
> 
> 
> 
> On 5/16/17, 5:11 PM, "Yin Lin"  wrote:
> 
> >Thanks Sai for the review! I fixed most of them and explained the remaining
> ones in the inline comments.
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC] packets: Do not initialize ct_orig_tuple.

[Joe Stringer]

On 17 May 2017 at 17:54, Joe Stringer  wrote:
> On 17 May 2017 at 16:26, Darrell Ball  wrote:
>>
>>
>> On 5/17/17, 2:19 PM, "Joe Stringer"  wrote:
>>
>> On 16 May 2017 at 21:01, Darrell Ball  wrote:
>> >
>> >
>> > On 5/15/17, 6:00 AM, "ovs-dev-boun...@openvswitch.org on behalf of 
>> Bodireddy, Bhanuprakash" > bhanuprakash.bodire...@intel.com> wrote:
>> >
>> > >Commit daf4d3c18da4("odp: Support conntrack orig tuple key.") 
>> introduced
>> > >new fields in struct 'pkt_metadata'.  pkt_metadata_init() is 
>> called for every
>> > >packet in the userspace datapath.  When testing a simple single 
>> flow case with
>> > >DPDK, we observe a lower throughput after the above commit (it 
>> was 14.88
>> > >Mpps before, it is 13 Mpps after).
>> > >
>> > >This patch skips initializing ct_orig_tuple in 
>> pkt_metadata_init().
>> > >It should be enough to initialize ct_state, because nobody should 
>> look at
>> > >ct_orig_tuple unless ct_state is != 0.
>> > >
>> > >CC: Jarno Rajahalme 
>> > >Signed-off-by: Daniele Di Proietto 
>> > >---
>> > >I'm sending this as an RFC because I didn't check very carefully 
>> if we can really
>> > >avoid initializing ct_orig_tuple.
>> > >
>> > >Maybe there are better solutions to this problem.
>> > >---
>> > > lib/packets.h | 2 +-
>> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > >diff --git a/lib/packets.h b/lib/packets.h index 
>> a5a483bc8..6f1791c7a 100644
>> > >--- a/lib/packets.h
>> > >+++ b/lib/packets.h
>> > >@@ -129,7 +129,7 @@ pkt_metadata_init(struct pkt_metadata *md,
>> > >odp_port_t port)
>> > > /* It can be expensive to zero out all of the tunnel 
>> metadata. However,
>> > >  * we can just zero out ip_dst and the rest of the data will 
>> never be
>> > >  * looked at. */
>> > >-memset(md, 0, offsetof(struct pkt_metadata, in_port));
>> > >+memset(md, 0, offsetof(struct pkt_metadata, ct_orig_tuple));
>> > > md->tunnel.ip_dst = 0;
>> > > md->tunnel.ipv6_dst = in6addr_any;
>> > >
>> >
>> > It's been a while this RFC patch has been submitted to fix 
>> performance drop on Master. This indeed fixes the OvS-DPDK performance drop 
>> that was introduced by the conntrack commit.
>> > Is there a better fix than what is suggested above?
>> >
>> >
>> > This affects both kernel and userspace.
>> > I tested this on both datapaths.
>> > LGTM
>>
>> How do we make sure that ct_orig_tuple isn't used uninitialized? Do we
>> need to zero out the ct_orig_tuple proto?
>>
>> There are a couple places where we explicitly set or reset the pkt metadata 
>> ct_orig_tuple;
>> one is in pkt_metadata_from_flow().
>>
>> I know there is a check for ct_orig_tuple proto in 
>> odp_key_from_pkt_metadata()
>> Can you find a case where can run this code without a set or reset of 
>> ct_orig_tuple pkt md
>> or you are not sure ?
>
> I wasn't sure, and I didn't see a response to the question that
> Daniele asked below the dashes in the original submission.
>
> Is miniflow_extract() safe wrt this? Seems like plausibly if the
> recirc_id is nonzero but there is no CT state (because, eg, bonds, or
> MPLS pop to IP, etc) it could end up populating the miniflow with
> garbage. In particular the path from emc_processing() seems suspect.

Perhaps this is actually OK, because the ct_state would be initialized
to zero in pkt_metadata_init(), and the classifier would never look at
ct_orig_tuple unless the ct_state is valid (according to the
ovs-fields(7) documentation). In which case, it shouldn't(?) matter
that garbage is written to the miniflow. Alternatively if md_is_valid
(eg, because it was recirculated), then it's up to the other callers
to ensure that md.ct_state was properly initialized. Effectively this
is done by the write_ct_md() function.

I wonder though if actually the miniflow_extract() "ct_nw_proto_p"
initialization should only occur if the md->ct_state is valid. That
could save a few extra cycles in a similar vein to how this patch
proposes.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] doc: Resolve pep8 warnings in conf.py

[Ben Pfaff]

On Fri, May 19, 2017 at 10:01:19AM +, Bodireddy, Bhanuprakash wrote:
> >-Original Message-
> >From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> >boun...@openvswitch.org] On Behalf Of Stephen Finucane
> >Sent: Friday, May 19, 2017 10:15 AM
> >To: d...@openvswitch.org
> >Subject: [ovs-dev] [PATCH] doc: Resolve pep8 warnings in conf.py
> >
> >flake8 doesn't like us redefining variables in loops.
> >
> >Signed-off-by: Stephen Finucane 
> >Reported-by: Bhanuprakash Bodireddy
> >
> >Fixes: f15010f ("doc: Reduce duplication in 'man_pages'")
> >Cc: Ben Pfaff 
> >---
> > Documentation/conf.py | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/Documentation/conf.py b/Documentation/conf.py index
> >d70ee6b..77c4df5 100644
> >--- a/Documentation/conf.py
> >+++ b/Documentation/conf.py
> >@@ -121,6 +121,6 @@ _man_pages = [
> >
> > # Generate list of (path, name, description, [author, ...], section)  
> > man_pages
> >= [
> >-('ref/%s' % filename, filename.split('.', 1)[0],
> >- description, [author], filename.split('.', 1)[1])
> >-for filename, description in _man_pages]
> >+('ref/%s' % file_name, file_name.split('.', 1)[0],
> >+ description, [author], file_name.split('.', 1)[1])
> >+for file_name, description in _man_pages]
> 
> Acked by: Bhanuprakash Bodireddy 

Thanks Stephen and Bhanu.  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] coding-style: Fix typo.

[Ben Pfaff]

On Fri, May 19, 2017 at 07:24:37AM -0700, Greg Rose wrote:
> On Thu, 2017-05-18 at 21:27 -0700, Ben Pfaff wrote:
> > Signed-off-by: Ben Pfaff 
> > ---
> >  Documentation/internals/contributing/coding-style.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/internals/contributing/coding-style.rst 
> > b/Documentation/internals/contributing/coding-style.rst
> > index 4694b2363775..666e887b1b68 100644
> > --- a/Documentation/internals/contributing/coding-style.rst
> > +++ b/Documentation/internals/contributing/coding-style.rst
> > @@ -210,7 +210,7 @@ accept and ignore a null pointer argument. Code that 
> > calls such a function
> >  null-pointer check. We find that this usually makes code easier to read.
> >  
> >  Functions in ``.c`` files should not normally be marked ``inline``, 
> > because it
> > -does not usually help code generation and it does suppress compilers 
> > warnings
> > +does not usually help code generation and it does suppress compiler 
> > warnings
> >  about unused functions. (Functions defined in .h usually should be marked
> >  inline.)
> >  
> 
> Acked-by: Greg Rose 

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

Re: [ovs-dev] [PATCH 3/3] netdev-dpdk: Use uint8_t for port_id.

[Darrell Ball]

On 5/19/17, 6:39 AM, "Ilya Maximets"  wrote:

On 18.05.2017 16:34, Aaron Conole wrote:
> Hi Ilya,
> 
> Ilya Maximets  writes:
> 
>> On 17.05.2017 18:32, Darrell Ball wrote:
>>>
>>>
>>> On 5/17/17, 7:59 AM, "Ilya Maximets"  wrote:
>>>
>>> I guess, we need some more opinions about this.
>>> 
>>> My comments inline.
>>> 
>>> Best regards, Ilya Maximets.
>>> 
>>> On 13.05.2017 07:00, Darrell Ball wrote:
>>> > 
>>> > 
>>> > On 5/12/17, 8:04 AM, "Ilya Maximets"  
wrote:
>>> > 
>>> > On 05.04.2017 22:34, Darrell Ball wrote:
>>> > > 
>>> > > 
> On 4/3/17, 8:04 AM, "ovs-dev-boun...@openvswitch.org on behalf
>> of Ilya Maximets" > i.maxim...@samsung.com> wrote:
>>> > > 
>>> > > Currently, signed integer is used for 'port_id' 
variable and
>>> > > '-1' as identifier of bad or uninitialized 'port_id'.
>>> > > 
>>> > > This inconsistent with dpdk library and, also, in few 
cases,
> leads to passing '-1' to dpdk functions where uint8_t expected.
>>> > > 
>>> > > Such behaviour doesn't produce any issues, but it's 
better to
>>> > > use same type as in dpdk library for consistency.
>>> > > 
>>> > > Also, magic number '-1' replaced with 
DPDK_ETH_PORT_ID_INVALID
>>> > > macro.
>>> > > 
>>> > > Signed-off-by: Ilya Maximets 
>>> > > ---
> lib/netdev-dpdk.c | 61
>> +++
>>> > >  1 file changed, 35 insertions(+), 26 deletions(-)
>>> > > 
>>> > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> > > index 658a454..216ced8 100644
>>> > > --- a/lib/netdev-dpdk.c
>>> > > +++ b/lib/netdev-dpdk.c
> @@ -140,6 +140,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
>> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
> #define OVS_VHOST_QUEUE_DISABLED (-2) /* Queue was disabled by
>> guest and not
> * yet mapped to another queue. */
>>> > >  
>>> > > +#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
>>> > > +
>>> > >  #define VHOST_ENQ_RETRY_NUM 8
> #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
>>> > >  
>>> > > @@ -309,7 +311,7 @@ struct dpdk_ring {
>>> > >  struct rte_ring *cring_tx;
>>> > >  struct rte_ring *cring_rx;
> unsigned int user_port_id; /* User given port no, parsed from
>> port name */
>>> > > -int eth_port_id; /* ethernet device port id */
>>> > > +uint8_t eth_port_id; /* ethernet device port id */
>>> > > 
>>> > > The rte does not have a typedef for port_id.
> One optional change is for OVS to have a typedef for this
>> external type.
>>> > > /* The dpdk rte uses uint8_t for port_id. */
>>> > > typedef uint8_t rte_port_id;
>>> > 
>>> > I don't think that it is a good change to do because we will 
have to
>>> > create additional typedef at least for PRIu8. This will look 
ugly.
>>> > 
>>> > No, see my following diff
>>> > 
>>> > Also, if DPDK someday will create typedef with different name
>>> > we will have typedef of the typedef?
>>> > 
>>> > I don’t follow this comment; see the diff below.
>>> > 
>>> > The reasons for the typedef here are:
>>> > 1) Easier to maintain if the size of type changes in future
>>> 
>>> In case of future type change we will have to change all the "PRIu8"
>>> format strings to something else. This is the half of all the 
changes.
>>> So, maintainability is in question.
>>>
>>> The alternative is to change both PRIu8 and the other code as well…
>>> This reasoning would imply that your proposed change is more 
maintainable
>>> because all code using the datatype would need to be re-written ?
>>
>> Maybe you're right, but both approaches are more or less poorly 
maintainable.
>>  
>>> The main purpose here is to differentiate b/w ovs and external port_id
>>> namespaces. 
>>> The reason why this patch exists is due to confusion in this regard –
>>> using the wrong datatype of int for port_ids derived from the RTE 
library.
>>>
>>> Your patch is trying again to manually replicate 

Re: [ovs-dev] [PATCH] coding-style: Fix typo.

[Greg Rose]

On Thu, 2017-05-18 at 21:27 -0700, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff 
> ---
>  Documentation/internals/contributing/coding-style.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/internals/contributing/coding-style.rst 
> b/Documentation/internals/contributing/coding-style.rst
> index 4694b2363775..666e887b1b68 100644
> --- a/Documentation/internals/contributing/coding-style.rst
> +++ b/Documentation/internals/contributing/coding-style.rst
> @@ -210,7 +210,7 @@ accept and ignore a null pointer argument. Code that 
> calls such a function
>  null-pointer check. We find that this usually makes code easier to read.
>  
>  Functions in ``.c`` files should not normally be marked ``inline``, because 
> it
> -does not usually help code generation and it does suppress compilers warnings
> +does not usually help code generation and it does suppress compiler warnings
>  about unused functions. (Functions defined in .h usually should be marked
>  inline.)
>  

Acked-by: Greg Rose 

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

[ovs-dev] Loligo & Cooked Shrimps

[Bonesca Import en Export BV]

Cooked Peeled Cat Tiger Shrimps
10 x 1 kilo / 750 grs net weight

Price per box - 10 box - palet
  61/70 € 6,25 - € 5,95 - € 5,75
  71/90 € 5,95 - € 5,65 - € 5,45
90/120 € 5,45 - € 5,15 - € 4,95
100/200 € 5,25 - € 4,95 - € 4,75
200/300 € 4,95 - € 4,65 - € 4,45
300/500 € 4,50 - € 4,30 - € 4,15
​Californian Squid / USA Loligo
10 kilo blockfrozen "Loligo Opalescens"
1 box € 3,99
10 box € 3,79

Palet (92 box) € 3,59 per kilo!!Click here for more offersClick here for 
contactpersons
If you no longer 
wish to receive mail from us, you can   
 unsubscribe



Bonesca, Schulpengat 9, Urk, Flevoland, 8321 WC, Netherlands

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

Re: [ovs-dev] [PATCH 3/3] netdev-dpdk: Use uint8_t for port_id.

[Ilya Maximets]

On 18.05.2017 16:34, Aaron Conole wrote:
> Hi Ilya,
> 
> Ilya Maximets  writes:
> 
>> On 17.05.2017 18:32, Darrell Ball wrote:
>>>
>>>
>>> On 5/17/17, 7:59 AM, "Ilya Maximets"  wrote:
>>>
>>> I guess, we need some more opinions about this.
>>> 
>>> My comments inline.
>>> 
>>> Best regards, Ilya Maximets.
>>> 
>>> On 13.05.2017 07:00, Darrell Ball wrote:
>>> > 
>>> > 
>>> > On 5/12/17, 8:04 AM, "Ilya Maximets"  wrote:
>>> > 
>>> > On 05.04.2017 22:34, Darrell Ball wrote:
>>> > > 
>>> > > 
> On 4/3/17, 8:04 AM, "ovs-dev-boun...@openvswitch.org on behalf
>> of Ilya Maximets" > i.maxim...@samsung.com> wrote:
>>> > > 
>>> > > Currently, signed integer is used for 'port_id' variable and
>>> > > '-1' as identifier of bad or uninitialized 'port_id'.
>>> > > 
>>> > > This inconsistent with dpdk library and, also, in few cases,
> leads to passing '-1' to dpdk functions where uint8_t expected.
>>> > > 
>>> > > Such behaviour doesn't produce any issues, but it's better 
>>> to
>>> > > use same type as in dpdk library for consistency.
>>> > > 
>>> > > Also, magic number '-1' replaced with 
>>> DPDK_ETH_PORT_ID_INVALID
>>> > > macro.
>>> > > 
>>> > > Signed-off-by: Ilya Maximets 
>>> > > ---
> lib/netdev-dpdk.c | 61
>> +++
>>> > >  1 file changed, 35 insertions(+), 26 deletions(-)
>>> > > 
>>> > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> > > index 658a454..216ced8 100644
>>> > > --- a/lib/netdev-dpdk.c
>>> > > +++ b/lib/netdev-dpdk.c
> @@ -140,6 +140,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
>> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
> #define OVS_VHOST_QUEUE_DISABLED (-2) /* Queue was disabled by
>> guest and not
> * yet mapped to another queue. */
>>> > >  
>>> > > +#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
>>> > > +
>>> > >  #define VHOST_ENQ_RETRY_NUM 8
> #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
>>> > >  
>>> > > @@ -309,7 +311,7 @@ struct dpdk_ring {
>>> > >  struct rte_ring *cring_tx;
>>> > >  struct rte_ring *cring_rx;
> unsigned int user_port_id; /* User given port no, parsed from
>> port name */
>>> > > -int eth_port_id; /* ethernet device port id */
>>> > > +uint8_t eth_port_id; /* ethernet device port id */
>>> > > 
>>> > > The rte does not have a typedef for port_id.
> One optional change is for OVS to have a typedef for this
>> external type.
>>> > > /* The dpdk rte uses uint8_t for port_id. */
>>> > > typedef uint8_t rte_port_id;
>>> > 
>>> > I don't think that it is a good change to do because we will have 
>>> to
>>> > create additional typedef at least for PRIu8. This will look ugly.
>>> > 
>>> > No, see my following diff
>>> > 
>>> > Also, if DPDK someday will create typedef with different name
>>> > we will have typedef of the typedef?
>>> > 
>>> > I don’t follow this comment; see the diff below.
>>> > 
>>> > The reasons for the typedef here are:
>>> > 1) Easier to maintain if the size of type changes in future
>>> 
>>> In case of future type change we will have to change all the "PRIu8"
>>> format strings to something else. This is the half of all the changes.
>>> So, maintainability is in question.
>>>
>>> The alternative is to change both PRIu8 and the other code as well…
>>> This reasoning would imply that your proposed change is more maintainable
>>> because all code using the datatype would need to be re-written ?
>>
>> Maybe you're right, but both approaches are more or less poorly maintainable.
>>  
>>> The main purpose here is to differentiate b/w ovs and external port_id
>>> namespaces. 
>>> The reason why this patch exists is due to confusion in this regard –
>>> using the wrong datatype of int for port_ids derived from the RTE library.
>>>
>>> Your patch is trying again to manually replicate the RTE port_id
>> datatype in OVS code and
>>> doing this without documenting that the port_id namespace is RTE and
>> not one of
>>> OVS native port ids.
>>> This is confusing to me and not maintainable.
>>>
>>> 
 2) Serves as documentation that the origin of the type is the rte
>> library and
>>> > and originates externally to ovs
>>> 
>>> IMHO, using 'rte_' prefix for something defined not inside DPDK is a
>>> bad style and may be misleading. We should 

[ovs-dev] [PATCH v4 3/3] netdev-dpdk: Use uint8_t for port_id.

[Ilya Maximets]

Currently, signed integer is used for 'port_id' variable and
'-1' as identifier of bad or uninitialized 'port_id'.

This inconsistent with dpdk library and, also, in few cases,
leads to passing '-1' to dpdk functions where uint8_t expected.

Such behaviour doesn't produce any issues, but it's better to
use same type as in dpdk library for consistency.
Introduced 'dpdk_port_t' typedef for better maintainability.

Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID
macro.

Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 63 ---
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index a41679f..b770b70 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -141,6 +141,11 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
 #define OVS_VHOST_QUEUE_DISABLED(-2) /* Queue was disabled by guest and not
   * yet mapped to another queue. */
 
+#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
+
+/* DPDK library uses uint8_t for port_id. */
+typedef uint8_t dpdk_port_t;
+
 #define VHOST_ENQ_RETRY_NUM 8
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
 
@@ -310,7 +315,7 @@ struct dpdk_ring {
 struct rte_ring *cring_tx;
 struct rte_ring *cring_rx;
 unsigned int user_port_id; /* User given port no, parsed from port name */
-int eth_port_id; /* ethernet device port id */
+dpdk_port_t eth_port_id; /* ethernet device port id */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 };
 
@@ -326,7 +331,7 @@ enum dpdk_hw_ol_features {
 
 struct netdev_dpdk {
 struct netdev up;
-int port_id;
+dpdk_port_t port_id;
 int max_packet_len;
 enum dpdk_dev_type type;
 
@@ -403,7 +408,7 @@ struct netdev_dpdk {
 
 struct netdev_rxq_dpdk {
 struct netdev_rxq up;
-int port_id;
+dpdk_port_t port_id;
 };
 
 static int netdev_dpdk_class_init(void);
@@ -604,12 +609,12 @@ check_link_status(struct netdev_dpdk *dev)
 dev->link_reset_cnt++;
 dev->link = link;
 if (dev->link.link_status) {
-VLOG_DBG_RL(, "Port %d Link Up - speed %u Mbps - %s",
+VLOG_DBG_RL(, "Port %"PRIu8" Link Up - speed %u Mbps - %s",
 dev->port_id, (unsigned) dev->link.link_speed,
 (dev->link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
  ("full-duplex") : ("half-duplex"));
 } else {
-VLOG_DBG_RL(, "Port %d Link Down", dev->port_id);
+VLOG_DBG_RL(, "Port %"PRIu8" Link Down", dev->port_id);
 }
 }
 }
@@ -727,8 +732,8 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
 if (rx_csum_ol_flag &&
 (info.rx_offload_capa & rx_chksm_offload_capa) !=
  rx_chksm_offload_capa) {
-VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d",
-   dev->port_id);
+VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8,
+   dev->port_id);
 dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
 return;
 }
@@ -739,7 +744,8 @@ static void
 dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
 {
 if (rte_eth_dev_flow_ctrl_set(dev->port_id, >fc_conf)) {
-VLOG_WARN("Failed to enable flow control on device %d", dev->port_id);
+VLOG_WARN("Failed to enable flow control on device %"PRIu8,
+  dev->port_id);
 }
 }
 
@@ -777,7 +783,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
 memset(_addr, 0x0, sizeof(eth_addr));
 rte_eth_macaddr_get(dev->port_id, _addr);
-VLOG_INFO_RL(, "Port %d: "ETH_ADDR_FMT,
+VLOG_INFO_RL(, "Port %"PRIu8": "ETH_ADDR_FMT,
 dev->port_id, ETH_ADDR_BYTES_ARGS(eth_addr.addr_bytes));
 
 memcpy(dev->hwaddr.ea, eth_addr.addr_bytes, ETH_ADDR_LEN);
@@ -789,7 +795,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 /* Get the Flow control configuration for DPDK-ETH */
 diag = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
 if (diag) {
-VLOG_DBG("cannot get flow control parameters on port=%d, err=%d",
+VLOG_DBG("cannot get flow control parameters on port=%"PRIu8", err=%d",
  dev->port_id, diag);
 }
 
@@ -834,7 +840,7 @@ netdev_dpdk_alloc_txq(unsigned int n_txqs)
 }
 
 static int
-common_construct(struct netdev *netdev, unsigned int port_no,
+common_construct(struct netdev *netdev, dpdk_port_t port_no,
  enum dpdk_dev_type type, int socket_id)
 OVS_REQUIRES(dpdk_mutex)
 {
@@ -919,7 +925,8 @@ vhost_common_construct(struct netdev *netdev)
 return ENOMEM;
 }
 
-return common_construct(netdev, -1, DPDK_DEV_VHOST, socket_id);
+return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID,
+DPDK_DEV_VHOST, socket_id);
 }
 
 

[ovs-dev] [PATCH v4 2/3] netdev-dpdk: Fix device leak on port deletion.

[Ilya Maximets]

Currently, once created device in dpdk will exist forever
even after del-port operation untill we manually call
'ovs-appctl netdev-dpdk/detach ', where  is not
the port's name but the name of dpdk eth device or pci address.

Few issues with current implementation:

1. Different API for usual (system) and DPDK devices.
   (We have to call 'ovs-appctl netdev-dpdk/detach' each
time after 'del-port' to actually free the device)
   This is a big issue mostly for virtual DPDK devices.

2. Follows from 1:
   For DPDK devices 'del-port' leads just to
   'rte_eth_dev_stop' and subsequent 'add-port' will
   just start the already existing device. Such behaviour
   will not reset the device to initial state as it could
   be expected. For example: virtual pcap pmd will continue
   reading input file instead of reading it from the beginning.

3. Follows from 2:
   After execution of the following commands 'port1' will be
   configured with the 'old-options' while 'ovs-vsctl show'
   will show us 'new-options' in dpdk-devargs field:

 ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
   options:dpdk-devargs=,
 ovs-vsctl del-port port1
 ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
   options:dpdk-devargs=,

4. Follows from 1:
   Not detached device consumes 'port_id'. Since we have very
   limited number of 'port_id's (32 in common case) this may
   lead to quick exhausting of id pool and inability to add any
   other port.

To avoid above issues we need to detach all the attached devices on
port destruction.
appctl 'netdev-dpdk/detach' removed because not needed anymore.

We need to use internal 'attached' variable to track ports on
which rte_eth_dev_attach() was called and returned successfully
to avoid closing and detaching devices that do not support hotplug or
by any other reason attached using the 'dpdk-extra' cmdline options.

CC: Ciara Loftus 
Fixes: 55e075e65ef9 ("netdev-dpdk: Arbitrary 'dpdk' port naming")
Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
Signed-off-by: Ilya Maximets 
---
 Documentation/howto/dpdk.rst |  5 ++-
 lib/netdev-dpdk.c| 72 
 2 files changed, 22 insertions(+), 55 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 3bd9e07..7c06239 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -342,10 +342,9 @@ Then it can be attached to OVS::
 $ ovs-vsctl add-port br0 dpdkx -- set Interface dpdkx type=dpdk \
 options:dpdk-devargs=:01:00.0
 
-It is also possible to detach a port from ovs, the user has to remove the
-port using the del-port command, then it can be detached using::
+Detaching will be performed while processing del-port command::
 
-$ ovs-appctl netdev-dpdk/detach :01:00.0
+$ ovs-vsctl del-port dpdkx
 
 This feature is not supported with VFIO and does not work with some NICs.
 For more information please refer to the `DPDK Port Hotplug Framework
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 1586e41..a41679f 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -360,6 +360,9 @@ struct netdev_dpdk {
 /* Device arguments for dpdk ports */
 char *devargs;
 
+/* If true, device was attached by rte_eth_dev_attach(). */
+bool attached;
+
 /* In dpdk_list. */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 
@@ -853,6 +856,7 @@ common_construct(struct netdev *netdev, unsigned int 
port_no,
 dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
 ovsrcu_index_init(>vid, -1);
 dev->vhost_reconfigured = false;
+dev->attached = false;
 
 ovsrcu_init(>qos_conf, NULL);
 
@@ -998,10 +1002,21 @@ static void
 netdev_dpdk_destruct(struct netdev *netdev)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+char devname[RTE_ETH_NAME_MAX_LEN];
 
 ovs_mutex_lock(_mutex);
 
 rte_eth_dev_stop(dev->port_id);
+
+if (dev->attached) {
+rte_eth_dev_close(dev->port_id);
+if (rte_eth_dev_detach(dev->port_id, devname) < 0) {
+VLOG_ERR("Device '%s' can not be detached", dev->devargs);
+} else {
+VLOG_INFO("Device '%s' detached", devname);
+}
+}
+
 free(dev->devargs);
 common_destruct(dev);
 
@@ -1113,7 +1128,8 @@ netdev_dpdk_lookup_by_port_id(int port_id)
 }
 
 static int
-netdev_dpdk_process_devargs(const char *devargs, char **errp)
+netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
+const char *devargs, char **errp)
 {
 /* Get the name up to the first comma. */
 char *name = xmemdup0(devargs, strcspn(devargs, ","));
@@ -1125,6 +1141,7 @@ 

[ovs-dev] [PATCH v4 1/3] netdev-dpdk: Fix double attaching of virtual devices.

[Ilya Maximets]

'devargs' for virtual devices contains not only name but
also a list of arguments like this:

'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap'
or
'eth_af_packet0,iface=eth0'

We must cut off the arguments from this string before calling
'rte_eth_dev_get_port_by_name()' to avoid double attaching of
the same device.

CC: Ciara Loftus 
Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 9941f88..1586e41 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1115,10 +1115,12 @@ netdev_dpdk_lookup_by_port_id(int port_id)
 static int
 netdev_dpdk_process_devargs(const char *devargs, char **errp)
 {
+/* Get the name up to the first comma. */
+char *name = xmemdup0(devargs, strcspn(devargs, ","));
 uint8_t new_port_id = UINT8_MAX;
 
 if (!rte_eth_dev_count()
-|| rte_eth_dev_get_port_by_name(devargs, _port_id)
+|| rte_eth_dev_get_port_by_name(name, _port_id)
 || !rte_eth_dev_is_valid_port(new_port_id)) {
 /* Device not found in DPDK, attempt to attach it */
 if (!rte_eth_dev_attach(devargs, _port_id)) {
@@ -1128,10 +1130,11 @@ netdev_dpdk_process_devargs(const char *devargs, char 
**errp)
 /* Attach unsuccessful */
 VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK",
   devargs);
-return -1;
+new_port_id = UINT8_MAX;
 }
 }
 
+free(name);
 return new_port_id;
 }
 
-- 
2.7.4

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

[ovs-dev] [PATCH v4 0/3] Hotplug fixes & port_id refactoring

[Ilya Maximets]

Version 4:
* 3rd patch: Introduced typedef dpdk_port_t.

Version 3:
* 1st patch: Fixed memory leak on error path. (Darrell Ball)
 (bad initial patch split)
* Discussion about patch #3 goes in replies to v1.
  Re-sent for now (rebased on modified patch #1).

Version 2:
* 1st patch: 'index' --> 'strcspn' (Ben Pfaff)
* One change was moved from patch 3 to patch 2.
  They were splitted in a wrong way.
  (passing 'dev' to 'netdev_dpdk_process_devargs()')


Ilya Maximets (3):
  netdev-dpdk: Fix double attaching of virtual devices.
  netdev-dpdk: Fix device leak on port deletion.
  netdev-dpdk: Use uint8_t for port_id.

 Documentation/howto/dpdk.rst |   5 +-
 lib/netdev-dpdk.c| 136 +++
 2 files changed, 61 insertions(+), 80 deletions(-)

-- 
2.7.4

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

Re: [ovs-dev] [PATCH v5 0/7] create tunnel devices using rtnetlink interface

[Eric Garver]

On Thu, May 18, 2017 at 07:48:56PM -0700, Greg Rose wrote:
> On Thu, 2017-05-18 at 15:53 -0700, Joe Stringer wrote:
> > On 18 May 2017 at 15:35, Greg Rose  wrote:
> > > On Thu, 2017-05-18 at 16:10 -0400, Eric Garver wrote:
> > >> This series adds support for the creation of tunnels using the rtnetlink
> > >> interface. This will open the possibility for new features and flags on 
> > >> those
> > >> vports without the need to change vport compatibility code.
> > >>
> > >> Support for STT and LISP have not been added because these are not 
> > >> upstream yet,
> > >> so we don't know how the interface will be like upstream. And there are 
> > >> no
> > >> features in the current drivers right now we could make use of.
> > >>
> > >> Note: This work originally started by Thadeu Lima de Souza Cascardo.
> > >>
> > >> Note: There is a known failure for GENEVE tunnels using rtnetlink on 
> > >> newer
> > >> kernels. This is being addressed with a separate kernel fix. The 
> > >> fallback to
> > >> compat will work however.
> > >
> > > I've applied these patches for testing and review and I get these errors
> > > when I run make check on a 4.12-rc1 kernel:
> > >  783: tunnel-push-pop.at:3 tunnel_push_pop - action
> > >  784: tunnel-push-pop.at:191 tunnel_push_pop - packet_out
> > >  785: tunnel-push-pop.at:229 tunnel_push_pop - underlay bridge match
> > >  786: tunnel-push-pop-ipv6.at:3 tunnel_push_pop_ipv6 - action
> > >
> > > Are these the known failures?  Was there something in the compat that
> > > needs fixing?  I'm sort of new to this so please excuse any obvious
> > > ignorance showing.
> > >
> > > I've downloaded the 4.9.23 kernel and will try against that next.
> > 
> > Hmm, I've tried it on my dev box with kernel 4.9.11 and I don't see
> > any failures.
> > 
> > If these reliably fail, could you post the testsuite.log?
> 
> It's the 4.12-rc1 kernel.  He mentions known failures on newer kernels.
> 

The known failure is for the "check-kernel" GENEVE tests. It affects the
rtnetlink interface, but it will fallback to the compat interface and
therefore the test case should still pass (assuming vport-geneve.ko is
available).

I just ran "check" on a RHEL-7 based VM with a 4.12-rc1 kernel. The
tests you mention above passed.

Are you still seeing failures after a re-run?

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

Re: [ovs-dev] [PATCH] Fix flake8 check

[Marcin Mirecki]

On Fri, May 19, 2017 at 2:16 PM, Numan Siddique  wrote:

>
> -- Forwarded message --
> From: Numan Siddique 
> Date: Fri, May 19, 2017 at 11:09 AM
> Subject: Re: [ovs-dev] [PATCH] Fix flake8 check
> To: Terry Wilson 
> Cc: ovs dev 
>
>
>
>
> On Fri, May 19, 2017 at 9:39 AM, Terry Wilson  wrote:
>
>> Stop occluding the previous definition of filename.
>>
>> Signed-off-by: Terry Wilson 
>>
>
> Acked-by: Numan Siddique 
>
>
>
>> ---
>>  Documentation/conf.py | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/conf.py b/Documentation/conf.py
>> index d70ee6b..62531dd 100644
>> --- a/Documentation/conf.py
>> +++ b/Documentation/conf.py
>> @@ -121,6 +121,6 @@ _man_pages = [
>>
>>  # Generate list of (path, name, description, [author, ...], section)
>>  man_pages = [
>> -('ref/%s' % filename, filename.split('.', 1)[0],
>> - description, [author], filename.split('.', 1)[1])
>> -for filename, description in _man_pages]
>> +('ref/%s' % fname, fname.split('.', 1)[0],
>> + description, [author], fname.split('.', 1)[1])
>> +for fname, description in _man_pages]
>> --
>> 1.8.3.1
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
>
Tested-by: Marcin Mirecki 


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

Re: [ovs-dev] [PATCH v2] python ovs: Fix SSL exceptions with pyOpenSSL v0.13

[Marcin Mirecki]

> From: nusid...@redhat.com
> To: d...@openvswitch.org
> Sent: Monday, 15 May, 2017 11:39:25 AM
> Subject: [ovs-dev] [PATCH v2] python ovs: Fix SSL exceptions with
pyOpenSSL   v0.13
>
> From: Numan Siddique 
>
> Centos provides pyOpenSSL version pyOpenSSL-0.13.1-3.el7.x86_64.
> There are 2 issues using this version, which this patch fixes
>
>  - The test case "simple idl verify notify - SSL" is skipped.
>This is because "python -m OpenSSL.SSL" is used to detect the
>presence of pyOpenSSL package. pyOpenSSL v0.13 has C python
>modules because of which the above command returns 1.
>So this patch fixes this by using 'python -c "import OpenSSL.SSL"'.
>
>  - The SSL.Context class do not have the function "set_session_cache_mode"
>defined. Setting the session cache mode has an effect for server-side
>sessions and doesn't make much sense for client-side sessions.
>Since python ovs doesn't support "pssl" connection mode, this patch
>deletes the reference to this function.
>
> I have not tested with older versions (< 0.13) of pyOpenSSL.
>
> Signed-off-by: Numan Siddique 
> ---

> LGTM

> Acked-by: Lance Richardson 

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

Re: [ovs-dev] [PATCH 4.4-only] openvswitch: clear sender cpu before forwarding packets

[Anoob Soman]

On 18/05/17 09:11, Greg KH wrote:

So backporting that one patch solves the issue here?  Can you please
verify it, and let me know before I apply it?

thanks,

greg k-h


yes, I can do that.

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

Re: [ovs-dev] [PATCH] doc: Resolve pep8 warnings in conf.py

[Bodireddy, Bhanuprakash]

>-Original Message-
>From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
>boun...@openvswitch.org] On Behalf Of Stephen Finucane
>Sent: Friday, May 19, 2017 10:15 AM
>To: d...@openvswitch.org
>Subject: [ovs-dev] [PATCH] doc: Resolve pep8 warnings in conf.py
>
>flake8 doesn't like us redefining variables in loops.
>
>Signed-off-by: Stephen Finucane 
>Reported-by: Bhanuprakash Bodireddy
>
>Fixes: f15010f ("doc: Reduce duplication in 'man_pages'")
>Cc: Ben Pfaff 
>---
> Documentation/conf.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/Documentation/conf.py b/Documentation/conf.py index
>d70ee6b..77c4df5 100644
>--- a/Documentation/conf.py
>+++ b/Documentation/conf.py
>@@ -121,6 +121,6 @@ _man_pages = [
>
> # Generate list of (path, name, description, [author, ...], section)  
> man_pages
>= [
>-('ref/%s' % filename, filename.split('.', 1)[0],
>- description, [author], filename.split('.', 1)[1])
>-for filename, description in _man_pages]
>+('ref/%s' % file_name, file_name.split('.', 1)[0],
>+ description, [author], file_name.split('.', 1)[1])
>+for file_name, description in _man_pages]

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

[ovs-dev] [PATCH v2 5/6] userspace: Complete Packet In handling

[Zoltán Balogh]

From: Jan Scheurich 

Send packet_in for non-Ethernet packets.
Include packet_type in Packet In for ptap bridges.

Signed-off-by: Jan Scheurich 
---
 lib/flow.c   |  4 
 lib/ofp-print.c  |  3 +--
 ofproto/ofproto-dpif-xlate.c | 10 +-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index 4f7a041..d3b3cee 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -991,6 +991,10 @@ flow_get_metadata(const struct flow *flow, struct match 
*flow_metadata)
 }
 
 match_set_in_port(flow_metadata, flow->in_port.ofp_port);
+if (flow->packet_type != htonl(PT_ETH)) {
+match_set_packet_type(flow_metadata, flow->packet_type);
+}
+
 if (flow->ct_state != 0) {
 match_set_ct_state(flow_metadata, flow->ct_state);
 if (is_ct_valid(flow, NULL, NULL) && flow->ct_nw_proto != 0) {
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 7ca9531..2408bff 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -217,10 +217,9 @@ ofp_print_packet_in(struct ds *string, const struct 
ofp_header *oh,
 }
 
 if (verbosity > 0) {
-/* Packet In can only carry Ethernet packets. */
 char *packet = ofp_packet_to_string(public->packet,
 public->packet_len,
-htonl(PT_ETH));
+
public->flow_metadata.flow.packet_type);
 ds_put_cstr(string, packet);
 free(packet);
 }
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7240a24..3a69c21 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4267,11 +4267,6 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
 return;
 }
 
-if (packet->packet_type != htonl(PT_ETH)) {
-dp_packet_delete(packet);
-return;
-}
-
 /* A packet sent by an action in a table-miss rule is considered an
  * explicit table miss.  OpenFlow before 1.3 doesn't have that concept so
  * it will get translated back to OFPR_ACTION for those versions. */
@@ -4305,6 +4300,11 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
 };
 flow_get_metadata(>xin->flow, >pin.up.public.flow_metadata);
 
+/* Send packet_type only from packet-type-aware bridges. */
+if (!ctx->xbridge->packet_type_aware) {
+am->pin.up.public.flow_metadata.wc.masks.packet_type = 0;
+}
+
 /* Async messages are only sent once, so if we send one now, no
  * xlate cache entry is created.  */
 if (ctx->xin->allow_side_effects) {
-- 
1.9.1

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

[ovs-dev] [PATCH v2 4/6] tests: Added unit tests in packet-type-aware.at

[Zoltán Balogh]

From: Jan Scheurich 

Packet type-aware unit tests.

 ptap - create packet-type-aware bridge
 ptap - legal flow entries in ptap bridge
 ptap - triangle bridge setup with L2 and L3 GRE tunnels

First and second unit tests perform basic verification.

The third one is a triangular bridge setup test case. It tests dataplane
in non-PTAP and ptap bridges in conjunction with L2 and L3 GRE tunnels.
It uses veth ports, therefore requires root privileges.

A simplified version of the third test is added to system userspace unit tests.

 GRE tunneling test setup for PTAP bridge

 192.168.10.10   192.168.10.20 192.168.10.30
  n1   n2n3
  || |
   +--o--+  +--o--+   +--o--+
   |br-in1   |  |br-in2   |   |br-in3   |
   | |  |   (PTAP)|   | |
   +--o--+  +--o--+   +--o--+
 gre  gre   gre
   10.0.0.1(10.0.0.2)(10.0.0.3)
  (20.0.0.1)20.0.0.2 (20.0.0.3)
  (30.0.0.1) LOCAL (30.0.0.2) LOCAL   30.0.0.3  LOCAL
   +---o-+  +---o-+   +---o-+
   |br-p1|  |br-p2|   |br-p3|
   +--o--+  +--o--+   +--o--+
 p1-0 || p2-0| p3-0
 p0-1 || p0-2| p0-3
   +--oo-o--+
   |  br0   |
   ++

   GRE tunnel ports:
  No Bridge  NameTypeRemote bridge & ports
 ---
  1020   br-in1  gre-12  l2  br-in2 2010 (versatile)
  1021   br-in1  gre-12_l3   l3same
  1030   br-in1  gre-13  l2  br-in3 3010 (l2)
  2010   br-in2  gre-21  versatile   br-in1 1020 (l2), 1021 (l3)
  2030   br-in2  gre-13  versatile   br-in3 3020 (l2), 3021 (l3)
  3010   br-in1  gre-31  l2  br-in1 1030 (l2)
  3020   br-in1  gre-32  l2  br-in2 2010 (versatile)
  3021   br-in1  gre-32_l3   l3same

Signed-off-by: Jan Scheurich 
---
 tests/automake.mk   |   6 +-
 tests/packet-type-aware.at  | 540 
 tests/system-userspace-packet-type-aware.at | 422 ++
 tests/system-userspace-testsuite.at |   1 +
 tests/testsuite.at  |   1 +
 5 files changed, 968 insertions(+), 2 deletions(-)
 create mode 100644 tests/packet-type-aware.at
 create mode 100644 tests/system-userspace-packet-type-aware.at

diff --git a/tests/automake.mk b/tests/automake.mk
index c6bd120..08d5a2d 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -95,7 +95,8 @@ TESTSUITE_AT = \
tests/ovn-sbctl.at \
tests/ovn-controller.at \
tests/ovn-controller-vtep.at \
-   tests/mcast-snooping.at
+   tests/mcast-snooping.at \
+   tests/packet-type-aware.at
 
 SYSTEM_KMOD_TESTSUITE_AT = \
tests/system-common-macros.at \
@@ -105,7 +106,8 @@ SYSTEM_KMOD_TESTSUITE_AT = \
 SYSTEM_USERSPACE_TESTSUITE_AT = \
tests/system-userspace-testsuite.at \
tests/system-ovn.at \
-   tests/system-userspace-macros.at
+   tests/system-userspace-macros.at \
+   tests/system-userspace-packet-type-aware.at
 
 SYSTEM_TESTSUITE_AT = \
tests/system-common-macros.at \
diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
new file mode 100644
index 000..976b0fd
--- /dev/null
+++ b/tests/packet-type-aware.at
@@ -0,0 +1,540 @@
+AT_BANNER([packet-type-aware pipeline])
+
+AT_SETUP([ptap - create packet-type-aware bridge])
+
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+ovs-vsctl set bridge br0 \
+datapath_type=dummy \
+other-config:packet-type-aware=true
+], [0])
+
+AT_CHECK([ovs-vsctl list bridge br0 | grep other_config], [0], [dnl
+other_config: {datapath-id="fedcba9876543210", 
hwaddr="aa:55:aa:55:00:00", packet-type-aware="true"}
+])
+
+AT_CHECK([ovs-ofctl -Oopenflow13 dump-table-features br0 | grep packet_type], 
[0], [dnl
+  packet_type: exact match or wildcard
+])
+AT_CHECK([ovs-ofctl -Oopenflow14 dump-table-features br0 | grep packet_type], 
[0], [dnl
+  packet_type: exact match or wildcard
+])
+AT_CHECK([ovs-ofctl -Oopenflow15 dump-table-features br0 | grep packet_type], 
[0], [dnl
+  packet_type: exact match or wildcard
+])
+
+AT_CHECK([
+

[ovs-dev] [PATCH v2 6/6] userspace: Introduce OF 1.5 packet-out

[Zoltán Balogh]

Introduce OpenFlow 1.5 packet-out

Partly based on Jean Tourrilhes's work.

Add test cases for OF1.5 packet-out
Add negative test case for OF1.5 packet-out
Modify wildcarding and packet-out test printout.

Signed-off-by: Jean Tourrilhes 
Signed-off-by: Zoltan Balogh 
Co-authored-by: Jan Scheurich 
---
 include/openflow/openflow-1.5.h |  17 +++
 include/openvswitch/ofp-msgs.h  |   7 ++-
 include/openvswitch/ofp-util.h  |   1 +
 lib/flow.c  |  37 --
 lib/ofp-parse.c |  11 
 lib/ofp-print.c |   3 +-
 lib/ofp-util.c  |  57 +++--
 ofproto/ofproto.c   |   8 +++
 tests/ofproto.at| 108 
 utilities/ovs-ofctl.c   |   1 +
 10 files changed, 226 insertions(+), 24 deletions(-)

diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 3649e6c..0f770d4 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -39,6 +39,23 @@
 
 #include 
 
+
+/* Send packet (controller -> datapath). */
+struct ofp15_packet_out {
+ovs_be32 buffer_id; /* ID assigned by datapath (OFP_NO_BUFFER (-1)
+   if none). */
+ovs_be16 actions_len;   /* Size of action array in bytes. */
+uint8_t  pad[2];/* Align to 64 bits. */
+/* struct ofp12_match match; */ /* Packet pipeline fields. Variable size. 
*/
+/* The variable size and padded match is followed by the list of actions. 
*/
+/* struct ofp_action_header actions[0]; */ /* Action list - 0 or more. */
+/* The variable size action list is optionally followed by packet data.
+ * This data is only present and meaningful if buffer_id = -1. */
+/* uint8_t data[0]; */  /* Packet data. The length is inferred
+   from the length field in the header. */
+};
+OFP_ASSERT(sizeof(struct ofp15_packet_out) == 8);
+
 /* Body for ofp15_multipart_request of type OFPMP_PORT_DESC. */
 struct ofp15_port_desc_request {
 ovs_be32 port_no; /* All ports if OFPP_ANY. */
diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 34708f3..6dc0b60 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -177,8 +177,10 @@ enum ofpraw {
 
 /* OFPT 1.0 (13): struct ofp10_packet_out, uint8_t[]. */
 OFPRAW_OFPT10_PACKET_OUT,
-/* OFPT 1.1+ (13): struct ofp11_packet_out, uint8_t[]. */
+/* OFPT 1.1-1.4 (13): struct ofp11_packet_out, uint8_t[]. */
 OFPRAW_OFPT11_PACKET_OUT,
+/* OFPT 1.5+ (13): struct ofp15_packet_out, uint8_t[]. */
+OFPRAW_OFPT15_PACKET_OUT,
 
 /* OFPT 1.0 (14): struct ofp10_flow_mod, uint8_t[8][]. */
 OFPRAW_OFPT10_FLOW_MOD,
@@ -561,7 +563,8 @@ enum ofptype {
 
 /* Controller command messages. */
 OFPTYPE_PACKET_OUT,  /* OFPRAW_OFPT10_PACKET_OUT.
-  * OFPRAW_OFPT11_PACKET_OUT. */
+  * OFPRAW_OFPT11_PACKET_OUT.
+  * OFPRAW_OFPT15_PACKET_OUT. */
 OFPTYPE_FLOW_MOD,/* OFPRAW_OFPT10_FLOW_MOD.
   * OFPRAW_OFPT11_FLOW_MOD.
   * OFPRAW_NXT_FLOW_MOD. */
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index f37d181..4e58cdc 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -527,6 +527,7 @@ struct ofputil_packet_out {
 size_t packet_len;  /* Length of packet data in bytes. */
 uint32_t buffer_id; /* Buffer id or UINT32_MAX if no buffer. */
 ofp_port_t in_port; /* Packet's input port. */
+ovs_be32 packet_type;   /* Packet's packet type. */
 struct ofpact *ofpacts; /* Actions. */
 size_t ofpacts_len; /* Size of ofpacts in bytes. */
 };
diff --git a/lib/flow.c b/lib/flow.c
index d3b3cee..ef0f3c3 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1421,6 +1421,8 @@ void
 flow_wildcards_init_for_packet(struct flow_wildcards *wc,
const struct flow *flow)
 {
+ovs_be16 dl_type = OVS_BE16_MAX;
+
 memset(>masks, 0x0, sizeof wc->masks);
 
 /* Update this function whenever struct flow changes. */
@@ -1472,26 +1474,29 @@ flow_wildcards_init_for_packet(struct flow_wildcards 
*wc,
 WC_MASK_FIELD(wc, in_port);
 
 /* actset_output wildcarded. */
-
-WC_MASK_FIELD(wc, dl_dst);
-WC_MASK_FIELD(wc, dl_src);
-WC_MASK_FIELD(wc, dl_type);
-
-/* No need to set mask of inner VLANs that don't exist. */
-for (int i = 0; i < FLOW_MAX_VLAN_HEADERS; i++) {
-/* Always show the first zero VLAN. */
-WC_MASK_FIELD(wc, vlans[i]);
-if (flow->vlans[i].tci == htons(0)) {
-break;
+if (flow->packet_type == htonl(PT_ETH)) {
+ 

[ovs-dev] [PATCH v2 2/6] userspace: Add bridge property 'packet-type-aware'

[Zoltán Balogh]

From: Jan Scheurich 

New boolean parameter "other-config:packet-type-aware' in bridge table.
Pass non-Ethernet packets unchanged into packet-type-aware bridges.
Do not convert packet type when sending packet from packet-type-aware
bridge to a port.

Only include field MFF_PACKET_TYPE in matchabale and maskable fields
if the ofproto bridge is packet-type-aware.

Add default match on packet_type Ethernet in case of packet-type-aware bridge.

Removed packet_type from list of matching properties in tests/ofproto.at.

Signed-off-by: Jan Scheurich 
---
 lib/nx-match.c   | 10 --
 lib/nx-match.h   |  4 ++--
 ofproto/ofproto-dpif-xlate.c | 37 +++--
 ofproto/ofproto-dpif-xlate.h |  1 +
 ofproto/ofproto-dpif.c   |  1 +
 ofproto/ofproto-provider.h   |  1 +
 ofproto/ofproto.c| 21 ++---
 ofproto/ofproto.h|  1 +
 tests/ofproto.at |  1 -
 vswitchd/bridge.c|  9 +
 vswitchd/vswitch.xml |  9 +
 11 files changed, 73 insertions(+), 22 deletions(-)

diff --git a/lib/nx-match.c b/lib/nx-match.c
index af19fb2..f9d8565 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -2071,12 +2071,15 @@ oxm_writable_fields(void)
 /* Returns a bitmap of fields that can be encoded in OXM and that can be
  * matched in a flow table.  */
 struct mf_bitmap
-oxm_matchable_fields(void)
+oxm_matchable_fields(bool packet_type_aware)
 {
 struct mf_bitmap b = MF_BITMAP_INITIALIZER;
 int i;
 
 for (i = 0; i < MFF_N_IDS; i++) {
+if (i == MFF_PACKET_TYPE && !packet_type_aware) {
+continue;
+}
 if (mf_oxm_header(i, 0)) {
 bitmap_set1(b.bm, i);
 }
@@ -2087,12 +2090,15 @@ oxm_matchable_fields(void)
 /* Returns a bitmap of fields that can be encoded in OXM and that can be
  * matched in a flow table with an arbitrary bitmask.  */
 struct mf_bitmap
-oxm_maskable_fields(void)
+oxm_maskable_fields(bool packet_type_aware)
 {
 struct mf_bitmap b = MF_BITMAP_INITIALIZER;
 int i;
 
 for (i = 0; i < MFF_N_IDS; i++) {
+if (i == MFF_PACKET_TYPE && !packet_type_aware) {
+continue;
+}
 if (mf_oxm_header(i, 0) && mf_from_id(i)->maskable == MFM_FULLY) {
 bitmap_set1(b.bm, i);
 }
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 6afb310..f75ce10 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -147,8 +147,8 @@ ovs_be64 oxm_bitmap_from_mf_bitmap(const struct mf_bitmap 
*, enum ofp_version);
 struct mf_bitmap oxm_bitmap_to_mf_bitmap(ovs_be64 oxm_bitmap,
  enum ofp_version);
 struct mf_bitmap oxm_writable_fields(void);
-struct mf_bitmap oxm_matchable_fields(void);
-struct mf_bitmap oxm_maskable_fields(void);
+struct mf_bitmap oxm_matchable_fields(bool packet_type_aware);
+struct mf_bitmap oxm_maskable_fields(bool packet_type_aware);
 
 /* Dealing with the 'ofs_nbits' members in several Nicira extensions. */
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index bcee839..7331b02 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -108,6 +108,7 @@ struct xbridge {
 
 bool has_in_band; /* Bridge has in band control? */
 bool forward_bpdu;/* Bridge forwards STP BPDUs? */
+bool packet_type_aware;   /* Bridge is packet-type-aware? */
 
 /* Datapath feature support. */
 struct dpif_backer_support support;
@@ -555,6 +556,7 @@ static void xlate_xbridge_set(struct xbridge *, struct dpif 
*,
   const struct dpif_ipfix *,
   const struct netflow *,
   bool forward_bpdu, bool has_in_band,
+  bool packet_type_aware,
   const struct dpif_backer_support *);
 static void xlate_xbundle_set(struct xbundle *xbundle,
   enum port_vlan_mode vlan_mode,
@@ -815,6 +817,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
   const struct dpif_ipfix *ipfix,
   const struct netflow *netflow,
   bool forward_bpdu, bool has_in_band,
+  bool packet_type_aware,
   const struct dpif_backer_support *support)
 {
 if (xbridge->ml != ml) {
@@ -860,6 +863,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
 xbridge->dpif = dpif;
 xbridge->forward_bpdu = forward_bpdu;
 xbridge->has_in_band = has_in_band;
+xbridge->packet_type_aware = packet_type_aware;
 xbridge->support = *support;
 }
 
@@ -950,7 +954,7 @@ xlate_xbridge_copy(struct xbridge *xbridge)
   xbridge->rstp, xbridge->ms, xbridge->mbridge,
   xbridge->sflow, xbridge->ipfix, xbridge->netflow,
   xbridge->forward_bpdu, xbridge->has_in_band,
- 

[ovs-dev] [PATCH v2 1/6] userspace: Add OXM field MFF_PACKET_TYPE

[Zoltán Balogh]

From: Jan Scheurich 

Allow packet type namespace OFPHTN_ETHERTYPE as alternative pre-requisite
for matching L3 protocols (MPLS, IP, IPv6, ARP etc).

Change the meta-flow definition of packet_type field to use the new
custom format MFS_PACKET_TYPE representing "(NS,NS_TYPE)".

Parsing routine for MFS_PACKET_TYPE added to meta-flow.c. Formatting
routine for field packet_type extracted from match_format() and moved to
flow.c to be used from meta-flow.c for formatting MFS_PACKET_TYPE.

Updated the ovs-fields man page source meta-flow.xml with documentation
for packet-type-aware bridges and added documentation for field packet_type.

Added packet_type to the matching properties in tests/ofproto.at. Should be
removed later, when packet_type_aware bridge attribute will be introduced.

Signed-off-by: Jan Scheurich 
---
 build-aux/extract-ofp-fields|   3 +-
 include/openvswitch/meta-flow.h |  20 
 lib/flow.c  |  18 +++
 lib/flow.h  |  28 ---
 lib/match.c |  52 +---
 lib/meta-flow.c |  85 +---
 lib/meta-flow.xml   | 106 +++-
 lib/nx-match.c  |  16 --
 lib/odp-util.c  |  48 --
 lib/ofp-parse.c |   6 +++
 lib/ofp-util.c  |  42 +++-
 tests/dpif-netdev.at|  14 +++---
 tests/odp.at|   1 +
 tests/ofproto-dpif.at   |  20 
 tests/ofproto.at|   1 +
 tests/pmd.at|   2 +-
 tests/tunnel-push-pop-ipv6.at   |   2 +-
 tests/tunnel-push-pop.at|   2 +-
 18 files changed, 361 insertions(+), 105 deletions(-)

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index d5b8a82..24dd756 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -36,7 +36,8 @@ FORMATTING = {"decimal":("MFS_DECIMAL",  1,   
8),
   "OpenFlow 1.1+ port": ("MFS_OFP_PORT_OXM", 4,   4),
   "frag":   ("MFS_FRAG", 1,   1),
   "tunnel flags":   ("MFS_TNL_FLAGS",2,   2),
-  "TCP flags":  ("MFS_TCP_FLAGS",2,   2)}
+  "TCP flags":  ("MFS_TCP_FLAGS",2,   2),
+  "packet type":("MFS_PACKET_TYPE",  4,   4)}
 
 PREREQS = {"none": "MFP_NONE",
"Ethernet": "MFP_ETHERNET",
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index c284ec6..c6b95df 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -132,6 +132,11 @@ struct ofputil_tlv_table_mod;
  *
  *   TCP flags: See the description of tcp_flags in ovs-ofctl(8).
  *
+ *   packet type: A pair of packet type namespace NS and NS_TYPE within
+ *   that namespace "(NS,NS_TYPE)". NS and NS_TYPE are formatted in
+ *   decimal or hexadecimal as and accept decimal and hexadecimal (with
+ *   0x prefix) at parsing.
+ *
  *   Prerequisites:
  *
  * The field's prerequisites.  The values should be straightfoward.
@@ -247,6 +252,20 @@ enum OVS_PACKED_ENUM mf_field_id {
  */
 MFF_RECIRC_ID,
 
+/* "packet_type".
+ *
+ * Define the packet type in OpenFlow 1.3+.
+ *
+ * Type: be32.
+ * Maskable: no.
+ * Formatting: packet type.
+ * Prerequisites: none.
+ * Access: read-only.
+ * NXM: none.
+ * OXM: OXM_OF_PACKET_TYPE(44) since OF1.3 and v2.8.
+ */
+MFF_PACKET_TYPE,
+
 /* "conj_id".
  *
  * ID for "conjunction" actions.  Please refer to ovs-ofctl(8)
@@ -1859,6 +1878,7 @@ enum OVS_PACKED_ENUM mf_string {
 MFS_FRAG,   /* no, yes, first, later, not_later */
 MFS_TNL_FLAGS,  /* FLOW_TNL_F_* flags */
 MFS_TCP_FLAGS,  /* TCP_* flags */
+MFS_PACKET_TYPE,/* "(NS,NS_TYPE)" */
 };
 
 struct mf_field {
diff --git a/lib/flow.c b/lib/flow.c
index 7f98a46..4f7a041 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1149,6 +1149,24 @@ format_flags_masked(struct ds *ds, const char *name,
 }
 }
 
+void
+format_packet_type_masked(struct ds *s, const ovs_be32 value, const ovs_be32 
mask)
+{
+if (pt_ns_type_be(mask) == 0) {
+ds_put_format(s, "packet_type=(%u,*),",
+  pt_ns(value));
+} else if (pt_ns_type_be(mask) == OVS_BE16_MAX) {
+ds_put_format(s, "packet_type=(%u,0x%"PRIx16"),",
+  pt_ns(value),
+  pt_ns_type(value));
+} else{
+ds_put_format(s, "packet_type=(%u,0x%"PRIx16"/0x%"PRIx16"),",
+  pt_ns(value),
+  pt_ns_type(value),
+  pt_ns_type(mask));
+}
+}
+
 /* Scans a string 's' of flags to determine their numerical value and
  * returns the number of 

[ovs-dev] [PATCH v2 0/6] userspace: Packet type-aware pipeline

[Zoltán Balogh]

This patch set is the 2nd part of an initiative presented by Jan Scheurich: 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332367.html

It takes the patch set referred by the link above as a bases.

The goal is to deal with non-Ethernet packets in OVS for advanced use cases 
like L3 tunneling or NSH. The initiative is centering on the new OpenFlow 
concepts of "Packet type-aware pipeline" (PTAP) and "Generic encap/decap 
actions" (EXT-382). The overall design is documented in:
https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8

These patches introduce the OXM field MFF_PACKET_TYPE, the 'packet-type-aware' 
bridge property, add support for versatile tunnel ports, provide new unit 
tests, implement packet-in for non-Ethernet packets and introduce OF 1.5 
packet-out handling.

The present series v2 supersedes v1 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331023.html), 
V2 was rebased to v6 of "userspace: Support for L3 tunneling" patch series 
mentioned before. It fixes warnings and some of its patches were merged 
together.

 userspace: Add OXM field MFF_PACKET_TYPE
 userspace: Add bridge property 'packet-type-aware'
 userspace: Handling of versatile tunnel ports
 tests: Added unit tests in packet-type-aware.at
 userspace: Complete Packet In handling
 userspace: Introduce OF 1.5 packet-out

Changes v1 -> v2:
* Rebased to v6 of "userspace: Support for L3 tunneling" patch series
* Fixed warnings

 build-aux/extract-ofp-fields|   3 +-
 include/openflow/openflow-1.5.h |  17 +
 include/openvswitch/meta-flow.h |  20 ++
 include/openvswitch/ofp-msgs.h  |   7 +-
 include/openvswitch/ofp-util.h  |   1 +
 lib/flow.c  |  59 ++-
 lib/flow.h  |  28 +-
 lib/match.c |  52 ++-
 lib/meta-flow.c |  85 -
 lib/meta-flow.xml   | 106 --
 lib/netdev-native-tnl.c |  20 +-
 lib/nx-match.c  |  26 +-
 lib/nx-match.h  |   4 +-
 lib/odp-util.c  |  48 ++-
 lib/ofp-parse.c |  17 +
 lib/ofp-print.c |   6 +-
 lib/ofp-util.c  |  99 -
 ofproto/ofproto-dpif-xlate.c|  49 +--
 ofproto/ofproto-dpif-xlate.h|   1 +
 ofproto/ofproto-dpif.c  |   1 +
 ofproto/ofproto-provider.h  |   1 +
 ofproto/ofproto.c   |  29 +-
 ofproto/ofproto.h   |   1 +
 ofproto/tunnel.c|  24 +-
 ofproto/tunnel.h|   2 +-
 tests/automake.mk   |   6 +-
 tests/dpif-netdev.at|  14 +-
 tests/odp.at|   1 +
 tests/ofproto-dpif.at   |  20 +-
 tests/ofproto.at| 108 ++
 tests/packet-type-aware.at  | 540 
 tests/pmd.at|   2 +-
 tests/system-userspace-packet-type-aware.at | 422 ++
 tests/system-userspace-testsuite.at |   1 +
 tests/testsuite.at  |   1 +
 tests/tunnel-push-pop-ipv6.at   |   2 +-
 tests/tunnel-push-pop.at|   2 +-
 utilities/ovs-ofctl.c   |   1 +
 vswitchd/bridge.c   |   9 +
 vswitchd/vswitch.xml|   9 +
 40 files changed, 1673 insertions(+), 171 deletions(-)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 1/2] doc: Reduce duplication in 'man_pages'

[Stephen Finucane]

On Fri, 2017-05-19 at 09:11 +, Bodireddy, Bhanuprakash wrote:
> The make fails with below error on my fedora 22 Target. 
> 
> error.log--
> 
> Documentation/conf.py:126:9: F812 list comprehension redefines
> 'filename' from line 59
> Makefile:5848: recipe for target 'flake8-check' failed
> make[2]: *** [flake8-check] Error 1
> make[2]: Leaving directory '/workspace/master/ovs'
> Makefile:5182: recipe for target 'all-recursive' failed
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory '/workspace/master/ovs'
> Makefile:3014: recipe for target 'all' failed
> make: *** [all] Error 2
> 
> I don't see this issue when I replace the filename with 'file_name'
> as below.  
> 
> diff --git a/Documentation/conf.py b/Documentation/conf.py
> index d70ee6b..77c4df5 100644
> --- a/Documentation/conf.py
> +++ b/Documentation/conf.py
> @@ -121,6 +121,6 @@ _man_pages = [
> 
>  # Generate list of (path, name, description, [author, ...], section)
>  man_pages = [
> -('ref/%s' % filename, filename.split('.', 1)[0],
> - description, [author], filename.split('.', 1)[1])
> -for filename, description in _man_pages]
> +('ref/%s' % file_name, file_name.split('.', 1)[0],
> + description, [author], file_name.split('.', 1)[1])
> +for file_name, description in _man_pages]
> 
> Regards,
> Bhanuprakash. 

Damn - I missed that. Patch submitted to fix this now.

Stephen

> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Ben Pfaff
> > Sent: Thursday, May 18, 2017 11:50 PM
> > To: Stephen Finucane 
> > Cc: d...@openvswitch.org
> > Subject: Re: [ovs-dev] [RFC 1/2] doc: Reduce duplication in
> > 'man_pages'
> > 
> > On Wed, May 10, 2017 at 09:32:18PM -0400, Stephen Finucane wrote:
> > > All these entries are going to be roughly the same, with only two
> > > key
> > > differences. Clarify things by focusing on those differences.
> > > 
> > > Signed-off-by: Stephen Finucane 
> > 
> > Fair enough!  Thanks, I applied this to master.
> > ___
> > 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] doc: Resolve pep8 warnings in conf.py

[Stephen Finucane]

flake8 doesn't like us redefining variables in loops.

Signed-off-by: Stephen Finucane 
Reported-by: Bhanuprakash Bodireddy 
Fixes: f15010f ("doc: Reduce duplication in 'man_pages'")
Cc: Ben Pfaff 
---
 Documentation/conf.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index d70ee6b..77c4df5 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -121,6 +121,6 @@ _man_pages = [
 
 # Generate list of (path, name, description, [author, ...], section)
 man_pages = [
-('ref/%s' % filename, filename.split('.', 1)[0],
- description, [author], filename.split('.', 1)[1])
-for filename, description in _man_pages]
+('ref/%s' % file_name, file_name.split('.', 1)[0],
+ description, [author], file_name.split('.', 1)[1])
+for file_name, description in _man_pages]
-- 
2.9.3

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

Re: [ovs-dev] [RFC 1/2] doc: Reduce duplication in 'man_pages'

[Bodireddy, Bhanuprakash]

The make fails with below error on my fedora 22 Target. 

error.log--
Documentation/conf.py:126:9: F812 list comprehension redefines 'filename' from 
line 59
Makefile:5848: recipe for target 'flake8-check' failed
make[2]: *** [flake8-check] Error 1
make[2]: Leaving directory '/workspace/master/ovs'
Makefile:5182: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/workspace/master/ovs'
Makefile:3014: recipe for target 'all' failed
make: *** [all] Error 2

I don't see this issue when I replace the filename with 'file_name' as below.  

diff --git a/Documentation/conf.py b/Documentation/conf.py
index d70ee6b..77c4df5 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -121,6 +121,6 @@ _man_pages = [

 # Generate list of (path, name, description, [author, ...], section)
 man_pages = [
-('ref/%s' % filename, filename.split('.', 1)[0],
- description, [author], filename.split('.', 1)[1])
-for filename, description in _man_pages]
+('ref/%s' % file_name, file_name.split('.', 1)[0],
+ description, [author], file_name.split('.', 1)[1])
+for file_name, description in _man_pages]

Regards,
Bhanuprakash. 

>-Original Message-
>From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
>boun...@openvswitch.org] On Behalf Of Ben Pfaff
>Sent: Thursday, May 18, 2017 11:50 PM
>To: Stephen Finucane 
>Cc: d...@openvswitch.org
>Subject: Re: [ovs-dev] [RFC 1/2] doc: Reduce duplication in 'man_pages'
>
>On Wed, May 10, 2017 at 09:32:18PM -0400, Stephen Finucane wrote:
>> All these entries are going to be roughly the same, with only two key
>> differences. Clarify things by focusing on those differences.
>>
>> Signed-off-by: Stephen Finucane 
>
>Fair enough!  Thanks, I applied this to master.
>___
>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 v5] netdev-dpdk: fix ifindex assignment for DPDK ports

[Przemyslaw Lal]

On 18/05/2017 22:41, Ben Pfaff wrote:


On Thu, May 18, 2017 at 06:09:21PM +, Darrell Ball wrote:


On 4/4/17, 5:47 PM, "Darrell Ball"  wrote:

 
 
 On 4/4/17, 3:09 AM, "Lal, PrzemyslawX"  wrote:
 
 On 04/04/2017 06:14, Darrell Ball wrote:
 
 >

 > On 4/3/17, 5:27 AM, "ovs-dev-boun...@openvswitch.org on behalf of Przemyslaw 
Lal"  wrote:
 >
 >  In current implementation port_id is used as an ifindex for all 
netdev-dpdk
 >  interfaces.
 >
 >  For physical DPDK interfaces using port_id as ifindex causes 
that '0' is set as
 >  ifindex for 'dpdk0' interface, '1' for 'dpdk1' and so on. For 
the DPDK vHost
 >  interfaces ifindexes are not even assigned (0 is used by 
default) due to the
 >  fact that vHost ports don't use port_id field from the DPDK 
library.
 >
 >  This causes multiple negative side-effects. First of all 0 is 
an invalid
 >  ifindex value. The other issue is possible overlapping of 
'dpdkX' interfaces
 >  ifindex values with the ifindexes of kernel space interfaces 
which may cause
 >  problems in any external tools that use those values. Neither 
'dpdk0', nor any
 >  DPDK vHost interfaces are visible in sFlow collector tools, as 
all interfaces
 >  with ifindexes smaller than 1 are ignored.
 >
 >  Proposed solution to these issues is to calculate a hash of 
interface's name
 >  and use calculated value as an ifindex. This way interfaces 
keep their
 >  ifindexes during OVS-DPDK restarts, ports re-initialization 
events, etc., show
 >  up in sFlow collectors and meet RFC 2863 specification 
regarding re-using
 >  ifindex values by the same virtual interfaces and maximum 
ifindex value.
 >
 >  Signed-off-by: Przemyslaw Lal 
 >  ---
 >   lib/netdev-dpdk.c | 6 +-
 >   1 file changed, 5 insertions(+), 1 deletion(-)
 >
 >  diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
 >  index ddc651b..687b0a5 100644
 >  --- a/lib/netdev-dpdk.c
 >  +++ b/lib/netdev-dpdk.c
 >  @@ -2215,7 +2215,11 @@ netdev_dpdk_get_ifindex(const struct 
netdev *netdev)
 >   int ifindex;
 >
 >   ovs_mutex_lock(>mutex);
 >  -ifindex = dev->port_id;
 >  +/* Calculate hash from the netdev name. Ensure that 
ifindex is a 24-bit
 >  + * postive integer to meet RFC 2863 recommendations.
 >  + */
 >  +uint32_t h = hash_string(netdev->name, 0);
 >  +ifindex = (int)(h % 0xfe + 1);
 >
 >
 > If user configuration was supported, enforcing uniqueness would be 
the
 > responsibility of the user.
 
 This was already discussed on this mailing list and outcome was that while hash is not perfect, it is the best solution for now.

 Also, please keep in mind that names of the physical DPDK devices and 
dpdkvhostuser interfaces are configurable, so user can still enforce
 uniqueness.
 
 I know uniqueness could be enforced by trial and error of name selection.

 I saw the comment
  “At some point, with vhost-pmd we will have port_ids also for vhost 
interfaces.
Maybe we can revisit this approach when that becomes available.”
 
 If others are fine, then so am I.


The uniqueness issue is understood and there is a workaround capability to
address it.

Let us just fold the patch in, since the patch has been out for long enough to
receive feedback.

Acked by: Darrell Ball 

Thanks Przemyslaw and Darrell.  I applied this to master.  I simplified
the code since the cast to int was a no-op (it does the same thing as
the conversion implied by the assignment:

--8<--cut here-->8--

From: Przemyslaw Lal 
Date: Mon, 3 Apr 2017 13:27:47 +0100
Subject: [PATCH] netdev-dpdk: fix ifindex assignment for DPDK ports

In current implementation port_id is used as an ifindex for all netdev-dpdk
interfaces.

For physical DPDK interfaces using port_id as ifindex causes that '0' is set as
ifindex for 'dpdk0' interface, '1' for 'dpdk1' and so on. For the DPDK vHost
interfaces ifindexes are not even assigned (0 is used by default) due to the
fact that vHost ports don't use port_id field from the DPDK library.

This causes multiple negative side-effects. First of all 0 is an invalid
ifindex value. The other issue is possible overlapping of 'dpdkX' interfaces
ifindex values with the ifindexes of kernel 

[ovs-dev] [PATCH v3 2/5] rstp: Add rstp port name for human reading.

[nickcooper-zhangtonghao]

This patch is useful to debug rstp subsystem and log the
port name instead of port number. This patch will also
be used to display rstp info for next patches.

Signed-off-by: nickcooper-zhangtonghao 
Acked-by: Jarno Rajahalme 
---
 lib/rstp-common.h  |  1 +
 lib/rstp.c | 14 +-
 lib/rstp.h |  3 ++-
 ofproto/ofproto-dpif.c |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/rstp-common.h b/lib/rstp-common.h
index 27e8079..c108232 100644
--- a/lib/rstp-common.h
+++ b/lib/rstp-common.h
@@ -262,6 +262,7 @@ struct rstp_port {
 struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
 struct hmap_node node OVS_GUARDED_BY(rstp_mutex); /* In rstp->ports. */
 void *aux OVS_GUARDED_BY(rstp_mutex);
+char *port_name;
 struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
 /*
  * MAC status parameters
diff --git a/lib/rstp.c b/lib/rstp.c
index 9ad1b0d..48df7ad 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -744,6 +744,14 @@ rstp_port_set_port_number__(struct rstp_port *port, 
uint16_t port_number)
 }
 }
 
+static void
+rstp_port_set_port_name__(struct rstp_port *port, const char *name)
+OVS_REQUIRES(rstp_mutex)
+{
+free(port->port_name);
+port->port_name = xstrdup(name);
+}
+
 /* Converts the link speed to a port path cost [Table 17-3]. */
 uint32_t
 rstp_convert_speed_to_cost(unsigned int speed)
@@ -1150,6 +1158,7 @@ rstp_add_port(struct rstp *rstp)
 rstp_port_set_priority__(p, RSTP_DEFAULT_PORT_PRIORITY);
 rstp_port_set_port_number__(p, 0);
 p->aux = NULL;
+p->port_name = NULL;
 rstp_initialize_port_defaults__(p);
 VLOG_DBG("%s: RSTP port "RSTP_PORT_ID_FMT" initialized.", rstp->name,
  p->port_id);
@@ -1185,6 +1194,7 @@ rstp_port_unref(struct rstp_port *rp)
 ovs_mutex_lock(_mutex);
 rstp = rp->rstp;
 rstp_port_set_state__(rp, RSTP_DISABLED);
+free(rp->port_name);
 hmap_remove(>ports, >node);
 VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
  rp->port_id);
@@ -1414,12 +1424,14 @@ void
 rstp_port_set(struct rstp_port *port, uint16_t port_num, int priority,
   uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
   enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state,
-  bool admin_port_state, bool do_mcheck, void *aux)
+  bool admin_port_state, bool do_mcheck, void *aux,
+  const char *name)
 {
 ovs_mutex_lock(_mutex);
 port->aux = aux;
 rstp_port_set_priority__(port, priority);
 rstp_port_set_port_number__(port, port_num);
+rstp_port_set_port_name__(port, name);
 rstp_port_set_path_cost__(port, path_cost);
 rstp_port_set_admin_edge__(port, is_admin_edge);
 rstp_port_set_auto_edge__(port, is_auto_edge);
diff --git a/lib/rstp.h b/lib/rstp.h
index 7a08228..b6f9089 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -194,7 +194,8 @@ uint32_t rstp_convert_speed_to_cost(unsigned int speed);
 void rstp_port_set(struct rstp_port *, uint16_t port_num, int priority,
uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
enum rstp_admin_point_to_point_mac_state 
admin_p2p_mac_state,
-   bool admin_port_state, bool do_mcheck, void *aux);
+   bool admin_port_state, bool do_mcheck, void *aux,
+   const char *name);
 
 enum rstp_state rstp_port_get_state(const struct rstp_port *);
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index dc5f004..6d3f1fb 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2722,7 +2722,7 @@ set_rstp_port(struct ofport *ofport_,
 rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
   s->admin_edge_port, s->auto_edge,
   s->admin_p2p_mac_state, s->admin_port_state, s->mcheck,
-  ofport);
+  ofport, netdev_get_name(ofport->up.netdev));
 update_rstp_port_state(ofport);
 /* Synchronize operational status. */
 rstp_port_set_mac_operational(rp, ofport->may_enable);
-- 
1.8.3.1



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

[ovs-dev] [PATCH v3 5/5] stp: Add link-state checking support for stp ports.

[nickcooper-zhangtonghao]

When bridge stp enabled, we can enable the stp ports
despite ports are down. When initializing, this patch checks
link-state of ports and enable or disable them according
to their link-state. This patch also allow user to enable
and disable a port when bridge stp is running. If a stp
port is in disable state, it can forward packets. If its
link is down and this patch sets it to disable, there is
no L2 loop.

Signed-off-by: nickcooper-zhangtonghao 
---
 ofproto/ofproto-dpif.c | 41 -
 tests/stp.at   | 71 +-
 2 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6d3f1fb..098d363 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2574,6 +2574,37 @@ update_stp_port_state(struct ofport_dpif *ofport)
 }
 }
 
+static void
+stp_check_and_update_link_state(struct ofproto_dpif *ofproto)
+{
+struct ofport *ofport_;
+struct ofport_dpif *ofport;
+bool up;
+
+HMAP_FOR_EACH (ofport_, hmap_node, >up.ports) {
+ofport = ofport_dpif_cast(ofport_);
+up = netdev_get_carrier(ofport_->netdev);
+
+if (ofport->stp_port &&
+up != (stp_port_get_state(ofport->stp_port) != STP_DISABLED)) {
+
+VLOG_DBG("bridge: %s, port: %s is %s, %s it", ofproto->up.name,
+ netdev_get_name(ofport->up.netdev),
+ up ? "up" : "down",
+ up ? "enabling" : "disabling");
+
+if (up) {
+stp_port_enable(ofport->stp_port);
+stp_port_set_aux(ofport->stp_port, ofport);
+} else {
+stp_port_disable(ofport->stp_port);
+}
+
+update_stp_port_state(ofport);
+}
+}
+}
+
 /* Configures STP on 'ofport_' using the settings defined in 's'.  The
  * caller is responsible for assigning STP port numbers and ensuring
  * there are no duplicates. */
@@ -2604,7 +2635,12 @@ set_stp_port(struct ofport *ofport_,
 /* Set name before enabling the port so that debugging messages can print
  * the name. */
 stp_port_set_name(sp, netdev_get_name(ofport->up.netdev));
-stp_port_enable(sp);
+
+if (netdev_get_carrier(ofport_->netdev)) {
+stp_port_enable(sp);
+} else {
+stp_port_disable(sp);
+}
 
 stp_port_set_aux(sp, ofport);
 stp_port_set_priority(sp, s->priority);
@@ -2666,6 +2702,9 @@ stp_run(struct ofproto_dpif *ofproto)
 stp_tick(ofproto->stp, MIN(INT_MAX, elapsed));
 ofproto->stp_last_tick = now;
 }
+
+stp_check_and_update_link_state(ofproto);
+
 while (stp_get_changed_port(ofproto->stp, )) {
 struct ofport_dpif *ofport = stp_port_get_aux(sp);
 
diff --git a/tests/stp.at b/tests/stp.at
index bd5d208..e27600e 100644
--- a/tests/stp.at
+++ b/tests/stp.at
@@ -420,6 +420,7 @@ AT_CHECK([ovs-vsctl add-port br1 p8 -- \
set port p8 other_config:stp-enable=false -- \
 ])
 
+ovs-appctl netdev-dummy/set-admin-state up
 ovs-appctl time/stop
 
 AT_CHECK([ovs-ofctl add-flow br0 "in_port=7 icmp actions=1"])
@@ -519,7 +520,7 @@ AT_CHECK([
 set interface p6 type=dummy options:pstream=punix:$OVS_RUNDIR/p6.sock 
ofport_request=6
 ], [0])
 
-
+ovs-appctl netdev-dummy/set-admin-state up
 ovs-appctl time/stop
 
 # give time for STP to move initially
@@ -626,5 +627,73 @@ AT_CHECK([ovs-appctl mdb/show br2], [0], [dnl
  port  VLAN  GROUPAge
 ])
 
+AT_CLEANUP
+
+AT_SETUP([STP - check link-state when stp is running])
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+ovs-vsctl -- \
+set port br0 other_config:stp-enable=false -- \
+set bridge br0 datapath-type=dummy stp_enable=true \
+other-config:hwaddr=aa:66:aa:66:00:00
+], [0])
+
+AT_CHECK([
+ovs-vsctl add-port br0 p1 -- \
+set interface p1 type=dummy -- \
+set port p1 other_config:stp-port-num=1
+ovs-vsctl add-port br0 p2 -- \
+set interface p2 type=dummy -- \
+set port p2 other_config:stp-port-num=2
+], [0])
+
+ovs-appctl netdev-dummy/set-admin-state up
+ovs-appctl time/stop
+
+# give time for STP to move initially
+for i in $(seq 0 30); do
+ovs-appctl time/warp 1000
+done
+
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+   p1 designated forwarding 19128.1
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
+   p2 designated forwarding 19128.2
+])
+
+# add a stp port
+AT_CHECK([
+ovs-vsctl add-port br0 p3 -- \
+set interface p3 type=dummy -- \
+set port p3 other_config:stp-port-num=3
+], [0])
+
+ovs-appctl netdev-dummy/set-admin-state p3 down
+
+# We should not show the p3 because its link-state is down
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+   p1 designated forwarding 19128.1
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
+   p2 

[ovs-dev] [PATCH v3 4/5] rstp: Increment the rstp port num counter.

[nickcooper-zhangtonghao]

OvS only supports RSTP_MAX_PORTS rstp ports while max port
num of stp is STP_MAX_PORTS. This patch increments the rstp
port num counter, otherwise the counter is 0 and the checking
above will always fail.

Signed-off-by: nickcooper-zhangtonghao 
---
 vswitchd/bridge.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 972146e..5a08219 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1485,6 +1485,10 @@ port_configure_rstp(const struct ofproto *ofproto, 
struct port *port,
 port_s->port_num = 0;
 }
 
+/* Increment the port num counter, because we only support
+ * RSTP_MAX_PORTS rstp ports. */
+(*port_num_counter) ++;
+
 config_str = smap_get(>cfg->other_config, "rstp-path-cost");
 if (config_str) {
 port_s->path_cost = strtoul(config_str, NULL, 10);
-- 
1.8.3.1



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

[ovs-dev] [PATCH v3 3/5] rstp: Add the 'ovs-appctl rstp/show' command.

[nickcooper-zhangtonghao]

The rstp/show command will help users and developers to
get more details about rstp. This patch works together with
the previous patches.

Signed-off-by: nickcooper-zhangtonghao 
---
 NEWS   |   3 +-
 lib/rstp.c | 114 +++--
 lib/rstp.h |   2 +-
 vswitchd/ovs-vswitchd.8.in |  11 -
 4 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 7a2b185..4576408 100644
--- a/NEWS
+++ b/NEWS
@@ -28,7 +28,8 @@ Post-v2.7.0
abbreviated to 4 hex digits.
  * "ovn-sbctl lflow-list" can now print OpenFlow flows that correspond
to logical flows.
-   - Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
+   - Add the command 'ovs-appctl stp/show' and 'ovs-appctl rstp/show'
+ (see ovs-vswitchd(8)).
- OpenFlow:
  * All features required by OpenFlow 1.4 are now implemented, so
ovs-vswitchd now enables OpenFlow 1.4 by default (in addition to
diff --git a/lib/rstp.c b/lib/rstp.c
index 48df7ad..095e01f 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -120,6 +120,10 @@ static void rstp_port_set_mcheck__(struct rstp_port *, 
bool mcheck)
 OVS_REQUIRES(rstp_mutex);
 static void reinitialize_port__(struct rstp_port *p)
 OVS_REQUIRES(rstp_mutex);
+static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
+ const char *argv[], void *aux);
+static void rstp_unixctl_show(struct unixctl_conn *, int argc,
+  const char *argv[], void *aux);
 
 const char *
 rstp_state_name(enum rstp_state state)
@@ -205,9 +209,6 @@ rstp_port_get_number(const struct rstp_port *p)
 return number;
 }
 
-static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
- const char *argv[], void *aux);
-
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
@@ -240,6 +241,8 @@ rstp_init(void)
 
 unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
  NULL);
+unixctl_command_register("rstp/show", "[bridge]", 0, 1,
+ rstp_unixctl_show, NULL);
 ovsthread_once_done();
 }
 }
@@ -1367,7 +1370,7 @@ rstp_get_designated_root(const struct rstp *rstp)
  * there is no such port.
  */
 struct rstp_port *
-rstp_get_root_port(struct rstp *rstp)
+rstp_get_root_port(const struct rstp *rstp)
 {
 struct rstp_port *p;
 
@@ -1506,3 +1509,106 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
 out:
 ovs_mutex_unlock(_mutex);
 }
+
+static void
+rstp_bridge_id_details(struct ds *ds, const rstp_identifier bridge_id,
+   const uint16_t hello_time, const uint16_t max_age,
+   const uint16_t forward_delay)
+OVS_REQUIRES(rstp_mutex)
+{
+uint16_t priority = bridge_id >> 48;
+ds_put_format(ds, "\tstp-priority\t%"PRIu16"\n", priority);
+
+struct eth_addr mac;
+const uint64_t mac_bits = (UINT64_C(1) << 48) - 1;
+eth_addr_from_uint64(bridge_id & mac_bits, );
+ds_put_format(ds, "\tstp-system-id\t"ETH_ADDR_FMT"\n", ETH_ADDR_ARGS(mac));
+ds_put_format(ds, "\tstp-hello-time\t%"PRIu16"s\n", hello_time);
+ds_put_format(ds, "\tstp-max-age\t%"PRIu16"s\n", max_age);
+ds_put_format(ds, "\tstp-fwd-delay\t%"PRIu16"s\n", forward_delay);
+}
+
+static void
+rstp_print_details(struct ds *ds, const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+ds_put_format(ds, " %s \n", rstp->name);
+ds_put_cstr(ds, "Root ID:\n");
+
+bool is_root = rstp_is_root_bridge(rstp);
+struct rstp_port *p = rstp_get_root_port(rstp);
+
+rstp_identifier bridge_id =
+is_root ? rstp->bridge_identifier : rstp_get_root_id(rstp);
+uint16_t hello_time =
+is_root ? rstp->bridge_hello_time : p->designated_times.hello_time;
+uint16_t max_age =
+is_root ? rstp->bridge_max_age : p->designated_times.max_age;
+uint16_t forward_delay =
+is_root ? rstp->bridge_forward_delay : 
p->designated_times.forward_delay;
+
+rstp_bridge_id_details(ds, bridge_id, hello_time, max_age, forward_delay);
+if (is_root) {
+ds_put_cstr(ds, "\tThis bridge is the root\n");
+} else {
+ds_put_format(ds, "\troot-port\t%s\n", p->port_name);
+ds_put_format(ds, "\troot-path-cost\t%u\n",
+  rstp_get_root_path_cost(rstp));
+}
+ds_put_cstr(ds, "\n");
+
+ds_put_cstr(ds, "Bridge ID:\n");
+rstp_bridge_id_details(ds, rstp->bridge_identifier,
+   rstp->bridge_hello_time,
+   rstp->bridge_max_age,
+   rstp->bridge_forward_delay);
+ds_put_cstr(ds, "\n");
+
+ds_put_format(ds, "\t%-11.10s%-11.10s%-11.10s%-9.8s%-8.7s\n",
+  "Interface", "Role", "State", "Cost", "Pri.Nbr");
+ds_put_cstr(ds, "\t-- -- --  

[ovs-dev] [PATCH v3 1/5] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

* This patch will be used for next patch. The 'rstp/show' command,
which uses the mutex, calls functions which also use the mutex.
We should init it as a recursive mutex.

* Because of recursive mutex, this patch remove the OVS_EXCLUDED in
list/rstp.[ch] files.

* Some rstp tests of OvS, which run with ovstest, does not run rstp_init
in the bridge_init. We should call rstp_init when creating the rstp
and stp also do that, otherwise tests fail.

Signed-off-by: nickcooper-zhangtonghao 
---
 lib/rstp.c |  54 ++---
 lib/rstp.h | 131 +
 2 files changed, 56 insertions(+), 129 deletions(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 907a907..9ad1b0d 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -50,7 +50,7 @@
 
 VLOG_DEFINE_THIS_MODULE(rstp);
 
-struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
+struct ovs_mutex rstp_mutex;
 
 static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(_rstps__);
 static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
_rstps__;
@@ -161,7 +161,6 @@ rstp_port_role_name(enum rstp_port_role role)
  * while taking a new reference. */
 struct rstp *
 rstp_ref(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 if (rstp) {
 ovs_refcount_ref(>ref_cnt);
@@ -172,7 +171,6 @@ rstp_ref(struct rstp *rstp)
 /* Frees RSTP struct when reference count reaches zero. */
 void
 rstp_unref(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 if (rstp && ovs_refcount_unref_relaxed(>ref_cnt) == 1) {
 ovs_mutex_lock(_mutex);
@@ -197,7 +195,6 @@ rstp_unref(struct rstp *rstp)
  * port_number). */
 int
 rstp_port_get_number(const struct rstp_port *p)
-OVS_EXCLUDED(rstp_mutex)
 {
 int number;
 
@@ -214,7 +211,6 @@ static void rstp_unixctl_tcn(struct unixctl_conn *, int 
argc,
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 decrease_rstp_port_timers__(rstp);
@@ -225,7 +221,6 @@ rstp_tick_timers(struct rstp *rstp)
 void
 rstp_port_received_bpdu(struct rstp_port *rp, const void *bpdu,
 size_t bpdu_size)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 /* Only process packets on ports that have RSTP enabled. */
@@ -237,10 +232,16 @@ rstp_port_received_bpdu(struct rstp_port *rp, const void 
*bpdu,
 
 void
 rstp_init(void)
-OVS_EXCLUDED(rstp_mutex)
 {
-unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
- NULL);
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+if (ovsthread_once_start()) {
+ovs_mutex_init_recursive(_mutex);
+
+unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
+ NULL);
+ovsthread_once_done();
+}
 }
 
 /* Creates and returns a new RSTP instance that initially has no ports. */
@@ -249,12 +250,13 @@ rstp_create(const char *name, rstp_identifier 
bridge_address,
 void (*send_bpdu)(struct dp_packet *bpdu, void *port_aux,
   void *rstp_aux),
 void *aux)
-OVS_EXCLUDED(rstp_mutex)
 {
 struct rstp *rstp;
 
 VLOG_DBG("Creating RSTP instance");
 
+rstp_init();
+
 rstp = xzalloc(sizeof *rstp);
 rstp->name = xstrdup(name);
 
@@ -338,7 +340,6 @@ rstp_set_bridge_address__(struct rstp *rstp, 
rstp_identifier bridge_address)
 /* Sets the bridge address. */
 void
 rstp_set_bridge_address(struct rstp *rstp, rstp_identifier bridge_address)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_address__(rstp, bridge_address);
@@ -347,7 +348,6 @@ rstp_set_bridge_address(struct rstp *rstp, rstp_identifier 
bridge_address)
 
 const char *
 rstp_get_name(const struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 char *name;
 
@@ -359,7 +359,6 @@ rstp_get_name(const struct rstp *rstp)
 
 rstp_identifier
 rstp_get_bridge_id(const struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 rstp_identifier bridge_id;
 
@@ -391,7 +390,6 @@ rstp_set_bridge_priority__(struct rstp *rstp, int 
new_priority)
 
 void
 rstp_set_bridge_priority(struct rstp *rstp, int new_priority)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_priority__(rstp, new_priority);
@@ -413,7 +411,6 @@ rstp_set_bridge_ageing_time__(struct rstp *rstp, int 
new_ageing_time)
 
 void
 rstp_set_bridge_ageing_time(struct rstp *rstp, int new_ageing_time)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_ageing_time__(rstp, new_ageing_time);
@@ -509,7 +506,6 @@ rstp_set_bridge_force_protocol_version__(struct rstp *rstp,
 void
 rstp_set_bridge_force_protocol_version(struct rstp *rstp,
 enum rstp_force_protocol_version new_force_protocol_version)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 

Re: [ovs-dev] [PATCH v2 1/5] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

> On May 19, 2017, at 6:47 AM, Ben Pfaff  wrote:
> 
> On Wed, May 10, 2017 at 04:15:02AM -0700, nickcooper-zhangtonghao wrote:
>> * This patch will be used for next patch. The 'rstp/show' command,
>> which uses the mutex, calls functions which also use the mutex.
>> We should init it as a recursive mutex.
>> 
>> * Some rstp tests of OvS, which run with ovstest, does not run rstp_init
>> in the bridge_init. We should call rstp_init when creating the rstp
>> and stp also do that, otherwise tests fail.
>> 
>> * This patch remove the rstp_mutex in header file and make it static
>> in c file because we only use it in lib/rstp.c
>> 
>> Signed-off-by: nickcooper-zhangtonghao 
> 
> Thanks for working on the rstp code!
> 
> This patch causes a huge number of build failures with Clang, like this:
> 
>In file included from ../lib/rstp.c:32:
>../lib/rstp.h:135:18: error: use of undeclared identifier 'rstp_mutex'
>../include/openvswitch/compiler.h:136:57: note: expanded from macro 
> 'OVS_EXCLUDED'
> 
> I think that's the only reason that rstp_mutex is global.


Thanks for your review, the v3 patches don’t remove the rstp_mutex from header 
file
and we can build it with gcc and clang. Because of recursive mutex, I remove 
the 
OVS_EXCLUDED in the lib/rstp.[ch] files. Thanks very much.


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