Re: [ovs-dev] 答复: Re: 答复: Re: 答复: Re: [PATCH v2] ovn: Support for taas(tap-as-a-service) function

[Takashi YAMAMOTO]

On Wed, Sep 6, 2017 at 3:57 AM, Russell Bryant  wrote:

> What if a mirror port *only* receives mirrored packets?  If the only
> packets it ever receives are mirrored packets, a new flag would not be
> necessary.
>
> Do you intend for the port to operate as both a regular port *and* to
> receive a mirror of traffic  for another port?
>

in taas, a destination port is supposed to receive both of mirrored traffic
and regular traffic.

(i haven't looked at this implementation yet)

>
>
> On Thu, Aug 24, 2017 at 10:31 PM,  wrote:
>
> > I know your mean.
> > The receiver need to distinguish the traffic is regular or mirror. This
> > may need some special flow table to deal with it.
> >
> > Thanks
> >
> >
> >
> > *Gao Zhenyu >*
> >
> > 2017/08/25 10:12
> >
> > 收件人：wang.qia...@zte.com.cn,
> > 抄送：ovs dev , Russell Bryant <
> > russ...@ovn.org>, xurong00037997 ,
> > zhou.huij...@zte.com.cn
> > 主题：Re: 答复: Re: [ovs-dev] 答复: Re: [PATCH v2] ovn: Support
> > for taas(tap-as-a-service) function
> >
> >
> > I mean for regular packet, ovs should not add the geneve option,  the new
> > geneve option is only for mirror traffic.
> >
> > Did you meant some mirror traffic has mirror flag and some would not
> have?
> >
> > Thanks
> > Zhenyu Gao
> >
> > 2017-08-25 9:44 GMT+08:00 <*wang.qia...@zte.com.cn*
> > >:
> > Hi zhenyu,
> > Thanks for your opinion.
> > The mirror flag is not always exist, so I do not think add a new geneve
> > option is a good idea.
> >
> > Thanks.
> >
> >
> > *Gao Zhenyu <**sysugaozhe...@gmail.com* *>*
> >
> > 2017/08/25 09:34
> >
> > 收件人：*wang.qia...@zte.com.cn* ,
> > 抄送：Russell Bryant <*russ...@ovn.org* >,
> > ovs dev <*d...@openvswitch.org* >,
> > *zhou.huij...@zte.com.cn* , xurong00037997 <
> > *xu.r...@zte.com.cn* >
> > 主题：Re: [ovs-dev] 答复: Re: [PATCH v2] ovn: Support for
> > taas(tap-as-a-service) function
> >
> >
> >
> > Although adding a new geneve option is more complicate but I think it
> > still worth having that.
> > Once the destination chassis found that geneve option, it can tag the
> > mirror flag on packet. And it make the whole process looks same no matter
> > on same chassis or not.
> >
> > Thanks
> > Zhenyu Gao
> >
> > 2017-08-25 9:15 GMT+08:00 <*wang.qia...@zte.com.cn*
> > >:
> > Hi Russell,
> >
> > Thanks for your review.
> >
> > When the mirror destination is in other chassis, the mirror flag which
> > used to mark the packet need be transmitted to the destination chassis.
> >
> > We could use the inport, geneve option or new type of out port to
> indicate
> > the packet as a mirrored packet.
> >
> > When we use inport to indicate the flag, this may need use inport as the
> > match field in the egress pipeline, I think this may conflict with the
> > egress pipeline.
> >
> > If use geneve option to deliver the mirror flag, this may be more
> > complicated. So, I add a new type of port as the destination of mirror
> > flow. The port types of mirror and taas corresponding to configurations
> of
> > tap-flow and tap-service.
> >
> > Thanks.
> >
> >
> >
> >
> >
> > Russell Bryant <*russ...@ovn.org* >
> > 2017/08/25 04:44
> >
> > 收件人：*wang.qia...@zte.com.cn* ,
> > 抄送：  ovs dev <*d...@openvswitch.org* >,
> > *zhou.huij...@zte.com.cn* ,
> > xurong00037997 <*xu.r...@zte.com.cn* >
> > 主题：  Re: [ovs-dev] [PATCH v2] ovn: Support for
> > taas(tap-as-a-service) function
> >
> >
> > Sorry for the delay in getting back to this ...
> >
> > On Tue, Aug 15, 2017 at 4:28 AM,  <*wang.qia...@zte.com.cn*
> > > wrote:
> > > Taas was designed to provide tenants and service providers a means of
> > > monitoring the traffic flowing in their Neutron provisioned virtual
> > > networks. It is useful for network trouble-shooting, security and
> > > analytics. The taas presentations could be found from
> > >
> >
> > *https://github.com/openstack/tap-as-a-service/blob/master/
> doc/source/presentations.rst*
> >  doc/source/presentations.rst>
> >
> > > , and the api reference could be found from
> > >
> >
> > *https://github.com/openstack/tap-as-a-service/blob/master/
> API_REFERENCE.rst*
> >  API_REFERENCE.rst>
> >
> > >
> > > To support taas function, this patch add two type of
> logica_switch_port,
> > > "mirror" and "taas". port with type "mirror" is used as inport for
> > monitor
> > > flow in logica_switch, and port with type 

Re: [ovs-dev] [PATCH] acinclude.m4: Avoid error from printf.

[Takashi YAMAMOTO]

oops, thank you.

On Tue, Jul 18, 2017 at 2:12 AM, Ben Pfaff  wrote:

> GNU (at least) printf interprets -I as an option, but we want to print it
> literally, so use %s.
>
> CC: YAMAMOTO Takashi 
> Fixes: 27d41afaa446 ("acinclude.m4: Avoid echo -n")
> Signed-off-by: Ben Pfaff 
> ---
>  acinclude.m4 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index 05c57b88d68e..e7affc514811 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -968,7 +968,7 @@ dnl ctags ignores symbols with extras identifiers.
> This builds a list of
>  dnl specially handled identifiers to be ignored.
>  AC_DEFUN([OVS_CTAGS_IDENTIFIERS],
>  AC_SUBST([OVS_CTAGS_IDENTIFIERS_LIST],
> -   [`printf '-I "'; sed -n 's/^#define
> \(OVS_[A-Z_]\+\)(\.\.\.)$/\1+/p' ${srcdir}/include/openvswitch/compiler.h
> | tr \\\n ' ' ; printf '"'`] ))
> +   [`printf %s '-I "'; sed -n 's/^#define
> \(OVS_[A-Z_]\+\)(\.\.\.)$/\1+/p' ${srcdir}/include/openvswitch/compiler.h
> | tr \\\n ' ' ; printf '"'`] ))
>
>  dnl OVS_PTHREAD_SET_NAME
>  dnl
> --
> 2.10.2
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] acinclude.m4: Avoid echo -n

[Takashi YAMAMOTO]

On Sun, Jul 16, 2017 at 7:45 AM, Ben Pfaff  wrote:

> On Sun, Jul 16, 2017 at 12:17:28AM +0900, YAMAMOTO Takashi wrote:
> > -n option for echo is not portable.  Use printf instead.
> > This fixes OSX build on travis-ci.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Good catch, thank you!
>
> Acked-by: Ben Pfaff 
>

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

Re: [ovs-dev] [PATCH 5/6] ovn.at: Use a correct operator in a test expression

[Takashi YAMAMOTO]

On Sat, Jul 15, 2017 at 6:56 AM, Ben Pfaff  wrote:

> On Sat, Jul 15, 2017 at 02:39:45AM +0900, YAMAMOTO Takashi wrote:
> > == is a GNU extension.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> I wish this didn't exist in the GNU version.  It's a useless extension.
>

i totally agree.


>
> Acked-by: Ben Pfaff 
>

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

Re: [ovs-dev] [PATCH 6/6] packet-type-aware.at: Avoid GNU extension

[Takashi YAMAMOTO]

On Sat, Jul 15, 2017 at 6:56 AM, Ben Pfaff  wrote:

> On Sat, Jul 15, 2017 at 02:39:46AM +0900, YAMAMOTO Takashi wrote:
> > \t is GNU sed extension.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 
>

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

Re: [ovs-dev] [PATCH 3/6] pmd.at: Avoid using GNU sed extension

[Takashi YAMAMOTO]

On Sat, Jul 15, 2017 at 6:55 AM, Ben Pfaff  wrote:

> On Sat, Jul 15, 2017 at 02:39:43AM +0900, YAMAMOTO Takashi wrote:
> > BRE alternative (\|) is an GNU sed extension. [1]
> > It isn't available in NetBSD sed.
> >
> > [1] http://www.gnu.org/software/sed/manual/sed.html#Regular-Expressions
> > regexp1\|regexp2
> > Matches either regexp1 or regexp2. Use parentheses to use
> > complex alternative regular expressions. The matching process
> > tries each alternative in turn, from left to right, and the
> > first one that succeeds is used. It is a GNU extension.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 
>

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

Re: [ovs-dev] [PATCH 2/6] NetBSD: Stop recommending pkg_alternatives

[Takashi YAMAMOTO]

On Sat, Jul 15, 2017 at 6:55 AM, Ben Pfaff  wrote:

> On Sat, Jul 15, 2017 at 02:39:42AM +0900, YAMAMOTO Takashi wrote:
> > Because:
> > - It's no longer necessary
> > - It can cause problems for some utilities (e.g. ovs-pcap)
> >   Cf. http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=51152
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 
>

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

Re: [ovs-dev] [PATCH 1/6] interface-reconfigure.at: Use $PYTHON explicitly

[Takashi YAMAMOTO]

On Sat, Jul 15, 2017 at 6:55 AM, Ben Pfaff  wrote:

> On Sat, Jul 15, 2017 at 02:39:41AM +0900, YAMAMOTO Takashi wrote:
> > Workaround pkg_alternative issue for NetBSD:
> > http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=51152
> >
> > An alternative would be generating xenserver scripts from *.in,
> > but i'm not sure if it's appropriate for those scripts.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Thank you!
>
> Acked-by: Ben Pfaff 
>

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

Re: [ovs-dev] [PATCH 4/6] testsuite: Drain the list of jobs in the shell

[Takashi YAMAMOTO]

Hi,

2017/07/15 6:55 "Ben Pfaff" :

On Sat, Jul 15, 2017 at 02:39:44AM +0900, YAMAMOTO Takashi wrote:
> SUS says:
> When jobs reports the termination status of a job,
> the shell removes its process ID from the list of those
> "known in the current shell execution environment";
>
> With NetBSD /bin/sh, the list involves zombie processes and
> ends up with "can not fork" during test runs.

Can you squash the new patch into the existing one?  And then we can
apply it unconditionally?  It isn't great to have conditional code here,
especially since the testsuite is distributed.


I thought the existing windows patch was conditional for some reasons. Do
you remember the reason?

I'll look at it closely to see if it's ok for non windows platforms.


Did you report this problem to the Autoconf maintainers?  It would be
best to fix it in upstream Autoconf.


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

Re: [ovs-dev] [PATCH] socket-util: Support sendmmsg() on Linux and non-Linux platforms.

[Takashi YAMAMOTO]

On Thu, Jul 13, 2017 at 1:23 AM, Ben Pfaff  wrote:

> This will have its first user in an upcoming commit.
>
> Signed-off-by: Ben Pfaff 
> ---
>  configure.ac|  3 ++-
>  include/sparse/sys/socket.h |  5 +
>  lib/socket-util.c   | 38 ++
>  lib/socket-util.h   | 25 +
>  4 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 6404b5fc1222..9ef6aad3a36e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -106,7 +106,8 @@ AC_CHECK_DECLS([sys_siglist], [], [], [[#include
> ]])
>  AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec],
>[], [], [[#include ]])
>  AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include
> ]])
> -AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r])
> +AC_CHECK_MEMBERS([struct mmsghdr.msg_len], [], [], [[#include
> ]])
> +AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r sendmmsg])
>  AC_CHECK_HEADERS([mntent.h sys/statvfs.h linux/types.h linux/if_ether.h
> stdatomic.h])
>  AC_CHECK_HEADERS([net/if_mib.h], [], [], [[#include 
>  #include ]])
> diff --git a/include/sparse/sys/socket.h b/include/sparse/sys/socket.h
> index 3212bf4b7f13..88a5387e7f9b 100644
> --- a/include/sparse/sys/socket.h
> +++ b/include/sparse/sys/socket.h
> @@ -75,6 +75,11 @@ __cmsg_nxthdr(struct msghdr *msg, struct cmsghdr *cmsg)
>  : NULL);
>  }
>
> +struct mmsghdr {
> +struct msghdr msg_hdr;
> +unsigned int msg_len;
> +};
> +
>  enum {
>  SCM_RIGHTS = 1
>  };
> diff --git a/lib/socket-util.c b/lib/socket-util.c
> index 2c0f1e62bd99..0927ac9ac6c4 100644
> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -1020,3 +1020,41 @@ sock_strerror(int error)
>  return ovs_strerror(error);
>  #endif
>  }
> +
> +static int
> +emulate_sendmmsg(int fd, struct mmsghdr *msgs, unsigned int n,
> + unsigned int flags)
> +{
> +for (unsigned int i = 0; i < n; i++) {
> +int retval = sendmsg(fd, [i].msg_hdr, flags);
>

ssize_t?

+if (retval < 0) {
> +return i ? i : retval;
> +}
> +msgs[i].msg_len = retval;
> +}
> +return n;
> +}
> +
> +#ifndef HAVE_SENDMMSG
> +int
> +sendmmsg(int fd, struct mmsghdr *msgs, unsigned int n, unsigned int flags)
> +{
> +return emulate_sendmmsg(fd, msgs, n, flags);
> +}
> +#else
> +int
> +wrap_sendmmsg(int fd, struct mmsghdr *msgs, unsigned int n, unsigned int
> flags)
> +{
> +static bool sendmmsg_broken = false;
> +if (!sendmmsg_broken) {
> +int save_errno = errno;
> +int retval = sendmmsg(fd, msgs, n, flags);
> +if (retval >= 0 || errno != ENOSYS) {
> +return retval;
> +}
> +sendmmsg_broken = true;
> +errno = save_errno;
> +}
> +return emulate_sendmmsg(fd, msgs, n, flags);
> +}
> +#endif
> diff --git a/lib/socket-util.h b/lib/socket-util.h
> index 5bf76a40eb84..0041345b0ba2 100644
> --- a/lib/socket-util.h
> +++ b/lib/socket-util.h
> @@ -87,6 +87,31 @@ int make_unix_socket(int style, bool nonblock,
>   const char *bind_path, const char *connect_path);
>  int get_unix_name_len(const struct sockaddr_un *sun, socklen_t sun_len);
>
> +/* sendmmsg support.
> + *
> + * We add the following infrastructure to allow all code to use sendmmsg()
> + * without caring what platform it runs on:
> + *
> + *   - Non-Linux platforms do not have sendmmsg.  For these, we emulate
> it.
>

FYI, it's available on non-linux platforms these days.
https://www.freebsd.org/cgi/man.cgi?query=sendmmsg=0=0=FreeBSD+11.0-RELEASE+and+Ports=default=html
http://netbsd.gw.com/cgi-bin/man-cgi?sendmmsg++NetBSD-current

+ *
> + *   - Linux platforms might have sendmmsg in their C library but not in
> the
> + * kernel, which means that it will always return ENOSYS.  To
> compensate,
> + * we wrap sendmmsg() with a handler that uses our emulation if
> sendmmsg()
> + * returns ENOSYS.
> + */
> +#ifndef HAVE_STRUCT_MMSGHDR_MSG_LEN
> +struct mmsghdr {
> +struct msghdr msg_hdr;
> +unsigned int msg_len;
> +};
> +#endif
> +#ifndef HAVE_SENDMMSG
> +int sendmmsg(int, struct mmsghdr *, unsigned int, unsigned int);
> +#else
> +#define sendmmsg wrap_sendmmsg
> +int wrap_sendmmsg(int, struct mmsghdr *, unsigned int, unsigned int);
> +#endif
> +
>  /* Helpers for calling ioctl() on an AF_INET socket. */
>  struct ifreq;
>  int af_inet_ioctl(unsigned long int command, const void *arg);
> --
> 2.10.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn.at: Fix "ovn -- 1 LR with distributed router gateway port" test

[Takashi YAMAMOTO]

On Tue, May 2, 2017 at 5:21 AM, Ben Pfaff  wrote:

> On Fri, Apr 21, 2017 at 10:32:57AM +0900, YAMAMOTO Takashi wrote:
> > NetBSD implementation of wc command outputs extra whitespaces
> > like the following.  Tweak the test to success on such environments.
> >
> > % echo hoge|wc -l|hexdump -C
> >   20 20 20 20 20 20 20 31  0a   |
>  1.|
> > 0009
> > %
> >
> > The failing test was introduced by
> > commit 41a15b71ed1ef35aa612a1128082219fbfc3f327
> > (ovn: Introduce distributed gateway port and "chassisredirect" port
> binding)
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Thanks a lot for this fix.
>
> This is not the first time we've had trouble with wc, and not the first
> project where I've had trouble with wc.  POSIX standardizes the output
> format for wc, but neither GNU or BSD honors the standard, and so it's
> not even that a particular OS is at fault.
>
> Anyway, how about the following?  I believe that it will fix the problem
> you're seeing, and it ought to avoid new problems of the same kind in
> the future.  What do you think?
>
> Thanks,
>
> Ben.
>
> --8<--cut here-->8--
>
> From: Ben Pfaff 
> Date: Mon, 1 May 2017 13:19:43 -0700
> Subject: [PATCH] ovs-macros: Add helper to make 'wc' use POSIX compliant
>  output format.
>
> Several times, we've had to fix tests that used 'wc' and expected a
> particular output format.  POSIX is specific about the output format, but
> neither GNU or BSD wc honors it.  This commit makes whatever 'wc' is on
> the system use the POSIX output format.
>
> Signed-off-by: Ben Pfaff 
>

good idea.  tested it on netbsd.
Acked-by: YAMAMOTO Takashi 


> ---
>  tests/ovs-macros.at | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 123c713395e4..37e72d992555 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -114,6 +114,16 @@ parent_pid () {
>  ps -o ppid= -p $1
>  fi
>  }
> +
> +# Normalize the output of 'wc' to match POSIX.
> +# POSIX says 'wc' should print "%d %d %d", but GNU prints "%7d %7d %7d".
> +# POSIX says 'wc -l' should print "%d %s", but BSD prints "%8d".
> +#
> +# This fixes all of those (it will screw up filenames that contain
> +# multiple sequential spaces, but that doesn't really matter).
> +wc () {
> +   command wc "$@" | tr -s ' ' ' ' | sed 's/^ *//'
> +}
>  ]
>  m4_divert_pop([PREPARE_TESTS])
>
> --
> 2.10.2
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn.at: Fix "ovn -- 1 LR with distributed router gateway port" test

[Takashi YAMAMOTO]

On Mon, Apr 24, 2017 at 9:48 AM, Mickey Spiegel 
wrote:

>
> On Thu, Apr 20, 2017 at 6:32 PM, YAMAMOTO Takashi 
> wrote:
>
>> NetBSD implementation of wc command outputs extra whitespaces
>> like the following.  Tweak the test to success on such environments.
>>
>> % echo hoge|wc -l|hexdump -C
>>   20 20 20 20 20 20 20 31  0a   |
>>  1.|
>> 0009
>> %
>>
>> The failing test was introduced by
>> commit 41a15b71ed1ef35aa612a1128082219fbfc3f327
>> (ovn: Introduce distributed gateway port and "chassisredirect" port
>> binding)
>>
>> Signed-off-by: YAMAMOTO Takashi 
>>
>
> There are three more in one of the tests in system-ovn.at, affecting make
> check-kernel.
>

sure.
but those tests seem heavily linux-centric anyway for other reasons.


>
> Acked-by: Mickey Spiegel 
>
>
>
>> ---
>>  tests/ovn.at | 18 ++
>>  1 file changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index af77c19..1bffc4c 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -6567,20 +6567,14 @@ as hv3 ovs-ofctl dump-flows br-int
>>  echo "--"
>>
>>  # Check that redirect mapping is programmed only on hv2
>> -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=33 | grep
>> =0x3,metadata=0x1 | wc -l], [0], [0
>> -])
>> -AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=33 | grep
>> =0x3,metadata=0x1 | grep load:0x2- | wc -l], [0], [1
>> -])
>> +AT_CHECK([test `as hv1 ovs-ofctl dump-flows br-int table=33 | grep
>> =0x3,metadata=0x1 | wc -l` -eq 0])
>> +AT_CHECK([test `as hv2 ovs-ofctl dump-flows br-int table=33 | grep
>> =0x3,metadata=0x1 | grep load:0x2- | wc -l` -eq 1])
>>  # Check that hv1 sends chassisredirect port traffic to hv2
>> -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=32 | grep
>> =0x3,metadata=0x1 | grep output | wc -l], [0], [1
>> -])
>> -AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=32 | grep
>> =0x3,metadata=0x1 | wc -l], [0], [0
>> -])
>> +AT_CHECK([test `as hv1 ovs-ofctl dump-flows br-int table=32 | grep
>> =0x3,metadata=0x1 | grep output | wc -l` -eq 1])
>> +AT_CHECK([test `as hv2 ovs-ofctl dump-flows br-int table=32 | grep
>> =0x3,metadata=0x1 | wc -l` -eq 0])
>>  # Check that arp reply on distributed gateway port is only programmed on
>> hv2
>> -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep arp | grep
>> =0x2,metadata=0x1 | wc -l], [0], [0
>> -])
>> -AT_CHECK([as hv2 ovs-ofctl dump-flows br-int | grep arp | grep
>> =0x2,metadata=0x1 | wc -l], [0], [1
>> -])
>> +AT_CHECK([test `as hv1 ovs-ofctl dump-flows br-int | grep arp | grep
>> =0x2,metadata=0x1 | wc -l` -eq 0])
>> +AT_CHECK([test `as hv2 ovs-ofctl dump-flows br-int | grep arp | grep
>> =0x2,metadata=0x1 | wc -l` -eq 1])
>>
>>
>>  ip_to_hex() {
>> --
>> 2.5.4 (Apple Git-61)
>>
>> ___
>> 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] Update netbsd install doc

[Takashi YAMAMOTO]

On Wed, Dec 21, 2016 at 3:25 AM, Hui Kang  wrote:

> - test ovs on netbsd 7.0.2
> - use gmake to compile and install
>
> Signed-off-by: Hui Kang 
> ---
>  Documentation/intro/install/netbsd.rst | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/intro/install/netbsd.rst
> b/Documentation/intro/install/netbsd.rst
> index b32da20..c2c23e5 100644
> --- a/Documentation/intro/install/netbsd.rst
> +++ b/Documentation/intro/install/netbsd.rst
> @@ -42,7 +42,7 @@ information.
>  Assuming you are running NetBSD/amd64 6.1.2, you can download and install
>  pre-built binary packages as the following::
>
> -$ PKG_PATH=http://ftp.netbsd.org/pub/pkgsrc/packages/
> NetBSD/amd64/6.1.2/All/
> +$ PKG_PATH=http://ftp.netbsd.org/pub/pkgsrc/packages/
> NetBSD/amd64/7.0.2/All/
>  $ export PKG_PATH
>  $ pkg_add automake libtool-base gmake python27 py27-six py27-xml \
>  pkg_alternatives
> @@ -55,7 +55,8 @@ NetBSD's ``/usr/bin/make`` is not GNU make.  GNU make is
> installed as
>  ``/usr/pkg/bin/gmake`` by the above mentioned ``gmake`` package.
>
>  As all executables installed with pkgsrc are placed in ``/usr/pkg/bin/``
> -directory, it might be a good idea to add it to your PATH.
> +directory, it might be a good idea to add it to your PATH. Or install ovs
> by
>

you mean And?


> +``gmake`` and ``gmake install``.
>
>  Open vSwitch on NetBSD is currently "userspace switch" implementation in
> the
>  sense described in :doc:`userspace` and :doc:`/topics/porting`.
> --
> 1.9.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] assertion failing in commit_set_ipv4_action(), flow->nw_proto != base_flow->nw_proto

[Takashi YAMAMOTO]

On Wed, Dec 14, 2016 at 9:02 AM, Jarno Rajahalme <ja...@ovn.org> wrote:

>
> On Dec 13, 2016, at 3:32 PM, Takashi YAMAMOTO <yamam...@ovn.org> wrote:
>
>
>
> On Tue, Dec 13, 2016 at 6:49 PM, Thomas Morin <thomas.mo...@orange.com>
> wrote:
>
>> Hi Jarno,
>>
>> 2016-12-10, Jarno Rajahalme:
>>
>> On Dec 9, 2016, at 7:04 AM, Thomas Morin <thomas.mo...@orange.com> wrote:
>>
>>
>> 2016-12-09, Thomas Morin:
>>
>> In the same setup as the one on which the bug was observed, [...]
>>
>>
>> I was confused, I in fact tested the patch (branch-2.5) on a /different/
>> setup as the one on which we hit the bug, using MPLS over a GRE tunnel
>> port, rather than plain MPLS over an eth port.
>> Sorry if any confusion arised... I can test on the first setup if
>> relevant.
>>
>>
>> Maybe the kernel datapath does not support MPLS over a GRE tunnel port.
>> Having ‘dmesg’ output for the test run might reveal why the actions
>> validation fails.
>>
>>
>> The dmesg output was the following:
>>
>> [171295.258939] openvswitch: netlink: Flow actions may not be safe on all
>> matching packets.
>>
>> I've tested the patch on the platform on which the bug was initially hit
>> (*not* using MPLS/GRE), and I have the following a few times in the logs
>> right after I do an "ovs-appctl fdb/flush":
>>
>> 2016-12-13T09:44:08.449Z|1|dpif(handler68)|WARN|Dropped 3 log
>> messages in last 1 seconds (most recently, 1 seconds ago) due to excessive
>> rate
>> 2016-12-13T09:44:08.449Z|2|dpif(handler68)|WARN|system@ovs-system:
>> failed to put[create] (Invalid argument) 
>> ufid:f046c4c4-b97f-436d-bd7c-91ed307275ac
>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(9),skb_m
>> ark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/
>> 0),eth(src=fa:16:3e:61:c0:b5,dst=00:00:5e:00:43:64),eth_
>> type(0x0800),ipv4(src=10.0.1.29,dst=10.0.0.3,proto=6,tos=0/
>> 0xfc,ttl=64,frag=no),tcp(src=54253,dst=8080),tcp_flags(0/0),
>> actions:set(ipv4(src=10.0.1.29,dst=10.0.0.3,ttl=63)),set(eth
>> (src=b8:2a:72:de:1b:e3,dst=00:17:cb:79:2c:01)),push_mpls(lab
>> el=433680,tc=0,ttl=63,bos=1,eth_type=0x8847),7,set(eth(src
>> =fa:16:3e:61:c0:b5,dst=00:00:5e:00:43:64)),pop_mpls(eth_typ
>> e=0x800),set(ipv4(src=10.0.1.29,dst=10.0.0.3,tos=0/0xfc,ttl
>> =64)),push_vlan(vid=1,pcp=0),3,8,pop_vlan,13
>>
>> And dmesg:
>> [926833.612372] openvswitch: netlink: Flow actions may not be safe on all
>> matching packets.
>>
>
> it's complaining about set ipv4 after pop_mpls because it doesn't know
> about the "restored" l3.
> while we can improve the kernel, i guess we need to stick with recirc for
> now.
>
>
>
> This should do it? Does not break any existing tests on branch-2.5, but I
> did not create a test for this yet.
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index fb25f63..6ee2a72 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2899,6 +2899,15 @@ xlate_commit_actions(struct xlate_ctx *ctx)
>  {
>  bool use_masked = ctx->xbridge->support.masked_set_action;
>
> +/* An MPLS packet can be implicitly popped back to a non-MPLS packet,
> if a
> + * patch port peer or a group bucket pushed MPLS.  Set the 'was_mpls'
> flag
> + * also in that case. */
> +if (eth_type_mpls(ctx->base_flow.dl_type)
> +&& !eth_type_mpls(ctx->xin->flow.dl_type)
> +&& ctx->xbridge->support.odp.recirc) {
> +ctx->was_mpls = true;
> +}
> +
>

i guess this check needs to be somewhere around "ctx->was_mpls =
old_was_mpls" in
affected functions.

 ctx->xout->slow |= commit_odp_actions(>xin->flow, >base_flow,
>ctx->odp_actions, ctx->wc,
>use_masked);
>
>   Jarno
>
>
>
>>
>> -Thomas
>>
>>
>>
>>
>>
>> On Dec 1, 2016, at 5:57 PM, Jarno Rajahalme <ja...@ovn.org> wrote:
>>
>>
>> On Nov 30, 2016, at 8:50 PM, Ben Pfaff <b...@ovn.org> wrote:
>>
>> On Wed, Nov 30, 2016 at 06:58:57PM -0800, Jarno Rajahalme wrote:
>>
>>
>> On Nov 30, 2016, at 8:41 AM, Ben Pfaff <b...@ovn.org> wrote:
>>
>> On Wed, Nov 30, 2016 at 12:05:46PM +0100, Thomas Morin wrote:
>>
>> Hi Ben,
>>
>> 2016-11-30, Ben Pfaff:
>>
>> Do you have any idea what in your OpenFlow pipeline might do th

Re: [ovs-dev] assertion failing in commit_set_ipv4_action(), flow->nw_proto != base_flow->nw_proto

[Takashi YAMAMOTO]

On Tue, Dec 13, 2016 at 6:49 PM, Thomas Morin 
wrote:

> Hi Jarno,
>
> 2016-12-10, Jarno Rajahalme:
>
> On Dec 9, 2016, at 7:04 AM, Thomas Morin  wrote:
>
>
> 2016-12-09, Thomas Morin:
>
> In the same setup as the one on which the bug was observed, [...]
>
>
> I was confused, I in fact tested the patch (branch-2.5) on a /different/
> setup as the one on which we hit the bug, using MPLS over a GRE tunnel
> port, rather than plain MPLS over an eth port.
> Sorry if any confusion arised... I can test on the first setup if relevant.
>
>
> Maybe the kernel datapath does not support MPLS over a GRE tunnel port.
> Having ‘dmesg’ output for the test run might reveal why the actions
> validation fails.
>
>
> The dmesg output was the following:
>
> [171295.258939] openvswitch: netlink: Flow actions may not be safe on all
> matching packets.
>
> I've tested the patch on the platform on which the bug was initially hit
> (*not* using MPLS/GRE), and I have the following a few times in the logs
> right after I do an "ovs-appctl fdb/flush":
>
> 2016-12-13T09:44:08.449Z|1|dpif(handler68)|WARN|Dropped 3 log
> messages in last 1 seconds (most recently, 1 seconds ago) due to excessive
> rate
> 2016-12-13T09:44:08.449Z|2|dpif(handler68)|WARN|system@ovs-system:
> failed to put[create] (Invalid argument) 
> ufid:f046c4c4-b97f-436d-bd7c-91ed307275ac
> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(9),skb_
> mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_
> label(0/0),eth(src=fa:16:3e:61:c0:b5,dst=00:00:5e:00:43:
> 64),eth_type(0x0800),ipv4(src=10.0.1.29,dst=10.0.0.3,proto=
> 6,tos=0/0xfc,ttl=64,frag=no),tcp(src=54253,dst=8080),tcp_flags(0/0),
> actions:set(ipv4(src=10.0.1.29,dst=10.0.0.3,ttl=63)),set(
> eth(src=b8:2a:72:de:1b:e3,dst=00:17:cb:79:2c:01)),push_mpls(
> label=433680,tc=0,ttl=63,bos=1,eth_type=0x8847),7,set(eth(
> src=fa:16:3e:61:c0:b5,dst=00:00:5e:00:43:64)),pop_mpls(eth_
> type=0x800),set(ipv4(src=10.0.1.29,dst=10.0.0.3,tos=0/0xfc,
> ttl=64)),push_vlan(vid=1,pcp=0),3,8,pop_vlan,13
>
> And dmesg:
> [926833.612372] openvswitch: netlink: Flow actions may not be safe on all
> matching packets.
>

it's complaining about set ipv4 after pop_mpls because it doesn't know
about the "restored" l3.
while we can improve the kernel, i guess we need to stick with recirc for
now.


>
>
> -Thomas
>
>
>
>
>
> On Dec 1, 2016, at 5:57 PM, Jarno Rajahalme  wrote:
>
>
> On Nov 30, 2016, at 8:50 PM, Ben Pfaff  wrote:
>
> On Wed, Nov 30, 2016 at 06:58:57PM -0800, Jarno Rajahalme wrote:
>
>
> On Nov 30, 2016, at 8:41 AM, Ben Pfaff  wrote:
>
> On Wed, Nov 30, 2016 at 12:05:46PM +0100, Thomas Morin wrote:
>
> Hi Ben,
>
> 2016-11-30, Ben Pfaff:
>
> Do you have any idea what in your OpenFlow pipeline might do that,
> i.e. is there anything especially tricky in the OpenFlow flows?
>
> Are you willing to show us your OpenFlow flow table?
>
>
> The setup involves three OVS bridges connected with patch-ports: br-int --
> br-tun -- br-mpls, with the traffic that triggers the assert being
> processed
> by br-int with a NORMAL action (ie. MAC learning).
>
> The flows in this setup aren't particularly tricky, I think, although I'm
> not sure what qualifies as tricky or non-tricky :)
>
> Anyway, since yesterday I managed to identify the event that trigger the
> assert, by adding more logging before the assert and displaying the actions
> taken:
>
> 2016-11-29T14:44:40.126Z|1|odp_util(revalidator45)|
> WARN|commit_set_ipv4_action
> assert would fail
> 2016-11-29T14:44:40.126Z|2|odp_util(revalidator45)|WARN|  base_flow:
> ip,in_port=5,dl_vlan=3,dl_vlan_pcp=0,dl_src=fa:16:3e:33:
> f7:fe,dl_dst=00:00:5e:00:43:64,nw_src=0.0.0.0,nw_dst=0.0.
> 0.0,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
> 2016-11-29T14:44:40.126Z|3|odp_util(revalidator45)|WARN|  flow:
> tcp,in_port=5,dl_vlan=3,dl_vlan_pcp=0,dl_src=fa:16:3e:33:
> f7:fe,dl_dst=00:00:5e:00:43:64,nw_src=10.0.1.22,nw_dst=10.
> 0.0.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=53295,tp_dst=
> 8080,tcp_flags=psh|ack
> 2016-11-29T14:44:40.126Z|4|odp_util(revalidator45)|WARN|  masks:
> recirc_id=0x,reg0=0x,in_port=4294967295,
> dl_vlan=4095,dl_vlan_pcp=7,dl_src=ff:ff:ff:ff:ff:ff,dl_dst=
> ff:ff:ff:ff:ff:ff,dl_type=0x
> 2016-11-29T14:44:40.126Z|5|odp_util(revalidator45)|WARN|  actions:
> set(ipv4(src=10.0.1.22,dst=10.0.0.3,ttl=63)),set(eth(src=b8:
> 2a:72:de:1b:e3,dst=00:17:cb:79:2c:01)),push_mpls(label=
> 410384,tc=0,ttl=63,bos=1,eth_type=0x8847),9,set(eth(src=fa:
> 16:3e:33:f7:fe,dst=00:00:5e:00:43:64)),pop_mpls(eth_type=
> 0x800),push_vlan(vid=3,pcp=0),1
>
>
> push_mpls clears L3/L4, while pop_mpls re-populates them, and then
> processing the output to port 1 hits the assert?
>
>
> That's what I'm thinking too.
>
> Jarno, is this something you have time to look into?  It'd be great, if
> you do.  I'm way behind.
>
>
> I’m looking at this.
>
> Based on the trace given it seems 

Re: [ovs-dev] [PATCH] mpls: Fix MPLS restoration after patch port and group bucket.

[Takashi YAMAMOTO]

On Sat, Dec 3, 2016 at 12:38 PM, Jarno Rajahalme <ja...@ovn.org> wrote:

> This patch fixes problems with MPLS handling related to patch ports
> and group buckets.
>
> If a group bucket or a peer bridge across a patch port pushes MPLS
> headers to a non-MPLS packet and outputs, the flow translation after
> returning from the group bucket or patch port would undo the packet
> transformations so that the processing could continue with the packet
> as it was before entering the patch port.  There were two problems
> with this:
>
> 1. As part of the first MPLS push on a non-MPLS packet, the flow
> translation would first clear the L3/4 headers of the 'flow' to mark
> those fields invalid.  Later, when committing 'flow' changes to
> datapath actions before output, the necessary datapath MPLS actions
> are created and the corresponding changes updated to the 'base flow'.
> This was done using the same flow_push_mpls() function that clears
> the L2/3 headers, so also the 'base flow' L2/3 headers were cleared.
>
> Then, when translation returns from a patch port or group bucket, the
> original 'flow' is restored, now showing no sign of the MPLS labels.
> Since the 'base flow' now has the MPLS labels, following translations
> know to issue MPLS POP actions before any output actions.  However, as
> part of checking for changes to IP headers we test that the IP
> protocol type was not changed.  But now the 'base flow's 'nw_proto'
> field is zero and an assert fail crashes OVS.
>
> This is solved by not clearing the L3/4 fields of the 'base
> flow'. This allows the processing after the patch port to continue
> with L3/4 fields as if no MPLS was done, after first issuing the
> necessary MPLS POP actions.
>
> 2. IP header updates were done before the MPLS POP actions were
> issued. This caused incorrect packet output after, e.g., group action
> or patch port.  For example, with actions:
>
> group 1234: all bucket=push_mpls,output:LOCAL
>
> ip actions=group:1234,dec_ttl,output:LOCAL,output:LOCAL
>
> the dec_ttl would only be executed before the last output to LOCAL,
> since at the time of committing IP changes after the group action the
> packet was still an MPLS packet.
>
> This is solved by checking the dl_type of both 'flow' and 'base flow'
> and issuing MPLS actions if they can transform the packet from an MPLS
> packet to a non-MPLS packet.  For an IP packet the change in ttl can
> then be correctly committed before the last two output actions.
>
> Two test cases are added to prevent future regressions.
>
> Reported-by: Thomas Morin <thomas.mo...@orange.com>
> Suggested-by: Takashi YAMAMOTO <yamam...@ovn.org>
> Fixes: 8bfd0fdac ("Enhance userspace support for MPLS, for up to 3
> labels.")
> Fixes: 1b035ef20 ("mpls: Allow l3 and l4 actions to prior to a push_mpls
> action")
> Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
>

Acked-by: YAMAMOTO Takashi <yamam...@ovn.org>

---
>  lib/flow.c   | 14 +-
>  lib/flow.h   |  2 +-
>  lib/odp-util.c   | 16 +--
>  ofproto/ofproto-dpif-xlate.c |  3 ++-
>  tests/mpls-xlate.at  | 64 ++
> ++
>  5 files changed, 89 insertions(+), 10 deletions(-)
>
> diff --git a/lib/flow.c b/lib/flow.c
> index f4ac8b3..b6d0d15 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -2096,7 +2096,7 @@ flow_count_common_mpls_labels(const struct flow *a,
> int an,
>   */
>  void
>  flow_push_mpls(struct flow *flow, int n, ovs_be16 mpls_eth_type,
> -   struct flow_wildcards *wc)
> +   struct flow_wildcards *wc, bool clear_flow_L3)
>  {
>  ovs_assert(eth_type_mpls(mpls_eth_type));
>  ovs_assert(n < FLOW_MAX_MPLS_LABELS);
> @@ -2134,11 +2134,13 @@ flow_push_mpls(struct flow *flow, int n, ovs_be16
> mpls_eth_type,
>
>  flow->mpls_lse[0] = set_mpls_lse_values(ttl, tc, 1, htonl(label));
>
> -/* Clear all L3 and L4 fields and dp_hash. */
> -BUILD_ASSERT(FLOW_WC_SEQ == 36);
> -memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
> -   sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
> -flow->dp_hash = 0;
> +if (clear_flow_L3) {
> +/* Clear all L3 and L4 fields and dp_hash. */
> +BUILD_ASSERT(FLOW_WC_SEQ == 36);
> +memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
> +   sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
> +flow->dp_hash = 0;
> +}
>  }
>  flow->dl_type = mpls_eth_type;
>  }
> diff --git a/lib/flow.h b/lib/flow.h
> index 35724b1..62315bc 10064

Re: [ovs-dev] assertion failing in commit_set_ipv4_action(), flow->nw_proto != base_flow->nw_proto

[Takashi YAMAMOTO]

hi,

On Fri, Dec 2, 2016 at 10:57 AM, Jarno Rajahalme  wrote:

>
> > On Nov 30, 2016, at 8:50 PM, Ben Pfaff  wrote:
> >
> > On Wed, Nov 30, 2016 at 06:58:57PM -0800, Jarno Rajahalme wrote:
> >>
> >>> On Nov 30, 2016, at 8:41 AM, Ben Pfaff  wrote:
> >>>
> >>> On Wed, Nov 30, 2016 at 12:05:46PM +0100, Thomas Morin wrote:
>  Hi Ben,
> 
>  2016-11-30, Ben Pfaff:
> > Do you have any idea what in your OpenFlow pipeline might do that,
> > i.e. is there anything especially tricky in the OpenFlow flows?
> >
> > Are you willing to show us your OpenFlow flow table?
> 
>  The setup involves three OVS bridges connected with patch-ports:
> br-int --
>  br-tun -- br-mpls, with the traffic that triggers the assert being
> processed
>  by br-int with a NORMAL action (ie. MAC learning).
> 
>  The flows in this setup aren't particularly tricky, I think, although
> I'm
>  not sure what qualifies as tricky or non-tricky :)
> 
>  Anyway, since yesterday I managed to identify the event that trigger
> the
>  assert, by adding more logging before the assert and displaying the
> actions
>  taken:
> 
>  2016-11-29T14:44:40.126Z|1|odp_util(revalidator45)|
> WARN|commit_set_ipv4_action
>  assert would fail
>  2016-11-29T14:44:40.126Z|2|odp_util(revalidator45)|WARN|
> base_flow: ip,in_port=5,dl_vlan=3,dl_vlan_pcp=0,dl_src=fa:16:3e:33:
> f7:fe,dl_dst=00:00:5e:00:43:64,nw_src=0.0.0.0,nw_dst=0.0.
> 0.0,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
>  2016-11-29T14:44:40.126Z|3|odp_util(revalidator45)|WARN|  flow:
> tcp,in_port=5,dl_vlan=3,dl_vlan_pcp=0,dl_src=fa:16:3e:33:
> f7:fe,dl_dst=00:00:5e:00:43:64,nw_src=10.0.1.22,nw_dst=10.
> 0.0.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=53295,tp_dst=
> 8080,tcp_flags=psh|ack
>  2016-11-29T14:44:40.126Z|4|odp_util(revalidator45)|WARN|  masks:
> recirc_id=0x,reg0=0x,in_port=4294967295,
> dl_vlan=4095,dl_vlan_pcp=7,dl_src=ff:ff:ff:ff:ff:ff,dl_dst=
> ff:ff:ff:ff:ff:ff,dl_type=0x
>  2016-11-29T14:44:40.126Z|5|odp_util(revalidator45)|WARN|
> actions: set(ipv4(src=10.0.1.22,dst=10.0.0.3,ttl=63)),set(eth(src=b8:
> 2a:72:de:1b:e3,dst=00:17:cb:79:2c:01)),push_mpls(label=
> 410384,tc=0,ttl=63,bos=1,eth_type=0x8847),9,set(eth(src=fa:
> 16:3e:33:f7:fe,dst=00:00:5e:00:43:64)),pop_mpls(eth_type=
> 0x800),push_vlan(vid=3,pcp=0),1
> >>
> >> push_mpls clears L3/L4, while pop_mpls re-populates them, and then
> processing the output to port 1 hits the assert?
> >
> > That's what I'm thinking too.
> >
> > Jarno, is this something you have time to look into?  It'd be great, if
> > you do.  I'm way behind.
>
> I’m looking at this.
>

> Based on the trace given it seems that:
> 1. Packet is received on br-int port 32, which outputs it via NORMAL
> action over a patch port to another bridge. The only patch-port on br-int
> is 2 (patch-tun). The NORMAL action adds dl_vlan=1.
> 2. br-tun receives the packet on in_port 1 (patch-int), and outputs it on
> it’s port 2 (patch-to-mpls)
> 3. br-mpls receives the packet on it’s in_port 2 (patch-to-tun), does
> pop_vlan, and outputs to it’s port 21 (ipvpn-pp-out), which is also an
> patch port.
> 4. br-mpls (?) receives the packet on it’s in_port 20 (ipvpn-pp-in), does
> dec_ttl,push_mpls:0x8847,load:0x644c0->OXM_OF_MPLS_LABEL[],
> set_field:b8:2a:72:de:1b:e3->eth_src,set_field:00:17:cb:79:
> 2c:01->eth_dst,output:1
>
> All this generates a megaflow: set(ipv4(src=10.0.1.23,dst=10.
> 0.0.3,ttl=63)),set(eth(src=b8:2a:72:de:1b:e3,dst=00:17:cb:
> 79:2c:01)),push_mpls(label=410816,tc=0,ttl=63,bos=1,eth_type=0x8847),9
>
> This is only the beginning part of the trouble-some megaflow, in which
> br-int sends the packet also to another port (vlan 3), and as part of that
> pops the MPLS and restores the original ethernet addresses. Maybe this
> would happen with the trace too, if you flushed MACs before the trace?
>
> The patch ports 21 and 20 appear to be in the same bridge and patched to
> each other. Is this the case?
>
> The crashing megaflow has in_port=5,dl_vlan=3. Is this also on br-int?
>
> Also, OVS 2.6 is a little bit less aggressive about avoiding recirculation
> after mpls operations, and I’d be interested to know if your case fails the
> same way with OVS 2.6?
>

as patch port restores ctx->flow, if the peer bridge does push_mpls
which clears L3, the assertion failure would happen.

i think master has the same issue.

i guess something like the following can trigger the failure
in a sandbox.

for NAME in hoge fuga; do
ovs-vsctl add-br $NAME -- set br $NAME datapath_type=netdev -- set br
$NAME "protocols=[OpenFlow13]"
ovs-ofctl -OOpenFlow13 del-flows $NAME
done

ovs-vsctl \
add-port hoge p1 -- \
set int p1 type=patch options:peer=p2 ofport_request=5 -- \
add-port fuga p2 -- \
set int p2 type=patch options:peer=p1

ovs-ofctl -OOpenFlow13 add-flow hoge "actions=5,IN_PORT,IN_PORT"

[ovs-dev] "nb7" branch

[Takashi YAMAMOTO]

hi,

i mistakenly pushed a branch "nb7" and then removed it.
sorry if it caused glitches to something.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/3] Fix parent_pid on NetBSD

[Takashi YAMAMOTO]

On Wed, Oct 19, 2016 at 1:46 AM, Ben Pfaff  wrote:

> On Tue, Oct 18, 2016 at 09:25:24PM +0900, YAMAMOTO Takashi wrote:
> > Fix recently introduced parent_pid macro on NetBSD 7.0.
> > On NetBSD, procfs status file looks like the following.
> >
> > n7% cat /proc/$$/status
> > zsh 18509 12970 18509 18509 5,8 ctty,sldr 1476344459,639266 0,15575
> 0,15575 pause 1000 100,100,0
> > n7%
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Thank you for the fix.  Would you mind updating the comment as well?
> Perhaps you could add a sentence like "We check the format of the status
> file to avoid the NetBSD file with the same name but different
> contents."
>
> Acked-by: Ben Pfaff 
>

thank you.
amended the comment and pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/3] ovn.at: Avoid using "printf -v"

[Takashi YAMAMOTO]

On Wed, Oct 19, 2016 at 1:47 AM, Ben Pfaff  wrote:

> On Tue, Oct 18, 2016 at 09:25:25PM +0900, YAMAMOTO Takashi wrote:
> > It seems like a non-portable bash extension.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 
>

thank you. pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] ovn-controller.at: Stop hardcoding a list of iface types

[Takashi YAMAMOTO]

On Wed, Oct 19, 2016 at 1:48 AM, Ben Pfaff  wrote:

> On Tue, Oct 18, 2016 at 09:25:26PM +0900, YAMAMOTO Takashi wrote:
> > The list of supported iface types hardcoded in the test
> > is wrong on NetBSD. (or any userland-only ports I guess)
> > Instead of adding another case for NetBSD following WIN32,
> > just get the list from ovsdb.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Good idea.
>
> Acked-by: Ben Pfaff 
>

thank you. pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Makefile.am: Add DocumentationStyle.rst to distribution.

[Takashi YAMAMOTO]

On Wed, Oct 19, 2016 at 9:13 AM, Daniele Di Proietto 
wrote:

> 2016-10-18 16:59 GMT-07:00 Jarno Rajahalme :
>
> > Compile fails otherwise.
> >
> > Signed-off-by: Jarno Rajahalme 
> >
>
> Acked-by: Daniele Di Proietto 
>

Acked-by: YAMAMOTO Takashi 


> Thanks!
>
>
> > ---
> >  Makefile.am | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index a4842c1..dc74886 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -69,6 +69,7 @@ docs = \
> > CONTRIBUTING.md \
> > CodingStyle.md \
> > DESIGN.md \
> > +   DocumentationStyle.rst \
> > FAQ.md \
> > INSTALL.rst \
> > INSTALL.Debian.rst \
> > --
> > 2.1.4
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn.at: Use = instead of == for test

[Takashi YAMAMOTO]

applied to master, thank you.

On Wed, Jul 6, 2016 at 12:36 AM, Ben Pfaff  wrote:

> On Tue, Jul 05, 2016 at 06:35:04PM +0900, YAMAMOTO Takashi wrote:
> > == is a GNU extension which might not be available.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-numa: Fix a compilation error

[Takashi YAMAMOTO]

On Thu, Jun 9, 2016 at 3:31 AM, Ben Pfaff  wrote:

> On Wed, Jun 08, 2016 at 03:19:46PM +0900, YAMAMOTO Takashi wrote:
> > Fix the following error on NetBSD 7.0.
> >
> > ../lib/ovs-numa.c: In function 'ovs_numa_set_cpu_mask':
> > ../lib/ovs-numa.c:555:9: error: array subscript has type 'char'
> [-Werror=char-subscripts]
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Thanks.
>
> Acked-by: Ben Pfaff 
>

applied, thank you
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] netdev-native-tnl: Fix a build error on NetBSD 7.0

[Takashi YAMAMOTO]

On Sat, May 21, 2016 at 3:42 AM, Jeff Feng  wrote:

> >
> > diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> > index 2e181f2..123f3b2 100644
> > --- a/lib/netdev-native-tnl.c
> > +++ b/lib/netdev-native-tnl.c
> > @@ -20,6 +20,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >
>
> Thanks YAMAMOTO. Now I can compile whole ovs code in OS X.
>
> Tested-by: Jeff Feng 
>

thank you.  pushed to master.


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

Re: [ovs-dev] [PATCH 3/4] ovn-controller-vtep.at: Pre-sort output before feeding to "sort -d"

[Takashi YAMAMOTO]

On Fri, May 20, 2016 at 2:30 PM, Takashi YAMAMOTO <yamam...@ovn.org> wrote:

