Re: [ovs-dev] [PATCH 03/10] windows: add includes to daemon-windows

[Alin Serdean]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Wednesday, April 19, 2017 7:46 AM
> To: Alin Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 03/10] windows: add includes to daemon-
> windows
> 
> On Wed, Apr 19, 2017 at 03:07:03AM +, Alin Serdean wrote:
> > > > -fprintf(filep_pidfile, "%d\n", _getpid());
> > > > +fprintf(filep_pidfile, "%d\n", getpid());
> > >
> > > This seems reasonable to me, except that usual practice would be
> > > more like
> > > this:
> > > fprintf(filep_pidfile, "%ld\n", (long int) getpid()); because
> > > pid_t might be short or int or long.
> > [Alin Serdean] Thanks for the review Ben, I totally missed that.
> > Although being MSFT specific if we replace getpid with
> GetCurrentProcessId (https://msdn.microsoft.com/en-
> us/library/windows/desktop/ms683180(v=vs.85).aspx) and from includes:
> > typedef unsigned long   DWORD;
> > I'm wondering if we should switch it to `%lu` and `(unsigned long)`.
> > What do you think?
> 
> Hmm.
> 
> POSIX says that pid_t must be a signed integer type.  An unsigned integer
> type for pid_t breaks things; for example, the POSIX wait() function has a
> pid_t return type and returns -1 to indicate an error.
> So it's somewhat distressing to have getpid() return an unsigned long, since
> that breaks real programs' fairly justified assumptions that
> getpid() returns a signed integer type.
> 
> What if, instead of "#define getpid() _getpid()", we used an inline function,
> e.g.
> 
> static inline pid_t getpid(void)
> {
> return GetCurrentProcessId();
> }
> 
> That would be cleaner from a POSIX point of view.
> 
> (On the other hand, if GetCurrentProcessId() might return a DWORD so big
> that it becomes a negative pid_t, we need to have a bigger discussion.)
[Alin Serdean] I agree it would look cleaner from a POSIX point of view, but I 
need to expand both functions to see if they call the same thing behind the 
scenes, so we can be sure it won't rollover. 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 02/10] windows: add definition of getpid and getcwd

[Alin Serdean]

The documentation is somewhat unclear on it. I'll try to expand 
getpid/GetCurrentProcessId to see if they call the same things under the scenes 
(one would assume yes), and let you know.

Thanks,
Alin.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Wednesday, April 19, 2017 7:55 AM
> To: Alin Serdean 
> Cc: Sairam Venugopal ; d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 02/10] windows: add definition of getpid and
> getcwd
> 
> I don't know how much risk there is.  If the values actually returned in
> practice as process IDs by GetCurrentProcessId() are all in the range
> 0...INT_MAX, then it's fine to use the implementation I suggested.  But if it
> might return any value, then it is safer to use _getpid().
> 
> On Wed, Apr 19, 2017 at 03:31:13AM +, Alin Serdean wrote:
> > To bring more to the table:
> > From includes
> > typedef unsigned long   DWORD;
> > Our pid_t is defined:
> >
> https://github.com/openvswitch/ovs/blob/master/include/windows/windef
> s
> > .h#L41
> >
> > I'm wondering if it would be best not to cut corners on this one and just
> stick to _getpid and do the same thing as we already have for `string.h`.
> >
> > What do you think?
> >
> > Thanks,
> > Alin.
> > > -Original Message-
> > > From: Ben Pfaff [mailto:b...@ovn.org]
> > > Sent: Saturday, April 15, 2017 6:27 AM
> > > To: Sairam Venugopal 
> > > Cc: Alin Serdean ;
> > > d...@openvswitch.org
> > > Subject: Re: [ovs-dev] [PATCH 02/10] windows: add definition of
> > > getpid and getcwd
> > >
> > > If GetCurrentProcessId() is a reasonable substitute for getpid(),
> > > but the return type is different, then I would suggest an inline function,
> like this:
> > >
> > > static inline pid_t
> > > getpid(void)
> > > {
> > > return GetCurrentProcessId();
> > > }
> > >
> > > Thanks,
> > >
> > > Ben.
> > >
> > > On Tue, Mar 07, 2017 at 09:07:55AM +, Sairam Venugopal wrote:
> > > > Shouldn’t we cast the DWORD to unsigned int for the
> > > GetCurrentProcessId?
> > > >
> > > >
> > > >
> > > >
> > > > On 2/5/17, 8:41 PM, "ovs-dev-boun...@openvswitch.org on behalf of
> > > > Alin
> > > Serdean"  > > aserd...@cloudbasesolutions.com> wrote:
> > > >
> > > > >getcwd - is used in lib/util.c. getcwd is deprecated on Windows
> > > > >but has _getcwd which is defined in :
> > > > >https://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__msdn.microsoft.c
> > > > >om_en-2Dus_library_sf98bd4y-28v-3Dvs.120-
> > > 29.aspx=DwICAg=uilaK90D4
> > > >
> > >
> >TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo
> > > =og4savU
> > > > >MMSe8GoOfKq6AMAirivJFLgVTMx5lx7hx6gk=CVjSRN456APj3-
> > > mMAQuXYxdJ4oUgdu
> > > > >wqZHzkod6cLvQ=
> > > > >
> > > > >getpid - is used in several files (i.e. lib/vlog.c). getpid is
> > > > >also and deprecated and _getpid should be used:
> > > > >https://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__msdn.microsoft.c
> > > > >om_en-2Dus_library_t2y34y40-28v-3Dvs.120-
> > > 29.aspx=DwICAg=uilaK90D4
> > > >
> > >
> >TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo
> > > =og4savU
> > > >
> > >
> >MMSe8GoOfKq6AMAirivJFLgVTMx5lx7hx6gk=pDh2W8ECiQdxZdHgHBdW
> > > HIDhLPcTJ9
> > > > >A6rb2n1YcRZ94= The problem using _getpid is that the definition
> > > > >is in .
> > > > >A file called process.h also exists in the lib folder. This will
> > > > >mess up includes.
> > > > >An option would be to use a wrapper like we use for
> > > > >lib/string.h(.in) but that would mean to also add it to the automake
> chain.
> > > > >A simple solution would be to map it to GetCurrentProcessId
> > > > >https://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__msdn.microsoft.c
> > > > >om_en-2Dus_library_windows_desktop_ms683180-28v-3Dvs.85-
> > > 29.aspx=DwI
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 02/10] windows: add definition of getpid and getcwd

[Ben Pfaff]

I don't know how much risk there is.  If the values actually returned in
practice as process IDs by GetCurrentProcessId() are all in the range
0...INT_MAX, then it's fine to use the implementation I suggested.  But
if it might return any value, then it is safer to use _getpid().

On Wed, Apr 19, 2017 at 03:31:13AM +, Alin Serdean wrote:
> To bring more to the table:
> From includes
> typedef unsigned long   DWORD;
> Our pid_t is defined:
> https://github.com/openvswitch/ovs/blob/master/include/windows/windefs.h#L41
> 
> I'm wondering if it would be best not to cut corners on this one and just 
> stick to _getpid and do the same thing as we already have for `string.h`.
> 
> What do you think?
> 
> Thanks,
> Alin.
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Saturday, April 15, 2017 6:27 AM
> > To: Sairam Venugopal 
> > Cc: Alin Serdean ;
> > d...@openvswitch.org
> > Subject: Re: [ovs-dev] [PATCH 02/10] windows: add definition of getpid and
> > getcwd
> > 
> > If GetCurrentProcessId() is a reasonable substitute for getpid(), but the
> > return type is different, then I would suggest an inline function, like 
> > this:
> > 
> > static inline pid_t
> > getpid(void)
> > {
> > return GetCurrentProcessId();
> > }
> > 
> > Thanks,
> > 
> > Ben.
> > 
> > On Tue, Mar 07, 2017 at 09:07:55AM +, Sairam Venugopal wrote:
> > > Shouldn’t we cast the DWORD to unsigned int for the
> > GetCurrentProcessId?
> > >
> > >
> > >
> > >
> > > On 2/5/17, 8:41 PM, "ovs-dev-boun...@openvswitch.org on behalf of Alin
> > Serdean"  > aserd...@cloudbasesolutions.com> wrote:
> > >
> > > >getcwd - is used in lib/util.c. getcwd is deprecated on Windows but
> > > >has _getcwd which is defined in :
> > > >https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__msdn.microsoft.c
> > > >om_en-2Dus_library_sf98bd4y-28v-3Dvs.120-
> > 29.aspx=DwICAg=uilaK90D4
> > >
> > >TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo
> > =og4savU
> > > >MMSe8GoOfKq6AMAirivJFLgVTMx5lx7hx6gk=CVjSRN456APj3-
> > mMAQuXYxdJ4oUgdu
> > > >wqZHzkod6cLvQ=
> > > >
> > > >getpid - is used in several files (i.e. lib/vlog.c). getpid is also
> > > >and deprecated and _getpid should be used:
> > > >https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__msdn.microsoft.c
> > > >om_en-2Dus_library_t2y34y40-28v-3Dvs.120-
> > 29.aspx=DwICAg=uilaK90D4
> > >
> > >TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo
> > =og4savU
> > >
> > >MMSe8GoOfKq6AMAirivJFLgVTMx5lx7hx6gk=pDh2W8ECiQdxZdHgHBdW
> > HIDhLPcTJ9
> > > >A6rb2n1YcRZ94= The problem using _getpid is that the definition is
> > > >in .
> > > >A file called process.h also exists in the lib folder. This will mess
> > > >up includes.
> > > >An option would be to use a wrapper like we use for lib/string.h(.in)
> > > >but that would mean to also add it to the automake chain.
> > > >A simple solution would be to map it to GetCurrentProcessId
> > > >https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__msdn.microsoft.c
> > > >om_en-2Dus_library_windows_desktop_ms683180-28v-3Dvs.85-
> > 29.aspx=DwI
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 03/10] windows: add includes to daemon-windows

[Ben Pfaff]

On Wed, Apr 19, 2017 at 03:07:03AM +, Alin Serdean wrote:
> > > -fprintf(filep_pidfile, "%d\n", _getpid());
> > > +fprintf(filep_pidfile, "%d\n", getpid());
> > 
> > This seems reasonable to me, except that usual practice would be more like
> > this:
> > fprintf(filep_pidfile, "%ld\n", (long int) getpid()); because pid_t 
> > might be
> > short or int or long.
> [Alin Serdean] Thanks for the review Ben, I totally missed that.
> Although being MSFT specific if we replace getpid with GetCurrentProcessId 
> (https://msdn.microsoft.com/en-us/library/windows/desktop/ms683180(v=vs.85).aspx)
>  and from includes:
> typedef unsigned long   DWORD;
> I'm wondering if we should switch it to `%lu` and `(unsigned long)`. 
> What do you think?

Hmm.

POSIX says that pid_t must be a signed integer type.  An unsigned
integer type for pid_t breaks things; for example, the POSIX wait()
function has a pid_t return type and returns -1 to indicate an error.
So it's somewhat distressing to have getpid() return an unsigned long,
since that breaks real programs' fairly justified assumptions that
getpid() returns a signed integer type.

What if, instead of "#define getpid() _getpid()", we used an inline
function, e.g.

static inline pid_t getpid(void)
{
return GetCurrentProcessId();
}

That would be cleaner from a POSIX point of view.

(On the other hand, if GetCurrentProcessId() might return a DWORD so big
that it becomes a negative pid_t, we need to have a bigger discussion.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Alin Serdean]

> > As a temporary solution issue an error when this warning is triggered.
> > This will help development on the Windows side.
> >
> > Suggested-by: Ben Pfaff 
> > Signed-off-by: Alin Gabriel Serdean 
> 
> This seems reasonable to me.  I assume that other patches in the series are
> needed before this one can be applied, so I'll wait for the next revision of 
> the
> series before applying it.
[Alin Serdean] You are correct in your assumption . I will post another 
revision once we clarify the rest of the reviews.

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

Re: [ovs-dev] [PATCH 02/10] windows: add definition of getpid and getcwd

[Alin Serdean]

To bring more to the table:
From includes
typedef unsigned long   DWORD;
Our pid_t is defined:
https://github.com/openvswitch/ovs/blob/master/include/windows/windefs.h#L41

I'm wondering if it would be best not to cut corners on this one and just stick 
to _getpid and do the same thing as we already have for `string.h`.

What do you think?

Thanks,
Alin.
> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Saturday, April 15, 2017 6:27 AM
> To: Sairam Venugopal 
> Cc: Alin Serdean ;
> d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 02/10] windows: add definition of getpid and
> getcwd
> 
> If GetCurrentProcessId() is a reasonable substitute for getpid(), but the
> return type is different, then I would suggest an inline function, like this:
> 
> static inline pid_t
> getpid(void)
> {
> return GetCurrentProcessId();
> }
> 
> Thanks,
> 
> Ben.
> 
> On Tue, Mar 07, 2017 at 09:07:55AM +, Sairam Venugopal wrote:
> > Shouldn’t we cast the DWORD to unsigned int for the
> GetCurrentProcessId?
> >
> >
> >
> >
> > On 2/5/17, 8:41 PM, "ovs-dev-boun...@openvswitch.org on behalf of Alin
> Serdean"  aserd...@cloudbasesolutions.com> wrote:
> >
> > >getcwd - is used in lib/util.c. getcwd is deprecated on Windows but
> > >has _getcwd which is defined in :
> > >https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__msdn.microsoft.c
> > >om_en-2Dus_library_sf98bd4y-28v-3Dvs.120-
> 29.aspx=DwICAg=uilaK90D4
> >
> >TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo
> =og4savU
> > >MMSe8GoOfKq6AMAirivJFLgVTMx5lx7hx6gk=CVjSRN456APj3-
> mMAQuXYxdJ4oUgdu
> > >wqZHzkod6cLvQ=
> > >
> > >getpid - is used in several files (i.e. lib/vlog.c). getpid is also
> > >and deprecated and _getpid should be used:
> > >https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__msdn.microsoft.c
> > >om_en-2Dus_library_t2y34y40-28v-3Dvs.120-
> 29.aspx=DwICAg=uilaK90D4
> >
> >TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo
> =og4savU
> >
> >MMSe8GoOfKq6AMAirivJFLgVTMx5lx7hx6gk=pDh2W8ECiQdxZdHgHBdW
> HIDhLPcTJ9
> > >A6rb2n1YcRZ94= The problem using _getpid is that the definition is
> > >in .
> > >A file called process.h also exists in the lib folder. This will mess
> > >up includes.
> > >An option would be to use a wrapper like we use for lib/string.h(.in)
> > >but that would mean to also add it to the automake chain.
> > >A simple solution would be to map it to GetCurrentProcessId
> > >https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__msdn.microsoft.c
> > >om_en-2Dus_library_windows_desktop_ms683180-28v-3Dvs.85-
> 29.aspx=DwI

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

Re: [ovs-dev] [PATCH 03/10] windows: add includes to daemon-windows

[Alin Serdean]