>
>
> On Fri, May 20, 2016 at 12:45 PM, Ben Pfaff <b...@ovn.org> wrote:
>
>> On Wed, May 18, 2016 at 03:30:30AM +0900, YAMAMOTO Takashi wrote:
>> > NetBSD's "sort -d" preserves the order of lines which doesn't have
>> > alphanumeric and blanks.  eg. empty lines and [].
>> > It means it sometimes preserve unstable order of the list output.
>> >
>> > Also, simply remove -d option where the expected output doesn't
>> > include [].
>> >
>> > Signed-off-by: YAMAMOTO Takashi <yamam...@ovn.org>
>>
>> Hmm, I hadn't noticed use of "sort -d" here before.  Do you think it is
>> worth using at all?
>>
>
> "sort -d" or some equivalent would be necessary here, because
> uuid can start with either numbers and alphabets, and "[]" is after
> numbers and before alphabets.
>
>
>> Acked-by: Ben Pfaff <b...@ovn.org>
>>
>
pushed to master.  thank you.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 4/4] utilities: Tweak python shebangs to use env

[Takashi YAMAMOTO]

On Fri, May 20, 2016 at 2:42 PM, Takashi YAMAMOTO <yamam...@ovn.org> wrote:

>
>
> On Fri, May 20, 2016 at 12:47 PM, Ben Pfaff <b...@ovn.org> wrote:
>
>> On Wed, May 18, 2016 at 03:30:31AM +0900, YAMAMOTO Takashi wrote:
>> > "python" command provided by pkg_alternatives is a shell script.
>> > At least on NetBSD-7, execve can't execute scripts whose interpreter
>> > is another shell script.  (While some "rich" shells like zsh seem
>> > to have handle the case by itself, NetBSD's /bin/sh doesn't.)
>> > Workaround the issue by using env command for shebangs for
>> > these scripts.
>> >
>> > Noticed with the recent tunnel-push-pop.at tests using ovs-pcap
>> command.
>> >
>> > Signed-off-by: YAMAMOTO Takashi <yamam...@ovn.org>
>>
>> Oh, how unfortunate.  (I'm surprised that this doesn't break practically
>> every Python program on NetBSD.)
>>
>
> i agree.  i'm inclined to report it as a bug there as well.
>
>
>> Acked-by: Ben Pfaff <b...@ovn.org>
>>
>
pushed to master.  thank you.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/4] ovsdb-server.at: Fix races

[Takashi YAMAMOTO]

On Fri, May 20, 2016 at 12:43 PM, Ben Pfaff  wrote:

> On Wed, May 18, 2016 at 03:30:29AM +0900, YAMAMOTO Takashi wrote:
> > As ovsdb-server creates pid file before unixctl socket, waiting
> > for pid file creation is not enough.  Fix the race by retrying
> > with "version" command before assuming the server is up.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Good catch, thank you.
>
> Acked-by: Ben Pfaff 
>

pushed to master.  thank you.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/4] dpif: Remove a warning

[Takashi YAMAMOTO]

On Fri, May 20, 2016 at 12:38 PM, Ben Pfaff  wrote:

> On Wed, May 18, 2016 at 03:30:28AM +0900, YAMAMOTO Takashi wrote:
> > Remove "attempted to unregister a datapath provider that is not
> registered"
> > warning.  It's normal for --enabled-dummy=system with userland-only
> build.
> > ovn-controller-vtep.at tests use the flag and fail on the extra warning.
> >
> > Alternatively, we can make the tests ignore this specific warning.
> > But currently it doesn't make much sense as dp_unregister_provider
> > is only used for --enabled-dummy.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 
>

pushed to master.  thank you.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 4/4] utilities: Tweak python shebangs to use env

[Takashi YAMAMOTO]

On Fri, May 20, 2016 at 12:47 PM, Ben Pfaff  wrote:

> On Wed, May 18, 2016 at 03:30:31AM +0900, YAMAMOTO Takashi wrote:
> > "python" command provided by pkg_alternatives is a shell script.
> > At least on NetBSD-7, execve can't execute scripts whose interpreter
> > is another shell script.  (While some "rich" shells like zsh seem
> > to have handle the case by itself, NetBSD's /bin/sh doesn't.)
> > Workaround the issue by using env command for shebangs for
> > these scripts.
> >
> > Noticed with the recent tunnel-push-pop.at tests using ovs-pcap command.
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Oh, how unfortunate.  (I'm surprised that this doesn't break practically
> every Python program on NetBSD.)
>

i agree.  i'm inclined to report it as a bug there as well.


> Acked-by: Ben Pfaff 
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/4] ovn-controller-vtep.at: Pre-sort output before feeding to "sort -d"

[Takashi YAMAMOTO]

On Fri, May 20, 2016 at 12:45 PM, Ben Pfaff  wrote:

> On Wed, May 18, 2016 at 03:30:30AM +0900, YAMAMOTO Takashi wrote:
> > NetBSD's "sort -d" preserves the order of lines which doesn't have
> > alphanumeric and blanks.  eg. empty lines and [].
> > It means it sometimes preserve unstable order of the list output.
> >
> > Also, simply remove -d option where the expected output doesn't
> > include [].
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Hmm, I hadn't noticed use of "sort -d" here before.  Do you think it is
> worth using at all?
>

"sort -d" or some equivalent would be necessary here, because
uuid can start with either numbers and alphabets, and "[]" is after
numbers and before alphabets.


> Acked-by: Ben Pfaff 
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] util.h: Restore stdarg.h which is necessary for va_list

[Takashi YAMAMOTO]

On Sat, Apr 23, 2016 at 12:28 PM, Ben Pfaff  wrote:

> On Sat, Apr 23, 2016 at 11:14:40AM +0900, YAMAMOTO Takashi wrote:
> > Fixes a regression in commit b44ff8d826535025f4f8d12808c4ef36a7a8 .
> > ("Misc cleanup with "util.h" header files")
> >
> > Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 
>

thank you. pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Add configurable OpenFlow port name.

[Takashi YAMAMOTO]

i don't have any problem with Xiao's approach.
just wanted to make sure alternatives considered.

wrt implementation, Xiao, can you rebase it?

On Fri, Apr 22, 2016 at 3:39 AM, Ben Pfaff <b...@ovn.org> wrote:

> Yamamoto-san, I could really use your opinion here: do you think that
> this should be done differently?  If you do, then I will not accept it.
> But if you do not feel strongly about it, then I'll start properly
> reviewing it.
>
> Thanks,
>
> Ben.
>
> On Mon, Apr 18, 2016 at 07:05:52PM +0800, Xiao Liang wrote:
> > On Mon, Apr 18, 2016 at 5:46 PM, Takashi YAMAMOTO <yamam...@ovn.org>
> wrote:
> > > for some reasons you want to change of_name without re-creating a port?
> > > why? (just curious)
> > >
> >
> > I don't have special use cases, just for convenience.
> >
> > >
> > > On Mon, Apr 18, 2016 at 4:19 PM, Xiao Liang <shaw.l...@gmail.com>
> wrote:
> > >>
> > >> By introducing of_name, ovs_name serves as a key of ports which is
> > >> shared by ofproto and netdev. It's easier to find and convert ports
> > >> back and forth. of_name and kernel_name could be configured (if
> > >> supported) independently of each other.
> > >>
> > >> On Mon, Apr 18, 2016 at 11:43 AM, Takashi YAMAMOTO <yamam...@ovn.org>
> > >> wrote:
> > >> > let me explain what netdev-bsd does first.
> > >> > on some platform "tap" interfaces are always named automatically by
> > >> > kernel
> > >> > itself
> > >> > and there's no way to rename them.  say, they always will have names
> > >> > like
> > >> > "tap0".
> > >> > so if you does "ovs-vsctl add-port br0 foo",
> > >> >   ovs_name = "foo"
> > >> >   kernel_name = "tap0"
> > >> >
> > >> > now, you are going to add another name for openflow. let's call it
> > >> > of_name.
> > >> > eg. "ovs-vsctl add-port br0 foo -- set int foo ofname=wan",
> > >> >   of_name = "wan"
> > >> >   ovs_name = "foo"
> > >> >   kernel_name = "tap0"
> > >> >
> > >> > while i don't have strong opinions either ways,
> > >> > i'm not sure why you want to use different names for of_name and
> > >> > ovs_name
> > >> > in the first place.  eg. what's wrong with "ovs-vsctl add-port br0
> wan".
> > >> > can you explain a little?
> > >> >
> > >> > On Mon, Apr 18, 2016 at 10:37 AM, Xiao Liang <shaw.l...@gmail.com>
> > >> > wrote:
> > >> >>
> > >> >> Hi Ben, Yamamoto-san,
> > >> >>
> > >> >> Kindly remind you of this thread. Would like to hear your
> preference
> > >> >> on the way to implement this feature.
> > >> >>
> > >> >> On Mon, Apr 11, 2016 at 11:18 PM, Ben Pfaff <b...@ovn.org> wrote:
> > >> >> > On Mon, Apr 11, 2016 at 04:30:04PM +0800, Xiao Liang wrote:
> > >> >> >> On Mon, Apr 11, 2016 at 3:42 PM, Takashi Yamamoto
> > >> >> >> <yamam...@midokura.com> wrote:
> > >> >> >> > hi,
> > >> >> >> >
> > >> >> >> > have you considered the opposite way?
> > >> >> >> > ie. have an ability to specify the device name.
> > >> >> >> >
> > >> >> >> > netdev-bsd already has a distinction between "kernel name" and
> > >> >> >> > "ovs
> > >> >> >> > name".
> > >> >> >> >
> > >> >> >>
> > >> >> >> Hi,
> > >> >> >>
> > >> >> >> I'm not familiar with netdev-bsd code, but I think this approach
> > >> >> >> will
> > >> >> >> make ports more difficult to manage and need much more effort.
> > >> >> >
> > >> >> > Yamamoto-san: thanks for bringing this up.  I'm going to wait
> for you
> > >> >> > and Xiao to talk this through a bit before continuing review.
> > >> >> ___
> > >> >> dev mailing list
> > >> >> dev@openvswitch.org
> > >> >> http://openvswitch.org/mailman/listinfo/dev
> > >> >
> > >> >
> > >
> > >
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Add configurable OpenFlow port name.

[Takashi YAMAMOTO]

for some reasons you want to change of_name without re-creating a port?
why? (just curious)

On Mon, Apr 18, 2016 at 4:19 PM, Xiao Liang <shaw.l...@gmail.com> wrote:

> By introducing of_name, ovs_name serves as a key of ports which is
> shared by ofproto and netdev. It's easier to find and convert ports
> back and forth. of_name and kernel_name could be configured (if
> supported) independently of each other.
>
> On Mon, Apr 18, 2016 at 11:43 AM, Takashi YAMAMOTO <yamam...@ovn.org>
> wrote:
> > let me explain what netdev-bsd does first.
> > on some platform "tap" interfaces are always named automatically by
> kernel
> > itself
> > and there's no way to rename them.  say, they always will have names like
> > "tap0".
> > so if you does "ovs-vsctl add-port br0 foo",
> >   ovs_name = "foo"
> >   kernel_name = "tap0"
> >
> > now, you are going to add another name for openflow. let's call it
> of_name.
> > eg. "ovs-vsctl add-port br0 foo -- set int foo ofname=wan",
> >   of_name = "wan"
> >   ovs_name = "foo"
> >   kernel_name = "tap0"
> >
> > while i don't have strong opinions either ways,
> > i'm not sure why you want to use different names for of_name and ovs_name
> > in the first place.  eg. what's wrong with "ovs-vsctl add-port br0 wan".
> > can you explain a little?
> >
> > On Mon, Apr 18, 2016 at 10:37 AM, Xiao Liang <shaw.l...@gmail.com>
> wrote:
> >>
> >> Hi Ben, Yamamoto-san,
> >>
> >> Kindly remind you of this thread. Would like to hear your preference
> >> on the way to implement this feature.
> >>
> >> On Mon, Apr 11, 2016 at 11:18 PM, Ben Pfaff <b...@ovn.org> wrote:
> >> > On Mon, Apr 11, 2016 at 04:30:04PM +0800, Xiao Liang wrote:
> >> >> On Mon, Apr 11, 2016 at 3:42 PM, Takashi Yamamoto
> >> >> <yamam...@midokura.com> wrote:
> >> >> > hi,
> >> >> >
> >> >> > have you considered the opposite way?
> >> >> > ie. have an ability to specify the device name.
> >> >> >
> >> >> > netdev-bsd already has a distinction between "kernel name" and "ovs
> >> >> > name".
> >> >> >
> >> >>
> >> >> Hi,
> >> >>
> >> >> I'm not familiar with netdev-bsd code, but I think this approach will
> >> >> make ports more difficult to manage and need much more effort.
> >> >
> >> > Yamamoto-san: thanks for bringing this up.  I'm going to wait for you
> >> > and Xiao to talk this through a bit before continuing review.
> >> ___
> >> dev mailing list
> >> dev@openvswitch.org
> >> http://openvswitch.org/mailman/listinfo/dev
> >
> >
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Add configurable OpenFlow port name.

[Takashi YAMAMOTO]

let me explain what netdev-bsd does first.
on some platform "tap" interfaces are always named automatically by kernel
itself
and there's no way to rename them.  say, they always will have names like
"tap0".
so if you does "ovs-vsctl add-port br0 foo",
  ovs_name = "foo"
  kernel_name = "tap0"

now, you are going to add another name for openflow. let's call it of_name.
eg. "ovs-vsctl add-port br0 foo -- set int foo ofname=wan",
  of_name = "wan"
  ovs_name = "foo"
  kernel_name = "tap0"

while i don't have strong opinions either ways,
i'm not sure why you want to use different names for of_name and ovs_name
in the first place.  eg. what's wrong with "ovs-vsctl add-port br0 wan".
can you explain a little?

On Mon, Apr 18, 2016 at 10:37 AM, Xiao Liang <shaw.l...@gmail.com> wrote:

> Hi Ben, Yamamoto-san,
>
> Kindly remind you of this thread. Would like to hear your preference
> on the way to implement this feature.
>
> On Mon, Apr 11, 2016 at 11:18 PM, Ben Pfaff <b...@ovn.org> wrote:
> > On Mon, Apr 11, 2016 at 04:30:04PM +0800, Xiao Liang wrote:
> >> On Mon, Apr 11, 2016 at 3:42 PM, Takashi Yamamoto <
> yamam...@midokura.com> wrote:
> >> > hi,
> >> >
> >> > have you considered the opposite way?
> >> > ie. have an ability to specify the device name.
> >> >
> >> > netdev-bsd already has a distinction between "kernel name" and "ovs
> name".
> >> >
> >>
> >> Hi,
> >>
> >> I'm not familiar with netdev-bsd code, but I think this approach will
> >> make ports more difficult to manage and need much more effort.
> >
> > Yamamoto-san: thanks for bringing this up.  I'm going to wait for you
> > and Xiao to talk this through a bit before continuing review.
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Add configurable OpenFlow port name.

[Takashi Yamamoto]

hi,

have you considered the opposite way?
ie. have an ability to specify the device name.

netdev-bsd already has a distinction between "kernel name" and "ovs name".