> > -fprintf(filep_pidfile, "%d\n", _getpid());
> > +fprintf(filep_pidfile, "%d\n", getpid());
> 
> This seems reasonable to me, except that usual practice would be more like
> this:
> fprintf(filep_pidfile, "%ld\n", (long int) getpid()); because pid_t might 
> be
> short or int or long.
[Alin Serdean] Thanks for the review Ben, I totally missed that.
Although being MSFT specific if we replace getpid with GetCurrentProcessId 
(https://msdn.microsoft.com/en-us/library/windows/desktop/ms683180(v=vs.85).aspx)
 and from includes:
typedef unsigned long   DWORD;
I'm wondering if we should switch it to `%lu` and `(unsigned long)`. 
What do you think?

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

Re: [ovs-dev] [PATCH branch-2.7 00/14] Backports for branch-2.7

[Jarno Rajahalme]

For the series:

Acked-by: Jarno Rajahalme 

One of the userspace changes (indicate if had labels) changes the OVS library 
interface, so leave it for you to decide what to do with that.

  Jarno

> On Apr 18, 2017, at 5:09 PM, Joe Stringer  wrote:
> 
> This is a trimmed down set of the backports presented by Jarno here:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329817.html
> 
> The fixes (including label inheritance and subsequent fixups) are included
> in this series, while the unrelated refactors and unnecessary compat changes
> are dropped.
> 
> Jarno acked this proposal already, but I'm sending it out to the list for
> completeness.
> 
> The following patch is new in this proposal, as suggested by Jarno:
> "datapath: Avoid struct copy on conntrack labels."
> 
> Jarno Rajahalme (9):
>  datapath: Use inverted tuple in ovs_ct_find_existing() if NATted.
>  datapath: Do not trigger events for unconfirmed connections.
>  lib: Indicate if netlink message had labels.
>  datapath: Unionize ovs_key_ct_label with a u32 array.
>  datapath: Simplify labels length logic.
>  datapath: Refactor labels initialization.
>  datapath: Inherit master's labels.
>  datapath: Avoid struct copy on conntrack labels.
>  ofp-util: Ignore unknown fields in ofputil_decode_packet_in2().
> 
> Jiri Benc (2):
>  datapath: remove unused functions
>  datapath: remove unnecessary EXPORT_SYMBOLs
> 
> Pablo Neira Ayuso (1):
>  datapath: handle NF_REPEAT from nf_conntrack_in()
> 
> Thadeu Lima de Souza Cascardo (1):
>  datapath: fix flow stats accounting when node 0 is not possible
> 
> Yi-Hung Wei (1):
>  nx-match: Fix oxm decode.
> 
> datapath/conntrack.c   | 172 ++---
> datapath/datapath.c|   2 -
> datapath/flow.c|   6 +-
> datapath/flow_table.c  |   3 +-
> datapath/linux/compat/include/linux/openvswitch.h  |   8 +-
> .../include/net/netfilter/nf_conntrack_core.h  |  21 +++
> datapath/vport-netdev.c|   1 -
> datapath/vport.c   |  17 --
> datapath/vport.h   |   1 -
> lib/ct-dpif.h  |   1 +
> lib/netlink-conntrack.c|   1 +
> lib/nx-match.c |  23 ++-
> lib/nx-match.h |   4 +-
> lib/ofp-util.c |   2 +-
> tests/system-traffic.at|  52 +++
> 15 files changed, 223 insertions(+), 91 deletions(-)
> 
> -- 
> 2.11.1
> 

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

Re: [ovs-dev] [PATCH branch-2.7 00/25] Backports for branch-2.7

[Joe Stringer]

On 13 April 2017 at 15:15, Jarno Rajahalme  wrote:
>
>> On Apr 13, 2017, at 2:08 PM, Joe Stringer  wrote:
>>
>> On 15 March 2017 at 16:31, Jarno Rajahalme  wrote:
>>> These are (mostly) datapath backports from master that fix existing
>>> features in branch-2.7 and/or make the datapath compilable with later
>>> Linux kernel code.
>>
>> Hi Jarno,
>>
>> This series was bigger than I expected, based on the general policy of
>> backporting fixes (in the least invasive way possible) and not
>> introducing new features or unrelated changes to existing branches.
>>
>> I have a counter-proposal patch series existing in this branch here,
>> which strips 15 of the patches out:
>>
>> https://github.com/joestringer/openvswitch/commits/dev/jarno_ct_27_bp_v1
>>
>> The nf_ct_delete() backport doesn't have a clear motivation described
>> in the commit message so I don't know whether that's appropriate for
>> backport.
>>
>> If you consider that master labels inheritance misfunctioning is a
>> prominent bug for v2.7, here's the same branch with the labels
>> refactor and inheritance fix for that on top (extra 4 patches); I
>> didn't try to minimize the size of these changes, eg by dropping some
>> of the refactoring patches:
>>
>> https://github.com/joestringer/openvswitch/tree/dev/jarno_ct_27_bp_v1%2Blabels
>>
>
> I think it would be good to fix the inconsistent labels inheritance behavior 
> also for OVS 2.7. I looked over this branch, and it looks good except it also 
> needs this backport fix from master:
>
> commit 280337736851 (“datapath: Avoid struct copy on conntrack labels.")

Thanks for taking a look. I posted the counter-proposal more formally here:

https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331068.html

With your ack I'd be happy to push it to branch-2.7.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-2.7 14/14] nx-match: Fix oxm decode.

[Joe Stringer]

From: Yi-Hung Wei 

decode_nx_packet_in2() may be used by the switch to parse NXT_RESUME messages,
where we need exact match on the oxm header. Therefore, change
oxm_decode_loose() to oxm_decode() that takes an extra argument to indicate 
whether
we want strict or loose match.

Fixes: 5a18e1055b88 ("ofp-util: Ignore unknown fields in 
ofputil_decode_packet_in2().")
Signed-off-by: Yi-Hung Wei 
Signed-off-by: Joe Stringer 
---
 lib/nx-match.c | 10 +-
 lib/nx-match.h |  4 ++--
 lib/ofp-util.c |  5 ++---
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 2e62e99c0098..43672cb4becf 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -683,14 +683,14 @@ oxm_pull_match_loose(struct ofpbuf *b, const struct 
tun_table *tun_table,
  *
  * Returns 0 if successful, otherwise an OpenFlow error code.
  *
- * Encountering unknown OXM headers or missing field prerequisites are not
- * considered as error conditions.
+ * If 'loose' is true, encountering unknown OXM headers or missing field
+ * prerequisites are not considered as error conditions.
  */
 enum ofperr
-oxm_decode_match_loose(const void *oxm, size_t oxm_len,
-   const struct tun_table *tun_table, struct match *match)
+oxm_decode_match(const void *oxm, size_t oxm_len, bool loose,
+ const struct tun_table *tun_table, struct match *match)
 {
-return nx_pull_raw(oxm, oxm_len, false, match, NULL, NULL, tun_table);
+return nx_pull_raw(oxm, oxm_len, !loose, match, NULL, NULL, tun_table);
 }
 
 /* Verify an array of OXM TLVs treating value of each TLV as a mask,
diff --git a/lib/nx-match.h b/lib/nx-match.h
index cee9e65e84b9..e103dd5fa74d 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -61,8 +61,8 @@ enum ofperr oxm_pull_match(struct ofpbuf *, const struct 
tun_table *,
struct match *);
 enum ofperr oxm_pull_match_loose(struct ofpbuf *, const struct tun_table *,
  struct match *);
-enum ofperr oxm_decode_match_loose(const void *, size_t,
-   const struct tun_table *, struct match *);
+enum ofperr oxm_decode_match(const void *, size_t, bool,
+ const struct tun_table *, struct match *);
 enum ofperr oxm_pull_field_array(const void *, size_t fields_len,
  struct field_array *);
 
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 9e8d4d25f114..d3153370f2e6 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3397,9 +3397,8 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool 
loose,
 }
 
 case NXPINT_METADATA:
-error = oxm_decode_match_loose(payload.msg,
-   ofpbuf_msgsize(),
-   tun_table, >flow_metadata);
+error = oxm_decode_match(payload.msg, ofpbuf_msgsize(),
+ loose, tun_table, >flow_metadata);
 break;
 
 case NXPINT_USERDATA:
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 13/14] ofp-util: Ignore unknown fields in ofputil_decode_packet_in2().

[Joe Stringer]

From: Jarno Rajahalme 

The decoder of packet_in messages should not fail on encountering
unknown metadata fields.  This allows the switch to add new features
without breaking controllers.  The controllers should, however, copy
the metadata fields from the packet_int to packet_out so that the
switch gets back the full metadata.  OVN is already doing this.

[Committer notes]

Changed mf_are_match_prereqs_ok() -> mf_are_prereqs_ok().

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 lib/nx-match.c | 25 -
 lib/nx-match.h |  4 ++--
 lib/ofp-util.c |  5 +++--
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 91401e2201c6..2e62e99c0098 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -504,6 +504,9 @@ nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie,
 return 0;
 }
 
+/* Prerequisites will only be checked when 'strict' is 'true'.  This allows
+ * decoding conntrack original direction 5-tuple IP addresses without the
+ * ethertype being present, when decoding metadata only. */
 static enum ofperr
 nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict,
 struct match *match, ovs_be64 *cookie, ovs_be64 *cookie_mask,
@@ -539,7 +542,7 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, bool 
strict,
 *cookie = value.be64;
 *cookie_mask = mask.be64;
 }
-} else if (!mf_are_prereqs_ok(field, >flow, NULL)) {
+} else if (strict && !mf_are_prereqs_ok(field, >flow, NULL)) {
 error = OFPERR_OFPBMC_BAD_PREREQ;
 } else if (!mf_is_all_wild(field, >wc)) {
 error = OFPERR_OFPBMC_DUP_FIELD;
@@ -607,7 +610,8 @@ nx_pull_match(struct ofpbuf *b, unsigned int match_len, 
struct match *match,
 }
 
 /* Behaves the same as nx_pull_match(), but skips over unknown NXM headers,
- * instead of failing with an error. */
+ * instead of failing with an error, and does not check for field
+ * prerequisities. */
 enum ofperr
 nx_pull_match_loose(struct ofpbuf *b, unsigned int match_len,
 struct match *match,
@@ -664,8 +668,9 @@ oxm_pull_match(struct ofpbuf *b, const struct tun_table 
*tun_table,
 return oxm_pull_match__(b, true, tun_table, match);
 }
 
-/* Behaves the same as oxm_pull_match() with one exception.  Skips over unknown
- * OXM headers instead of failing with an error when they are encountered. */
+/* Behaves the same as oxm_pull_match() with two exceptions.  Skips over
+ * unknown OXM headers instead of failing with an error when they are
+ * encountered, and does not check for field prerequisities. */
 enum ofperr
 oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table,
  struct match *match)
@@ -676,14 +681,16 @@ oxm_pull_match_loose(struct ofpbuf *b, const struct 
tun_table *tun_table,
 /* Parses the OXM match description in the 'oxm_len' bytes in 'oxm'.  Stores
  * the result in 'match'.
  *
- * Fails with an error when encountering unknown OXM headers.
+ * Returns 0 if successful, otherwise an OpenFlow error code.
  *
- * Returns 0 if successful, otherwise an OpenFlow error code. */
+ * Encountering unknown OXM headers or missing field prerequisites are not
+ * considered as error conditions.
+ */
 enum ofperr
-oxm_decode_match(const void *oxm, size_t oxm_len,
- const struct tun_table *tun_table, struct match *match)
+oxm_decode_match_loose(const void *oxm, size_t oxm_len,
+   const struct tun_table *tun_table, struct match *match)
 {
-return nx_pull_raw(oxm, oxm_len, true, match, NULL, NULL, tun_table);
+return nx_pull_raw(oxm, oxm_len, false, match, NULL, NULL, tun_table);
 }
 
 /* Verify an array of OXM TLVs treating value of each TLV as a mask,
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 5dca24a01a49..cee9e65e84b9 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -61,8 +61,8 @@ enum ofperr oxm_pull_match(struct ofpbuf *, const struct 
tun_table *,
struct match *);
 enum ofperr oxm_pull_match_loose(struct ofpbuf *, const struct tun_table *,
  struct match *);
-enum ofperr oxm_decode_match(const void *, size_t, const struct tun_table *,
- struct match *);
+enum ofperr oxm_decode_match_loose(const void *, size_t,
+   const struct tun_table *, struct match *);
 enum ofperr oxm_pull_field_array(const void *, size_t fields_len,
  struct field_array *);
 
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 0c9343ec400b..9e8d4d25f114 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3397,8 +3397,9 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool 
loose,
 }
 
 case NXPINT_METADATA:
-error = oxm_decode_match(payload.msg, ofpbuf_msgsize(),
-   

[ovs-dev] [PATCH branch-2.7 11/14] datapath: Inherit master's labels.

[Joe Stringer]

From: Jarno Rajahalme 

Upstream commit:

commit 09aa98ad496d6b11a698b258bc64d7f64c55d682
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:58 2017 -0800

openvswitch: Inherit master's labels.

We avoid calling into nf_conntrack_in() for expected connections, as
that would remove the expectation that we want to stick around until
we are ready to commit the connection.  Instead, we do a lookup in the
expectation table directly.  However, after a successful expectation
lookup we have set the flow key label field from the master
connection, whereas nf_conntrack_in() does not do this.  This leads to
master's labels being inherited after an expectation lookup, but those
labels not being inherited after the corresponding conntrack action
with a commit flag.

This patch resolves the problem by changing the commit code path to
also inherit the master's labels to the expected connection.
Resolving this conflict in favor of inheriting the labels allows more
information be passed from the master connection to related
connections, which would otherwise be much harder if the 32 bits in
the connmark are not enough.  Labels can still be set explicitly, so
this change only affects the default values of the labels in presense
of a master connection.

Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
Signed-off-by: Jarno Rajahalme 
Acked-by: Pravin B Shelar 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Fixes: a94ebc39996b ("datapath: Add conntrack action")
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 datapath/conntrack.c | 45 +++--
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 743a88a990f0..87306a2c5e07 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -80,6 +80,8 @@ struct ovs_conntrack_info {
 #endif
 };
 
+static bool labels_nonzero(const struct ovs_key_ct_labels *labels);
+
 static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info);
 
 static u16 key_to_nfproto(const struct sw_flow_key *key)
@@ -275,18 +277,32 @@ static int ovs_ct_init_labels(struct nf_conn *ct, struct 
sw_flow_key *key,
  const struct ovs_key_ct_labels *labels,
  const struct ovs_key_ct_labels *mask)
 {
-   struct nf_conn_labels *cl;
-   u32 *dst;
-   int i;
+   struct nf_conn_labels *cl, *master_cl;
+   bool have_mask = labels_nonzero(mask);
+
+   /* Inherit master's labels to the related connection? */
+   master_cl = ct->master ? nf_ct_labels_find(ct->master) : NULL;
+
+   if (!master_cl && !have_mask)
+   return 0;   /* Nothing to do. */
 
cl = ovs_ct_get_conn_labels(ct);
if (!cl)
return -ENOSPC;
 
-   dst = (u32 *)cl->bits;
-   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
-   dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
-   (labels->ct_labels_32[i] & mask->ct_labels_32[i]);
+   /* Inherit the master's labels, if any. */
+   if (master_cl)
+   *cl = *master_cl;
+
+   if (have_mask) {
+   u32 *dst = (u32 *)cl->bits;
+   int i;
+
+   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
+   dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
+   (labels->ct_labels_32[i]
+& mask->ct_labels_32[i]);
+   }
 
/* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
 * IPCT_LABEL bit it set in the event cache.
@@ -945,13 +961,14 @@ static int ovs_ct_commit(struct net *net, struct 
sw_flow_key *key,
if (err)
return err;
}
-   if (labels_nonzero(>labels.mask)) {
-   if (!nf_ct_is_confirmed(ct))
-   err = ovs_ct_init_labels(ct, key, >labels.value,
->labels.mask);
-   else
-   err = ovs_ct_set_labels(ct, key, >labels.value,
-   >labels.mask);
+   if (!nf_ct_is_confirmed(ct)) {
+   err = ovs_ct_init_labels(ct, key, >labels.value,
+>labels.mask);
+   if (err)
+   return err;
+   } else if (labels_nonzero(>labels.mask)) {
+   err = ovs_ct_set_labels(ct, key, >labels.value,
+   >labels.mask);
if (err)
return err;
}
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 09/14] datapath: Simplify labels length logic.

[Joe Stringer]

From: Jarno Rajahalme 

Upstream commit:

commit b87cec3814ccc7f6afb0a1378ee7e5110d07cdd3
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:56 2017 -0800

openvswitch: Simplify labels length logic.

Since 23014011ba42 ("netfilter: conntrack: support a fixed size of 128
distinct labels"), the size of conntrack labels extension has fixed to
128 bits, so we do not need to check for labels sizes shorter than 128
at run-time.  This patch simplifies labels length logic accordingly,
but allows the conntrack labels size to be increased in the future
without breaking the build.  In the event of conntrack labels
increasing in size OVS would still be able to deal with the 128 first
label bits.

Suggested-by: Joe Stringer 
Signed-off-by: Jarno Rajahalme 
Acked-by: Pravin B Shelar 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 datapath/conntrack.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 2286a6987052..a1106e66a942 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -145,22 +145,20 @@ static size_t ovs_ct_get_labels_len(struct nf_conn_labels 
*cl)
 #endif
 }
 
+/* Guard against conntrack labels max size shrinking below 128 bits. */
+#if NF_CT_LABELS_MAX_SIZE < 16
+#error NF_CT_LABELS_MAX_SIZE must be at least 16 bytes
+#endif
+
 static void ovs_ct_get_labels(const struct nf_conn *ct,
  struct ovs_key_ct_labels *labels)
 {
struct nf_conn_labels *cl = ct ? nf_ct_labels_find(ct) : NULL;
 
-   if (cl) {
-   size_t len = ovs_ct_get_labels_len(cl);
-
-   if (len > OVS_CT_LABELS_LEN)
-   len = OVS_CT_LABELS_LEN;
-   else if (len < OVS_CT_LABELS_LEN)
-   memset(labels, 0, OVS_CT_LABELS_LEN);
-   memcpy(labels, cl->bits, len);
-   } else {
+   if (cl)
+   memcpy(labels, cl->bits, OVS_CT_LABELS_LEN);
+   else
memset(labels, 0, OVS_CT_LABELS_LEN);
-   }
 }
 
 static void __ovs_ct_update_key(struct sw_flow_key *key, u8 state,
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 10/14] datapath: Refactor labels initialization.

[Joe Stringer]

From: Jarno Rajahalme 

Upstream commit:

Refactoring conntrack labels initialization makes changes in later
patches easier to review.

Signed-off-by: Jarno Rajahalme 
Acked-by: Pravin B Shelar 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 datapath/conntrack.c | 120 +++
 1 file changed, 64 insertions(+), 56 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index a1106e66a942..743a88a990f0 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -136,15 +136,6 @@ static u32 ovs_ct_get_mark(const struct nf_conn *ct)
 #endif
 }
 
-static size_t ovs_ct_get_labels_len(struct nf_conn_labels *cl)
-{
-#ifdef HAVE_NF_CONN_LABELS_WITH_WORDS
-   return cl->words * sizeof(long);
-#else
-   return sizeof(cl->bits);
-#endif
-}
-
 /* Guard against conntrack labels max size shrinking below 128 bits. */
 #if NF_CT_LABELS_MAX_SIZE < 16
 #error NF_CT_LABELS_MAX_SIZE must be at least 16 bytes
@@ -243,19 +234,12 @@ int ovs_ct_put_key(const struct sw_flow_key *key, struct 
sk_buff *skb)
return 0;
 }
 
-static int ovs_ct_set_mark(struct sk_buff *skb, struct sw_flow_key *key,
+static int ovs_ct_set_mark(struct nf_conn *ct, struct sw_flow_key *key,
   u32 ct_mark, u32 mask)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
-   enum ip_conntrack_info ctinfo;
-   struct nf_conn *ct;
u32 new_mark;
 
-   /* The connection could be invalid, in which case set_mark is no-op. */
-   ct = nf_ct_get(skb, );
-   if (!ct)
-   return 0;
-
new_mark = ct_mark | (ct->mark & ~(mask));
if (ct->mark != new_mark) {
ct->mark = new_mark;
@@ -270,18 +254,9 @@ static int ovs_ct_set_mark(struct sk_buff *skb, struct 
sw_flow_key *key,
 #endif
 }
 
-static int ovs_ct_set_labels(struct sk_buff *skb, struct sw_flow_key *key,
-const struct ovs_key_ct_labels *labels,
-const struct ovs_key_ct_labels *mask)
+static struct nf_conn_labels *ovs_ct_get_conn_labels(struct nf_conn *ct)
 {
-   enum ip_conntrack_info ctinfo;
struct nf_conn_labels *cl;
-   struct nf_conn *ct;
-
-   /* The connection could be invalid, in which case set_label is no-op.*/
-   ct = nf_ct_get(skb, );
-   if (!ct)
-   return 0;
 
cl = nf_ct_labels_find(ct);
if (!cl) {
@@ -289,37 +264,59 @@ static int ovs_ct_set_labels(struct sk_buff *skb, struct 
sw_flow_key *key,
cl = nf_ct_labels_find(ct);
}
 
-   if (!cl || ovs_ct_get_labels_len(cl) < OVS_CT_LABELS_LEN)
+   return cl;
+}
+
+/* Initialize labels for a new, yet to be committed conntrack entry.  Note that
+ * since the new connection is not yet confirmed, and thus no-one else has
+ * access to it's labels, we simply write them over.
+ */
+static int ovs_ct_init_labels(struct nf_conn *ct, struct sw_flow_key *key,
+ const struct ovs_key_ct_labels *labels,
+ const struct ovs_key_ct_labels *mask)
+{
+   struct nf_conn_labels *cl;
+   u32 *dst;
+   int i;
+
+   cl = ovs_ct_get_conn_labels(ct);
+   if (!cl)
return -ENOSPC;
 
-   if (nf_ct_is_confirmed(ct)) {
-   /* Triggers a change event, which makes sense only for
-* confirmed connections.
-*/
-   int err = nf_connlabels_replace(ct, labels->ct_labels_32,
-   mask->ct_labels_32,
-   OVS_CT_LABELS_LEN_32);
-   if (err)
-   return err;
-   } else {
-   u32 *dst = (u32 *)cl->bits;
-   const u32 *msk = mask->ct_labels_32;
-   const u32 *lbl = labels->ct_labels_32;
-   int i;
+   dst = (u32 *)cl->bits;
+   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
+   dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
+   (labels->ct_labels_32[i] & mask->ct_labels_32[i]);
 
-   /* No-one else has access to the non-confirmed entry, copy
-* labels over, keeping any bits we are not explicitly setting.
-*/
-   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
-   dst[i] = (dst[i] & ~msk[i]) | (lbl[i] & msk[i]);
+   /* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
+* IPCT_LABEL bit it set in the event cache.
+*/
+   nf_conntrack_event_cache(IPCT_LABEL, ct);
 
-   /* Labels are included in the IPCTNL_MSG_CT_NEW event only if
-* the IPCT_LABEL bit it set in the event cache.
-*/
-   

[ovs-dev] [PATCH branch-2.7 08/14] datapath: Unionize ovs_key_ct_label with a u32 array.

[Joe Stringer]

From: Jarno Rajahalme 

Upstream commit:

commit cb80d58fae76d8ea93555149b2b16e19b89a1f4f
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:55 2017 -0800

openvswitch: Unionize ovs_key_ct_label with a u32 array.

Make the array of labels in struct ovs_key_ct_label an union, adding a
u32 array of the same byte size as the existing u8 array.  It is
faster to loop through the labels 32 bits at the time, which is also
the alignment of netlink attributes.

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 datapath/conntrack.c  | 15 ---
 datapath/linux/compat/include/linux/openvswitch.h |  8 ++--
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index a4f2549deff8..2286a6987052 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -298,20 +298,21 @@ static int ovs_ct_set_labels(struct sk_buff *skb, struct 
sw_flow_key *key,
/* Triggers a change event, which makes sense only for
 * confirmed connections.
 */
-   int err = nf_connlabels_replace(ct, (u32 *)labels, (u32 *)mask,
-   OVS_CT_LABELS_LEN / 
sizeof(u32));
+   int err = nf_connlabels_replace(ct, labels->ct_labels_32,
+   mask->ct_labels_32,
+   OVS_CT_LABELS_LEN_32);
if (err)
return err;
} else {
u32 *dst = (u32 *)cl->bits;
-   const u32 *msk = (const u32 *)mask->ct_labels;
-   const u32 *lbl = (const u32 *)labels->ct_labels;
+   const u32 *msk = mask->ct_labels_32;
+   const u32 *lbl = labels->ct_labels_32;
int i;
 
/* No-one else has access to the non-confirmed entry, copy
 * labels over, keeping any bits we are not explicitly setting.
 */
-   for (i = 0; i < OVS_CT_LABELS_LEN / sizeof(u32); i++)
+   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
dst[i] = (dst[i] & ~msk[i]) | (lbl[i] & msk[i]);
 
/* Labels are included in the IPCTNL_MSG_CT_NEW event only if
@@ -914,8 +915,8 @@ static bool labels_nonzero(const struct ovs_key_ct_labels 
*labels)
 {
size_t i;
 
-   for (i = 0; i < sizeof(*labels); i++)
-   if (labels->ct_labels[i])
+   for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
+   if (labels->ct_labels_32[i])
return true;
 
return false;
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 12260d8bfd64..76a19ed3eb4e 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -472,9 +472,13 @@ struct ovs_key_nd {
__u8nd_tll[ETH_ALEN];
 };
 
-#define OVS_CT_LABELS_LEN  16
+#define OVS_CT_LABELS_LEN_32   4
+#define OVS_CT_LABELS_LEN  (OVS_CT_LABELS_LEN_32 * sizeof(__u32))
 struct ovs_key_ct_labels {
-   __u8ct_labels[OVS_CT_LABELS_LEN];
+   union {
+   __u8ct_labels[OVS_CT_LABELS_LEN];
+   __u32   ct_labels_32[OVS_CT_LABELS_LEN_32];
+   };
 };
 
 /* OVS_KEY_ATTR_CT_STATE flags */
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 06/14] datapath: Do not trigger events for unconfirmed connections.

[Joe Stringer]

From: Jarno Rajahalme 

Upstream commit:

commit 193e30967897f3a8b6f9f137ac30571d832c2c5c
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:54 2017 -0800

openvswitch: Do not trigger events for unconfirmed connections.
Receiving change events before the 'new' event for the connection has
been received can be confusing.  Avoid triggering change events for
setting conntrack mark or labels before the conntrack entry has been
confirmed.

Fixes: 182e3042e15d ("openvswitch: Allow matching on conntrack mark")
Fixes: c2ac66735870 ("openvswitch: Allow matching on conntrack label")
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Upstream commit:

commit 2317c6b51e4249dbfa093e1b88cab0a9f0564b7f
Author: Jarno Rajahalme 
Date:   Fri Feb 17 18:11:58 2017 -0800

openvswitch: Set event bit after initializing labels.

Connlabels are included in conntrack netlink event messages only if
the IPCT_LABEL bit is set in the event cache (see
ctnetlink_conntrack_event()).  Set it after initializing labels for a
new connection.

Found upon further system testing, where it was noticed that labels
were missing from the conntrack events.

Fixes: 193e30967897 ("openvswitch: Do not trigger events for unconfirmed con
nections.")
Signed-off-by: Jarno Rajahalme 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Fixes: 372ce9737d2b ("datapath: Allow matching on conntrack mark")
Fixes: 038e34abaa31 ("datapath: Allow matching on conntrack label")
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 datapath/conntrack.c | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index f3e604a2ef19..a4f2549deff8 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -261,7 +261,8 @@ static int ovs_ct_set_mark(struct sk_buff *skb, struct 
sw_flow_key *key,
new_mark = ct_mark | (ct->mark & ~(mask));
if (ct->mark != new_mark) {
ct->mark = new_mark;
-   nf_conntrack_event_cache(IPCT_MARK, ct);
+   if (nf_ct_is_confirmed(ct))
+   nf_conntrack_event_cache(IPCT_MARK, ct);
key->ct.mark = new_mark;
}
 
@@ -278,7 +279,6 @@ static int ovs_ct_set_labels(struct sk_buff *skb, struct 
sw_flow_key *key,
enum ip_conntrack_info ctinfo;
struct nf_conn_labels *cl;
struct nf_conn *ct;
-   int err;
 
/* The connection could be invalid, in which case set_label is no-op.*/
ct = nf_ct_get(skb, );
@@ -294,10 +294,31 @@ static int ovs_ct_set_labels(struct sk_buff *skb, struct 
sw_flow_key *key,
if (!cl || ovs_ct_get_labels_len(cl) < OVS_CT_LABELS_LEN)
return -ENOSPC;
 
-   err = nf_connlabels_replace(ct, (u32 *)labels, (u32 *)mask,
-   OVS_CT_LABELS_LEN / sizeof(u32));
-   if (err)
-   return err;
+   if (nf_ct_is_confirmed(ct)) {
+   /* Triggers a change event, which makes sense only for
+* confirmed connections.
+*/
+   int err = nf_connlabels_replace(ct, (u32 *)labels, (u32 *)mask,
+   OVS_CT_LABELS_LEN / 
sizeof(u32));
+   if (err)
+   return err;
+   } else {
+   u32 *dst = (u32 *)cl->bits;
+   const u32 *msk = (const u32 *)mask->ct_labels;
+   const u32 *lbl = (const u32 *)labels->ct_labels;
+   int i;
+
+   /* No-one else has access to the non-confirmed entry, copy
+* labels over, keeping any bits we are not explicitly setting.
+*/
+   for (i = 0; i < OVS_CT_LABELS_LEN / sizeof(u32); i++)
+   dst[i] = (dst[i] & ~msk[i]) | (lbl[i] & msk[i]);
+
+   /* Labels are included in the IPCTNL_MSG_CT_NEW event only if
+* the IPCT_LABEL bit it set in the event cache.
+*/
+   nf_conntrack_event_cache(IPCT_LABEL, ct);
+   }
 
ovs_ct_get_labels(ct, >ct.labels);
return 0;
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 07/14] lib: Indicate if netlink message had labels.

[Joe Stringer]

From: Jarno Rajahalme 

Conntrack update events include labels only if they have changed.
Record the presence of labels in the netlink message to OVS internal
representation, so that the user may keep the old labels when an
update does not modify them.

Fixes: 6830a0c0e6bf ("netlink-conntrack: New module.")
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 lib/ct-dpif.h   | 1 +
 lib/netlink-conntrack.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 5da3c2c8e3d1..e8e159a75f3a 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -163,6 +163,7 @@ struct ct_dpif_entry {
 struct ct_dpif_protoinfo protoinfo;
 
 ovs_u128 labels;
+bool have_labels;
 uint32_t status;
 /* Timeout for this entry in seconds */
 uint32_t timeout;
diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c
index 2aadb1e16f75..f0e2aeac8e62 100644
--- a/lib/netlink-conntrack.c
+++ b/lib/netlink-conntrack.c
@@ -780,6 +780,7 @@ nl_ct_attrs_to_ct_dpif_entry(struct ct_dpif_entry *entry,
 entry->mark = ntohl(nl_attr_get_be32(attrs[CTA_MARK]));
 }
 if (attrs[CTA_LABELS]) {
+entry->have_labels = true;
 memcpy(>labels, nl_attr_get(attrs[CTA_LABELS]),
MIN(sizeof entry->labels, nl_attr_get_size(attrs[CTA_LABELS])));
 }
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 04/14] datapath: handle NF_REPEAT from nf_conntrack_in()

[Joe Stringer]

From: Pablo Neira Ayuso 

Upstream commit:
commit 08733a0cb7decce40bbbd0331a0449465f13c444
Author: Pablo Neira Ayuso 
Date:   Thu Nov 3 10:56:43 2016 +0100

netfilter: handle NF_REPEAT from nf_conntrack_in()

NF_REPEAT is only needed from nf_conntrack_in() under a very specific
case required by the TCP protocol tracker, we can handle this case
without returning to the core hook path. Handling of NF_REPEAT from the
nf_reinject() is left untouched.

Signed-off-by: Pablo Neira Ayuso 

[Committer notes]
Shift the functionality into the compat code, protected by v4.10
version check. This allows the datapath/conntrack.c to match
upstream.

Signed-off-by: Jarno Rajahalme 
Signed-off-by: Joe Stringer 
---
 datapath/conntrack.c|  8 ++--
 .../include/net/netfilter/nf_conntrack_core.h   | 21 +
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index a0c54438728e..36db32abbf63 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -770,12 +770,8 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
skb->nfctinfo = IP_CT_NEW;
}
 
-   /* Repeat if requested, see nf_iterate(). */
-   do {
-   err = nf_conntrack_in(net, info->family,
- NF_INET_PRE_ROUTING, skb);
-   } while (err == NF_REPEAT);
-
+   err = nf_conntrack_in(net, info->family,
+ NF_INET_PRE_ROUTING, skb);
if (err != NF_ACCEPT)
return -ENOENT;
 
diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h 
b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
index 09a53c3251b0..16b57a647772 100644
--- a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
+++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h
@@ -67,4 +67,25 @@ static inline bool rpl_nf_ct_get_tuple(const struct sk_buff 
*skb,
 #define nf_ct_get_tuple rpl_nf_ct_get_tuple
 #endif /* HAVE_NF_CT_GET_TUPLEPR_TAKES_STRUCT_NET */
 
+/* Commit 08733a0cb7de ("netfilter: handle NF_REPEAT from nf_conntrack_in()")
+ * introduced behavioural changes to this function which cannot be detected
+ * in the headers. Unconditionally backport to kernels older than the one which
+ * contains this commit. */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0)
+static unsigned int rpl_nf_conntrack_in(struct net *net, u_int8_t pf,
+   unsigned int hooknum,
+   struct sk_buff *skb)
+{
+   int err;
+
+   /* Repeat if requested, see nf_iterate(). */
+   do {
+   err = nf_conntrack_in(net, pf, hooknum, skb);
+   } while (err == NF_REPEAT);
+
+   return err;
+}
+#define nf_conntrack_in rpl_nf_conntrack_in
+#endif /* < 4.10 */
+
 #endif /* _NF_CONNTRACK_CORE_WRAPPER_H */
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 05/14] datapath: Use inverted tuple in ovs_ct_find_existing() if NATted.

[Joe Stringer]

From: Jarno Rajahalme 

Upstream commit:

commit 9ff464db50e437eef131f719cc2e9902eea9c607
Author: Jarno Rajahalme 
Date:   Thu Feb 9 11:21:53 2017 -0800

openvswitch: Use inverted tuple in ovs_ct_find_existing() if NATted.

The conntrack lookup for existing connections fails to invert the
packet 5-tuple for NATted packets, and therefore fails to find the
existing conntrack entry.  Conntrack only stores 5-tuples for incoming
packets, and there are various situations where a lookup on a packet
that has already been transformed by NAT needs to be made.  Looking up
an existing conntrack entry upon executing packet received from the
userspace is one of them.

This patch fixes ovs_ct_find_existing() to invert the packet 5-tuple
for the conntrack lookup whenever the packet has already been
transformed by conntrack from its input form as evidenced by one of
the NAT flags being set in the conntrack state metadata.

Fixes: 05752523e565 ("openvswitch: Interface with NAT.")
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

This patch also adds a test case to OVS system tests to verify the
behavior.

The following is a more thorough explanation of what is going on:

When we have evidence that an existing conntrack entry could exist, we
must invert the tuple if NAT has already been applied, as the current
packet headers do not match any tuple stored in conntrack.  For
example, if a packet from private address X to a public address B is
source-NATted to A, the conntrack entry will have the following tuples
(ignoring the protocol and port numbers) after the conntrack entry is
committed:

Original direction tuple: (X,B)
Reply direction tuple: (B,A)

Now, if a reply packet is already transformed back to the private
address space (e.g., with a CT(nat) action), the tuple corresponding
to the current packet headers is:

Current packet tuple: (B,X)

This does not match either of the conntrack tuples above.  Normally
this does not matter, as the conntrack lookup was already done using
the tuple (B,A), but if the current packet does not match any flow in
the OVS datapath, the packet is sent to userspace via an upcall,
during which the packet's skb is freed, and the conntrack entry
pointer in the skb is lost.  When the packet is reintroduced to the
datapath, any further conntrack action will need to perform a new
conntrack lookup to find the entry again.  Prior to this patch this
second lookup failed.  The datapath flow setup corresponding to the
upcall can succeed, however, allowing all further packets in the reply
direction to re-use the conntrack entry pointer in the skb, so
typically the lookup failure only causes a packet drop.

The solution is to invert the tuple derived from the current packet
headers in case the conntrack state stored in the packet metadata
indicates that the packet has been transformed by NAT:

Inverted tuple: (X,B)

With this the conntrack entry can be found, matching the original
direction tuple.

This same logic also works for the original direction packets:

Current packet tuple (after reverse NAT): (A,B)
Inverted tuple: (B,A)

While the current packet tuple (A,B) does not match either of the
conntrack tuples, the inverted one (B,A) does match the reply
direction tuple.

Since the inverted tuple matches the reverse direction tuple the
direction of the packet must be reversed as well.

Fixes: c5f6c06b58d6 ("datapath: Interface with NAT.")
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
 datapath/conntrack.c| 24 +--
 tests/system-traffic.at | 52 +
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 36db32abbf63..f3e604a2ef19 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -471,7 +471,7 @@ ovs_ct_get_info(const struct nf_conntrack_tuple_hash *h)
  */
 static struct nf_conn *
 ovs_ct_find_existing(struct net *net, const struct nf_conntrack_zone *zone,
-u8 l3num, struct sk_buff *skb)
+u8 l3num, struct sk_buff *skb, bool natted)
 {
struct nf_conntrack_l3proto *l3proto;
struct nf_conntrack_l4proto *l4proto;
@@ -494,6 +494,17 @@ ovs_ct_find_existing(struct net *net, const struct 
nf_conntrack_zone *zone,
return NULL;
}
 
+   /* Must invert the tuple if skb has been transformed by NAT. */
+   if (natted) {
+   struct nf_conntrack_tuple inverse;
+
+   if (!nf_ct_invert_tuple(, , l3proto, l4proto)) {
+   pr_debug("ovs_ct_find_existing: Inversion failed!\n");
+   return NULL;
+   }
+   tuple = inverse;
+   }

[ovs-dev] [PATCH branch-2.7 03/14] datapath: remove unnecessary EXPORT_SYMBOLs

[Joe Stringer]

From: Jiri Benc 

Upstream commit:
commit 76e4cc7731a1e0c07e202999b9834f9d9be66de4
Author: Jiri Benc 
Date:   Wed Oct 19 11:26:37 2016 +0200

openvswitch: remove unnecessary EXPORT_SYMBOLs

Some symbols exported to other modules are really used only by
openvswitch.ko. Remove the exports.

Tested by loading all 4 openvswitch modules, nothing breaks.

Signed-off-by: Jiri Benc 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Jarno Rajahalme 
Signed-off-by: Joe Stringer 
---
 datapath/datapath.c | 2 --
 datapath/vport-netdev.c | 1 -
 datapath/vport.c| 1 -
 3 files changed, 4 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index ce2364a983f5..12ecb95067f2 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -62,7 +62,6 @@
 #include "vport-netdev.h"
 
 int ovs_net_id __read_mostly;
-EXPORT_SYMBOL_GPL(ovs_net_id);
 
 static struct genl_family dp_packet_genl_family;
 static struct genl_family dp_flow_genl_family;
@@ -135,7 +134,6 @@ int lockdep_ovsl_is_held(void)
else
return 1;
 }
-EXPORT_SYMBOL_GPL(lockdep_ovsl_is_held);
 #endif
 
 static int queue_gso_packets(struct datapath *dp, struct sk_buff *,
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index 970f7d3e56a9..fd97246c19a3 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -167,7 +167,6 @@ void ovs_netdev_detach_dev(struct vport *vport)
netdev_master_upper_dev_get(vport->dev));
dev_set_promiscuity(vport->dev, -1);
 }
-EXPORT_SYMBOL_GPL(ovs_netdev_detach_dev);
 
 static void netdev_destroy(struct vport *vport)
 {
diff --git a/datapath/vport.c b/datapath/vport.c
index 7ac463254d4e..9c8c0f171620 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -507,7 +507,6 @@ int ovs_vport_receive(struct vport *vport, struct sk_buff 
*skb,
ovs_dp_process_packet(skb, );
return 0;
 }
-EXPORT_SYMBOL_GPL(ovs_vport_receive);
 
 static unsigned int packet_length(const struct sk_buff *skb)
 {
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 01/14] datapath: fix flow stats accounting when node 0 is not possible

[Joe Stringer]

From: Thadeu Lima de Souza Cascardo 

Upstream commit:
commit 40773966ccf1985a1b2bb570a03cbeaf1cbd4e00
Author: Thadeu Lima de Souza Cascardo 
Date:   Thu Sep 15 19:11:52 2016 -0300

openvswitch: fix flow stats accounting when node 0 is not possible

On a system with only node 1 as possible, all statistics is going to be
accounted on node 0 as it will have a single writer.

However, when getting and clearing the statistics, node 0 is not going
to be considered, as it's not a possible node.

Tested that statistics are not zero on a system with only node 1
possible. Also compile-tested with CONFIG_NUMA off.

Signed-off-by: Thadeu Lima de Souza Cascardo 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

This patch contained a memory leak that is fixed in this backport.
The next patch silently fixed that in upstream, too.

Signed-off-by: Jarno Rajahalme 
Signed-off-by: Joe Stringer 
---
 datapath/flow.c   | 6 --
 datapath/flow_table.c | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index 390286c9889e..6d5664497430 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -141,7 +141,8 @@ void ovs_flow_stats_get(const struct sw_flow *flow,
*tcp_flags = 0;
memset(ovs_stats, 0, sizeof(*ovs_stats));
 
-   for_each_node(node) {
+   /* We open code this to make sure node 0 is always considered */
+   for (node = 0; node < MAX_NUMNODES; node = next_node(node, 
node_possible_map)) {
struct flow_stats *stats = 
rcu_dereference_ovsl(flow->stats[node]);
 
if (stats) {
@@ -164,7 +165,8 @@ void ovs_flow_stats_clear(struct sw_flow *flow)
 {
int node;
 
-   for_each_node(node) {
+   /* We open code this to make sure node 0 is always considered */
+   for (node = 0; node < MAX_NUMNODES; node = next_node(node, 
node_possible_map)) {
struct flow_stats *stats = ovsl_dereference(flow->stats[node]);
 
if (stats) {
diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index d4204e5038d2..3829b920b057 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -154,7 +154,8 @@ static void flow_free(struct sw_flow *flow)
kfree(flow->id.unmasked_key);
if (flow->sf_acts)
ovs_nla_free_flow_actions((struct sw_flow_actions __force 
*)flow->sf_acts);
-   for_each_node(node)
+   /* We open code this to make sure node 0 is always considered */
+   for (node = 0; node < MAX_NUMNODES; node = next_node(node, 
node_possible_map))
if (flow->stats[node])
kmem_cache_free(flow_stats_cache,
rcu_dereference_raw(flow->stats[node]));
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 00/14] Backports for branch-2.7

[Joe Stringer]

This is a trimmed down set of the backports presented by Jarno here:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329817.html

The fixes (including label inheritance and subsequent fixups) are included
in this series, while the unrelated refactors and unnecessary compat changes
are dropped.

Jarno acked this proposal already, but I'm sending it out to the list for
completeness.

The following patch is new in this proposal, as suggested by Jarno:
"datapath: Avoid struct copy on conntrack labels."

Jarno Rajahalme (9):
  datapath: Use inverted tuple in ovs_ct_find_existing() if NATted.
  datapath: Do not trigger events for unconfirmed connections.
  lib: Indicate if netlink message had labels.
  datapath: Unionize ovs_key_ct_label with a u32 array.
  datapath: Simplify labels length logic.
  datapath: Refactor labels initialization.
  datapath: Inherit master's labels.
  datapath: Avoid struct copy on conntrack labels.
  ofp-util: Ignore unknown fields in ofputil_decode_packet_in2().

Jiri Benc (2):
  datapath: remove unused functions
  datapath: remove unnecessary EXPORT_SYMBOLs

Pablo Neira Ayuso (1):
  datapath: handle NF_REPEAT from nf_conntrack_in()

Thadeu Lima de Souza Cascardo (1):
  datapath: fix flow stats accounting when node 0 is not possible

Yi-Hung Wei (1):
  nx-match: Fix oxm decode.

 datapath/conntrack.c   | 172 ++---
 datapath/datapath.c|   2 -
 datapath/flow.c|   6 +-
 datapath/flow_table.c  |   3 +-
 datapath/linux/compat/include/linux/openvswitch.h  |   8 +-
 .../include/net/netfilter/nf_conntrack_core.h  |  21 +++
 datapath/vport-netdev.c|   1 -
 datapath/vport.c   |  17 --
 datapath/vport.h   |   1 -
 lib/ct-dpif.h  |   1 +
 lib/netlink-conntrack.c|   1 +
 lib/nx-match.c |  23 ++-
 lib/nx-match.h |   4 +-
 lib/ofp-util.c |   2 +-
 tests/system-traffic.at|  52 +++
 15 files changed, 223 insertions(+), 91 deletions(-)

-- 
2.11.1

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

Re: [ovs-dev] [PATCH 1/5] netlink: extended ACK reporting

[Joe Stringer]

On 18 April 2017 at 02:41, Johannes Berg  wrote:
> On Thu, 2017-04-13 at 14:44 -0700, Joe Stringer wrote
> (something that never made it to the list, due to HTML formatting)
>>
>> I think that OVS was doing some more elaborate validation than most
>> users, so over time we picked up a bunch of extra parsing code that
>> layers on top of nla_parse(). I took a look at trying to broaden this
>> and make it useful to other users a while ago, but when I posted
>> there wasn't much interest from others on it so I just moved on.
>> Maybe it's about time to pick that back up.
>
> Ah, ok. I didn't realize it was actually on top of nla_parse(). Some of
> this does seem rather useful though, and having more expressive policy
> would seem very useful too - I'd love to be able to express nesting
> better, for example.

Ah, correction - nla_parse() is used in some parts but not all of it.
More expressive policy sounds useful for OVS cases too.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [datapath backport v2 03/10] compat: ipv6: orphan skbs in reassembly unit.

[Joe Stringer]

On 14 April 2017 at 16:06, Andy Zhou  wrote:
> From: Eric Dumazet 
>
> Upstream commit:
> ipv6: orphan skbs in reassembly unit
>
> Andrey reported a use-after-free in IPv6 stack.
>
> Issue here is that we free the socket while it still has skb
> in TX path and in some queues.
>
> It happens here because IPv6 reassembly unit messes skb->truesize,
> breaking skb_set_owner_w() badly.
>
> We fixed a similar issue for IPV4 in commit 8282f27449bf ("inet: frag:
> Always orphan skbs inside ip_defrag()")
> Acked-by: Joe Stringer 
>
> ==
> BUG: KASAN: use-after-free in sock_wfree+0x118/0x120
> Read of size 8 at addr 880062da0060 by task a.out/4140
>
> page:ea00018b6800 count:1 mapcount:0 mapping:  (null)
> index:0x0 compound_mapcount: 0
> flags: 0x1008100(slab|head)
> raw: 01008100   000180130013
> raw: dead0100 dead0200 88006741f140 
> page dumped because: kasan: bad access detected
>
> CPU: 0 PID: 4140 Comm: a.out Not tainted 4.10.0-rc3+ #59
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 
> 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:15
>  dump_stack+0x292/0x398 lib/dump_stack.c:51
>  describe_address mm/kasan/report.c:262
>  kasan_report_error+0x121/0x560 mm/kasan/report.c:370
>  kasan_report mm/kasan/report.c:392
>  __asan_report_load8_noabort+0x3e/0x40 mm/kasan/report.c:413
>  sock_flag ./arch/x86/include/asm/bitops.h:324
>  sock_wfree+0x118/0x120 net/core/sock.c:1631
>  skb_release_head_state+0xfc/0x250 net/core/skbuff.c:655
>  skb_release_all+0x15/0x60 net/core/skbuff.c:668
>  __kfree_skb+0x15/0x20 net/core/skbuff.c:684
>  kfree_skb+0x16e/0x4e0 net/core/skbuff.c:705
>  inet_frag_destroy+0x121/0x290 net/ipv4/inet_fragment.c:304
>  inet_frag_put ./include/net/inet_frag.h:133
>  nf_ct_frag6_gather+0x1125/0x38b0 
> net/ipv6/netfilter/nf_conntrack_reasm.c:617
>  ipv6_defrag+0x21b/0x350 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68
>  nf_hook_entry_hookfn ./include/linux/netfilter.h:102
>  nf_hook_slow+0xc3/0x290 net/netfilter/core.c:310
>  nf_hook ./include/linux/netfilter.h:212
>  __ip6_local_out+0x52c/0xaf0 net/ipv6/output_core.c:160
>  ip6_local_out+0x2d/0x170 net/ipv6/output_core.c:170
>  ip6_send_skb+0xa1/0x340 net/ipv6/ip6_output.c:1722
>  ip6_push_pending_frames+0xb3/0xe0 net/ipv6/ip6_output.c:1742
>  rawv6_push_pending_frames net/ipv6/raw.c:613
>  rawv6_sendmsg+0x2cff/0x4130 net/ipv6/raw.c:927
>  inet_sendmsg+0x164/0x5b0 net/ipv4/af_inet.c:744
>  sock_sendmsg_nosec net/socket.c:635
>  sock_sendmsg+0xca/0x110 net/socket.c:645
>  sock_write_iter+0x326/0x620 net/socket.c:848
>  new_sync_write fs/read_write.c:499
>  __vfs_write+0x483/0x760 fs/read_write.c:512
>  vfs_write+0x187/0x530 fs/read_write.c:560
>  SYSC_write fs/read_write.c:607
>  SyS_write+0xfb/0x230 fs/read_write.c:599
>  entry_SYSCALL_64_fastpath+0x1f/0xc2 arch/x86/entry/entry_64.S:203
> RIP: 0033:0x7ff26e6f5b79
> RSP: 002b:7ff268e0ed98 EFLAGS: 0206 ORIG_RAX: 0001
> RAX: ffda RBX: 7ff268e0f9c0 RCX: 7ff26e6f5b79
> RDX: 0010 RSI: 20f50fe1 RDI: 0003
> RBP: 7ff26ebc1220 R08:  R09: 
> R10:  R11: 0206 R12: 
> R13: 7ff268e0f9c0 R14: 7ff26efec040 R15: 0003
>
> The buggy address belongs to the object at 880062da
>  which belongs to the cache RAWv6 of size 1504
> The buggy address 880062da0060 is located 96 bytes inside
>  of 1504-byte region [880062da, 880062da05e0)
>
> Freed by task 4113:
>  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:502
>  set_track mm/kasan/kasan.c:514
>  kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:578
>  slab_free_hook mm/slub.c:1352
>  slab_free_freelist_hook mm/slub.c:1374
>  slab_free mm/slub.c:2951
>  kmem_cache_free+0xb2/0x2c0 mm/slub.c:2973
>  sk_prot_free net/core/sock.c:1377
>  __sk_destruct+0x49c/0x6e0 net/core/sock.c:1452
>  sk_destruct+0x47/0x80 net/core/sock.c:1460
>  __sk_free+0x57/0x230 net/core/sock.c:1468
>  sk_free+0x23/0x30 net/core/sock.c:1479
>  sock_put ./include/net/sock.h:1638
>  sk_common_release+0x31e/0x4e0 net/core/sock.c:2782
>  rawv6_close+0x54/0x80 net/ipv6/raw.c:1214
>  inet_release+0xed/0x1c0 net/ipv4/af_inet.c:425
>  inet6_release+0x50/0x70 net/ipv6/af_inet6.c:431
>  sock_release+0x8d/0x1e0 net/socket.c:599
>  sock_close+0x16/0x20 net/socket.c:1063
>  

Re: [ovs-dev] [datapath backport v2 02/10] datapath: Pack struct sw_flow_key.

[Joe Stringer]

On 14 April 2017 at 16:06, Andy Zhou  wrote:
> From: Jarno Rajahalme 
>
> Upstream commit:
> openvswitch: Pack struct sw_flow_key.
>
> struct sw_flow_key has two 16-bit holes. Move the most matched
> conntrack match fields there.  In some typical cases this reduces the
> size of the key that needs to be hashed into half and into one cache
> line.
>
> Signed-off-by: Jarno Rajahalme 
> Acked-by: Joe Stringer 
> Acked-by: Pravin B Shelar 
> Signed-off-by: David S. Miller 
>
> Upstream: 316d4d78cf9b ("openvswitch: Pack struct sw_flow_key.")
> Signed-off-by: Joe Stringer 
> Signed-off-by: Andy Zhou 

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

Re: [ovs-dev] [datapath backport v2 01/10] datapath: Always define NF_CT_LABELS_MAX_SIZE

[Joe Stringer]

On 14 April 2017 at 16:06, Andy Zhou  wrote:
> When CONFIG_NF_CONNTRACK_LABLES is not set, upstream code still make
> use of NF_CT_LABLES_MAX_SIZE. Always define it in the compat code
> to keep back ports close to the upstream.
>
> Signed-off-by: Andy Zhou 
>
> ---

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

Re: [ovs-dev] [PATCH] system-ovn.at: Add test for ping other router's port on distributed router

[Joe Stringer]

On 18 April 2017 at 04:49, Guoshuai Li  wrote:
> Signed-off-by: Guoshuai Li 
> ---

Hi Guoshuai,

Thanks for the patch. Is this intended for debugging some current issue?

I can't get these new tests to pass "make check-kmod".

Furthermore, for "make check-system-userspace", the "datapath - SNAT
and UNSNAT" test needs to check for CT, NAT features, eg using the
following incremental:

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 295e6062579e..6f637f257b75 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3680,6 +3680,8 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/dropping VLAN
\(0\|300\) packet received on dot1q-t
AT_CLEANUP

AT_SETUP([datapath - SNAT and UNSNAT])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
OVS_TRAFFIC_VSWITCHD_START()

AT_CHECK([ovs-ofctl add-flow br0 "table=0,
priority=100,in_port=1,ip,nw_dst=20.0.0.2
actions=dec_ttl(),mod_dl_src:00:00:02:01:02:01,mod_dl_dst:00:00:02:01:02:02,resubmit(,1)"])



> --
> 2.10.1.windows.1
>
> This patch is used to analyze "ovn: unsnat handling error for Distributed 
> Gateway" problems:
>
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331033.html

Ah :-)

Usually this appears immediately below a "---" after the commit
message. I'll mark this in patchwork as 'Deferred'. When a fix is
supplied, we could apply this patch at the same time.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] netdev-dpdk: Implement Tx intermediate queue for dpdk ports.

[Bodireddy, Bhanuprakash]

Hi Eelco,

Please find my comments inline. 

>
>Hi Bhanuprakash,
>
>I was doing some Physical to Virtual tests, and whenever the number of flows
>reaches the rx batch size performance dropped a lot. I created an
>experimental patch where I added an intermediate queue and flush it at the
>end of the rx batch.
>
>When I found your patch I decided to give it try to see how it behaves.
>I also modified you patch in such a way that it will flush the queue after 
>every
>call to dp_netdev_process_rxq_port().

I presume you were doing like below in the pmd_thread_main receive loop?

for (i = 0; i < poll_cnt; i++) {
dp_netdev_process_rxq_port(pmd, poll_list[i].rx,
   poll_list[i].port_no);
dp_netdev_drain_txq_ports(pmd);
}

>
>Here are some pkt forwarding stats for the Physical to Physical scenario, for
>two 82599ES 10G port with 64 byte packets being send at wire speed:
>
>Number  plainpatch +
>of flows  git clonepatch  flush
>  =  =  =
>   10   10727283   13527752   13393844
>   327042253   11285572 11228799
>   5075154919642650  9607791
>  10058386999461239  9430730
>  50052850667859123  7845807
>100052264777146404  7135601

Thanks for sharing the numbers, I do agree with your findings and I saw very 
similar results with our v3 patch.
In any case we see significant throughput improvement with the patch.

>
>
>I do not have an IXIA to do the latency tests you performed, however I do
>have a XENA tester which has a basic latency measurement feature.
>I used the following script to get the latency numbers:
>
>https://github.com/chaudron/XenaPythonLib/blob/latency/examples/latenc
>y.py

Thanks for pointing this, it could be useful for users with no IXIA setup.

>
>
>As you can see in the numbers below, the default queue introduces quite
>some latency, however doing the flush every rx batch brings the latency down
>to almost the original values. The results mimics your test case 2, sending 10G
>traffic @ wire speed:
>
>   = GIT CLONE
>   Pkt size  min(ns)  avg(ns)  max(ns)
>512  4,631  5,022309,914
>   1024  5,545  5,749104,294
>   1280  5,978  6,159 45,306
>   1518  6,419  6,774946,850
>
>   = PATCH
>   Pkt size  min(ns)  avg(ns)  max(ns)
>512  4,928492,228  1,995,026
>   1024  5,761499,206  2,006,628
>   1280  6,186497,975  1,986,175
>   1518  6,579494,434  2,005,947
>
>   = PATCH + FLUSH
>   Pkt size  min(ns)  avg(ns)  max(ns)
>512  4,711  5,064182,477
>   1024  5,601  5,888701,654
>   1280  6,018  6,491533,037
>   1518  6,467  6,734312,471

The latency numbers above are very encouraging indeed. However with RFC2544 
tests especially on IXIA, we do have lot of parameters to tune.
I see that the latency stats fluctuate a lot with change in acceptable 'Frame 
Loss'.  I am not expert of IXIA myself, but trying to figure out acceptable
settings and trying to measure latency/throughput. 

>
>Maybe it will be good to re-run your latency tests with the flush for every rx
>batch. This might get ride of your huge latency while still increasing the
>performance in the case the rx batch shares the same egress port.
>
>The overall patchset looks fine to me, see some comments inline.
Thanks for reviewing the patch.

>>
>> +#define MAX_LOOP_TO_DRAIN 128
>Is defining this inline ok?
I see that this convention is used in ovs. 

>>   NULL,
>>   NULL,
>>   netdev_dpdk_vhost_reconfigure,
>> -netdev_dpdk_vhost_rxq_recv);
>> +netdev_dpdk_vhost_rxq_recv,
>> +NULL);
>We need this patch even more in the vhost case as there is an even bigger
>drop in performance when we exceed the rx batch size. I measured around
>40%, when reducing the rx batch size to 4, and using 1 vs 5 flows (single PMD).

Completely Agree. Infact we did a quick patch doing batching for vhost ports as 
well and found significant performance improvement(though it's not thoroughly 
tested for all corner cases).
We have that in our backlog and we will trying posting that patch as an RFC 
atleast to get feedback from the community.

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

[ovs-dev] [PATCH 5/5] ovn-trace: Implement ct_next and ct_clear actions.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 NEWS  |  1 +
 ovn/utilities/ovn-trace.8.xml | 58 +
 ovn/utilities/ovn-trace.c | 99 ++-
 3 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index eb825ac72161..f0ada38b4019 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,7 @@ Post-v2.7.0
  * Gratuitous ARP for NAT addresses on a distributed logical router.
  * Allow ovn-controller SSL configuration to be obtained from vswitchd
database.
+ * ovn-trace now has basic support for tracing distributed firewalls.
- Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
- OpenFlow:
  * Increased support for OpenFlow 1.6 (draft).
diff --git a/ovn/utilities/ovn-trace.8.xml b/ovn/utilities/ovn-trace.8.xml
index 78914240d88c..8bb329bfbd71 100644
--- a/ovn/utilities/ovn-trace.8.xml
+++ b/ovn/utilities/ovn-trace.8.xml
@@ -307,6 +307,64 @@
 
   
 
+
+--ct=flags
+
+  
+This option sets the ct_state flags that a
+ct_next logical action will report.  The flags
+must be a comma- or space-separated list of the following connection
+tracking flags:
+  
+
+  
+
+  trk: Include to indicate connection tracking has taken
+  place.  (This bit is set automatically even if not listed in
+  flags.
+
+new: Include to indicate a new flow.
+est: Include to indicate an established flow.
+rel: Include to indicate a related flow.
+rpl: Include to indicate a reply flow.
+inv: Include to indicate a connection entry in a
+bad state.
+dnat: Include to indicate a packet whose
+destination IP address has been changed.
+snat: Include to indicate a packet whose source IP
+address has been changed.
+  
+
+  
+The ct_next action is used to implement the OVN
+distributed firewall.  For testing, useful flag combinations include:
+  
+
+  
+trk,new: A packet in a flow in either direction
+through a firewall that has not yet been committed (with
+ct_commit).
+trk,est: A packet in an established flow going out
+through a firewall.
+trk,rpl: A packet coming in through a firewall in
+reply to an established flow.
+trk,inv: An invalid packet in either direction.
+  
+
+  
+A packet might pass through the connection tracker twice in one trip
+through OVN: once following egress from a VM as it passes outward
+through a firewall, and once preceding ingress to a second VM as it
+passes inward through a firewall.  Use multiple --ct
+options to specify the flags for multiple ct_next actions.
+  
+
+  
+When --ct is unspecified, or when there are fewer
+--ct options than ct_next actions, the
+flags default to trk,est.
+  
+
   
 
   Daemon Options
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 66844b11ac1d..e9463f02a70f 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -69,6 +69,11 @@ static bool minimal;
 static const char *ovs;
 static struct vconn *vconn;
 
+/* --ct: Connection tracking state to use for ct_next() actions. */
+static uint32_t *ct_states;
+static size_t n_ct_states;
+static size_t ct_state_idx;
+
 OVS_NO_RETURN static void usage(void);
 static void parse_options(int argc, char *argv[]);
 static char *trace(const char *datapath, const char *flow);
@@ -156,6 +161,42 @@ default_ovs(void)
 }
 
 static void
+parse_ct_option(const char *state_s_)
+{
+uint32_t state = CS_TRACKED;
+
+char *state_s = xstrdup(state_s_);
+char *save_ptr = NULL;
+for (char *cs = strtok_r(state_s, ", ", _ptr); cs;
+ cs = strtok_r(NULL, ", ", _ptr)) {
+uint32_t bit = ct_state_from_string(cs);
+if (!bit) {
+ovs_fatal(0, "%s: unknown connection tracking state flag", cs);
+}
+state |= bit;
+}
+free(state_s);
+
+/* Check constraints listed in ovs-fields(7). */
+if (state & CS_INVALID && state & ~(CS_TRACKED | CS_INVALID)) {
+VLOG_WARN("%s: invalid connection state: "
+  "when \"inv\" is set, only \"trk\" may also be set",
+  state_s_);
+}
+if (state & CS_NEW && state & CS_ESTABLISHED) {
+VLOG_WARN("%s: invalid connection state: "
+  "\"new\" and \"est\" are mutually exclusive", state_s_);
+}
+if (state & CS_NEW && state & CS_REPLY_DIR) {
+VLOG_WARN("%s: invalid connection state: "
+  "\"new\" and \"rpy\" are mutually exclusive", state_s_);
+}
+
+ct_states = xrealloc(ct_states, (n_ct_states + 1) * sizeof *ct_states);
+ct_states[n_ct_states++] = state;
+}
+
+static void
 parse_options(int argc, char *argv[])
 {
 enum {
@@ -166,6 

[ovs-dev] [PATCH 4/5] flow: New function flow_clear_conntrack().

[Ben Pfaff]

This will have a new user in an upcoming commit.

Signed-off-by: Ben Pfaff 
---
 lib/flow.c   | 21 +
 lib/flow.h   |  1 +
 ofproto/ofproto-dpif-xlate.c | 18 +-
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index 7552cd72a173..2ba51214ef80 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1024,6 +1024,27 @@ ct_state_from_string(const char *s)
 return 0;
 }
 
+/* Clears the fields in 'flow' associated with connection tracking. */
+void
+flow_clear_conntrack(struct flow *flow)
+{
+flow->ct_state = 0;
+flow->ct_zone = 0;
+flow->ct_mark = 0;
+flow->ct_label = OVS_U128_ZERO;
+
+flow->ct_nw_proto = 0;
+flow->ct_tp_src = 0;
+flow->ct_tp_dst = 0;
+if (flow->dl_type == htons(ETH_TYPE_IP)) {
+flow->ct_nw_src = 0;
+flow->ct_nw_dst = 0;
+} else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
+memset(>ct_ipv6_src, 0, sizeof flow->ct_ipv6_src);
+memset(>ct_ipv6_dst, 0, sizeof flow->ct_ipv6_dst);
+}
+}
+
 char *
 flow_to_string(const struct flow *flow)
 {
diff --git a/lib/flow.h b/lib/flow.h
index 9d4ae49ccfbd..2957108a68db 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -74,6 +74,7 @@ void flow_get_metadata(const struct flow *, struct match 
*flow_metadata);
 
 const char *ct_state_to_string(uint32_t state);
 uint32_t ct_state_from_string(const char *);
+void flow_clear_conntrack(struct flow *);
 
 char *flow_to_string(const struct flow *);
 void format_flags(struct ds *ds, const char *(*bit_to_string)(uint32_t),
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a24aef9a43a1..76b8a3aa729a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3222,23 +3222,7 @@ static void
 clear_conntrack(struct xlate_ctx *ctx)
 {
 ctx->conntracked = false;
-
-struct flow *flow = >xin->flow;
-flow->ct_state = 0;
-flow->ct_zone = 0;
-flow->ct_mark = 0;
-flow->ct_label = OVS_U128_ZERO;
-
-flow->ct_nw_proto = 0;
-flow->ct_tp_src = 0;
-flow->ct_tp_dst = 0;
-if (flow->dl_type == htons(ETH_TYPE_IP)) {
-flow->ct_nw_src = 0;
-flow->ct_nw_dst = 0;
-} if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
-memset(>ct_ipv6_src, 0, sizeof flow->ct_ipv6_src);
-memset(>ct_ipv6_dst, 0, sizeof flow->ct_ipv6_dst);
-}
+flow_clear_conntrack(>xin->flow);
 }
 
 static bool
-- 
2.10.2

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

[ovs-dev] [PATCH 2/5] packets: Reduce redundant copies of connection states.

[Ben Pfaff]

I was about to add another complete list of all the connection states but
this eliminates the need.

Signed-off-by: Ben Pfaff 
---
 lib/flow.c   | 21 --
 lib/odp-util.c   | 56 +---
 lib/packets.h| 45 +++---
 ovn/lib/logical-fields.c | 37 
 4 files changed, 51 insertions(+), 108 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index 2b1ec4fed7ef..5f9c3d0e3a88 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017 Nicira, 
Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1003,22 +1003,9 @@ flow_get_metadata(const struct flow *flow, struct match 
*flow_metadata)
 const char *ct_state_to_string(uint32_t state)
 {
 switch (state) {
-case CS_REPLY_DIR:
-return "rpl";
-case CS_TRACKED:
-return "trk";
-case CS_NEW:
-return "new";
-case CS_ESTABLISHED:
-return "est";
-case CS_RELATED:
-return "rel";
-case CS_INVALID:
-return "inv";
-case CS_SRC_NAT:
-return "snat";
-case CS_DST_NAT:
-return "dnat";
+#define CS_STATE(ENUM, INDEX, NAME) case CS_##ENUM: return NAME;
+CS_STATES
+#undef CS_STATE
 default:
 return NULL;
 }
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 874777858906..ebb572dc1acc 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3533,30 +3533,12 @@ ovs_to_odp_ct_state(uint8_t state)
 {
 uint32_t odp = 0;
 
-if (state & CS_NEW) {
-odp |= OVS_CS_F_NEW;
-}
-if (state & CS_ESTABLISHED) {
-odp |= OVS_CS_F_ESTABLISHED;
-}
-if (state & CS_RELATED) {
-odp |= OVS_CS_F_RELATED;
-}
-if (state & CS_INVALID) {
-odp |= OVS_CS_F_INVALID;
-}
-if (state & CS_REPLY_DIR) {
-odp |= OVS_CS_F_REPLY_DIR;
-}
-if (state & CS_TRACKED) {
-odp |= OVS_CS_F_TRACKED;
-}
-if (state & CS_SRC_NAT) {
-odp |= OVS_CS_F_SRC_NAT;
-}
-if (state & CS_DST_NAT) {
-odp |= OVS_CS_F_DST_NAT;
+#define CS_STATE(ENUM, INDEX, NAME) \
+if (state & CS_##ENUM) {\
+odp |= OVS_CS_F_##ENUM; \
 }
+CS_STATES
+#undef CS_STATE
 
 return odp;
 }
@@ -3566,30 +3548,12 @@ odp_to_ovs_ct_state(uint32_t flags)
 {
 uint32_t state = 0;
 
-if (flags & OVS_CS_F_NEW) {
-state |= CS_NEW;
-}
-if (flags & OVS_CS_F_ESTABLISHED) {
-state |= CS_ESTABLISHED;
-}
-if (flags & OVS_CS_F_RELATED) {
-state |= CS_RELATED;
-}
-if (flags & OVS_CS_F_INVALID) {
-state |= CS_INVALID;
-}
-if (flags & OVS_CS_F_REPLY_DIR) {
-state |= CS_REPLY_DIR;
-}
-if (flags & OVS_CS_F_TRACKED) {
-state |= CS_TRACKED;
-}
-if (flags & OVS_CS_F_SRC_NAT) {
-state |= CS_SRC_NAT;
-}
-if (flags & OVS_CS_F_DST_NAT) {
-state |= CS_DST_NAT;
+#define CS_STATE(ENUM, INDEX, NAME) \
+if (flags & OVS_CS_F_##ENUM) {  \
+state |= CS_##ENUM; \
 }
+CS_STATES
+#undef CS_STATE
 
 return state;
 }
diff --git a/lib/packets.h b/lib/packets.h
index d8f32fc5f3f7..6776be36dcc1 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -794,33 +794,34 @@ struct tcp_header {
 };
 BUILD_ASSERT_DECL(TCP_HEADER_LEN == sizeof(struct tcp_header));
 
-/* Connection states */
-enum {
-CS_NEW_BIT = 0,
-CS_ESTABLISHED_BIT = 1,
-CS_RELATED_BIT = 2,
-CS_REPLY_DIR_BIT =   3,
-CS_INVALID_BIT = 4,
-CS_TRACKED_BIT = 5,
-CS_SRC_NAT_BIT = 6,
-CS_DST_NAT_BIT = 7,
-};
+/* Connection states.
+ *
+ * Names like CS_RELATED are bit values, e.g. 1 << 2.
+ * Names like CS_RELATED_BIT are bit indexes, e.g. 2. */
+#define CS_STATES   \
+CS_STATE(NEW, 0, "new") \
+CS_STATE(ESTABLISHED, 1, "est") \
+CS_STATE(RELATED, 2, "rel") \
+CS_STATE(REPLY_DIR,   3, "rpl") \
+CS_STATE(INVALID, 4, "inv") \
+CS_STATE(TRACKED, 5, "trk") \
+CS_STATE(SRC_NAT, 6, "snat")\
+CS_STATE(DST_NAT, 7, "dnat")
 
 enum {
-CS_NEW = (1 << CS_NEW_BIT),
-CS_ESTABLISHED = (1 << CS_ESTABLISHED_BIT),
-CS_RELATED = (1 << CS_RELATED_BIT),
-CS_REPLY_DIR =   (1 << CS_REPLY_DIR_BIT),
-CS_INVALID = (1 << CS_INVALID_BIT),
-CS_TRACKED = (1 << CS_TRACKED_BIT),
-CS_SRC_NAT = (1 << CS_SRC_NAT_BIT),
-CS_DST_NAT = (1 << CS_DST_NAT_BIT),
+#define CS_STATE(ENUM, INDEX, NAME) \
+CS_##ENUM = 1 << INDEX, \
+CS_##ENUM##_BIT = INDEX,

[ovs-dev] [PATCH 0/5] Add basic ct_next support to ovn-trace

[Ben Pfaff]

This makes ovn-trace much more usable with OpenStack.

Ben Pfaff (5):
  ovn-sb.xml: Document ct.trk and improve wording for other ct flags.
  packets: Reduce redundant copies of connection states.
  flow: New function ct_state_from_string().
  flow: New function flow_clear_conntrack().
  ovn-trace: Implement ct_next and ct_clear actions.

 NEWS  |  1 +
 lib/flow.c| 57 +
 lib/flow.h|  5 ++-
 lib/odp-util.c| 56 +---
 lib/packets.h | 45 ++--
 ofproto/ofproto-dpif-xlate.c  | 18 +---
 ovn/lib/logical-fields.c  | 37 ++--
 ovn/ovn-sb.xml| 18 
 ovn/utilities/ovn-trace.8.xml | 58 +
 ovn/utilities/ovn-trace.c | 99 ++-
 10 files changed, 257 insertions(+), 137 deletions(-)

-- 
2.10.2

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

Re: [ovs-dev] [PATCH 4/5] dpif-netdev: Fix comments for dp_netdev_pmd_thread struct.

[Bodireddy, Bhanuprakash]

>On Sun, Mar 12, 2017 at 05:33:27PM +, Bhanuprakash Bodireddy wrote:
>> The sorted subtable ranking patch introduced a classifier instance per
>> ingress port with its subtables ranked on the frequency of hits. The
>> pmd thread can have more classifier instances now and solely depends
>> on the number of ingress ports currently handled by the pmd thread.
>>
>> Signed-off-by: Bhanuprakash Bodireddy
>> 
>
>Thank you for improving comments!
>
>I'm not the right person to review this patch, but from a process perspective I
>find myself wondering whether it corrects a comment that was already wrong
>before the beginning of the series (in which case it is fine as-is) or whether 
>it
>only needs correction following the series (in which case it should be folded
>into whichever patch made it incorrect).
>
>Thanks again!

Hi Ben,
Please note that the comments were right initially but after the "subtable 
ranking" feature got introduced the comments needed correction.  The subtable 
ranking patch series got merged a while ago  but this particular comment wasn't 
fixed then. I happened to find this during code inspection.  What's the best 
way to handle this now?

- Bhanuprakash.

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

Re: [ovs-dev] [PATCH 3/3] ofproto-dpif: Check support for resubmit with conntrack action.

[Jarno Rajahalme]

> On Apr 18, 2017, at 8:56 AM, Ben Pfaff  wrote:
> 
> On Fri, Apr 14, 2017 at 05:25:49PM -0700, Jarno Rajahalme wrote:
>> Use the existing probed support flag for the original direction tuple
>> to determine if resubmit(ct) can be executed or not.
>> 
>> Signed-off-by: Jarno Rajahalme 
> 
> Thank you!  I think that this will help to make for less confusing
> errors in the future.  I did not study this carefully, but what I saw
> looked good.
> 
> Acked-by: Ben Pfaff 

Thanks for the reviews!

Series pushed to master,

  Jarno

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

[ovs-dev] [PATCH v2] Avoid update probe interval to non-zero for unix socket.

[Han Zhou]

In commit c1bfdd9d it disables probe when not needed, but commit
715038b6 updates ovn-controller probe interval for OVNSB DB
periodically according to ovn-remote-probe-interval config, and sets
it to DEFAULT_PROBE_INTERVAL_MSEC if not configured, even if the
connection type is unix socket which doesn't need probe.

This fix avoids probe interval update if not needed (always set to 0).

Signed-off-by: Han Zhou 
---

Notes:
v1->v2: fix commit id mentioned in commit message.

 lib/reconnect.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/reconnect.c b/lib/reconnect.c
index 471fb7f..6b52481 100644
--- a/lib/reconnect.c
+++ b/lib/reconnect.c
@@ -16,6 +16,7 @@
 
 #include 
 #include "reconnect.h"
+#include "stream.h"
 
 #include 
 
@@ -243,6 +244,9 @@ reconnect_set_backoff(struct reconnect *fsm, int 
min_backoff, int max_backoff)
 void
 reconnect_set_probe_interval(struct reconnect *fsm, int probe_interval)
 {
+if (!stream_or_pstream_needs_probes(fsm->name)) {
+probe_interval = 0;
+}
 fsm->probe_interval = probe_interval ? MAX(1000, probe_interval) : 0;
 }
 
-- 
2.1.0

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

Re: [ovs-dev] [PATCH] Avoid update probe interval to non-zero for unix socket.

[Han Zhou]

sorry :D
I will update it.

On Tue, Apr 18, 2017 at 10:25 AM, Russell Bryant  wrote:

> On Tue, Apr 18, 2017 at 1:08 PM, Han Zhou  wrote:
> > In commit  it disables probe when not needed, but commit
>
> I assume you meant to update this "xxx" with a commit ID?
>
> > 715038b6 updates ovn-controller probe interval for OVNSB DB
> > periodically according to ovn-remote-probe-interval config, and sets
> > it to DEFAULT_PROBE_INTERVAL_MSEC if not configured, even if the
> > connection type is unix socket which doesn't need probe.
> >
> > This fix avoids probe interval update if not needed (always set to 0).
> >
> > Signed-off-by: Han Zhou 
> > ---
> >  lib/reconnect.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/reconnect.c b/lib/reconnect.c
> > index 471fb7f..6b52481 100644
> > --- a/lib/reconnect.c
> > +++ b/lib/reconnect.c
> > @@ -16,6 +16,7 @@
> >
> >  #include 
> >  #include "reconnect.h"
> > +#include "stream.h"
> >
> >  #include 
> >
> > @@ -243,6 +244,9 @@ reconnect_set_backoff(struct reconnect *fsm, int
> min_backoff, int max_backoff)
> >  void
> >  reconnect_set_probe_interval(struct reconnect *fsm, int probe_interval)
> >  {
> > +if (!stream_or_pstream_needs_probes(fsm->name)) {
> > +probe_interval = 0;
> > +}
> >  fsm->probe_interval = probe_interval ? MAX(1000, probe_interval) :
> 0;
> >  }
> >
> > --
> > 2.1.0
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
>
> --
> Russell Bryant
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Avoid update probe interval to non-zero for unix socket.

[Russell Bryant]

On Tue, Apr 18, 2017 at 1:08 PM, Han Zhou  wrote:
> In commit  it disables probe when not needed, but commit

I assume you meant to update this "xxx" with a commit ID?

> 715038b6 updates ovn-controller probe interval for OVNSB DB
> periodically according to ovn-remote-probe-interval config, and sets
> it to DEFAULT_PROBE_INTERVAL_MSEC if not configured, even if the
> connection type is unix socket which doesn't need probe.
>
> This fix avoids probe interval update if not needed (always set to 0).
>
> Signed-off-by: Han Zhou 
> ---
>  lib/reconnect.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/reconnect.c b/lib/reconnect.c
> index 471fb7f..6b52481 100644
> --- a/lib/reconnect.c
> +++ b/lib/reconnect.c
> @@ -16,6 +16,7 @@
>
>  #include 
>  #include "reconnect.h"
> +#include "stream.h"
>
>  #include 
>
> @@ -243,6 +244,9 @@ reconnect_set_backoff(struct reconnect *fsm, int 
> min_backoff, int max_backoff)
>  void
>  reconnect_set_probe_interval(struct reconnect *fsm, int probe_interval)
>  {
> +if (!stream_or_pstream_needs_probes(fsm->name)) {
> +probe_interval = 0;
> +}
>  fsm->probe_interval = probe_interval ? MAX(1000, probe_interval) : 0;
>  }
>
> --
> 2.1.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev



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

[ovs-dev] [PATCH] Avoid update probe interval to non-zero for unix socket.

[Han Zhou]

In commit  it disables probe when not needed, but commit
715038b6 updates ovn-controller probe interval for OVNSB DB
periodically according to ovn-remote-probe-interval config, and sets
it to DEFAULT_PROBE_INTERVAL_MSEC if not configured, even if the
connection type is unix socket which doesn't need probe.

This fix avoids probe interval update if not needed (always set to 0).

Signed-off-by: Han Zhou 
---
 lib/reconnect.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/reconnect.c b/lib/reconnect.c
index 471fb7f..6b52481 100644
--- a/lib/reconnect.c
+++ b/lib/reconnect.c
@@ -16,6 +16,7 @@
 
 #include 
 #include "reconnect.h"
+#include "stream.h"
 
 #include 
 
@@ -243,6 +244,9 @@ reconnect_set_backoff(struct reconnect *fsm, int 
min_backoff, int max_backoff)
 void
 reconnect_set_probe_interval(struct reconnect *fsm, int probe_interval)
 {
+if (!stream_or_pstream_needs_probes(fsm->name)) {
+probe_interval = 0;
+}
 fsm->probe_interval = probe_interval ? MAX(1000, probe_interval) : 0;
 }
 
-- 
2.1.0

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

Re: [ovs-dev] [PATCH ovs] Documentation: fix broken links in maintainers page

[Stephen Finucane]

On Tue, 2017-04-18 at 15:08 +0300, Roi Dayan wrote:
> The links were pointing to static non-existent location instead
> of internal doc link.
> 
> Signed-off-by: Roi Dayan 
> ---
>  MAINTAINERS.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MAINTAINERS.rst b/MAINTAINERS.rst
> index 28831ab..36c8d58 100644
> --- a/MAINTAINERS.rst
> +++ b/MAINTAINERS.rst
> @@ -29,10 +29,10 @@ Open vSwitch committers are the people who have
> been granted access to push
>  changes to to the Open vSwitch git repository.
>  
>  The responsibilities of an Open vSwitch committer are documented
> -`here `__.
> +:doc:`here `.

I don't think we can do this: these files are in the top-level and as
such are liable to be rendered on GitHub, which doesn't support Sphinx
directives like this. This is mentioned in the documentation guide [1].

We can do this, but be aware it will render funnily on GitHub.
Personally, I think updating the link would be easier.

>  The process for adding or removing committers is documented
> -`here `__.
> +:doc:`here `.

Ditto.

>  This is the current list of Open vSwitch committers:

Cheers,
Stephen

[1] http://docs.openvswitch.org/en/latest/internals/contributing/docume
ntation-style/
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto-dpif: Send QinQ related sFlow counters

[Eric Garver]

Hi Lukasz,

Thanks for expanding the 802.1ad support.
I'm not familiar with sFlow, so I'll provide what feedback I can.

On Tue, Apr 18, 2017 at 12:08:22PM +0100, Lukasz Rzasik wrote:
> This patch implements QinQ related sFlow counters.
> It is implemented according to sFlow Version 5,
> http://www.sflow.org/sflow_version_5.txt
> Open vSwitch will send a stack of stripped VLAN tags
> to sFlow collector if the original packets have multiple
> VLAN tags.
> 
> Unit tests have been updated accordingly.
> 
> The patch is based on commit f0fb825a37 (Add support
> for 802.1ad (QinQ tunneling))
> 
> Signed-off-by: Lukasz Rzasik 
> CC: Thomas F Herbert 
> CC: Xiao Liang 
> CC: Eric Garver 
> CC: Neil McKee 
> ---
>  lib/packets.h|  4 +++
>  ofproto/ofproto-dpif-sflow.c | 60 +---
>  ofproto/ofproto-dpif-sflow.h |  9 +
>  tests/ofproto-dpif.at| 83 
> 
>  tests/test-sflow.c   | 25 +
>  5 files changed, 177 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/packets.h b/lib/packets.h
> index 755f08d..7555dfc 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -384,6 +384,10 @@ static inline bool eth_type_vlan(ovs_be16 eth_type)
>  eth_type == htons(ETH_TYPE_VLAN_8021AD);
>  }
>  
> +static inline bool eth_type_8021Q(ovs_be16 eth_type)
> +{
> +return eth_type == htons(ETH_TYPE_VLAN_8021Q);
> +}
>  
>  /* Minimum value for an Ethernet type.  Values below this are IEEE 802.2 
> frame
>   * lengths. */
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index 59fafa1..41972e2 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -1084,6 +1084,31 @@ dpif_sflow_capture_input_mpls(const struct flow *flow,
>  }
>  }
>  
> +static void
> +dpif_sflow_pop_vlan(const struct flow *flow,
> +struct dpif_sflow_actions *sflow_actions)
> +{
> +union flow_vlan_hdr vlan = flow->vlans[0];
> +if (eth_type_vlan(vlan.tpid)) {
> +int depth = 0;
> +int ii;
> +/* Calculate depth by detecting 8021Q TPID. */
> +for (ii = 0; ii < FLOW_MAX_VLAN_HEADERS; ii++) {
> +vlan = flow->vlans[ii];
> +depth++;
> +if (eth_type_8021Q(vlan.tpid)) {
> +break;
> +}

This seems to be using 0x8100 as a marker to stop. Does this mean double
stacked 0x8100 will not work?

flow_count_vlan_headers() can be used to find the number of tags.

> +}
> +
> +if (depth > 1) {
> +sflow_actions->vlan_qtag[sflow_actions->vlan_stack_depth] =
> +ntohl(flow->vlans[sflow_actions->vlan_stack_depth].qtag);
> +sflow_actions->vlan_stack_depth++;
> +}

AFAICS, this only adds the outermost tag to the stack. Is that
intentional? It's unclear to me if sFlow expects one or two VLANs to be
in the stack.

> +}
> +}
> +
>  void
>  dpif_sflow_read_actions(const struct flow *flow,
>   const struct nlattr *actions, size_t actions_len,
> @@ -1170,11 +1195,10 @@ dpif_sflow_read_actions(const struct flow *flow,
>   break;
>  
>   case OVS_ACTION_ATTR_PUSH_VLAN:
> +break;
> +
>   case OVS_ACTION_ATTR_POP_VLAN:
> - /* TODO: 802.1AD(QinQ) is not supported by OVS (yet), so do not
> -  * construct a VLAN-stack. The sFlow user-action cookie already
> -  * captures the egress VLAN ID so there is nothing more to do here.
> -  */
> +dpif_sflow_pop_vlan(flow, sflow_actions);
>   break;
>  
>   case OVS_ACTION_ATTR_PUSH_MPLS: {
> @@ -1225,6 +1249,19 @@ dpif_sflow_encode_mpls_stack(SFLLabelStack *stack,
>  stack->stack[stack->depth - 1] |= MPLS_BOS_MASK;
>  }
>  
> +static void
> +dpif_sflow_encode_vlan_stack(SFLVlanStack *stack,
> + uint32_t *vlans_buf,
> + const struct dpif_sflow_actions *sflow_actions)
> +{
> +int ii;
> +stack->depth = sflow_actions->vlan_stack_depth;
> +stack->stack = vlans_buf;
> +for (ii = 0; ii < stack->depth; ii++) {
> +stack->stack[ii] = sflow_actions->vlan_qtag[ii];
> +}
> +}
> +
>  /* Extract the output port count from the user action cookie.
>   * See http://sflow.org/sflow_version_5.txt "Input/Output port information"
>   */
> @@ -1258,6 +1295,8 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
> dp_packet *packet,
>  SFLFlow_sample_element vniInElem, vniOutElem;
>  SFLFlow_sample_element mplsElem;
>  uint32_t mpls_lse_buf[FLOW_MAX_MPLS_LABELS];
> +SFLFlow_sample_element vlanTunnelElem;
> +uint32_t vlans_buf[FLOW_MAX_VLAN_HEADERS];
>  SFLSampler *sampler;
>  struct dpif_sflow_port *in_dsp;
>  struct dpif_sflow_port *out_dsp;
> @@ -1372,6 +1411,19 @@ 

[ovs-dev] [PATCH] rhel-systemd: start vswitchd after udev

[Aaron Conole]

It's possible to race with the udev service, such that dpdk ports are
not finished being bound until after ovs-vswitchd has been started.
This means that attempts to use the port will fail.  While it is
possible to work around this for some NICs using port hotplug, not all
port types are supported (for instance vfio), and it requires manual
intervention.

Fixes: 36af136b690c ("rhel-systemd: Delay shutting down the services")
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1397299
Suggested-by: Flavio Leitner 
Signed-off-by: Aaron Conole 
---
 rhel/usr_lib_systemd_system_ovs-vswitchd.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service 
b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
index 39627e9..22a4c63 100644
--- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service
+++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
@@ -1,6 +1,6 @@
 [Unit]
 Description=Open vSwitch Forwarding Unit
-After=ovsdb-server.service network-pre.target
+After=ovsdb-server.service network-pre.target systemd-udev-settle.service
 Before=network.target network.service
 Requires=ovsdb-server.service
 ReloadPropagatedFrom=ovsdb-server.service
-- 
2.9.3

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

Re: [ovs-dev] [PATCH] doc: Don't override default theme

[Matthew Thode via dev]

On 04/18/2017 05:26 AM, Stephen Finucane wrote:
> Sphinx 1.3 renamed the 'default' theme to 'classic' and configured the
> 'alabaster' theme as the new default. To prevent breaking existing
> builds, the 'default' name was reserved as an alias for 'classic' [1].
> However, initially this raised a warning [1] with a message to use
> 'classic' instead. This warning was removed in 1.3.2 [2], but it will
> result in errors (due to the use of the '-W' flag) for Sphinx 1.3.0 and
> 1.3.1 users.
> 
> Mitigate the issue by not setting a theme if the 'ovs_sphinx_theme'
> package is absent. This will result in Sphinx using its default theme,
> be that 'classic' (Sphinx < 1.3) or 'alabaster'.
> 
> [1] https://github.com/sphinx-doc/sphinx/commit/68021b0bd
> [2] https://github.com/sphinx-doc/sphinx/commit/034c4e942
> 
> Signed-off-by: Stephen Finucane 
> Cc: Matthew Thode 
> ---
> We might want to backport this if there are any releases with the Sphinx
> docs present.
> ---
>  Documentation/conf.py | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/Documentation/conf.py b/Documentation/conf.py
> index 49514ec..97f402e 100644
> --- a/Documentation/conf.py
> +++ b/Documentation/conf.py
> @@ -145,8 +145,6 @@ linkcheck_anchors = False
>  #
>  if use_ovs_theme:
>  html_theme = 'ovs'
> -else:
> -html_theme = 'default'
>  
>  # Theme options are theme-specific and customize the look and feel of a theme
>  # further.  For a list of options available for each theme, see the
> 
Thanks for this, looks good and keeps default behaviour by default
instead of hard coding a default.

-- 
Matthew Thode

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

[ovs-dev] [PATCH 07/10] match: Format match with packet_type (OFPHTN_ETHERTYPE, x)

[Zoltán Balogh]

From: Jan Scheurich 

Include the L3 match fields in the match string also when there
is no dl_type in the match, but the Ethertype encoded in the packet
type.

Signed-off-by: Jan Scheurich 
---
 lib/match.c | 40 ++--
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/lib/match.c b/lib/match.c
index 4855c74..eddbc7d 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1171,8 +1171,8 @@ match_format(const struct match *match, struct ds *s, int 
priority)
 size_t start_len = s->length;
 const struct flow *f = >flow;
 bool skip_type = false;
-
 bool skip_proto = false;
+ovs_be16 dl_type;
 
 int i;
 
@@ -1267,11 +1267,15 @@ match_format(const struct match *match, struct ds *s, 
int priority)
   pt_ns_type(f->packet_type),
   pt_ns_type(wc->masks.packet_type));
 }
+if (pt_ns(f->packet_type) == OFPHTN_ETHERTYPE) {
+dl_type = pt_ns_type_be(f->packet_type);
+}
 }
 
 if (wc->masks.dl_type) {
+dl_type = f->dl_type;
 skip_type = true;
-if (f->dl_type == htons(ETH_TYPE_IP)) {
+if (dl_type == htons(ETH_TYPE_IP)) {
 if (wc->masks.nw_proto) {
 skip_proto = true;
 if (f->nw_proto == IPPROTO_ICMP) {
@@ -1291,7 +1295,7 @@ match_format(const struct match *match, struct ds *s, int 
priority)
 } else {
 ds_put_format(s, "%sip%s,", colors.value, colors.end);
 }
-} else if (f->dl_type == htons(ETH_TYPE_IPV6)) {
+} else if (dl_type == htons(ETH_TYPE_IPV6)) {
 if (wc->masks.nw_proto) {
 skip_proto = true;
 if (f->nw_proto == IPPROTO_ICMPV6) {
@@ -1309,13 +1313,13 @@ match_format(const struct match *match, struct ds *s, 
int priority)
 } else {
 ds_put_format(s, "%sipv6%s,", colors.value, colors.end);
 }
-} else if (f->dl_type == htons(ETH_TYPE_ARP)) {
+} else if (dl_type == htons(ETH_TYPE_ARP)) {
 ds_put_format(s, "%sarp%s,", colors.value, colors.end);
-} else if (f->dl_type == htons(ETH_TYPE_RARP)) {
+} else if (dl_type == htons(ETH_TYPE_RARP)) {
 ds_put_format(s, "%srarp%s,", colors.value, colors.end);
-} else if (f->dl_type == htons(ETH_TYPE_MPLS)) {
+} else if (dl_type == htons(ETH_TYPE_MPLS)) {
 ds_put_format(s, "%smpls%s,", colors.value, colors.end);
-} else if (f->dl_type == htons(ETH_TYPE_MPLS_MCAST)) {
+} else if (dl_type == htons(ETH_TYPE_MPLS_MCAST)) {
 ds_put_format(s, "%smplsm%s,", colors.value, colors.end);
 } else {
 skip_type = false;
@@ -1387,9 +1391,9 @@ match_format(const struct match *match, struct ds *s, int 
priority)
 
 if (!skip_type && wc->masks.dl_type) {
 ds_put_format(s, "%sdl_type=%s0x%04"PRIx16",",
-  colors.param, colors.end, ntohs(f->dl_type));
+  colors.param, colors.end, ntohs(dl_type));
 }
-if (f->dl_type == htons(ETH_TYPE_IPV6)) {
+if (dl_type == htons(ETH_TYPE_IPV6)) {
 format_ipv6_netmask(s, "ipv6_src", >ipv6_src, >masks.ipv6_src);
 format_ipv6_netmask(s, "ipv6_dst", >ipv6_dst, >masks.ipv6_dst);
 if (wc->masks.ipv6_label) {
@@ -1403,8 +1407,8 @@ match_format(const struct match *match, struct ds *s, int 
priority)
   ntohl(wc->masks.ipv6_label));
 }
 }
-} else if (f->dl_type == htons(ETH_TYPE_ARP) ||
-   f->dl_type == htons(ETH_TYPE_RARP)) {
+} else if (dl_type == htons(ETH_TYPE_ARP) ||
+   dl_type == htons(ETH_TYPE_RARP)) {
 format_ip_netmask(s, "arp_spa", f->nw_src, wc->masks.nw_src);
 format_ip_netmask(s, "arp_tpa", f->nw_dst, wc->masks.nw_dst);
 } else {
@@ -1412,8 +1416,8 @@ match_format(const struct match *match, struct ds *s, int 
priority)
 format_ip_netmask(s, "nw_dst", f->nw_dst, wc->masks.nw_dst);
 }
 if (!skip_proto && wc->masks.nw_proto) {
-if (f->dl_type == htons(ETH_TYPE_ARP) ||
-f->dl_type == htons(ETH_TYPE_RARP)) {
+if (dl_type == htons(ETH_TYPE_ARP) ||
+dl_type == htons(ETH_TYPE_RARP)) {
 ds_put_format(s, "%sarp_op=%s%"PRIu8",",
   colors.param, colors.end, f->nw_proto);
 } else {
@@ -1421,8 +1425,8 @@ match_format(const struct match *match, struct ds *s, int 
priority)
   colors.param, colors.end, f->nw_proto);
 }
 }
-if (f->dl_type == htons(ETH_TYPE_ARP) ||
-f->dl_type == htons(ETH_TYPE_RARP)) {
+if (dl_type == htons(ETH_TYPE_ARP) ||
+dl_type == htons(ETH_TYPE_RARP)) {
 format_eth_masked(s, "arp_sha", f->arp_sha, wc->masks.arp_sha);
 format_eth_masked(s, 

Re: [ovs-dev] [PATCH 01/10] userspace: Add OXM field MFF_PACKET_TYPE

[Greg Rose]

On Tue, 2017-04-18 at 11:19 +, Zoltán Balogh wrote:
> From: Jan Scheurich 
> 
> Allow packet type namespace OFPHTN_ETHERTYPE as alternative pre-requisite
> for matching L3 protocols (MPLS, IP, IPv6, ARP etc).
> 
> Scan packet_type in odp_flow string.
> 
> Added dummy entry for MFF_PACKET_TYPE to lib/meta-flow.xml
> 
> Added packet_type to the matching properties in tests/ofproto.at. Should be
> removed later, when packet_type_aware bridge attribute will be introduced.
> 
> Signed-off-by: Jan Scheurich 
> ---
>  include/openvswitch/meta-flow.h | 16 +-
>  lib/flow.h  | 26 --
>  lib/meta-flow.c | 30 --
>  lib/meta-flow.xml   |  4 
>  lib/nx-match.c  | 16 ++
>  lib/odp-util.c  | 48 
> ++---
>  lib/ofp-parse.c |  6 ++
>  lib/ofp-util.c  | 42 +---
>  tests/dpif-netdev.at| 14 ++--
>  tests/odp.at|  1 +
>  tests/ofproto-dpif.at   | 20 -
>  tests/ofproto.at|  1 +
>  tests/pmd.at|  2 +-
>  tests/tunnel-push-pop-ipv6.at   |  2 +-
>  tests/tunnel-push-pop.at|  2 +-
>  15 files changed, 177 insertions(+), 53 deletions(-)
> 
> diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
> index c284ec6..172cc3b 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -247,6 +247,20 @@ enum OVS_PACKED_ENUM mf_field_id {
>   */
>  MFF_RECIRC_ID,
>  
> +/* "packet_type".
> + *
> + * Define the packet type in OpenFlow 1.3+.
> + *
> + * Type: be32.
> + * Maskable: no.
> + * Formatting: hexadecimal.
> + * Prerequisites: none.
> + * Access: read-only.
> + * NXM: none.
> + * OXM: OXM_OF_PACKET_TYPE(44) since OF1.3 and v2.7.
> + */
> +MFF_PACKET_TYPE,
> +
>  /* "conj_id".
>   *
>   * ID for "conjunction" actions.  Please refer to ovs-ofctl(8)
> @@ -589,7 +603,7 @@ enum OVS_PACKED_ENUM mf_field_id {
>   */
>  MFF_METADATA,
>  
> -/* "in_port".
> + /* "in_port".

Why the formatting change in the middle of the patch?  And even in that
case it looks off a bit.

- Greg

>   *
>   * 16-bit (OpenFlow 1.0) view of the physical or virtual port on which 
> the
>   * packet was received.
> diff --git a/lib/flow.h b/lib/flow.h
> index a8772c5..fcc7d13 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -959,9 +959,23 @@ static inline bool is_ethernet(const struct flow *flow,
>  return flow->packet_type == htonl(PT_ETH);
>  }
>  
> +static inline ovs_be16 get_dl_type(const struct flow *flow)
> +{
> +if (flow->packet_type == htonl(PT_ETH)) {
> +return flow->dl_type;
> +} else if (pt_ns(flow->packet_type) == OFPHTN_ETHERTYPE) {
> +return pt_ns_type_be(flow->packet_type);
> +} else {
> +return FLOW_DL_TYPE_NONE;
> +}
> +}
> +
>  static inline bool is_vlan(const struct flow *flow,
> struct flow_wildcards *wc)
>  {
> +if (!is_ethernet(flow, wc)) {
> +return false;
> +}
>  if (wc) {
>  WC_MASK_FIELD_MASK(wc, vlans[0].tci, htons(VLAN_CFI));
>  }
> @@ -970,7 +984,7 @@ static inline bool is_vlan(const struct flow *flow,
>  
>  static inline bool is_ip_any(const struct flow *flow)
>  {
> -return dl_type_is_ip_any(flow->dl_type);
> +return dl_type_is_ip_any(get_dl_type(flow));
>  }
>  
>  static inline bool is_ip_proto(const struct flow *flow, uint8_t ip_proto,
> @@ -1006,7 +1020,7 @@ static inline bool is_sctp(const struct flow *flow,
>  static inline bool is_icmpv4(const struct flow *flow,
>   struct flow_wildcards *wc)
>  {
> -if (flow->dl_type == htons(ETH_TYPE_IP)) {
> +if (get_dl_type(flow) == htons(ETH_TYPE_IP)) {
>  if (wc) {
>  memset(>masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
>  }
> @@ -1018,7 +1032,7 @@ static inline bool is_icmpv4(const struct flow *flow,
>  static inline bool is_icmpv6(const struct flow *flow,
>   struct flow_wildcards *wc)
>  {
> -if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
> +if (get_dl_type(flow) == htons(ETH_TYPE_IPV6)) {
>  if (wc) {
>  memset(>masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
>  }
> @@ -1049,7 +1063,7 @@ static inline bool is_nd(const struct flow *flow,
>  
>  static inline bool is_igmp(const struct flow *flow, struct flow_wildcards 
> *wc)
>  {
> -if (flow->dl_type == htons(ETH_TYPE_IP)) {
> +if (get_dl_type(flow) == htons(ETH_TYPE_IP)) {
>  if (wc) {
>  memset(>masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
>  }
> @@ -1093,8 +1107,8 @@ static inline 

Re: [ovs-dev] [PATCH v3] netdev-dpdk: Implement Tx intermediate queue for dpdk ports.

[Eelco Chaudron]

Hi Bhanuprakash,

I was doing some Physical to Virtual tests, and whenever the number of flows
reaches the rx batch size performance dropped a lot. I created an 
experimental

patch where I added an intermediate queue and flush it at the end of the
rx batch.

When I found your patch I decided to give it try to see how it behaves.
I also modified you patch in such a way that it will flush the queue
after every call to dp_netdev_process_rxq_port().

Here are some pkt forwarding stats for the Physical to Physical scenario,
for two 82599ES 10G port with 64 byte packets being send at wire speed:

Number  plainpatch +
of flows  git clonepatch  flush
  =  =  =
  10   10727283   13527752   13393844
  327042253   11285572 11228799
  5075154919642650  9607791
 10058386999461239  9430730
 50052850667859123  7845807
100052264777146404  7135601


I do not have an IXIA to do the latency tests you performed, however I
do have a XENA tester which has a basic latency measurement feature.
I used the following script to get the latency numbers:

https://github.com/chaudron/XenaPythonLib/blob/latency/examples/latency.py


As you can see in the numbers below, the default queue introduces quite
some latency, however doing the flush every rx batch brings the latency
down to almost the original values. The results mimics your test case 2,
sending 10G traffic @ wire speed:

  = GIT CLONE
  Pkt size  min(ns)  avg(ns)  max(ns)
   512  4,631  5,022309,914
  1024  5,545  5,749104,294
  1280  5,978  6,159 45,306
  1518  6,419  6,774946,850

  = PATCH
  Pkt size  min(ns)  avg(ns)  max(ns)
   512  4,928492,228  1,995,026
  1024  5,761499,206  2,006,628
  1280  6,186497,975  1,986,175
  1518  6,579494,434  2,005,947

  = PATCH + FLUSH
  Pkt size  min(ns)  avg(ns)  max(ns)
   512  4,711  5,064182,477
  1024  5,601  5,888701,654
  1280  6,018  6,491533,037
  1518  6,467  6,734312,471

Maybe it will be good to re-run your latency tests with the flush for 
every rx

batch. This might get ride of your huge latency while still increasing the
performance in the case the rx batch shares the same egress port.

The overall patchset looks fine to me, see some comments inline.

Cheers,

Eelco


On 02/02/17 23:14, Bhanuprakash Bodireddy wrote:

After packet classification, packets are queued in to batches depending
on the matching netdev flow. Thereafter each batch is processed to
execute the related actions. This becomes particularly inefficient if
there are few packets in each batch as rte_eth_tx_burst() incurs expensive
MMIO writes.

This commit adds back intermediate queue implementation. Packets are
queued and burst when the packet count exceeds threshold. Also drain
logic is refactored to handle packets hanging in the tx queues. Testing
shows significant performance gains with this implementation.

Fixes: b59cc14e032d("netdev-dpdk: Use instant sending instead of
queueing of packets.")
Signed-off-by: Bhanuprakash Bodireddy >
Signed-off-by: Antonio Fischetti 
Co-authored-by: Antonio Fischetti 
Signed-off-by: Markus Magnusson 
Co-authored-by: Markus Magnusson 
---
v2->v3
   * Refactor the code
   * Use thread local copy 'send_port_cache' instead of 'tx_port' while draining
   * Invoke dp_netdev_drain_txq_port() to drain the packets from the queue as
 part of pmd reconfiguration that gets triggered due to port 
addition/deletion
 or change in pmd-cpu-mask.
   * Invoke netdev_txq_drain() from xps_get_tx_qid() to drain packets in old
 queue. This is possible in XPS case where the tx queue can change after
 timeout.
   * Fix another bug in netdev_dpdk_eth_tx_burst() w.r.t 'txq->count'.

Latency stats:
Collected the latency stats with PHY2PHY loopback case using 30 IXIA streams
/UDP packets/uni direction traffic. All the stats are in nanoseconds. Results
below compare latency results between Master vs patch.

case 1: Matching IP flow rules for each IXIA stream
 Eg:  For an IXIA stream with src Ip: 2.2.2.1, dst tip: 5.5.5.1
 ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=2.2.2.1,actions=output:2

 For an IXIA stream with src Ip: 4.4.4.1, dst tip: 15.15.15.1
 ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=4.4.4.1,actions=output:2

Packet64 128 256 512
Branch  Master  Patch   Master  Patch   Master  Patch   Master  Patch   
case 1  26100   222000  26190   217930  23890   199000  30370   212440 (min 
latency ns)
 1239100 906910  1168740 691040  575470  574240  724360  734050 (max 
latency ns)
 1189501763908  913602  

Re: [ovs-dev] [PATCH ovs V7 02/24] netdev: Adding a new netdev api to be used for offloading flows

[Roi Dayan]

On 14/04/2017 04:11, Joe Stringer wrote:

On 7 April 2017 at 06:12, Roi Dayan  wrote:

From: Paul Blakey 

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
Reviewed-by: Simon Horman 
---





diff --git a/lib/netdev.h b/lib/netdev.h
index d6c07c1..6d2db7d 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -156,6 +156,29 @@ int netdev_send(struct netdev *, int qid, struct 
dp_packet_batch *,
 bool may_steal, bool concurrent_txq);
 void netdev_send_wait(struct netdev *, int qid);

+/* Flow offloading. */
+struct offload_info {
+const void *port_hmap_obj; /* To query ports info from netdev port map */
+ovs_be16 tp_dst_port; /* Destination port for tunnel in SET action */


Is this assuming there is only ever one tunnel destination port? What
about multiple output?



Yes. we currently only support single dst port output. if there is more 
than 1 we fail with EOPNOTSUPP and fallback to OVS datapath.

the check is done in dpif-netlink.c parse_flow_put()

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

[ovs-dev] [PATCH ovs] Documentation: fix broken links in maintainers page

[Roi Dayan]

The links were pointing to static non-existent location instead
of internal doc link.

Signed-off-by: Roi Dayan 
---
 MAINTAINERS.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS.rst b/MAINTAINERS.rst
index 28831ab..36c8d58 100644
--- a/MAINTAINERS.rst
+++ b/MAINTAINERS.rst
@@ -29,10 +29,10 @@ Open vSwitch committers are the people who have been 
granted access to push
 changes to to the Open vSwitch git repository.
 
 The responsibilities of an Open vSwitch committer are documented
-`here `__.
+:doc:`here `.
 
 The process for adding or removing committers is documented
-`here `__.
+:doc:`here `.
 
 This is the current list of Open vSwitch committers:
 
-- 
2.7.4

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

Re: [ovs-dev] [ovs-discuss] ovn: unsnat handling error for Distributed Gateway

[Guoshuai Li]

Sorry, This patch "git am" error, Please use this:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331034.html





On Sun, Apr 9, 2017 at 3:23 PM, Mickey Spiegel > wrote:




On Thu, Apr 6, 2017 at 7:34 AM, Guoshuai Li > wrote:


revese my topology:

 +-++
 |  VM 172.16.1.7  |
 +-++
   |
 +-++
 |  Logical Switch  |
 +-++
 |172.16.1.254
  10.157.142.3 +---++
  ++  Logical Router 1  +
  | ++
+-++
|  Logical Switch  |
+--+
  | ++
  ++  Logical Router 2  |
  10.157.142.1 ++


Hi All, I am having a problem for ovn and need help, thanks.


I created two logical routes and connected the two
LogicalRoutes through a external LogicalSwitch (connected
to the external network) .

And then LogicalRoute-1 connected to the VM through the
internal LogicalSwitch .

my topology:

  10.157.142.3 172.16.1.254
 ++ +-++
 +-++
  ++  Logical Router 1
+--|  Logical Switch 
+---+ VM 172.16.1.7   |

  | ++ +--+
 +--+
+-++
|  Logical Switch  |
+--+
  | ++
  ++  Logical Router 2  |
 ++
  10.157.142.1

I tested the master and Branch2.7, it Can not be
transferred from VM (172.16.1.7) to LogicaRouter-2 's
port (10.157.142.

Sorry, The destination address is 10.157.142.1, And The
SNAT/unSNAT address is 10.157.142.3.

) via ping.
My logical router is a distributed gateway, and the two
logical router ports that connect external LogicalSwitch
are on the same chassis.
If the two logical router ports are not on the same
chassis ping is also OK, And ping from VM (172.16.1.7) to
external network is also OK.

I looked at the openflow tables on gateway chassis,  I
suspected unsnat handling error in Router1 input for icmp
replay.
I think it is necessary to replace the destination
address 10.157.142.3 with 172.16.1.7 in Table 19 and
route 172.16.1.7 in Table 21, but now the route match is
10.157.142.0/24 .

cookie=0x92bd0055, duration=68.468s, table=16,
n_packets=1, n_bytes=98, idle_age=36,
priority=50,reg14=0x4,metadata=0x7,dl_dst=fa:16:3e:58:1c:8a
actions=resubmit(,17)
cookie=0x45765344, duration=68.467s, table=17,
n_packets=1, n_bytes=98, idle_age=36,
priority=0,metadata=0x7 actions=resubmit(,18)
cookie=0xaeaaed29, duration=68.479s, table=18,
n_packets=1, n_bytes=98, idle_age=36,
priority=0,metadata=0x7 actions=resubmit(,19)
cookie=0xce785d51, duration=68.479s, table=19,
n_packets=1, n_bytes=98, idle_age=36,
priority=100,ip,reg14=0x4,metadata=0x7,nw_dst=10.157.142.3
actions=ct(table=20,zone=NXM_NX_REG12[0..15],nat)
cookie=0xbd994421, duration=68.481s, table=20,
n_packets=1, n_bytes=98, idle_age=36,
priority=0,metadata=0x7 actions=resubmit(,21)
cookie=0xaea3a6ae, duration=68.479s, table=21,
n_packets=1, n_bytes=98, idle_age=36,
priority=49,ip,metadata=0x7,nw_dst=10.157.142.0/24


actions=dec_ttl(),move:NXM_OF_IP_DST[]->NXM_NX_XXREG0[96..127],load:0xa9d8e03->NXM_NX_XXREG0[64..95],mod_dl_src:fa:16:3e:58:1c:8a,load:0x4->NXM_NX_REG15[],load:0x1->NXM_NX_REG10[0],resubmit(,22)
cookie=0xce6e8d4e, duration=68.482s, table=22,
n_packets=1, n_bytes=98, idle_age=36,
priority=0,ip,metadata=0x7

actions=push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],pop:NXM_NX_REG0[],mod_dl_dst:00:00:00:00:00:00,resubmit(,66),pop:NXM_NX_REG0[],resubmit(,23)
cookie=0xce89c4ed, duration=68.481s, table=23,

[ovs-dev] [PATCH] system-ovn.at: Add test for ping other router's port on distributed router

[Guoshuai Li]

Signed-off-by: Guoshuai Li 
---
 tests/system-ovn.at | 101 
 tests/system-traffic.at |  20 ++
 2 files changed, 121 insertions(+)

diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index dd62bd1..68da38a 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -1396,3 +1396,104 @@ as
 OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
 /connection dropped.*/d"])
 AT_CLEANUP
+
+AT_SETUP([ovn -- ping other router port on distributed router])
+AT_KEYWORDS([ovnnat])
+
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+-- set Open_vSwitch . external-ids:system-id=hv1 \
+-- set Open_vSwitch . 
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+-- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+-- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+-- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+# Logical network:
+# Two LRs - R1 and R2 that are connected to LS "join" (20.0.0.0/24).
+# R1 has switchess foo (192.168.1.0/24).
+# The port between R1/R2 and "join" is the router gateway port where
+# the NAT rules are applied.
+#
+#foo -- R1 -- join -- R2
+#
+
+ovn-nbctl lr-add R1
+ovn-nbctl lr-add R2
+
+ovn-nbctl ls-add foo
+ovn-nbctl ls-add join
+
+ovn-nbctl lrp-add R1 foo 00:00:01:01:02:01 192.168.1.1/24
+ovn-nbctl lrp-add R1 join1 00:00:02:01:02:01 20.0.0.1/24 \
+-- set Logical_Router_Port join1 options:redirect-chassis=hv1
+ovn-nbctl lrp-add R2 join2 00:00:02:01:02:02 20.0.0.2/24 \
+-- set Logical_Router_Port join2 options:redirect-chassis=hv1
+
+# Connect foo to R1
+ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo \
+type=router options:router-port=foo \
+-- lsp-set-addresses rp-foo router
+
+# Connect join to R1
+ovn-nbctl lsp-add join rp-join1 -- set Logical_Switch_Port rp-join1 \
+type=router options:router-port=join1 \
+-- lsp-set-addresses rp-join1 router
+
+# Connect join to R2
+ovn-nbctl lsp-add join rp-join2 -- set Logical_Switch_Port rp-join2 \
+type=router options:router-port=join2 \
+-- lsp-set-addresses rp-join2 router
+
+# Logical port 'foo1' in switch 'foo'.
+ADD_NAMESPACES(foo1)
+ADD_VETH(foo1, foo1, br-int, "192.168.1.2/24", "f0:00:00:01:02:01", \
+ "192.168.1.1")
+ovn-nbctl lsp-add foo foo1 \
+-- lsp-set-addresses foo1 "f0:00:00:01:02:01 192.168.1.2"
+
+# Add SNAT rule
+ovn-nbctl lr-nat-add R1 snat 20.0.0.1 192.168.1.0/24
+
+ovn-nbctl --wait=hv sync
+
+echo "-- hv dump --"
+ovs-ofctl show br-int
+ovs-ofctl dump-flows br-int
+echo "-"
+
+# East-West No NAT: 'foo1' pings 'R2' using 20.0.0.2
+NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 20.0.0.2 | FORMAT_PING], \
+[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+# We verify that SNAT indeed happened via 'dump-conntrack' command.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.1.2) | \
+sed -e 's/zone=[[0-9]]*/zone=/'], [0], [dnl
+icmp,orig=(src=192.168.1.2,dst=20.0.0.2,id=,type=8,code=0),reply=(src=20.0.0.2,dst=20.0.0.1,id=,type=0,code=0),zone=
+])
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index c042773..295e606 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3678,3 +3678,23 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 1 -w 3 10.4.2.2], 
[1], [ignore])
 
 OVS_TRAFFIC_VSWITCHD_STOP(["/dropping VLAN \(0\|300\) packet received on 
dot1q-tunnel port/d"])
 AT_CLEANUP
+
+AT_SETUP([datapath - SNAT and UNSNAT])
+OVS_TRAFFIC_VSWITCHD_START()
+
+AT_CHECK([ovs-ofctl add-flow br0 "table=0, 
priority=100,in_port=1,ip,nw_dst=20.0.0.2 
actions=dec_ttl(),mod_dl_src:00:00:02:01:02:01,mod_dl_dst:00:00:02:01:02:02,resubmit(,1)"])
+AT_CHECK([ovs-ofctl add-flow br0 "table=1, priority=100,ip,nw_src=192.168.1.2 
actions=ct(commit,table=2,zone=6,nat(src=20.0.0.1))"])
+AT_CHECK([ovs-ofctl add-flow br0 "table=2, 
priority=100,icmp,nw_dst=20.0.0.2,icmp_type=8,icmp_code=0 
actions=push:NXM_OF_IP_SRC[],push:NXM_OF_IP_DST[],pop:NXM_OF_IP_SRC[],pop:NXM_OF_IP_DST[],load:0xff->NXM_NX_IP_TTL[],load:0->NXM_OF_ICMP_TYPE[],dec_ttl(),mod_dl_src:00:00:02:01:02:02,mod_dl_dst:00:00:02:01:02:01,resubmit(,3)"])
+AT_CHECK([ovs-ofctl add-flow br0 "table=3, priority=100,ip,nw_dst=20.0.0.1 
actions=ct(table=4,zone=6,nat)"])
+AT_CHECK([ovs-ofctl add-flow br0 "table=4, priority=100,ip,nw_dst=192.168.1.2 

Re: [ovs-dev] [ovs-discuss] ovn: unsnat handling error for Distributed Gateway

[Guoshuai Li]

On Sun, Apr 9, 2017 at 3:23 PM, Mickey Spiegel > wrote:




On Thu, Apr 6, 2017 at 7:34 AM, Guoshuai Li > wrote:


revese my topology:

 +-++
 |  VM 172.16.1.7  |
 +-++
   |
 +-++
 |  Logical Switch  |
 +-++
 |172.16.1.254
  10.157.142.3 +---++
  ++  Logical Router 1  +
  | ++
+-++
|  Logical Switch  |
+--+
  | ++
  ++  Logical Router 2  |
  10.157.142.1 ++


Hi All, I am having a problem for ovn and need help, thanks.


I created two logical routes and connected the two
LogicalRoutes through a external LogicalSwitch (connected
to the external network) .

And then LogicalRoute-1 connected to the VM through the
internal LogicalSwitch .

my topology:

  10.157.142.3   172.16.1.254
 ++ +-++
 +-++
  ++  Logical Router 1
+--|  Logical Switch 
+---+ VM 172.16.1.7   |

  | ++ +--+
 +--+
+-++
|  Logical Switch  |
+--+
  | ++
  ++  Logical Router 2  |
 ++
  10.157.142.1

I tested the master and Branch2.7, it Can not be
transferred from VM (172.16.1.7) to LogicaRouter-2 's port
(10.157.142.

Sorry, The destination address is 10.157.142.1, And The
SNAT/unSNAT address is 10.157.142.3.

) via ping.
My logical router is a distributed gateway, and the two
logical router ports that connect external LogicalSwitch
are on the same chassis.
If the two logical router ports are not on the same
chassis ping is also OK, And ping from VM (172.16.1.7) to
external network is also OK.

I looked at the openflow tables on gateway chassis,  I
suspected unsnat handling error in Router1 input for icmp
replay.
I think it is necessary to replace the destination address
10.157.142.3 with 172.16.1.7 in Table 19 and route
172.16.1.7 in Table 21, but now the route match is
10.157.142.0/24 .

cookie=0x92bd0055, duration=68.468s, table=16,
n_packets=1, n_bytes=98, idle_age=36,
priority=50,reg14=0x4,metadata=0x7,dl_dst=fa:16:3e:58:1c:8a
actions=resubmit(,17)
cookie=0x45765344, duration=68.467s, table=17,
n_packets=1, n_bytes=98, idle_age=36,
priority=0,metadata=0x7 actions=resubmit(,18)
cookie=0xaeaaed29, duration=68.479s, table=18,
n_packets=1, n_bytes=98, idle_age=36,
priority=0,metadata=0x7 actions=resubmit(,19)
cookie=0xce785d51, duration=68.479s, table=19,
n_packets=1, n_bytes=98, idle_age=36,
priority=100,ip,reg14=0x4,metadata=0x7,nw_dst=10.157.142.3
actions=ct(table=20,zone=NXM_NX_REG12[0..15],nat)
cookie=0xbd994421, duration=68.481s, table=20,
n_packets=1, n_bytes=98, idle_age=36,
priority=0,metadata=0x7 actions=resubmit(,21)
cookie=0xaea3a6ae, duration=68.479s, table=21,
n_packets=1, n_bytes=98, idle_age=36,
priority=49,ip,metadata=0x7,nw_dst=10.157.142.0/24


actions=dec_ttl(),move:NXM_OF_IP_DST[]->NXM_NX_XXREG0[96..127],load:0xa9d8e03->NXM_NX_XXREG0[64..95],mod_dl_src:fa:16:3e:58:1c:8a,load:0x4->NXM_NX_REG15[],load:0x1->NXM_NX_REG10[0],resubmit(,22)
cookie=0xce6e8d4e, duration=68.482s, table=22,
n_packets=1, n_bytes=98, idle_age=36,
priority=0,ip,metadata=0x7

actions=push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],pop:NXM_NX_REG0[],mod_dl_dst:00:00:00:00:00:00,resubmit(,66),pop:NXM_NX_REG0[],resubmit(,23)
cookie=0xce89c4ed, duration=68.481s, table=23,
n_packets=1, n_bytes=98, idle_age=36,
priority=150,reg15=0x4,metadata=0x7,dl_dst=00:00:00:00:00:00

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

[Zoltán Balogh]

Partly based on Jean Tourrilhes's work.

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

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

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

[ovs-dev] [PATCH 06/10] ofproto: Add default match on packet_type Ethernet

[Zoltán Balogh]

From: Jan Scheurich 

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

Signed-off-by: Jan Scheurich 
---
 ofproto/ofproto.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c21a6e1..31e64b6 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -4709,6 +4709,7 @@ add_flow_init(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm,
 OVS_EXCLUDED(ofproto_mutex)
 {
 struct oftable *table;
+struct match match = fm->match;
 struct cls_rule cr;
 uint8_t table_id;
 enum ofperr error;
@@ -4749,8 +4750,16 @@ add_flow_init(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm,
 return OFPERR_OFPBRC_EPERM;
 }
 
+if (ofproto->packet_type_aware) {
+if (!match.wc.masks.packet_type) {
+/* Add default match on packet_type Ethernet.*/
+match.flow.packet_type = htonl(PT_ETH);
+match.wc.masks.packet_type = OVS_BE32_MAX;
+}
+}
+
 if (!ofm->temp_rule) {
-cls_rule_init(, >match, fm->priority);
+cls_rule_init(, , fm->priority);
 
 /* Allocate new rule.  Destroys 'cr'. */
 error = ofproto_rule_create(ofproto, , table - ofproto->tables,
-- 
2.7.4

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

[ovs-dev] [PATCH 04/10] userspace: Handling of versatile tunnel ports

[Zoltán Balogh]

From: Jan Scheurich 

Not yet fully functional in this commit.

Signed-off-by: Jan Scheurich 
---
 ofproto/ofproto-dpif-xlate.c |  2 +-
 ofproto/tunnel.c | 21 -
 ofproto/tunnel.h |  2 +-
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 6dd8a07..5039758 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -6162,7 +6162,7 @@ xlate_wc_init(struct xlate_ctx *ctx)
 netflow_mask_wc(>xin->flow, ctx->wc);
 }
 
-tnl_wc_init(>xin->flow, ctx->wc);
+tnl_wc_init(>xin->flow, ctx->wc, ctx->xbridge->packet_type_aware);
 }
 
 static void
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 0da4f42..0aca81f 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -361,7 +361,8 @@ tnl_process_ecn(struct flow *flow)
 }
 
 void
-tnl_wc_init(struct flow *flow, struct flow_wildcards *wc)
+tnl_wc_init(struct flow *flow, struct flow_wildcards *wc,
+bool packet_type_aware)
 {
 if (tnl_port_should_receive(flow)) {
 wc->masks.tunnel.tun_id = OVS_BE64_MAX;
@@ -386,8 +387,10 @@ tnl_wc_init(struct flow *flow, struct flow_wildcards *wc)
 && IP_ECN_is_ce(flow->tunnel.ip_tos)) {
 wc->masks.nw_tos |= IP_ECN_MASK;
 }
-/* Match on packet_type for tunneled packets.*/
-wc->masks.packet_type = OVS_BE32_MAX;
+if (!packet_type_aware) {
+/* Match on packet_type for tunneled packets.*/
+wc->masks.packet_type = OVS_BE32_MAX;
+}
 }
 }
 
@@ -566,8 +569,16 @@ tnl_find(const struct flow *flow) OVS_REQ_RDLOCK(rwlock)
 match.in_key_flow = in_key_flow;
 match.ip_dst_flow = ip_dst_flow;
 match.ip_src_flow = ip_src == IP_SRC_FLOW;
-match.is_layer3 = flow->packet_type != htonl(PT_ETH);
-
+if (pt_ns(flow->packet_type) == OFPHTN_ETHERTYPE) {
+/* Try to find a layer3 port first. */
+match.is_layer3 = true;
+tnl_port = tnl_find_exact(, map);
+if (tnl_port) {
+return tnl_port;
+}
+}
+/* Check for a non-layer3 or versatile tunnel port. */
+match.is_layer3 = false;
 tnl_port = tnl_find_exact(, map);
 if (tnl_port) {
 return tnl_port;
diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
index b0ec67c..dc004cc 100644
--- a/ofproto/tunnel.h
+++ b/ofproto/tunnel.h
@@ -39,7 +39,7 @@ int tnl_port_add(const struct ofport_dpif *, const struct 
netdev *,
 void tnl_port_del(const struct ofport_dpif *);
 
 const struct ofport_dpif *tnl_port_receive(const struct flow *);
-void tnl_wc_init(struct flow *, struct flow_wildcards *);
+void tnl_wc_init(struct flow *, struct flow_wildcards *, bool);
 bool tnl_process_ecn(struct flow *);
 odp_port_t tnl_port_send(const struct ofport_dpif *, struct flow *,
  struct flow_wildcards *wc);
-- 
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 05/10] netdev-native-tnl: Selection of gre protocol based on packet_type

[Zoltán Balogh]

From: Jan Scheurich 

In netdev_gre_build_header(), GRE protocol is set based on packet_type of flow.
If it's about an Ethernet packet, it is set to ETP_TYPE_TEB. Otherwise, if the 
name space is OFPHTN_ETHERNET, it is set according to the name space type.

Signed-off-by: Jan Scheurich 
---
 lib/netdev-native-tnl.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 2d947c2..76dd07e 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -462,10 +462,12 @@ netdev_gre_build_header(const struct netdev *netdev,
 
 greh = netdev_tnl_ip_build_header(data, params, IPPROTO_GRE);
 
-if (tnl_cfg->is_layer3) {
-greh->protocol = params->flow->dl_type;
-} else {
+if (params->flow->packet_type == htonl(PT_ETH)) {
 greh->protocol = htons(ETH_TYPE_TEB);
+} else if (pt_ns(params->flow->packet_type) == OFPHTN_ETHERTYPE) {
+greh->protocol = pt_ns_type_be(params->flow->packet_type);
+} else {
+return 1;
 }
 greh->flags = 0;
 
-- 
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 03/10] userspace: Filter field packet_type in table_features

[Zoltán Balogh]

From: Jan Scheurich 

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

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

Signed-off-by: Jan Scheurich 
---
 lib/nx-match.c| 10 --
 lib/nx-match.h|  4 ++--
 ofproto/ofproto.c |  4 ++--
 tests/ofproto.at  |  1 -
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/lib/nx-match.c b/lib/nx-match.c
index af19fb2..f9d8565 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -2071,12 +2071,15 @@ oxm_writable_fields(void)
 /* Returns a bitmap of fields that can be encoded in OXM and that can be
  * matched in a flow table.  */
 struct mf_bitmap
-oxm_matchable_fields(void)
+oxm_matchable_fields(bool packet_type_aware)
 {
 struct mf_bitmap b = MF_BITMAP_INITIALIZER;
 int i;
 
 for (i = 0; i < MFF_N_IDS; i++) {
+if (i == MFF_PACKET_TYPE && !packet_type_aware) {
+continue;
+}
 if (mf_oxm_header(i, 0)) {
 bitmap_set1(b.bm, i);
 }
@@ -2087,12 +2090,15 @@ oxm_matchable_fields(void)
 /* Returns a bitmap of fields that can be encoded in OXM and that can be
  * matched in a flow table with an arbitrary bitmask.  */
 struct mf_bitmap
-oxm_maskable_fields(void)
+oxm_maskable_fields(bool packet_type_aware)
 {
 struct mf_bitmap b = MF_BITMAP_INITIALIZER;
 int i;
 
 for (i = 0; i < MFF_N_IDS; i++) {
+if (i == MFF_PACKET_TYPE && !packet_type_aware) {
+continue;
+}
 if (mf_oxm_header(i, 0) && mf_from_id(i)->maskable == MFM_FULLY) {
 bitmap_set1(b.bm, i);
 }
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 6afb310..f75ce10 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -147,8 +147,8 @@ ovs_be64 oxm_bitmap_from_mf_bitmap(const struct mf_bitmap 
*, enum ofp_version);
 struct mf_bitmap oxm_bitmap_to_mf_bitmap(ovs_be64 oxm_bitmap,
  enum ofp_version);
 struct mf_bitmap oxm_writable_fields(void);
-struct mf_bitmap oxm_matchable_fields(void);
-struct mf_bitmap oxm_maskable_fields(void);
+struct mf_bitmap oxm_matchable_fields(bool packet_type_aware);
+struct mf_bitmap oxm_maskable_fields(bool packet_type_aware);
 
 /* Dealing with the 'ofs_nbits' members in several Nicira extensions. */
 
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 81fe02a..c21a6e1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3213,8 +3213,8 @@ query_tables(struct ofproto *ofproto,
  struct ofputil_table_stats **statsp)
 {
 struct mf_bitmap rw_fields = oxm_writable_fields();
-struct mf_bitmap match = oxm_matchable_fields();
-struct mf_bitmap mask = oxm_maskable_fields();
+struct mf_bitmap match = oxm_matchable_fields(ofproto->packet_type_aware);
+struct mf_bitmap mask = oxm_maskable_fields(ofproto->packet_type_aware);
 
 struct ofputil_table_features *features;
 struct ofputil_table_stats *stats;
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 66ee695..5c0d076 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2390,7 +2390,6 @@ metadata in_port in_port_oxm pkt_mark ct_mark ct_label 
reg0 reg1 reg2 reg3 reg4
 matching:
   dp_hash: arbitrary mask
   recirc_id: exact match or wildcard
-  packet_type: exact match or wildcard
   conj_id: exact match or wildcard
   tun_id: arbitrary mask
   tun_src: arbitrary mask
-- 
2.7.4

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

[ovs-dev] [PATCH 02/10] userspace: Add bridge property 'packet-type-aware'

[Zoltán Balogh]

From: Jan Scheurich 

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

Signed-off-by: Jan Scheurich 
---
 ofproto/ofproto-dpif-xlate.c | 33 +
 ofproto/ofproto-dpif-xlate.h |  1 +
 ofproto/ofproto-dpif.c   |  1 +
 ofproto/ofproto-provider.h   |  1 +
 ofproto/ofproto.c|  6 ++
 ofproto/ofproto.h|  1 +
 vswitchd/bridge.c|  9 +
 vswitchd/vswitch.xml |  8 
 8 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9a5a878..6dd8a07 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -108,6 +108,7 @@ struct xbridge {
 
 bool has_in_band; /* Bridge has in band control? */
 bool forward_bpdu;/* Bridge forwards STP BPDUs? */
+bool packet_type_aware;   /* Bridge is packet-type-aware? */
 
 /* Datapath feature support. */
 struct dpif_backer_support support;
@@ -553,6 +554,7 @@ static void xlate_xbridge_set(struct xbridge *, struct dpif 
*,
   const struct dpif_ipfix *,
   const struct netflow *,
   bool forward_bpdu, bool has_in_band,
+  bool packet_type_aware,
   const struct dpif_backer_support *);
 static void xlate_xbundle_set(struct xbundle *xbundle,
   enum port_vlan_mode vlan_mode,
@@ -813,6 +815,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
   const struct dpif_ipfix *ipfix,
   const struct netflow *netflow,
   bool forward_bpdu, bool has_in_band,
+  bool packet_type_aware,
   const struct dpif_backer_support *support)
 {
 if (xbridge->ml != ml) {
@@ -858,6 +861,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
 xbridge->dpif = dpif;
 xbridge->forward_bpdu = forward_bpdu;
 xbridge->has_in_band = has_in_band;
+xbridge->packet_type_aware = packet_type_aware;
 xbridge->support = *support;
 }
 
@@ -948,7 +952,7 @@ xlate_xbridge_copy(struct xbridge *xbridge)
   xbridge->rstp, xbridge->ms, xbridge->mbridge,
   xbridge->sflow, xbridge->ipfix, xbridge->netflow,
   xbridge->forward_bpdu, xbridge->has_in_band,
-  >support);
+  xbridge->packet_type_aware, >support);
 LIST_FOR_EACH (xbundle, list_node, >xbundles) {
 xlate_xbundle_copy(new_xbridge, xbundle);
 }
@@ -1100,6 +1104,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const 
char *name,
   const struct dpif_ipfix *ipfix,
   const struct netflow *netflow,
   bool forward_bpdu, bool has_in_band,
+  bool packet_type_aware,
   const struct dpif_backer_support *support)
 {
 struct xbridge *xbridge;
@@ -1118,7 +1123,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const 
char *name,
 xbridge->name = xstrdup(name);
 
 xlate_xbridge_set(xbridge, dpif, ml, stp, rstp, ms, mbridge, sflow, ipfix,
-  netflow, forward_bpdu, has_in_band, support);
+  netflow, forward_bpdu, has_in_band, packet_type_aware, 
support);
 }
 
 static void
@@ -3504,16 +3509,20 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
 memcpy(flow_vlans, flow->vlans, sizeof flow_vlans);
 flow_nw_tos = flow->nw_tos;
 
-if (flow->packet_type == htonl(PT_ETH) && xport->is_layer3 ) {
-/* Ethernet packet to L3 outport -> pop ethernet header. */
-flow->packet_type = htonl(PACKET_TYPE(OFPHTN_ETHERTYPE,
-  ntohs(flow->dl_type)));
-}
-else if (flow->packet_type != htonl(PT_ETH) && !xport->is_layer3) {
-/* L2 outport and non-ethernet packet_type -> add dummy eth header. */
-flow->packet_type = htonl(PT_ETH);
-flow->dl_dst = eth_addr_zero;
-flow->dl_src = eth_addr_zero;
+/* In a bridge that is not packet type-aware convert the packet to what
+ * the output port expects. */
+if (!ctx->xbridge->packet_type_aware) {
+if (flow->packet_type == htonl(PT_ETH) && xport->is_layer3 ) {
+/* Ethernet packet to L3 outport -> pop ethernet header. */
+flow->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,
+   ntohs(flow->dl_type));
+}
+else if (flow->packet_type != htonl(PT_ETH) && !xport->is_layer3) {
+/* L2 outport and non-ethernet packet_type -> add dummy eth 
header. */
+

[ovs-dev] [PATCH 01/10] userspace: Add OXM field MFF_PACKET_TYPE

[Zoltán Balogh]

From: Jan Scheurich 

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

Scan packet_type in odp_flow string.

Added dummy entry for MFF_PACKET_TYPE to lib/meta-flow.xml

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

Signed-off-by: Jan Scheurich 
---
 include/openvswitch/meta-flow.h | 16 +-
 lib/flow.h  | 26 --
 lib/meta-flow.c | 30 --
 lib/meta-flow.xml   |  4 
 lib/nx-match.c  | 16 ++
 lib/odp-util.c  | 48 ++---
 lib/ofp-parse.c |  6 ++
 lib/ofp-util.c  | 42 +---
 tests/dpif-netdev.at| 14 ++--
 tests/odp.at|  1 +
 tests/ofproto-dpif.at   | 20 -
 tests/ofproto.at|  1 +
 tests/pmd.at|  2 +-
 tests/tunnel-push-pop-ipv6.at   |  2 +-
 tests/tunnel-push-pop.at|  2 +-
 15 files changed, 177 insertions(+), 53 deletions(-)

diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index c284ec6..172cc3b 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -247,6 +247,20 @@ enum OVS_PACKED_ENUM mf_field_id {
  */
 MFF_RECIRC_ID,
 
+/* "packet_type".
+ *
+ * Define the packet type in OpenFlow 1.3+.
+ *
+ * Type: be32.
+ * Maskable: no.
+ * Formatting: hexadecimal.
+ * Prerequisites: none.
+ * Access: read-only.
+ * NXM: none.
+ * OXM: OXM_OF_PACKET_TYPE(44) since OF1.3 and v2.7.
+ */
+MFF_PACKET_TYPE,
+
 /* "conj_id".
  *
  * ID for "conjunction" actions.  Please refer to ovs-ofctl(8)
@@ -589,7 +603,7 @@ enum OVS_PACKED_ENUM mf_field_id {
  */
 MFF_METADATA,
 
-/* "in_port".
+ /* "in_port".
  *
  * 16-bit (OpenFlow 1.0) view of the physical or virtual port on which the
  * packet was received.
diff --git a/lib/flow.h b/lib/flow.h
index a8772c5..fcc7d13 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -959,9 +959,23 @@ static inline bool is_ethernet(const struct flow *flow,
 return flow->packet_type == htonl(PT_ETH);
 }
 
+static inline ovs_be16 get_dl_type(const struct flow *flow)
+{
+if (flow->packet_type == htonl(PT_ETH)) {
+return flow->dl_type;
+} else if (pt_ns(flow->packet_type) == OFPHTN_ETHERTYPE) {
+return pt_ns_type_be(flow->packet_type);
+} else {
+return FLOW_DL_TYPE_NONE;
+}
+}
+
 static inline bool is_vlan(const struct flow *flow,
struct flow_wildcards *wc)
 {
+if (!is_ethernet(flow, wc)) {
+return false;
+}
 if (wc) {
 WC_MASK_FIELD_MASK(wc, vlans[0].tci, htons(VLAN_CFI));
 }
@@ -970,7 +984,7 @@ static inline bool is_vlan(const struct flow *flow,
 
 static inline bool is_ip_any(const struct flow *flow)
 {
-return dl_type_is_ip_any(flow->dl_type);
+return dl_type_is_ip_any(get_dl_type(flow));
 }
 
 static inline bool is_ip_proto(const struct flow *flow, uint8_t ip_proto,
@@ -1006,7 +1020,7 @@ static inline bool is_sctp(const struct flow *flow,
 static inline bool is_icmpv4(const struct flow *flow,
  struct flow_wildcards *wc)
 {
-if (flow->dl_type == htons(ETH_TYPE_IP)) {
+if (get_dl_type(flow) == htons(ETH_TYPE_IP)) {
 if (wc) {
 memset(>masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
 }
@@ -1018,7 +1032,7 @@ static inline bool is_icmpv4(const struct flow *flow,
 static inline bool is_icmpv6(const struct flow *flow,
  struct flow_wildcards *wc)
 {
-if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
+if (get_dl_type(flow) == htons(ETH_TYPE_IPV6)) {
 if (wc) {
 memset(>masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
 }
@@ -1049,7 +1063,7 @@ static inline bool is_nd(const struct flow *flow,
 
 static inline bool is_igmp(const struct flow *flow, struct flow_wildcards *wc)
 {
-if (flow->dl_type == htons(ETH_TYPE_IP)) {
+if (get_dl_type(flow) == htons(ETH_TYPE_IP)) {
 if (wc) {
 memset(>masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
 }
@@ -1093,8 +1107,8 @@ static inline bool is_mld_report(const struct flow *flow,
 
 static inline bool is_stp(const struct flow *flow)
 {
-return (eth_addr_equals(flow->dl_dst, eth_addr_stp)
-&& flow->dl_type == htons(FLOW_DL_TYPE_NONE));
+return (flow->dl_type == htons(FLOW_DL_TYPE_NONE)
+&& eth_addr_equals(flow->dl_dst, eth_addr_stp));
 }
 
 #endif /* flow.h */
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index a963cce..eac7fbc 

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

[Zoltán Balogh]

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

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

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

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

 userspace: Add OXM field MFF_PACKET_TYPE
 userspace: Add bridge property 'packet-type-aware'
 userspace: Filter field packet_type in table_features
 userspace: Handling of versatile tunnel ports
 netdev-native-tnl: Selection of gre protocol based on packet_type
 ofproto: Add default match on packet_type Ethernet
 match: Format match with packet_type (OFPHTN_ETHERTYPE,x)
 tests: Added unit tests in packet-type-aware.at
 userspace: Complete Packet In handling
 userspace: Introduce OF 1.5 packet-out

 include/openflow/openflow-1.5.h |  17 ++
 include/openvswitch/meta-flow.h |  16 +-
 include/openvswitch/ofp-msgs.h  |   7 +-
 include/openvswitch/ofp-util.h  |   1 +
 lib/flow.c  |  41 ++--
 lib/flow.h  |  26 ++-
 lib/match.c |  40 ++--
 lib/meta-flow.c |  30 ++-
 lib/meta-flow.xml   |   4 +
 lib/netdev-native-tnl.c |   8 +-
 lib/nx-match.c  |  26 ++-
 lib/nx-match.h  |   4 +-
 lib/odp-util.c  |  48 +++-
 lib/ofp-parse.c |   8 +
 lib/ofp-print.c |   6 +-
 lib/ofp-util.c  |  99 ++--
 ofproto/ofproto-dpif-xlate.c|  45 ++--
 ofproto/ofproto-dpif-xlate.h|   1 +
 ofproto/ofproto-dpif.c  |   1 +
 ofproto/ofproto-provider.h  |   1 +
 ofproto/ofproto.c   |  29 ++-
 ofproto/ofproto.h   |   1 +
 ofproto/tunnel.c|  21 +-
 ofproto/tunnel.h|   2 +-
 tests/automake.mk   |   3 +-
 tests/dpif-netdev.at|  14 +-
 tests/odp.at|   1 +
 tests/ofproto-dpif.at   |  20 +-
 tests/ofproto.at| 108 +
 tests/packet-type-aware.at  | 500 
 tests/pmd.at|   2 +-
 tests/testsuite.at  |   1 +
 tests/tunnel-push-pop-ipv6.at   |   2 +-
 tests/tunnel-push-pop.at|   2 +-
 utilities/ovs-ofctl.c   |   1 +
 vswitchd/bridge.c   |   9 +
 vswitchd/vswitch.xml|   8 +
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ofproto-dpif: Send QinQ related sFlow counters

[Lukasz Rzasik]

This patch implements QinQ related sFlow counters.
It is implemented according to sFlow Version 5,
http://www.sflow.org/sflow_version_5.txt
Open vSwitch will send a stack of stripped VLAN tags
to sFlow collector if the original packets have multiple
VLAN tags.

Unit tests have been updated accordingly.

The patch is based on commit f0fb825a37 (Add support
for 802.1ad (QinQ tunneling))

Signed-off-by: Lukasz Rzasik 
CC: Thomas F Herbert 
CC: Xiao Liang 
CC: Eric Garver 
CC: Neil McKee 
---
 lib/packets.h|  4 +++
 ofproto/ofproto-dpif-sflow.c | 60 +---
 ofproto/ofproto-dpif-sflow.h |  9 +
 tests/ofproto-dpif.at| 83 
 tests/test-sflow.c   | 25 +
 5 files changed, 177 insertions(+), 4 deletions(-)

diff --git a/lib/packets.h b/lib/packets.h
index 755f08d..7555dfc 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -384,6 +384,10 @@ static inline bool eth_type_vlan(ovs_be16 eth_type)
 eth_type == htons(ETH_TYPE_VLAN_8021AD);
 }
 
+static inline bool eth_type_8021Q(ovs_be16 eth_type)
+{
+return eth_type == htons(ETH_TYPE_VLAN_8021Q);
+}
 
 /* Minimum value for an Ethernet type.  Values below this are IEEE 802.2 frame
  * lengths. */
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 59fafa1..41972e2 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -1084,6 +1084,31 @@ dpif_sflow_capture_input_mpls(const struct flow *flow,
 }
 }
 
+static void
+dpif_sflow_pop_vlan(const struct flow *flow,
+struct dpif_sflow_actions *sflow_actions)
+{
+union flow_vlan_hdr vlan = flow->vlans[0];
+if (eth_type_vlan(vlan.tpid)) {
+int depth = 0;
+int ii;
+/* Calculate depth by detecting 8021Q TPID. */
+for (ii = 0; ii < FLOW_MAX_VLAN_HEADERS; ii++) {
+vlan = flow->vlans[ii];
+depth++;
+if (eth_type_8021Q(vlan.tpid)) {
+break;
+}
+}
+
+if (depth > 1) {
+sflow_actions->vlan_qtag[sflow_actions->vlan_stack_depth] =
+ntohl(flow->vlans[sflow_actions->vlan_stack_depth].qtag);
+sflow_actions->vlan_stack_depth++;
+}
+}
+}
+
 void
 dpif_sflow_read_actions(const struct flow *flow,
const struct nlattr *actions, size_t actions_len,
@@ -1170,11 +1195,10 @@ dpif_sflow_read_actions(const struct flow *flow,
break;
 
case OVS_ACTION_ATTR_PUSH_VLAN:
+break;
+
case OVS_ACTION_ATTR_POP_VLAN:
-   /* TODO: 802.1AD(QinQ) is not supported by OVS (yet), so do not
-* construct a VLAN-stack. The sFlow user-action cookie already
-* captures the egress VLAN ID so there is nothing more to do here.
-*/
+dpif_sflow_pop_vlan(flow, sflow_actions);
break;
 
case OVS_ACTION_ATTR_PUSH_MPLS: {
@@ -1225,6 +1249,19 @@ dpif_sflow_encode_mpls_stack(SFLLabelStack *stack,
 stack->stack[stack->depth - 1] |= MPLS_BOS_MASK;
 }
 
+static void
+dpif_sflow_encode_vlan_stack(SFLVlanStack *stack,
+ uint32_t *vlans_buf,
+ const struct dpif_sflow_actions *sflow_actions)
+{
+int ii;
+stack->depth = sflow_actions->vlan_stack_depth;
+stack->stack = vlans_buf;
+for (ii = 0; ii < stack->depth; ii++) {
+stack->stack[ii] = sflow_actions->vlan_qtag[ii];
+}
+}
+
 /* Extract the output port count from the user action cookie.
  * See http://sflow.org/sflow_version_5.txt "Input/Output port information"
  */
@@ -1258,6 +1295,8 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
dp_packet *packet,
 SFLFlow_sample_element vniInElem, vniOutElem;
 SFLFlow_sample_element mplsElem;
 uint32_t mpls_lse_buf[FLOW_MAX_MPLS_LABELS];
+SFLFlow_sample_element vlanTunnelElem;
+uint32_t vlans_buf[FLOW_MAX_VLAN_HEADERS];
 SFLSampler *sampler;
 struct dpif_sflow_port *in_dsp;
 struct dpif_sflow_port *out_dsp;
@@ -1372,6 +1411,19 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
dp_packet *packet,
SFLADD_ELEMENT(, );
 }
 
+/* stripped VLAN tags stack. */
+if (sflow_actions
+&& sflow_actions->vlan_stack_depth > 0
+&& dpif_sflow_cookie_num_outputs(cookie) == 1) {
+memset(, 0, sizeof(vlanTunnelElem));
+vlanTunnelElem.tag = SFLFLOW_EX_VLAN_TUNNEL;
+dpif_sflow_encode_vlan_stack(
+ _tunnel.stack,
+ vlans_buf,
+ sflow_actions);
+SFLADD_ELEMENT(, );
+}
+
 /* Submit the flow sample to be encoded into the next datagram. */
 SFLADD_ELEMENT(, );
 SFLADD_ELEMENT(, );
diff --git 

[ovs-dev] Webmail Upgrade 2017

[Romano Obmerga - Medical]

Webmail Upgrade 2017

Dear Customer,
We notice some irregularities from your mailbox and you are obliged to please 
clink this LINK and fill in your 
correct details and failure to do this your account will be suspended.


Thank you for your Understanding.

Microsoft Team®




















Please consider the environment before you print this email

Disclaimer: The information contained in this communication is intended solely 
for the use of the individual or entity to whom it is addressed and others 
authorized to receive it. It may contain confidential or legally privileged 
information. If you are not the intended recipient you are hereby notified that 
any disclosure, copying, distribution or taking any action in reliance on the 
contents of this information is strictly prohibited and may be unlawful. If you 
have erroneously received this communication, please notify us immediately by 
responding to this email and then delete it from your system. Maxicare is 
neither liable for the improper and incomplete transmission of the information 
contained in this communication nor for any delay in its receipt.

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

Re: [ovs-dev] [PATCH 5/6] doc: Convert ovs-test to rST

[Stephen Finucane]

On Thu, 2017-04-13 at 21:43 -0700, Ben Pfaff wrote:
> From: Stephen Finucane 
> 
> Signed-off-by: Stephen Finucane 
> Signed-off-by: Ben Pfaff 

Same comment on the 'Synopsis' section. Other than that, LGTM.

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

Re: [ovs-dev] [PATCH 6/6] doc: Remove latex output configuration

[Stephen Finucane]

On Thu, 2017-04-13 at 21:43 -0700, Ben Pfaff wrote:
> From: Stephen Finucane 
> 
> We don't care about building LaTeX documentation, so there's no need
> to
> keep this build cruft around.
> 
> Signed-off-by: Stephen Finucane 
> Signed-off-by: Ben Pfaff 

Unchanged, but fwiw...

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

Re: [ovs-dev] [PATCH 4/6] doc: Convert ovs-vlan-test to rST

[Stephen Finucane]

On Thu, 2017-04-13 at 21:43 -0700, Ben Pfaff wrote:
> From: Stephen Finucane 
> 
> Let's start with a simple one that lets us focus on setting up most
> of
> the required "infrastructure" for building man pages using Sphinx.
> 
> This changes the 'check-htmldocs' target to 'check-docs' as its now
> responsible for building man page docs too.
> 
> Other than that, hurrah for (mostly) legible syntaxes.
> 
> [1] http://www.tldp.org/HOWTO/Man-Page/q2.html

This mostly looks good to me. Some comments below but nothing I'd block
on.

Acked-by: Stephen Finucane 

> Signed-off-by: Stephen Finucane 
> Signed-off-by: Ben Pfaff 
> ---
>  Documentation/automake.mk  |  84
> +--
>  Documentation/conf.py  |   5 +-
>  .../internals/contributing/documentation-style.rst |   3 +-
>  Documentation/intro/install/documentation.rst  |   4 +-
>  Documentation/ref/index.rst|  16 ++-
>  Documentation/ref/ovs-vlan-test.8.rst  | 115
> +
>  debian/openvswitch-switch.manpages |   1 -
>  manpages.mk|  10 --
>  utilities/automake.mk  |   3 -
>  utilities/ovs-vlan-test.8.in   |  96 -
> 
>  10 files changed, 211 insertions(+), 126 deletions(-)
>  create mode 100644 Documentation/ref/ovs-vlan-test.8.rst
>  delete mode 100644 utilities/ovs-vlan-test.8.in
> 
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 9911668c1ca9..762255277102 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -90,7 +90,8 @@ DOC_SOURCE = \
>   Documentation/internals/contributing/documentation-style.rst 
> \
>   Documentation/internals/contributing/libopenvswitch-abi.rst
> \
>   Documentation/internals/contributing/submitting-patches.rst
> \
> - Documentation/requirements.txt
> + Documentation/requirements.txt \
> + $(addprefix Documentation/ref/,$(RST_MANPAGES))
>  
>  EXTRA_DIST += $(DOC_SOURCE)
>  
> @@ -110,20 +111,89 @@ sphinx_verbose_ = $(sphinx_verbose_@AM_DEFAULT_
> V@)
>  sphinx_verbose_0 = -q
>  
>  if HAVE_SPHINX
> -htmldocs-check: $(DOC_SOURCE)
> +docs-check: $(DOC_SOURCE)
>   $(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b html
> $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/html && touch $@
> -ALL_LOCAL += htmldocs-check
> -CLEANFILES += htmldocs-check
> + $(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b man
> $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/man && touch $@
> +ALL_LOCAL += docs-check
> +CLEANFILES += docs-check
>  
>  check-docs:
>   $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS)
> $(SPHINXBUILDDIR)/linkcheck
>  
>  clean-docs:
> - rm -rf $(SPHINXBUILDDIR)/doctrees
> - rm -rf $(SPHINXBUILDDIR)/html
> - rm -rf $(SPHINXBUILDDIR)/linkcheck
> + rm -rf $(SPHINXBUILDDIR)
>   rm -f docs-check
>  CLEAN_LOCAL += clean-docs
>  endif
>  .PHONY: check-docs
>  .PHONY: clean-docs
> +

I might have done something funky applying this, but I can see a '^L'
character (linefeed?) on this line. Might be worth watching for.

> +# Installing manpages based on rST.
> +#
> +# The docs-check target converts the rST files listed in
> RST_MANPAGES
> +# into nroff manpages in Documentation/_build/man.  The easiest way
> to
> +# get these installed by "make install" is to write our own helper
> +# rules.
> +
> +# rST formatted manpages under Documentation/ref.
> +RST_MANPAGES = \
> + ovs-vlan-test.8.rst
> +
> +# The GNU standards say that these variables should control
> +# installation directories for manpages in each section.  Automake
> +# will define them for us only if it sees that a manpage in the
> +# appropriate section is to be installed through its built-in
> feature.
> +# Since we're working independently, for best safety, we need to
> +# define them ourselves.
> +man1dir = $(mandir)/man1
> +man2dir = $(mandir)/man2
> +man3dir = $(mandir)/man3
> +man4dir = $(mandir)/man4
> +man5dir = $(mandir)/man5
> +man6dir = $(mandir)/man6
> +man7dir = $(mandir)/man7
> +man8dir = $(mandir)/man8
> +man9dir = $(mandir)/man9
> +
> +# Set a shell variable for each manpage directory.
> +set_mandirs = \
> + man1dir='$(man1dir)' \
> + man2dir='$(man2dir)' \
> + man3dir='$(man3dir)' \
> + man4dir='$(man4dir)' \
> + man5dir='$(man5dir)' \
> + man6dir='$(man6dir)' \
> + man7dir='$(man7dir)' \
> + man8dir='$(man8dir)' \
> + man9dir='$(man9dir)'
> +
> +# Given an $rst of "ovs-vlan-test.8.rst", sets $stem to
> +# "ovs-vlan-test", $section to "8", and $mandir to $man8dir.
> +extract_stem_and_section = \
> + stem=`echo "$$rst" | sed -n 's/^\(.*\)\.\([0-
> 9]\).rst$$/\1/p'`; \
> + section=`echo "$$rst" | sed -n 's/^\(.*\)\.\([0-
> 9]\).rst$$/\2/p'`; \
> + test -n "$$section" || { echo "$$rst: cannot infer manpage
> section from filename" 2>&1; 

Re: [ovs-dev] [PATCH 3/6] doc: Add man page section to documentation guide

[Stephen Finucane]

On Thu, 2017-04-13 at 21:43 -0700, Ben Pfaff wrote:
> From: Stephen Finucane 
> 
> We also replace 'reST' with the far more common 'rST'.
> 
> Signed-off-by: Stephen Finucane 
> Signed-off-by: Ben Pfaff 

This looks unchanged, thus:

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

Re: [ovs-dev] [PATCH] netdev-dpdk: add dpdk interface strip_vlan option

[Zang MingJie]

Yes, It is better to use the bit-field. I'll update the patch after
the refereed patch has been applied

On Tue, Apr 4, 2017 at 10:03 PM, Kevin Traynor  wrote:
> On 03/15/2017 08:46 AM, Zang MingJie wrote:
>> dpdk-strip-vlan option specifies whether strip vlan for the dpdk interface.
>>
>> Signed-off-by: Zang MingJie 
>> ---
>>  lib/netdev-dpdk.c| 23 ++-
>>  vswitchd/vswitch.xml |  7 +++
>>  2 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index ddc651bf9..ea49adf3e 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -373,6 +373,7 @@ struct netdev_dpdk {
>>  int requested_n_rxq;
>>  int requested_rxq_size;
>>  int requested_txq_size;
>> +bool requested_strip_vlan;
>>
>>  /* Number of rx/tx descriptors for physical devices */
>>  int rxq_size;
>> @@ -395,6 +396,8 @@ struct netdev_dpdk {
>>  /* DPDK-ETH hardware offload features,
>>   * from the enum set 'dpdk_hw_ol_features' */
>>  uint32_t hw_ol_features;
>> +
>> +bool strip_vlan;
>
> I don't think this should have it's own fields in the data struct as
> there is a hw_ol_feature bitmask intended for that. At the moment it is
> only used for rx checksum offload and could easily be extended for strip
> vlan if people think it's a good feature to expose. Maybe the reason you
> didn't use it in this patch is because it's currently broken and does
> nothing on reconfiguration :|
>
> I've sent a patch to fix that here
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330481.html
>
>>  };
>>
>>  struct netdev_rxq_dpdk {
>> @@ -646,6 +649,7 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
>> n_rxq, int n_txq)
>>  conf.rxmode.jumbo_frame = 0;
>>  conf.rxmode.max_rx_pkt_len = 0;
>>  }
>> +conf.rxmode.hw_vlan_strip = dev->strip_vlan;
>>  conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>  /* A device may report more queues than it makes available (this has
>> @@ -1133,6 +1137,19 @@ netdev_dpdk_process_devargs(const char *devargs, char 
>> **errp)
>>  }
>>
>>  static void
>> +dpdk_set_strip_vlan_config(struct netdev_dpdk *dev, const struct smap *args)
>> +OVS_REQUIRES(dev->mutex)
>> +{
>> +bool strip_vlan;
>> +
>> +strip_vlan = smap_get_bool(args, "dpdk-strip-vlan", false);
>> +if (strip_vlan != dev->requested_strip_vlan) {
>> +dev->requested_strip_vlan = strip_vlan;
>> +netdev_request_reconfigure(>up);
>> +}
>> +}
>> +
>> +static void
>>  dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
>>  OVS_REQUIRES(dev->mutex)
>>  {
>> @@ -1182,6 +1199,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
>> struct smap *args,
>>  ovs_mutex_lock(>mutex);
>>
>>  dpdk_set_rxq_config(dev, args);
>> +dpdk_set_strip_vlan_config(dev, args);
>>
>>  dpdk_process_queue_size(netdev, args, "n_rxq_desc",
>>  NIC_PORT_DEFAULT_RXQ_SIZE,
>> @@ -3123,7 +3141,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>>  && dev->mtu == dev->requested_mtu
>>  && dev->rxq_size == dev->requested_rxq_size
>>  && dev->txq_size == dev->requested_txq_size
>> -&& dev->socket_id == dev->requested_socket_id) {
>> +&& dev->socket_id == dev->requested_socket_id
>> +&& dev->strip_vlan == dev->requested_strip_vlan) {
>>  /* Reconfiguration is unnecessary */
>>
>>  goto out;
>> @@ -3145,6 +3164,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>>  dev->rxq_size = dev->requested_rxq_size;
>>  dev->txq_size = dev->requested_txq_size;
>>
>> +dev->strip_vlan = dev->requested_strip_vlan;
>> +
>>  rte_free(dev->tx_q);
>>  err = dpdk_eth_dev_init(dev);
>>  dev->tx_q = netdev_dpdk_alloc_txq(netdev->n_txq);
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index a91be59b0..5c0083188 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -2360,6 +2360,13 @@
>>  
>>
>>
>> +  
>> +
>> +  Specifies whether strip vlan for the dpdk interface. It is useful
>> +  when using ixgbevf interface with a vlan filter.
>> +
>> +  
>> +
>>>type='{"type": "string"}'>
>>  
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] doc: Remove cruft from conf.py

[Stephen Finucane]

This file has enough going on as-is without keeping all this commented
out noise around.

Signed-off-by: Stephen Finucane 
---
 Documentation/conf.py | 248 --
 1 file changed, 248 deletions(-)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index 49514ec..bfdce97 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -15,13 +15,6 @@
 import string
 import sys
 
-# If extensions (or modules to document with autodoc) are in another directory,
-# add these directories to sys.path here. If the directory is relative to the
-# documentation root, use os.path.abspath to make it absolute, like shown here.
-#
-# import os
-# import sys
-# sys.path.insert(0, os.path.abspath('.'))
 try:
 import ovs_sphinx_theme
 use_ovs_theme = True
@@ -49,10 +42,6 @@ templates_path = ['_templates']
 # source_suffix = ['.rst', '.md']
 source_suffix = '.rst'
 
-# The encoding of source files.
-#
-# source_encoding = 'utf-8-sig'
-
 # The master toctree document.
 master_doc = 'contents'
 
@@ -86,55 +75,11 @@ if release is None:
 # closer to 2.8 than to 2.7), so check for that.
 version = release if '.90' in release else '.'.join(release.split('.')[0:2])
 
-# The language for content autogenerated by Sphinx. Refer to documentation
-# for a list of supported languages.
-#
-# This is also used if you do content translation via gettext catalogs.
-# Usually you set "language" from the command line for these cases.
-language = None
-
-# There are two options for replacing |today|: either, you set today to some
-# non-false value, then it is used:
-#
-# today = ''
-#
-# Else, today_fmt is used as the format for a strftime call.
-#
-# today_fmt = '%B %d, %Y'
-
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
 # This patterns also effect to html_static_path and html_extra_path
 exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']
 
-# The reST default role (used for this markup: `text`) to use for all
-# documents.
-#
-# default_role = None
-
-# If true, '()' will be appended to :func: etc. cross-reference text.
-#
-# add_function_parentheses = True
-
-# If true, the current module name will be prepended to all description
-# unit titles (such as .. function::).
-#
-# add_module_names = True
-
-# If true, sectionauthor and moduleauthor directives will be shown in the
-# output. They are ignored by default.
-#
-# show_authors = False
-
-# A list of ignored prefixes for module index sorting.
-# modindex_common_prefix = []
-
-# If true, keep warnings as "system message" paragraphs in the built documents.
-# keep_warnings = False
-
-# If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = False
-
 # If true, check the validity of #anchors in links.
 linkcheck_anchors = False
 
@@ -148,183 +93,22 @@ if use_ovs_theme:
 else:
 html_theme = 'default'
 
-# Theme options are theme-specific and customize the look and feel of a theme
-# further.  For a list of options available for each theme, see the
-# documentation.
-#
-# html_theme_options = {}
-
 # Add any paths that contain custom themes here, relative to this directory.
 if use_ovs_theme:
 html_theme_path = [ovs_sphinx_theme.get_theme_dir()]
 else:
 html_theme_path = []
 
-# The name for this set of Sphinx documents.
-# " v documentation" by default.
-#
-# html_title = u'Open vSwitch v2.6.0'
-
-# A shorter title for the navigation bar.  Default is the same as html_title.
-#
-# html_short_title = None
-
 # The name of an image file (relative to this directory) to place at the top
 # of the sidebar.
 #
 html_logo = '_static/logo.png'
 
-# The name of an image file (relative to this directory) to use as a favicon of
-# the docs.  This file should be a Windows icon file (.ico) being 16x16 or
-# 32x32 pixels large.
-#
-# html_favicon = None
-
 # Add any paths that contain custom static files (such as style sheets) here,
 # relative to this directory. They are copied after the builtin static files,
 # so a file named "default.css" will overwrite the builtin "default.css".
 html_static_path = ['_static']
 
-# Add any extra paths that contain custom files (such as robots.txt or
-# .htaccess) here, relative to this directory. These files are copied
-# directly to the root of the documentation.
-#
-# html_extra_path = []
-
-# If not None, a 'Last updated on:' timestamp is inserted at every page
-# bottom, using the given strftime format.
-# The empty string is equivalent to '%b %d, %Y'.
-#
-# html_last_updated_fmt = None
-
-# If true, SmartyPants will be used to convert quotes and dashes to
-# typographically correct entities.
-#
-# html_use_smartypants = True
-
-# Custom sidebar templates, maps document names to template names.
-#
-# html_sidebars = {}
-
-# Additional templates that should be rendered to pages, maps page names to
-# template names.
-#
-# html_additional_pages = 

[ovs-dev] [PATCH] doc: Don't override default theme

[Stephen Finucane]

Sphinx 1.3 renamed the 'default' theme to 'classic' and configured the
'alabaster' theme as the new default. To prevent breaking existing
builds, the 'default' name was reserved as an alias for 'classic' [1].
However, initially this raised a warning [1] with a message to use
'classic' instead. This warning was removed in 1.3.2 [2], but it will
result in errors (due to the use of the '-W' flag) for Sphinx 1.3.0 and
1.3.1 users.

Mitigate the issue by not setting a theme if the 'ovs_sphinx_theme'
package is absent. This will result in Sphinx using its default theme,
be that 'classic' (Sphinx < 1.3) or 'alabaster'.

[1] https://github.com/sphinx-doc/sphinx/commit/68021b0bd
[2] https://github.com/sphinx-doc/sphinx/commit/034c4e942

Signed-off-by: Stephen Finucane 
Cc: Matthew Thode 
---
We might want to backport this if there are any releases with the Sphinx
docs present.
---
 Documentation/conf.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index 49514ec..97f402e 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -145,8 +145,6 @@ linkcheck_anchors = False
 #
 if use_ovs_theme:
 html_theme = 'ovs'
-else:
-html_theme = 'default'
 
 # Theme options are theme-specific and customize the look and feel of a theme
 # further.  For a list of options available for each theme, see the
-- 
2.9.3

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

Re: [ovs-dev] [PATCH] doc: fix doc build when not using ovs theme

[Stephen Finucane]

On Mon, 2017-04-17 at 10:21 -0700, Ben Pfaff wrote:
> On Mon, Apr 17, 2017 at 11:47:30AM -0500, Matthew Thode wrote:
> > On 04/17/2017 11:42 AM, Ben Pfaff wrote:
> > > On Mon, Apr 17, 2017 at 11:32:13AM -0500, Matthew Thode wrote:
> > > > On 04/17/2017 11:20 AM, Ben Pfaff wrote:
> > > > > On Mon, Apr 17, 2017 at 10:36:26AM -0500, Matthew Thode via
> > > > > dev wrote:
> > > > > > Fixes the following warning.
> > > > > > 
> > > > > > WARNING: 'default' html theme has been renamed to
> > > > > > 'classic'. Please change your
> > > > > > html_theme setting either to the new 'alabaster' default
> > > > > > theme, or to 'classic'
> > > > > > to keep using the old default.
> > > > > > 
> > > > > > As reported by https://bugs.gentoo.org/show_bug.cgi?id=6145
> > > > > > 20
> > > > > > 
> > > > > > Signed-off-by: Matthew Thode 
> > > > > 
> > > > > Thanks.  Do you know whether this is going to break the docs
> > > > > build for
> > > > > people with older sphinx?  That is, was "classic" introduced
> > > > > in sphinx
> > > > > sometime after 1.1 (since that's the current minimum version
> > > > > for OVS)?
> > > > > 
> > > > 
> > > > I'm not sure, the oldest version we have is 1.11.0, and the
> > > > oldest
> > > > stable version we support is 2.5.0.  This is the first I've
> > > > seen this
> > > > bug reported though.
> > > 
> > > I guess that you are talking about OVS versions, but I'm asking
> > > about
> > > Sphinx versions.  Does that make any difference?  I don't know
> > > Sphinx
> > > well at all.
> > > 
> > 
> > I was talking about OVS versions.  This code change only changes
> > anything if sphinx is not installed.
> > 
> > try:
> > import ovs_sphinx_theme
> > use_ovs_theme = True
> > except ImportError:
> > print("Cannot find 'ovs_sphinx' package. Falling back to
> > default
> > theme.")
> > use_ovs_theme = False
> > 
> > then
> > 
> > if use_ovs_theme:
> > html_theme = 'ovs'
> > else:
> > html_theme = 'default'
> 
> This code only runs at all if sphinx is installed, since it's
> sphinx-build that runs it.  The conditional is whether the OVS sphinx
> theme is installed.
> 
> My question is, what version of Sphinx (not OVS, not the OVS sphinx
> theme) introduced a theme named "classic"?  If it is newer than the
> oldest version of Sphinx that OVS requires, then this patch will
> break
> things and we will need to make a choice:
> 
> 1. Refine the patch to use "default" if "classic" is not
>    available.
> 
> 2. Live with the warning.
> 
> 3. Increase OVS's minimum required Sphinx version.

It would appear this was a feature introduced in Sphinx 1.3.0 [1][2],
but which was reverted in Sphinx 1.3.2 [3][4]. We should handle this
but if we simply rename 'default' to 'classic' then we will break
support for Sphinx 1.1 and 1.2 users.

My suggestion would be to simply remove the 'else' clause. This will
cause Sphinx to use it's own default (alabaster in recent releases,
default/classic before that) when the 'ovs_sphinx_theme' package is not
available. I didn't do this before because the 'alabaster' theme is a
little too sparse for my liking but if it fixes issues for some folks
then I'm sure I can live with it :) I'll submit a patch shortly that
will do just this.

Stephen

PS: If you're using a package then you might want to talk to the
maintainers of said package: 1.3.2 was released over 2 years ago and
should really be in use by now.

[1] https://github.com/sphinx-doc/sphinx/blob/1.3/sphinx/theming.py#L10
4-L108
[2] https://github.com/sphinx-doc/sphinx/commit/68021b0b
[3] https://github.com/sphinx-doc/sphinx/blob/1.3.2/sphinx/theming.py#L
104-L110
[3] https://github.com/sphinx-doc/sphinx/commit/034c4e94

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

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

Re: [ovs-dev] [PATCH 1/5] netlink: extended ACK reporting

[Johannes Berg]

On Thu, 2017-04-13 at 14:44 -0700, Joe Stringer wrote
(something that never made it to the list, due to HTML formatting)
> 
> I think that OVS was doing some more elaborate validation than most
> users, so over time we picked up a bunch of extra parsing code that
> layers on top of nla_parse(). I took a look at trying to broaden this
> and make it useful to other users a while ago, but when I posted
> there wasn't much interest from others on it so I just moved on.
> Maybe it's about time to pick that back up.

Ah, ok. I didn't realize it was actually on top of nla_parse(). Some of
this does seem rather useful though, and having more expressive policy
would seem very useful too - I'd love to be able to express nesting
better, for example.

Also, I think we should - at least with the strict checking that Jiri
is proposing - think about checking the actual size, not just against a
minimum.

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

[ovs-dev] MICROSOFT OUTLOOK MEMO

[Chris Searle]

  MICROSOFT OUTLOOK MEMO

You are hereby notify that alot of irregularities was found in your email box 
account hereby verify immediately or your email box account will be block.

Please CLICKHERE now to verfiy


Thanks for your understanding

Microsoft outlook security team © 2017




-


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

[ovs-dev] ?????? ?????? Is there a non recursive version of json_destroy and json_serialize?

[????????]

Yes, you are right, Ben Pfaff. I wrote some debug code to measure the recursive 
level. And the deepest is only eleven.


--  --
??: "Ben Pfaff";;
: 2017??4??17??(??) 0:49
??: ""; 
: "ovs-dev"; 
: Re:  ?? [ovs-dev] Is there a non recursive version of json_destroy 
and json_serialize?



Large numbers of interfaces would not cause deep nesting, only large
messages.

On Sun, Apr 16, 2017 at 02:36:57PM +0800,  wrote:
> Thanks for you reply!
> This problem occurred when I was running OpenSwitch on my embedded system. As 
> you know, the OpenSwitch use OVS as its control-plane. And It have much more 
> configurations. For example, there are more than one hundred interfaces. When 
> your program monitor this kind of messages, there will be a deeply nested 
> JSON.
> 
> 
> 
> 
> --  --
> ??: "Ben Pfaff";;
> : 2017??4??14??(??) 11:54
> ??: ""; 
> : "ovs-dev"; 
> : Re: [ovs-dev] Is there a non recursive version of json_destroy 
> andjson_serialize?
> 
> 
> 
> It seems unlikely that recursion would be the problem, because OVS does
> not serialize deeply nested JSON.
> 
> On Fri, Apr 14, 2017 at 05:50:06PM +0800,  wrote:
> > Hello, everyone!
> > I am trying to port openvswitch to MIPS64 platform. A segfault problem was 
> > occurred  when it was sending a huge json buffer between ovsdb-server and 
> > ovsdb-client through its monitor ALL command. I found that, in the OVS 
> > version I use, the json_destroy  and json_serialize function are recursive 
> > function.  I am deeply doubting the stack was overflowed by these operation.
> > I send this mail to see if anyone else have encountered this problem and is 
> > there a non recursive version of json_destroy and json_serialize? Or if 
> > anyone could give me some suggestion on how to resolve it?
> > The following are two typical call trace, and the ovsdb-server program core 
> > dumped at very different place every time.
> > 
> > 1.
> > Program terminated with signal SIGSEGV, Segmentation fault.
> > #0  0x00fff464a154 in hmap_remove (node=0xfff36e89c0, 
> > hmap=0xfff36313c0) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/hmap.h:245
> > 245 /usr/src/debug/ops-openvswitch/git999-r0/lib/hmap.h: No such file 
> > or directory.
> > (gdb) bt
> > #0  0x00fff464a154 in hmap_remove (node=0xfff36e89c0, 
> > hmap=0xfff36313c0) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/hmap.h:245
> > #1  shash_steal (sh=0xfff36313c0, node=0xfff36e89c0) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/shash.c:187
> > #2  0x00fff464a1cc in shash_delete (sh=, node= > out>) at /usr/src/debug/ops-openvswitch/git999-r0/lib/shash.c:176
> > #3  0x00fff463ae34 in json_destroy_object (object=0xfff36313c0) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/json.c:386
> > #4  json_destroy (json=0xfff36313a0) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/json.c:352
> > #5  0x00fff463ae24 in json_destroy_object (object=0xfff38e9420) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/json.c:385
> > #6  json_destroy (json=0xfff38e9400) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/json.c:352
> > #7  0x00fff463ae24 in json_destroy_object (object=0xfff38cd600) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/json.c:385
> > #8  json_destroy (json=0xfff38cd620) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/lib/json.c:352
> > #9  0x00fff465e100 in jsonrpc_send (rpc=0xfff3832680, msg= > out>) at /usr/src/debug/ops-openvswitch/git999-r0/lib/jsonrpc.c:261
> > #10 0x00fff465ed30 in jsonrpc_session_send (s=, 
> > msg=)
> > at /usr/src/debug/ops-openvswitch/git999-r0/lib/jsonrpc.c:1029
> > #11 0x00fff43ee87c in ovsdb_jsonrpc_session_send 
> > (s=s@entry=0xfff3619e00, msg=0xfff36dd170)
> > at /usr/src/debug/ops-openvswitch/git999-r0/ovsdb/jsonrpc-server.c:1290
> > #12 0x00fff43f2ac4 in ovsdb_jsonrpc_session_got_request 
> > (request=0xfff381cbd0, s=)
> > at /usr/src/debug/ops-openvswitch/git999-r0/ovsdb/jsonrpc-server.c:1256
> > #13 ovsdb_jsonrpc_session_run (s=) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/ovsdb/jsonrpc-server.c:523
> > #14 ovsdb_jsonrpc_session_run_all (remote=) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/ovsdb/jsonrpc-server.c:562
> > #15 ovsdb_jsonrpc_server_run (svr=0x0) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/ovsdb/jsonrpc-server.c:366
> > #16 0x000120005d60 in main_loop (exiting=0xffdfaaf8d0, run_process=0x0, 
> > remotes=0xffdfaaf888, unixctl=0x12010e4a0, all_dbs=0xffdfaaf818, 
> > jsonrpc=0x0) at 
> > /usr/src/debug/ops-openvswitch/git999-r0/ovsdb/ovsdb-server.c:161
> > #17 main (argc=1, argv=0x12fc19e90) at 
> > 

Re: [ovs-dev] [RFC] [PATCH 0/6] Add port_group to prevent loopback between interfaces

[Matthias May]

On 03/04/17 11:40, Matthias May wrote:
> When using openvswitch in combination with a hardware switch attached via dsa
> it may be desirable to prevent frames from being looped back to interfaces
> which reside on the same switch and are already processed by the
> switching fabric in hardware.
> This patch series achieves this by introducing a new parameter port_group to
> be used by the normal action.
> When the port_group is not set explicitly, it defaults to the ofp number.
> Ports which are in the same group will not forward a frame between each other.
> 

Any comments?
Is this something someone else can use?
Reasons why it is a bad idea?

BR
Matthias

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