On Mon, Apr 11, 2016 at 4:23 PM, Xiao Liang  wrote:
> Add new column "ofname" in Interface table to configure port name reported
> to controllers with OpenFlow protocol, thus decouple OpenFlow port name from
> device name.
>
> For example:
> # ovs-vsctl set Interface eth0 ofname=wan
> # ovs-vsctl set Interface eth1 ofname=lan0
> then controllers can recognize ports by their names.
>
> Signed-off-by: Xiao Liang 
> ---
> v2: Added test for ofname
> Increased db schema version
> ---
>  lib/db-ctl-base.h  |  2 +-
>  ofproto/ofproto-provider.h |  1 +
>  ofproto/ofproto.c  | 67 
> --
>  ofproto/ofproto.h  |  9 ++-
>  tests/ofproto.at   | 60 +
>  utilities/checkpatch.py|  2 +-
>  utilities/ovs-vsctl.c  |  1 +
>  vswitchd/bridge.c  | 10 +--
>  vswitchd/vswitch.ovsschema |  6 +++--
>  vswitchd/vswitch.xml   | 14 ++
>  10 files changed, 163 insertions(+), 9 deletions(-)
>
> diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
> index f8f576b..5bd62d5 100644
> --- a/lib/db-ctl-base.h
> +++ b/lib/db-ctl-base.h
> @@ -177,7 +177,7 @@ struct weak_ref_table {
>  struct cmd_show_table {
>  const struct ovsdb_idl_table_class *table;
>  const struct ovsdb_idl_column *name_column;
> -const struct ovsdb_idl_column *columns[3]; /* Seems like a good number. 
> */
> +const struct ovsdb_idl_column *columns[4]; /* Seems like a good number. 
> */
>  const struct weak_ref_table wref_table;
>  };
>
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 9373a2c..c34047c 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -84,6 +84,7 @@ struct ofproto {
>  struct hmap ports;  /* Contains "struct ofport"s. */
>  struct shash port_by_name;
>  struct simap ofp_requests;  /* OpenFlow port number requests. */
> +struct smap ofp_names;  /* OpenFlow port names. */
>  uint16_t alloc_port_no; /* Last allocated OpenFlow port number. */
>  uint16_t max_ports; /* Max possible OpenFlow port num, plus one. 
> */
>  struct hmap ofport_usage;   /* Map ofport to last used time. */
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 6e74e5e..a47d139 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -549,6 +549,7 @@ ofproto_create(const char *datapath_name, const char 
> *datapath_type,
>  hmap_init(>ofport_usage);
>  shash_init(>port_by_name);
>  simap_init(>ofp_requests);
> +smap_init(>ofp_names);
>  ofproto->max_ports = ofp_to_u16(OFPP_MAX);
>  ofproto->eviction_group_timer = LLONG_MIN;
>  ofproto->tables = NULL;
> @@ -1545,6 +1546,7 @@ ofproto_destroy__(struct ofproto *ofproto)
>  hmap_destroy(>ofport_usage);
>  shash_destroy(>port_by_name);
>  simap_destroy(>ofp_requests);
> +smap_destroy(>ofp_names);
>
>  OFPROTO_FOR_EACH_TABLE (table, ofproto) {
>  oftable_destroy(table);
> @@ -1944,7 +1946,7 @@ ofproto_port_open_type(const char *datapath_type, const 
> char *port_type)
>   * 'ofp_portp' is non-null). */
>  int
>  ofproto_port_add(struct ofproto *ofproto, struct netdev *netdev,
> - ofp_port_t *ofp_portp)
> + ofp_port_t *ofp_portp, const char *ofp_name)
>  {
>  ofp_port_t ofp_port = ofp_portp ? *ofp_portp : OFPP_NONE;
>  int error;
> @@ -1955,6 +1957,9 @@ ofproto_port_add(struct ofproto *ofproto, struct netdev 
> *netdev,
>
>  simap_put(>ofp_requests, netdev_name,
>ofp_to_u16(ofp_port));
> +if (ofp_name) {
> +smap_replace(>ofp_names, netdev_name, ofp_name);
> +}
>  error = update_port(ofproto, netdev_name);
>  }
>  if (ofp_portp) {
> @@ -2008,6 +2013,8 @@ ofproto_port_del(struct ofproto *ofproto, ofp_port_t 
> ofp_port)
>  simap_delete(>ofp_requests, ofp_request_node);
>  }
>
> +smap_remove(>ofp_names, name);
> +
>  error = ofproto->ofproto_class->port_del(ofproto, ofp_port);
>  if (!error && ofport) {
>  /* 'name' is the netdev's name and update_port() is going to close 
> the
> @@ -2284,6 +2291,7 @@ ofport_open(struct ofproto *ofproto,
>  {
>  enum netdev_flags flags;
>  struct netdev *netdev;
> +const char *ofp_name;
>  int error;
>
>  error = netdev_open(ofproto_port->name, ofproto_port->type, );
> @@ -2306,7 +2314,13 @@ ofport_open(struct ofproto *ofproto,
>  }
>  pp->port_no = ofproto_port->ofp_port;
>  netdev_get_etheraddr(netdev, >hw_addr);
> -ovs_strlcpy(pp->name, ofproto_port->name, sizeof pp->name);
> +
> +ofp_name = smap_get(>ofp_names, ofproto_port->name);
> +if (!ofp_name) {
> +   

Re: [ovs-dev] [PATCH 4/4] ofproto/trace: Fix "unchanged" output for Final flow

[Takashi Yamamoto]

On Thu, Mar 24, 2016 at 1:12 AM, Takashi Yamamoto <yamam...@midokura.com> wrote:
> On Thu, Mar 24, 2016 at 12:50 AM, Ben Pfaff <b...@ovn.org> wrote:
>> On Wed, Mar 16, 2016 at 09:39:34PM +0900, YAMAMOTO Takashi wrote:
>>> Clear actset_output so that it can be compared via flow_equal.
>>> Note: trace->key has actset_output == 0.
>>>
>>> Found by OVS flow tests under development for Neutron. [1]
>>>
>>> [1] 
>>> https://review.openstack.org/#/c/235155/10/neutron/tests/functional/agent/test_ovs_flows.py@399
>>>
>>> Signed-off-by: YAMAMOTO Takashi <yamam...@midokura.com>
>>
>> Thanks for noticing that, I hadn't myself.
>>
>> Acked-by: Ben Pfaff <b...@ovn.org>
>
> thank you. pushed to master.

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

Re: [ovs-dev] [PATCH 3/4] vlog.at: Avoid using GNU sed extension

[Takashi Yamamoto]

On Thu, Mar 24, 2016 at 12:46 AM, Ben Pfaff  wrote:
> On Wed, Mar 16, 2016 at 09:39:33PM +0900, YAMAMOTO Takashi wrote:
>> BRE alternative (\|) is an GNU sed extension. [1]
>> It isn't available in NetBSD sed.
>>
>> [1] http://www.gnu.org/software/sed/manual/sed.html#Regular-Expressions
>> regexp1\|regexp2
>> Matches either regexp1 or regexp2. Use parentheses to use
>> complex alternative regular expressions. The matching process
>> tries each alternative in turn, from left to right, and the
>> first one that succeeds is used. It is a GNU extension.
>>
>> Signed-off-by: YAMAMOTO Takashi 
>
> Oops.  Thanks for the fix.
>
> Acked-by: Ben Pfaff 

thank you. pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 4/4] ofproto/trace: Fix "unchanged" output for Final flow

[Takashi Yamamoto]

On Thu, Mar 24, 2016 at 12:50 AM, Ben Pfaff  wrote:
> On Wed, Mar 16, 2016 at 09:39:34PM +0900, YAMAMOTO Takashi wrote:
>> Clear actset_output so that it can be compared via flow_equal.
>> Note: trace->key has actset_output == 0.
>>
>> Found by OVS flow tests under development for Neutron. [1]
>>
>> [1] 
>> https://review.openstack.org/#/c/235155/10/neutron/tests/functional/agent/test_ovs_flows.py@399
>>
>> Signed-off-by: YAMAMOTO Takashi 
>
> Thanks for noticing that, I hadn't myself.
>
> Acked-by: Ben Pfaff 

thank you. pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/4] ovs_strerror, ovs_format_message: Always use "Success" for errno 0

[Takashi Yamamoto]

On Thu, Mar 24, 2016 at 12:44 AM, Ben Pfaff  wrote:
> On Wed, Mar 16, 2016 at 09:39:32PM +0900, YAMAMOTO Takashi wrote:
>> So that testsuite can compare log messages including the string.
>>
>> Signed-off-by: YAMAMOTO Takashi 
>
> Thanks.  I didn't know about this problem.  Maybe the testsuite should
> be more lenient, but I guess that this is OK too, and less work.
>
> Acked-by: Ben Pfaff 

thank you. pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/4] INSTALL.NetBSD.md: Update for six

[Takashi Yamamoto]

2016-03-24 0:42 GMT+09:00 Ben Pfaff :
> On Wed, Mar 16, 2016 at 09:39:31PM +0900, YAMAMOTO Takashi wrote:
>> Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 

thank you. pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] ovn.at: Fix races

[Takashi Yamamoto]

hi,

On Thu, Dec 3, 2015 at 3:26 AM, Flavio Fernandes  wrote:
> On Thu, Nov 26, 2015 at 4:41 AM, YAMAMOTO Takashi 
> wrote:
>
>> These tests are racy as nothing prevents packet re-ordering.
>> Fix them by sorting outputs before comparing.
>>
>> Signed-off-by: YAMAMOTO Takashi 
>>
>
> Reviewed-by: Flavio Fernandes 

i've already merged this patch.
anyway, thank you for review!

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

Re: [ovs-dev] [PATCH] fix compile error on FreeBSD

[Takashi Yamamoto]

On Wed, Dec 2, 2015 at 3:21 PM, Kevin Lo  wrote:
> Hi,
>
> Missing a include file caused compile error:
> http://dpaste.com/22E4MHK

why to add it to lib/util.h, rather than ovs-router.c?

>
> FreeBSD (and NetBSD?) needs to include netinet/in.h to define
> struct in6_addr.
>
> Signed-off-by: Kevin Lo 
> ---
>
> diff --git a/lib/util.h b/lib/util.h
> index 340ef65..9db65c5 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/3] ovn.at: Use {} to make this less ambiguous

[Takashi Yamamoto]

On Sat, Nov 28, 2015 at 3:17 AM, Ben Pfaff  wrote:
> On Thu, Nov 26, 2015 at 06:41:30PM +0900, YAMAMOTO Takashi wrote:
>> While (surprisingly to me) bash interprets $10 as ${1}0,
>> many other shells, including NetBSD's /bin/sh, interpret it as ${10}.
>>
>> Signed-off-by: YAMAMOTO Takashi 
>
> Acked-by: Ben Pfaff 

pushed with the commit message updated as Simon suggested.

>
> I guess that this is documented in the Autoconf manual, but I had never
> really paid attention before:

thank you.

>
> '${10}'
>  The 10th, 11th, ... positional parameters can be accessed only
>  after a 'shift'.  The 7th Edition shell reported an error if given
>  '${10}', and Solaris 10 '/bin/sh' still acts that way:
>
>   $ set 1 2 3 4 5 6 7 8 9 10
>   $ echo ${10}
>   bad substitution
>
>  Conversely, not all shells obey the Posix rule that when braces are
>  omitted, multiple digits beyond a '$' imply the single-digit
>  positional parameter expansion concatenated with the remaining
>  literal digits.  To work around the issue, you must use braces.
>
>   $ bash -c 'set a b c d e f g h i j; echo $10 ${1}0'
>   a0 a0
>   $ dash -c 'set a b c d e f g h i j; echo $10 ${1}0'
>   j a0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] fix compile error on FreeBSD

[Takashi Yamamoto]

On Wed, Dec 2, 2015 at 4:12 PM, Kevin Lo <ke...@freebsd.org> wrote:
> On Wed, Dec 02, 2015 at 03:47:57PM +0900, Takashi Yamamoto wrote:
>>
>> On Wed, Dec 2, 2015 at 3:21 PM, Kevin Lo <ke...@freebsd.org> wrote:
>> > Hi,
>> >
>> > Missing a include file caused compile error:
>> > http://dpaste.com/22E4MHK
>>
>> why to add it to lib/util.h, rather than ovs-router.c?
>
> Because the declarations for ovs_router_lookup() and ovs_router_insert() are
> in ovs-router.h, adding netinet/in.h to ovs-router.c won't help.
> please see the log, thanks.

well, i can understand if it's ovs-router.h.
(although a forward decl seems enough there.)
my question is why lib/util.h.

>
>> > FreeBSD (and NetBSD?) needs to include netinet/in.h to define
>> > struct in6_addr.
>> >
>> > Signed-off-by: Kevin Lo <ke...@freebsd.org>
>> > ---
>> >
>> > diff --git a/lib/util.h b/lib/util.h
>> > index 340ef65..9db65c5 100644
>> > --- a/lib/util.h
>> > +++ b/lib/util.h
>> > @@ -20,6 +20,7 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> >  #include 
>> >  #include 
>> >  #include 
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/3] ovn.at: Avoid using GNU sed extension

[Takashi Yamamoto]

hi,

On Sat, Nov 28, 2015 at 3:20 AM, Ben Pfaff  wrote:
> On Thu, Nov 26, 2015 at 06:41:31PM +0900, YAMAMOTO Takashi wrote:
>> Signed-off-by: YAMAMOTO Takashi 
>> ---
>>  tests/ovn.at | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index de0a830..a4dbf96 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -566,7 +566,7 @@ ovn-sbctl dump-flows -- list multicast_group
>>  # more) list the VIFs on which the packet should be received.  INPORT and 
>> the
>>  # OUTPORTs are specified as lport numbers, e.g. 11 for vif11.
>>  trim_zeros() {
>> -sed 's/\(00\)\{1,\}$//'
>> +sed 's/\(00\)\(00\)*$//'
>>  }
>
> I'm pretty sure this is a standard POSIX basic regular expression, see
> 9.3.6 item 5 at http://pubs.opengroup.org/onlinepubs/9699919799/.
>
> BSD doesn't support it?

hm, you are right.  and sed in NetBSD seems to support it.
i guess i was confused by some other things.
i withdraw this patch.  thank you!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 17/21] ofproto-dpif.at: Workaround a race

[Takashi Yamamoto]

i forgot it too.  i'll investigate if it can be used.

On Tue, Nov 10, 2015 at 1:55 PM, Ben Pfaff  wrote:
> On Mon, Nov 09, 2015 at 08:53:14PM -0800, Joe Stringer wrote:
>> On 18 October 2015 at 21:29, YAMAMOTO Takashi  wrote:
>> > Signed-off-by: YAMAMOTO Takashi 
>> > ---
>> >  tests/ofproto-dpif.at | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>> > index f64c56c..b93c8fb 100644
>> > --- a/tests/ofproto-dpif.at
>> > +++ b/tests/ofproto-dpif.at
>> > @@ -844,6 +844,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
>> >  AT_CHECK([ovs-ofctl mod-port br0 5 noforward])
>> >  AT_CHECK([ovs-ofctl mod-port br0 6 noflood])
>> >
>> > +sleep 1  # wait for revalidation
>>
>> Is it possible to use ovs-appctl revalidator/wait?
>
> I forgot we had that!  I hope it works too.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 10/21] ofproto-macros.at: Add another strerror(0) value

[Takashi Yamamoto]

hi,

On Tue, Nov 10, 2015 at 1:19 PM, Ben Pfaff  wrote:
> On Mon, Oct 19, 2015 at 01:29:01PM +0900, YAMAMOTO Takashi wrote:
>> On NetBSD, strerror(0) is "Undefined error: 0".
>>
>> Signed-off-by: YAMAMOTO Takashi 
>> ---
>>  tests/ofproto-macros.at | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
>> index 94d7c86..368dade 100644
>> --- a/tests/ofproto-macros.at
>> +++ b/tests/ofproto-macros.at
>> @@ -28,7 +28,8 @@ prt==1 { sub(/[ \t]*$/, ""); print $0 }
>>  vconn_sub() {
>>  sed '
>>  s/tcp:127.0.0.1:[0-9][0-9]*:/unix:/
>> -s/No error/Success/
>> +s/(No error)/(Success)/
>> +s/(Undefined error: 0)/(Success)/
>>  '
>>  }
>
> Since we now have two different OSes that need special treatment here,
> it might be better to do something more uniform to avoid the special
> treatment.  One idea would be to modify ovs_strerror() to always provide
> the same string for error 0.  Another would be to modify vconn.c to
> avoid indicating any error when there is none, e.g.:

it's a good idea.

>
> diff --git a/lib/vconn.c b/lib/vconn.c
> index d835943..f50f3e8 100644
> --- a/lib/vconn.c
> +++ b/lib/vconn.c
> @@ -683,7 +683,9 @@ do_send(struct vconn *vconn, struct ofpbuf *msg)
>  } else {
>  char *s = ofp_to_string(msg->data, msg->size, 1);
>  retval = (vconn->vclass->send)(vconn, msg);
> -if (retval != EAGAIN) {
> +if (!retval) {
> +VLOG_DBG_RL(_rl, "%s: sent: %s", vconn->name, s);
> +} else if (retval != EAGAIN) {
>  VLOG_DBG_RL(_rl, "%s: sent (%s): %s",
>  vconn->name, ovs_strerror(retval), s);
>  }
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 06/21] daemon_switch_user: Improve portablility

[Takashi Yamamoto]

hi,

On Mon, Oct 19, 2015 at 3:14 PM, Andy Zhou  wrote:
> On Sun, Oct 18, 2015 at 9:28 PM, YAMAMOTO Takashi  
> wrote:
>> NetBSD doesn't have [gs]etres[ug]id.
>>
>> Signed-off-by: YAMAMOTO Takashi 
>> ---
>>  lib/daemon-unix.c | 40 ++--
>>  1 file changed, 18 insertions(+), 22 deletions(-)
>>
> Thanks for testing on NetBSD.
>
> I am concerned that on platforms supports saved uid, Would this patch
> leave that value not changed, thus open up a security risk?
>
> How about we add a stub version of [gs]etres[ug]id for the NetBSD
> platform that can safely ignore the saved uid/ gid for that platform?

NetBSD has saved uid/gid.
saved ids are expected to be changed by set[ug]id.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/setuid.html
http://man.netbsd.org/HEAD/usr/share/man/html2/setuid.html

i'm not sure what security risks you are concerning about.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 06/21] daemon_switch_user: Improve portablility

[Takashi Yamamoto]

hi,

On Tue, Oct 20, 2015 at 7:14 AM, Andy Zhou <az...@nicira.com> wrote:
> I am going by the advice of paper " The Murky Issue of Changing
> Process Identity: Revising “Setuid Demystified” "
>
> On page 7, it says:
>
> Specifically, all OSes that support getresuid (see Figure 3) also
> support setresuid and setresgid. These offer the clearest and most
> consistent semantics, and can be used by privileged and non-privileged
> processes alike.
>
> According to the paper,  setuid() may or may not change saved uid, it
> is OS dependent and may only change effective uid in cause current uid
> is not
> zero.
>
> Also according to the same paper in Figure 3, getresuid() is supported
> by Linux, HPUX, FreeBSD and OpenBSD, it would be nice to let those OS
> use this API. For NetBSD, we can resolve this by emulating the
> getresuid() call.  Make sense?

well, this fallback code is currently for FreeBSD and NetBSD,
for which the semantics are consistent, right?

>
>
>
> On Sun, Oct 18, 2015 at 11:48 PM, Takashi Yamamoto
> <yamam...@midokura.com> wrote:
>> hi,
>>
>> On Mon, Oct 19, 2015 at 3:14 PM, Andy Zhou <az...@nicira.com> wrote:
>>> On Sun, Oct 18, 2015 at 9:28 PM, YAMAMOTO Takashi <yamam...@midokura.com> 
>>> wrote:
>>>> NetBSD doesn't have [gs]etres[ug]id.
>>>>
>>>> Signed-off-by: YAMAMOTO Takashi <yamam...@midokura.com>
>>>> ---
>>>>  lib/daemon-unix.c | 40 ++--
>>>>  1 file changed, 18 insertions(+), 22 deletions(-)
>>>>
>>> Thanks for testing on NetBSD.
>>>
>>> I am concerned that on platforms supports saved uid, Would this patch
>>> leave that value not changed, thus open up a security risk?
>>>
>>> How about we add a stub version of [gs]etres[ug]id for the NetBSD
>>> platform that can safely ignore the saved uid/ gid for that platform?
>>
>> NetBSD has saved uid/gid.
>> saved ids are expected to be changed by set[ug]id.
>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/setuid.html
>> http://man.netbsd.org/HEAD/usr/share/man/html2/setuid.html
>>
>> i'm not sure what security risks you are concerning about.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/4] ovs-ofctl: Fix OpenFlow versions with '--bundle'

[Takashi Yamamoto]

On Fri, Oct 16, 2015 at 6:28 AM, Jarno Rajahalme  wrote:
> While the presence of the '--bundle' option implicitly added the
> OpenFlow 1.4 to the allowed protocols, it failed to remove OpenFlow
> 1.0 from the allowed protocols.  This is changed so that '--bundle'
> option now also implicitly removes versions lesser than 1.4 from the
> allowed protocols.  This has no behavioral difference when ovs-ofctl
> is paired with OVS that supports OpenFlow 1.4, as the greatest common
> version is negotiated, but prevents negotiation of OpenFlow 1.0 when
> OVS does not support OpenFlow 1.4.
>
> Found by inspection.
>
> Signed-off-by: Jarno Rajahalme 

Acked-by: YAMAMOTO Takashi 

> ---
>  tests/ofproto.at  | 12 ++--
>  tests/ovs-ofctl.at|  8 
>  utilities/ovs-ofctl.c |  3 +++
>  3 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 5e4441c..4c6dd29 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -3962,8 +3962,8 @@ vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY:
>  vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
>   version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
>  vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
> - version bitmap: 0x01, 0x05
> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports versions 0x01, 0x05)
> + version bitmap: 0x05
> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports version 0x05)
>  vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
>  vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
> @@ -4014,8 +4014,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply:
>  vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
>   version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
>  vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
> - version bitmap: 0x01, 0x05
> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports versions 0x01, 0x05)
> + version bitmap: 0x05
> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports version 0x05)
>  vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
>  vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
> @@ -4045,8 +4045,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply:
>  vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
>   version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
>  vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
> - version bitmap: 0x01, 0x05
> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports versions 0x01, 0x05)
> + version bitmap: 0x05
> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports version 0x05)
>  vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
>  vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 6f03adb..7375cad 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -2889,8 +2889,8 @@ AT_CHECK([print_vconn_debug | vconn_windows_sub | 
> ofctl_strip], [0], [dnl
>  vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
>   version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
>  vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
> - version bitmap: 0x01, 0x05
> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports versions 0x01, 0x05)
> + version bitmap: 0x05
> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports version 0x05)
>  vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
>  vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
> @@ -2926,8 +2926,8 @@ vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL 
> (OF1.4):
>  vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
>   version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
>  vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
> - version bitmap: 0x01, 0x05
> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports versions 0x01, 0x05)
> + version bitmap: 0x05
> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 
> and earlier, peer supports version 0x05)
>  vconn|DBG|unix: received: OFPST_FLOW request (OF1.4):
>  vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4):
>   importance=1, dl_vlan=1 actions=drop
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index fbc9da4..ee15e1a 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -312,6 +312,9 @@ parse_options(int argc, 

Re: [ovs-dev] shell substitution "${withval/:/}"

[Takashi Yamamoto]

On Wed, Oct 14, 2015 at 1:35 PM, IWAMOTO Toshihiro
<iwam...@valinux.co.jp> wrote:
> At Wed, 14 Oct 2015 13:06:01 +0900,
> Takashi Yamamoto wrote:
>>
>> hi,
>>
>> m4/openvswitch.m4 has the following substitution.
>> NetBSD's /bin/sh complains on that.
>>
>> PTHREAD_WIN32_DIR_DLL=/${withval/:/}/dll/x64
>>
>> i've looked at bash's man page but i couldn't find this substitution.
>
> My bash manpage has the following:
>
>${parameter/pattern/string}
>   Pattern substitution.  The pattern is expanded to produce a pat-
>   tern just as in pathname expansion.  Parameter is  expanded  and
>   the  longest match of pattern against its value is replaced with

thank you!

>
>> can you explain what does it mean, and even better, replace it
>> with something more portable?  thank you.
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 02/12] ovs-ofctl: Add bundle support and unit testing.

[Takashi Yamamoto]

hi,

On Wed, Jun 10, 2015 at 9:24 AM, Jarno Rajahalme  wrote:
> All existing ovs-ofctl flow mod commands now take an optional
> '--bundle' argument, which executes the flow mods as a single
> transaction.  OpenFlow 1.4+ is implicitly assumed when '--bundle' is
> specified.
>
> ovs-ofctl 'add-flow' and 'add-flows' commands now accept flow
> specifications that start with an optional 'add', 'modify', 'delete',
> 'modify_strict', or 'delete_strict' keyword, so that arbitrary flow
> table modifications may be specified.  For backwards compatibility, a
> missing keyword is treated as an 'add'.  With the new '--bundle'
> option all the modifications are executed as a single transaction
> using an OpenFlow 1.4 bundle.
>
> OpenFlow 1.4 requires bundles to support at least flow and port mods.
> This implementation does not yet support port mods in bundles.
>
> Another restriction is that the atomic transactions are not yet
> supported.
>
> Signed-off-by: Jarno Rajahalme 
> ---
>  NEWS|   19 +++-
>  include/openvswitch/vconn.h |3 +
>  lib/ofp-parse.c |   40 ++-
>  lib/ofp-parse.h |6 +-
>  lib/ofp-util.c  |   30 ++
>  lib/ofp-util.h  |2 +
>  lib/ofp-version-opt.c   |7 ++
>  lib/ofp-version-opt.h   |1 +
>  lib/vconn.c |  238 +
>  tests/ofproto-macros.at |   10 ++
>  tests/ofproto.at|  244 
> +++
>  tests/ovs-ofctl.at  |  107 +++
>  utilities/ovs-ofctl.8.in|   60 ---
>  utilities/ovs-ofctl.c   |   88 +++-
>  14 files changed, 813 insertions(+), 42 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index a480607..5bea237 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,6 @@
>  Post-v2.3.0
>  -
> -   - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
> +   - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
> - Add bash command-line completion support for ovs-vsctl Please check
>   utilities/ovs-command-compgen.INSTALL.md for how to use.
> - The MAC learning feature now includes per-port fairness to mitigate
> @@ -27,6 +27,11 @@ Post-v2.3.0
>   commands are now redundant and will be removed in a future
>   release.  See ovs-vswitchd(8) for details.
> - OpenFlow:
> + * OpenFlow 1.4 bundles are now supported, but for flow mod
> +   messages only. 'atomic' bundles are not yet supported, and
> +   'ordered' bundles are trivially supported, as all bundled
> +   messages are executed in the order they were added to the
> +   bundle regardless of the presence of the 'ordered' flag.
>   * IPv6 flow label and neighbor discovery fields are now modifiable.
>   * OpenFlow 1.5 extended registers are now supported.
>   * The OpenFlow 1.5 actset_output field is now supported.
> @@ -41,6 +46,18 @@ Post-v2.3.0
>   * A new Netronome extension to OpenFlow 1.5+ allows control over the
> fields hashed for OpenFlow select groups.  See "selection_method" and
> related options in ovs-ofctl(8) for details.
> +   - ovs-ofctl has a new '--bundle' option that makes the flow mod commands
> + ('add-flow', 'add-flows', 'mod-flows', 'del-flows', and 'replace-flows')
> + use an OpenFlow 1.4 bundle to operate the modifications as a single
> + transaction.  If any of the flow mods in a transaction fail, none of
> + them are executed.
> +   - ovs-ofctl 'add-flow' and 'add-flows' commands now accept arbitrary flow
> + mods as an input by allowing the flow specification to start with an
> + explicit 'add', 'modify', 'modify_strict', 'delete', or 'delete_strict'
> + keyword.  A missing keyword is treated as 'add', so this is fully
> + backwards compatible.  With the new '--bundle' option all the flow mods
> + are executed as a single transaction using the new OpenFlow 1.4 bundles
> + support.
> - ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because
>   MD5 is no longer secure and some operating systems have started to 
> disable
>   it in OpenSSL.
> diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
> index 3b157e1..f8b6655 100644
> --- a/include/openvswitch/vconn.h
> +++ b/include/openvswitch/vconn.h
> @@ -55,6 +55,9 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct 
> ofpbuf **);
>  int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf 
> **);
>  int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list 
> *requests,
>  struct ofpbuf **replyp);
> +int vconn_bundle_transact(struct vconn *, struct ovs_list *requests,
> +  uint16_t bundle_flags,
> +  void (*error_reporter)(const struct ofp_header *));
>
>  void vconn_run(struct vconn *);
>  void 

Re: [ovs-dev] [RFC PATCH] userspace: Define and use struct eth_addr.

[Takashi Yamamoto]

hi,

On Thu, Sep 17, 2015 at 12:44 AM, Ben Pfaff <b...@nicira.com> wrote:
> On Wed, Sep 16, 2015 at 10:55:10AM +0900, Takashi Yamamoto wrote:
>> hi,
>>
>> > --- a/build-aux/extract-odp-netlink-h
>> > +++ b/build-aux/extract-odp-netlink-h
>> > @@ -20,11 +20,14 @@ $i\
>> >  #include "OvsDpInterfaceExt.h"\
>> >  #endif\
>> >
>> > +# Use OVS's own struct eth_addr instead of a 6-byte char array.
>> > +s,,"openvswitch/types.h",
>> > +s,#.*,,
>> > +s/__u8[ \t]*\([a-zA-Z0-9_]*\)[ \t]*\[[ \t]*ETH_ALEN[ \t]*\]/struct 
>> > eth_addr \1/
>> >
>> >  # Transform most Linux-specific __u types into C99 uint_t types,
>> >  # and most Linux-specific __be into Open vSwitch ovs_be,
>> >  # and use the appropriate userspace header.
>> > -s,,"openvswitch/types.h",
>> >  s/__u32/uint32_t/g
>> >  s/__u16/uint16_t/g
>> >  s/__u8/uint8_t/g
>> > @@ -36,7 +39,3 @@ s/__be16/ovs_be16/g
>> >  # boundary.
>> >  s/__u64/ovs_32aligned_u64/g
>> >  s/__be64/ovs_32aligned_be64/g
>> > -
>> > -# Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
>> > -s,,"packets.h",
>> > -s/ETH_ALEN/ETH_ADDR_LEN/
>>
>> isn't this still necessary for ovs_key_ethernet etc?
>
> It gets transformed into struct eth_addr, see the output:
>
> struct ovs_key_ethernet {
> struct eth_addr eth_src;
> struct eth_addr eth_dst;
> };

thank you.
it turned out to be a portability problem in the script.
i'll send out a patch later.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] shell substitution "${withval/:/}"

[Takashi Yamamoto]

hi,

m4/openvswitch.m4 has the following substitution.
NetBSD's /bin/sh complains on that.

PTHREAD_WIN32_DIR_DLL=/${withval/:/}/dll/x64

i've looked at bash's man page but i couldn't find this substitution.
can you explain what does it mean, and even better, replace it
with something more portable?  thank you.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH] userspace: Define and use struct eth_addr.

[Takashi Yamamoto]

hi,

> --- a/build-aux/extract-odp-netlink-h
> +++ b/build-aux/extract-odp-netlink-h
> @@ -20,11 +20,14 @@ $i\
>  #include "OvsDpInterfaceExt.h"\
>  #endif\
>
> +# Use OVS's own struct eth_addr instead of a 6-byte char array.
> +s,,"openvswitch/types.h",
> +s,#.*,,
> +s/__u8[ \t]*\([a-zA-Z0-9_]*\)[ \t]*\[[ \t]*ETH_ALEN[ \t]*\]/struct eth_addr 
> \1/
>
>  # Transform most Linux-specific __u types into C99 uint_t types,
>  # and most Linux-specific __be into Open vSwitch ovs_be,
>  # and use the appropriate userspace header.
> -s,,"openvswitch/types.h",
>  s/__u32/uint32_t/g
>  s/__u16/uint16_t/g
>  s/__u8/uint8_t/g
> @@ -36,7 +39,3 @@ s/__be16/ovs_be16/g
>  # boundary.
>  s/__u64/ovs_32aligned_u64/g
>  s/__be64/ovs_32aligned_be64/g
> -
> -# Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
> -s,,"packets.h",
> -s/ETH_ALEN/ETH_ADDR_LEN/

isn't this still necessary for ovs_key_ethernet etc?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Update my email address

[Takashi Yamamoto]

Justin, Miguel, thank you!

applied to master, branch-2.3, 2.2, and 2.1.

On Tue, Jun 2, 2015 at 5:12 PM, Miguel Ángel Ajo majop...@redhat.com wrote:
 Congratulations :)

 Midokura did something very smart ;)

 Miguel Ángel Ajo

 On Tuesday, 2 de June de 2015 at 8:03, Justin Pettit wrote:

 Congratulations on the new job!

 Acked-by: Justin Pettit jpet...@nicira.com

 --Justin


 On Jun 1, 2015, at 10:35 PM, YAMAMOTO Takashi yamam...@midokura.com wrote:

 Signed-off-by: YAMAMOTO Takashi yamam...@midokura.com
 ---
 AUTHORS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/AUTHORS b/AUTHORS
 index a2b40a7..f155105 100644
 --- a/AUTHORS
 +++ b/AUTHORS
 @@ -179,7 +179,7 @@ Vivien Bernet-Rollande v...@soprive.net
 Wang Sheng-Hui shh...@gmail.com
 Wei Yongjun yj...@cn.fujitsu.com
 William Fulton
 -YAMAMOTO Takashi yamam...@valinux.co.jp
 +YAMAMOTO Takashi yamam...@midokura.com
 Yasuhito Takamiya yasuh...@gmail.com
 yinpeijun yinpei...@huawei.com
 Yu Zhiguo y...@cn.fujitsu.com
 --
 2.3.2 (Apple Git-55)

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


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


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

Re: [ovs-dev] Can we trivially enable group chaining?

[Takashi Yamamoto]

hi,

On Wed, Jun 3, 2015 at 3:36 PM, Ben Pfaff b...@nicira.com wrote:
 We have the following comment in ofproto-dpif-xlate.c:

 /* Prevent nested translation of OpenFlow groups.
  *
  * OpenFlow allows this restriction.  We enforce this restriction only
  * because, with the current architecture, we would otherwise have to
  * take a possibly recursive read lock on the ofgroup rwlock, which is
  * unsafe given that POSIX allows taking a read lock to block if there
  * is a thread blocked on taking the write lock.  Other solutions
  * without this restriction are also possible, but seem unwarranted
  * given the current limited use of groups. */

 But I think that the reasoning no longer holds.  At the time the comment
 was written, each ofgroup had its own internal rwlock, which was held
 for reading whenever the ofgroup was being translated.  Since then,
 ofgroups have transitioned to using RCU for protection, and no rwlock is
 held during translation.

 I think that, therefore, we can trivially enable group chaining with a
 commit like this (untested):

 diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
 index 71b8bef..ce90b2a 100644
 --- a/ofproto/ofproto-dpif-xlate.c
 +++ b/ofproto/ofproto-dpif-xlate.c
 @@ -3302,33 +3302,9 @@ xlate_group_action__(struct xlate_ctx *ctx, struct 
 group_dpif *group)
  }

  static bool
 -xlate_group_resource_check(struct xlate_ctx *ctx)
 -{
 -if (!xlate_resubmit_resource_check(ctx)) {
 -return false;
 -} else if (ctx-in_group) {
 -/* Prevent nested translation of OpenFlow groups.
 - *
 - * OpenFlow allows this restriction.  We enforce this restriction 
 only
 - * because, with the current architecture, we would otherwise have to
 - * take a possibly recursive read lock on the ofgroup rwlock, which 
 is
 - * unsafe given that POSIX allows taking a read lock to block if 
 there
 - * is a thread blocked on taking the write lock.  Other solutions
 - * without this restriction are also possible, but seem unwarranted
 - * given the current limited use of groups. */
 -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
 -
 -VLOG_ERR_RL(rl, cannot recursively translate OpenFlow group);
 -return false;
 -} else {
 -return true;
 -}
 -}
 -
 -static bool
  xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
  {
 -if (xlate_group_resource_check(ctx)) {
 +if (xlate_resubmit_resource_check(ctx)) {
  struct group_dpif *group;
  bool got_group;

 Anyone know a reason this might be a bad idea?

i think it's a good idea.
in_group needs to be a counter if we allow recursion.

YAMAMOTO Takashi

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