Re: [ovs-dev] Bug: ovsdb monitor2 - includes unknown object

[Justin Pettit]

> On Dec 15, 2015, at 7:13 PM, Han Zhou  wrote:
> 
> For some reason my patch was blocked by the mailinglist, so I just removed
> the http link from commit message and resent. Now it is seen in the
> mailinglist:
> 
> http://openvswitch.org/pipermail/dev/2015-December/063461.html

Ben is right that our spam filter isn't great, but I went through the logs, and 
it showed it allowed your messages.  I also received this message, which is 
also in the archives that appears to have the URL:

http://openvswitch.org/pipermail/dev/2015-December/063459.html

I'm happy to add you to the white-list, but I'm curious if there was actually a 
problem.

--Justin


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

Re: [ovs-dev] [PATCH v2 RFC] ovs-lib: try to call exit before killing

[Ben Pfaff]

On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote:
> 
> 
> On 16.12.2015 09:53, Ben Pfaff wrote:
> > On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote:
> >> While killing OVS may not free all allocated resources.
> >>
> >> Eample:
> >>Socket for vhost-user port will stay in a system
> >>after 'systemctl stop openvswitch' and opening
> >>that port after restart will fail.
> >>
> >> Signed-off-by: Ilya Maximets 
> >> ---
> >>
> >> version 2:
> >>* added '-T 1'
> >>* '-t $1' --> '-t $rundir/$1.$pid.ctl'
> > 
> > Thanks.
> > 
> > Have you tested it?
> > 
> 
> Yes. On my setup it works.
> Only reason why it put RFC to that patch:
> Is all daemons have unixctl exit command? Or we can just let ovs-appctl to 
> fail here
> in that case?

I think that all of our current daemons do have an "exit" command, but
it might be worth removing the "2" after EXIT.  The purpose of the
delays in this chain is to wait a bit for a given signal to be processed
through the operating system and the daemon, but EXIT should wait until
the process has actually exited, or at least get very close to that.
Maybe a short delay like .1 would be reasonable, to allow for a brief
race window.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 RFC] ovs-lib: try to call exit before killing

[Ilya Maximets]

On 16.12.2015 11:27, Ben Pfaff wrote:
> On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote:
>>
>>
>> On 16.12.2015 09:53, Ben Pfaff wrote:
>>> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote:
 While killing OVS may not free all allocated resources.

 Eample:
Socket for vhost-user port will stay in a system
after 'systemctl stop openvswitch' and opening
that port after restart will fail.

 Signed-off-by: Ilya Maximets 
 ---

 version 2:
* added '-T 1'
* '-t $1' --> '-t $rundir/$1.$pid.ctl'
>>>
>>> Thanks.
>>>
>>> Have you tested it?
>>>
>>
>> Yes. On my setup it works.
>> Only reason why it put RFC to that patch:
>> Is all daemons have unixctl exit command? Or we can just let ovs-appctl to 
>> fail here
>> in that case?
> 
> I think that all of our current daemons do have an "exit" command, but
> it might be worth removing the "2" after EXIT.  The purpose of the
> delays in this chain is to wait a bit for a given signal to be processed
> through the operating system and the daemon, but EXIT should wait until
> the process has actually exited, or at least get very close to that.
> Maybe a short delay like .1 would be reasonable, to allow for a brief
> race window.

I don't think so, because ovs-vswitchd replies on connection to ovs-appctl
immediately, but actual exiting performed while stopping execution in
main loop.

Best regards, Ilya Maximets.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/5] debian: Add a package for ovn-controller.

[Ben Pfaff]

On Tue, Dec 15, 2015 at 09:52:04AM -0800, Gurucharan Shetty wrote:
> Signed-off-by: Gurucharan Shetty 

I wonder whether the name of this package is well chosen.  I know that
the program that it starts is ovn-controller, and I know that that runs
on every hypervisor in OVN, but still at first I assumed that any
package with "controller" in the name would run centrally, that is, my
first guess was that it would run ovn-northd or the nb or sb
ovsdb-server.  So maybe it would confuse others too.  If you agree, then
maybe we should name it openvswitch-ovn-hypervisor or -host or something
of that sort.

On another naming note, I wonder whether we should drop the
"openvswitch-" prefix from these package names.  I'm not sure it's
helpful.

Thanks,

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

Re: [ovs-dev] [PATCH 5/5] debian: Add a package for OVN docker drivers.

[Ben Pfaff]

On Tue, Dec 15, 2015 at 09:52:06AM -0800, Gurucharan Shetty wrote:
> Signed-off-by: Gurucharan Shetty 

I think that this is architecture-independent (there's no actual
binaries in it, just Python source), so that it should be Architecture:
"all" instead of "linux-any".

Is there a reason someone wouldn't want to install this?  That is, is
there a reason not to fold it into the -common package?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 RFC] ovs-lib: try to call exit before killing

[Ben Pfaff]

On Wed, Dec 16, 2015 at 12:07:23PM +0300, Ilya Maximets wrote:
> On 16.12.2015 11:27, Ben Pfaff wrote:
> > On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote:
> >>
> >>
> >> On 16.12.2015 09:53, Ben Pfaff wrote:
> >>> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote:
>  While killing OVS may not free all allocated resources.
> 
>  Eample:
>   Socket for vhost-user port will stay in a system
>   after 'systemctl stop openvswitch' and opening
>   that port after restart will fail.
> 
>  Signed-off-by: Ilya Maximets 
>  ---
> 
>  version 2:
>   * added '-T 1'
>   * '-t $1' --> '-t $rundir/$1.$pid.ctl'
> >>>
> >>> Thanks.
> >>>
> >>> Have you tested it?
> >>>
> >>
> >> Yes. On my setup it works.
> >> Only reason why it put RFC to that patch:
> >> Is all daemons have unixctl exit command? Or we can just let ovs-appctl to 
> >> fail here
> >> in that case?
> > 
> > I think that all of our current daemons do have an "exit" command, but
> > it might be worth removing the "2" after EXIT.  The purpose of the
> > delays in this chain is to wait a bit for a given signal to be processed
> > through the operating system and the daemon, but EXIT should wait until
> > the process has actually exited, or at least get very close to that.
> > Maybe a short delay like .1 would be reasonable, to allow for a brief
> > race window.
> 
> I don't think so, because ovs-vswitchd replies on connection to ovs-appctl
> immediately, but actual exiting performed while stopping execution in
> main loop.

Then that means that this patch will generally make it take about 4
longer to stop OVS, since killing each daemon will sleep for 2 seconds.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 RFC] ovs-lib: try to call exit before killing

[Ilya Maximets]

On 16.12.2015 13:02, Ben Pfaff wrote:
> On Wed, Dec 16, 2015 at 12:07:23PM +0300, Ilya Maximets wrote:
>> On 16.12.2015 11:27, Ben Pfaff wrote:
>>> On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote:


 On 16.12.2015 09:53, Ben Pfaff wrote:
> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote:
>> While killing OVS may not free all allocated resources.
>>
>> Eample:
>>  Socket for vhost-user port will stay in a system
>>  after 'systemctl stop openvswitch' and opening
>>  that port after restart will fail.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>
>> version 2:
>>  * added '-T 1'
>>  * '-t $1' --> '-t $rundir/$1.$pid.ctl'
>
> Thanks.
>
> Have you tested it?
>

 Yes. On my setup it works.
 Only reason why it put RFC to that patch:
 Is all daemons have unixctl exit command? Or we can just let ovs-appctl to 
 fail here
 in that case?
>>>
>>> I think that all of our current daemons do have an "exit" command, but
>>> it might be worth removing the "2" after EXIT.  The purpose of the
>>> delays in this chain is to wait a bit for a given signal to be processed
>>> through the operating system and the daemon, but EXIT should wait until
>>> the process has actually exited, or at least get very close to that.
>>> Maybe a short delay like .1 would be reasonable, to allow for a brief
>>> race window.
>>
>> I don't think so, because ovs-vswitchd replies on connection to ovs-appctl
>> immediately, but actual exiting performed while stopping execution in
>> main loop.
> 
> Then that means that this patch will generally make it take about 4
> longer to stop OVS, since killing each daemon will sleep for 2 seconds.

Ok. May be something like 'EXIT .1 .25 .65 1 1 TERM' will be better?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 05/14] Add OpenFlow support for new "arp" action.

[Ben Pfaff]

On Wed, Dec 09, 2015 at 05:57:47PM -0800, Justin Pettit wrote:
> 
> > On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> > 
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,5 +1,7 @@
> > Post-v2.5.0
> > -
> > +   - OpenFlow:
> > + * New "arp" OpenFlow extension action for generating ARP packets.
> 
> Once you have a design for packet injection from a controller, I think
> we should revisit this patch.  My initial reaction is that I'd prefer
> to not introduce changes to OVS unless they're really needed, and it's
> not clear to me whether that's the case here.  We're going to be
> handling the IPv6 equivalent from the controller, so there'd be some
> parity.

That's true.

> > + * @OVS_ACTION_ATTR_ARP: Transform IPv4 packet into ARP packet, execute 
> > nested
> > + * actions, restore IPv4 packet.
> 
> Another advantage of initiating the ARP in the controller is that we
> can buffer the packet, since this means that an ARP miss will always
> require a retransmission of the original packet.  We'd also have the
> ability to rate-limit ARP generation from the controller.

I want to equivocate here.

Rate-limiting always works better the closer you get to the source.  If
we rate-limit at the controller, then we're not rate-limiting the
packets sent to the controller by the switch.  That makes it hard to
achieve fairness.  That is, if we send all the packets that need an ARP
to the controller, and there's a big stream of those that the controller
is going to drop, and some packet slips in that won't be dropped, then
the socket buffer between the switch and controller is what's actually
going to drop them, and statistically the fairness there isn't going to
be good.

Therefore, I don't think we should rate-limit from the controller.  We
do have some mechanisms to rate-limit from the switch.  One way we can
do it today is to "learn" a flow that drops packets with a suitable
(e.g. 1-second) hard timeout; I don't know whether this is the best way
though.

I don't think that ARP generation from the controller is mutually
exclusive with buffering packets.

> > +/* Parses 'arg' as the argument to a "arp" action, and appends such an
> > + * action to 'ofpacts'.
> 
> s/a "arp"/an "arp"/

Thanks, fixed.

> > --- a/lib/packets.h
> > +++ b/lib/packets.h
> > @@ -1041,6 +1041,7 @@ void packet_set_nd(struct dp_packet *, const ovs_be32 
> > target[4],
> > 
> > void packet_format_tcp_flags(struct ds *, uint16_t);
> > const char *packet_tcp_flag_to_string(uint32_t flag);
> > +void compose_arp__(struct dp_packet *);
> 
> Somehow it seems kind of strange to have a "__" function defined in a
> header file for external users.

Sorry.

> Acked-by: Justin Pettit 

I'm going to hold off on pushing this one for the moment.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 RFC] ovs-lib: try to call exit before killing

[Ben Pfaff]

On Wed, Dec 16, 2015 at 01:31:03PM +0300, Ilya Maximets wrote:
> 
> 
> On 16.12.2015 13:02, Ben Pfaff wrote:
> > On Wed, Dec 16, 2015 at 12:07:23PM +0300, Ilya Maximets wrote:
> >> On 16.12.2015 11:27, Ben Pfaff wrote:
> >>> On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote:
> 
> 
>  On 16.12.2015 09:53, Ben Pfaff wrote:
> > On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote:
> >> While killing OVS may not free all allocated resources.
> >>
> >> Eample:
> >>Socket for vhost-user port will stay in a system
> >>after 'systemctl stop openvswitch' and opening
> >>that port after restart will fail.
> >>
> >> Signed-off-by: Ilya Maximets 
> >> ---
> >>
> >> version 2:
> >>* added '-T 1'
> >>* '-t $1' --> '-t $rundir/$1.$pid.ctl'
> >
> > Thanks.
> >
> > Have you tested it?
> >
> 
>  Yes. On my setup it works.
>  Only reason why it put RFC to that patch:
>  Is all daemons have unixctl exit command? Or we can just let ovs-appctl 
>  to fail here
>  in that case?
> >>>
> >>> I think that all of our current daemons do have an "exit" command, but
> >>> it might be worth removing the "2" after EXIT.  The purpose of the
> >>> delays in this chain is to wait a bit for a given signal to be processed
> >>> through the operating system and the daemon, but EXIT should wait until
> >>> the process has actually exited, or at least get very close to that.
> >>> Maybe a short delay like .1 would be reasonable, to allow for a brief
> >>> race window.
> >>
> >> I don't think so, because ovs-vswitchd replies on connection to ovs-appctl
> >> immediately, but actual exiting performed while stopping execution in
> >> main loop.
> > 
> > Then that means that this patch will generally make it take about 4
> > longer to stop OVS, since killing each daemon will sleep for 2 seconds.
> 
> Ok. May be something like 'EXIT .1 .25 .65 1 1 TERM' will be better?

I'm more comfortable with that.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 06/14] actions: Factor out new helper function add_prerequisite().

[Ben Pfaff]

Thanks, applied to master.

On Thu, Dec 10, 2015 at 05:13:58PM -0800, Justin Pettit wrote:
> Acked-by: Justin Pettit 
> 
> --Justin
> 
> 
> > On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> > 
> > This will acquire new users in upcoming commits.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> > ovn/lib/actions.c | 24 +++-
> > 1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> > index 693b1c1..581dbae 100644
> > --- a/ovn/lib/actions.c
> > +++ b/ovn/lib/actions.c
> > @@ -183,6 +183,19 @@ parse_next_action(struct action_context *ctx)
> > }
> > }
> > 
> > +/* Parses 'prerequisite' as an expression in the context of 'ctx', then 
> > adds it
> > + * as a conjunction with the existing 'ctx->prereqs'. */
> > +static void
> > +add_prerequisite(struct action_context *ctx, const char *prerequisite)
> > +{
> > +struct expr *expr;
> > +char *error;
> > +
> > +expr = expr_parse_string(prerequisite, ctx->symtab, &error);
> > +ovs_assert(!error);
> > +ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
> > +}
> > +
> > static void
> > emit_ct(struct action_context *ctx, bool recirc_next, bool commit)
> > {
> > @@ -209,12 +222,7 @@ emit_ct(struct action_context *ctx, bool recirc_next, 
> > bool commit)
> > ct->alg = 0;
> > 
> > /* CT only works with IP, so set up a prerequisite. */
> > -struct expr *expr;
> > -char *error;
> > -
> > -expr = expr_parse_string("ip", ctx->symtab, &error);
> > -ovs_assert(!error);
> > -ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
> > +add_prerequisite(ctx, "ip");
> > }
> > 
> > static void
> > @@ -249,9 +257,7 @@ parse_actions(struct action_context *ctx)
> > emit_resubmit(ctx, ctx->output_ptable);
> > } else if (lexer_match_id(ctx->lexer, "ip.ttl")) {
> > if (lexer_match(ctx->lexer, LEX_T_DECREMENT)) {
> > -struct expr *e = expr_parse_string("ip", ctx->symtab,
> > -   &ctx->error);
> > -ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, e);
> > +add_prerequisite(ctx, "ip");
> > ofpact_put_DEC_TTL(ctx->ofpacts);
> > } else {
> > action_syntax_error(ctx, "expecting `--'");
> > -- 
> > 2.1.3
> > 
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 06/14] actions: Factor out new helper function add_prerequisite().

[Ben Pfaff]

Thanks, applied to master.

On Thu, Dec 10, 2015 at 05:13:58PM -0800, Justin Pettit wrote:
> Acked-by: Justin Pettit 
> 
> --Justin
> 
> 
> > On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> > 
> > This will acquire new users in upcoming commits.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> > ovn/lib/actions.c | 24 +++-
> > 1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> > index 693b1c1..581dbae 100644
> > --- a/ovn/lib/actions.c
> > +++ b/ovn/lib/actions.c
> > @@ -183,6 +183,19 @@ parse_next_action(struct action_context *ctx)
> > }
> > }
> > 
> > +/* Parses 'prerequisite' as an expression in the context of 'ctx', then 
> > adds it
> > + * as a conjunction with the existing 'ctx->prereqs'. */
> > +static void
> > +add_prerequisite(struct action_context *ctx, const char *prerequisite)
> > +{
> > +struct expr *expr;
> > +char *error;
> > +
> > +expr = expr_parse_string(prerequisite, ctx->symtab, &error);
> > +ovs_assert(!error);
> > +ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
> > +}
> > +
> > static void
> > emit_ct(struct action_context *ctx, bool recirc_next, bool commit)
> > {
> > @@ -209,12 +222,7 @@ emit_ct(struct action_context *ctx, bool recirc_next, 
> > bool commit)
> > ct->alg = 0;
> > 
> > /* CT only works with IP, so set up a prerequisite. */
> > -struct expr *expr;
> > -char *error;
> > -
> > -expr = expr_parse_string("ip", ctx->symtab, &error);
> > -ovs_assert(!error);
> > -ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
> > +add_prerequisite(ctx, "ip");
> > }
> > 
> > static void
> > @@ -249,9 +257,7 @@ parse_actions(struct action_context *ctx)
> > emit_resubmit(ctx, ctx->output_ptable);
> > } else if (lexer_match_id(ctx->lexer, "ip.ttl")) {
> > if (lexer_match(ctx->lexer, LEX_T_DECREMENT)) {
> > -struct expr *e = expr_parse_string("ip", ctx->symtab,
> > -   &ctx->error);
> > -ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, e);
> > +add_prerequisite(ctx, "ip");
> > ofpact_put_DEC_TTL(ctx->ofpacts);
> > } else {
> > action_syntax_error(ctx, "expecting `--'");
> > -- 
> > 2.1.3
> > 
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 07/14] actions: Factor parsing a single action into a new function parse_action().

[Ben Pfaff]

Thanks, applied to master.

On Thu, Dec 10, 2015 at 05:16:02PM -0800, Justin Pettit wrote:
> Acked-by: Justin Pettit 
> 
> --Justin
> 
> 
> > On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> > 
> > This will have another user in an upcoming commit.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> > ovn/lib/actions.c | 70 
> > +++
> > 1 file changed, 39 insertions(+), 31 deletions(-)
> > 
> > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> > index 581dbae..6bc452a 100644
> > --- a/ovn/lib/actions.c
> > +++ b/ovn/lib/actions.c
> > @@ -52,6 +52,8 @@ struct action_context {
> > struct expr *prereqs;   /* Prerequisites to apply to match. */
> > };
> > 
> > +static bool parse_action(struct action_context *);
> > +
> > static bool
> > action_error_handle_common(struct action_context *ctx)
> > {
> > @@ -225,6 +227,42 @@ emit_ct(struct action_context *ctx, bool recirc_next, 
> > bool commit)
> > add_prerequisite(ctx, "ip");
> > }
> > 
> > +static bool
> > +parse_action(struct action_context *ctx)
> > +{
> > +if (ctx->lexer->token.type != LEX_T_ID) {
> > +action_syntax_error(ctx, NULL);
> > +return false;
> > +}
> > +
> > +enum lex_type lookahead = lexer_lookahead(ctx->lexer);
> > +if (lookahead == LEX_T_EQUALS || lookahead == LEX_T_EXCHANGE
> > +|| lookahead == LEX_T_LSQUARE) {
> > +parse_set_action(ctx);
> > +} else if (lexer_match_id(ctx->lexer, "next")) {
> > +parse_next_action(ctx);
> > +} else if (lexer_match_id(ctx->lexer, "output")) {
> > +emit_resubmit(ctx, ctx->output_ptable);
> > +} else if (lexer_match_id(ctx->lexer, "ip.ttl")) {
> > +if (lexer_match(ctx->lexer, LEX_T_DECREMENT)) {
> > +add_prerequisite(ctx, "ip");
> > +ofpact_put_DEC_TTL(ctx->ofpacts);
> > +} else {
> > +action_syntax_error(ctx, "expecting `--'");
> > +}
> > +} else if (lexer_match_id(ctx->lexer, "ct_next")) {
> > +emit_ct(ctx, true, false);
> > +} else if (lexer_match_id(ctx->lexer, "ct_commit")) {
> > +emit_ct(ctx, false, true);
> > +} else {
> > +action_syntax_error(ctx, "expecting action");
> > +}
> > +if (!lexer_match(ctx->lexer, LEX_T_SEMICOLON)) {
> > +action_syntax_error(ctx, "expecting ';'");
> > +}
> > +return !ctx->error;
> > +}
> > +
> > static void
> > parse_actions(struct action_context *ctx)
> > {
> > @@ -242,37 +280,7 @@ parse_actions(struct action_context *ctx)
> > }
> > 
> > while (ctx->lexer->token.type != LEX_T_END) {
> > -if (ctx->lexer->token.type != LEX_T_ID) {
> > -action_syntax_error(ctx, NULL);
> > -break;
> > -}
> > -
> > -enum lex_type lookahead = lexer_lookahead(ctx->lexer);
> > -if (lookahead == LEX_T_EQUALS || lookahead == LEX_T_EXCHANGE
> > -|| lookahead == LEX_T_LSQUARE) {
> > -parse_set_action(ctx);
> > -} else if (lexer_match_id(ctx->lexer, "next")) {
> > -parse_next_action(ctx);
> > -} else if (lexer_match_id(ctx->lexer, "output")) {
> > -emit_resubmit(ctx, ctx->output_ptable);
> > -} else if (lexer_match_id(ctx->lexer, "ip.ttl")) {
> > -if (lexer_match(ctx->lexer, LEX_T_DECREMENT)) {
> > -add_prerequisite(ctx, "ip");
> > -ofpact_put_DEC_TTL(ctx->ofpacts);
> > -} else {
> > -action_syntax_error(ctx, "expecting `--'");
> > -}
> > -} else if (lexer_match_id(ctx->lexer, "ct_next")) {
> > -emit_ct(ctx, true, false);
> > -} else if (lexer_match_id(ctx->lexer, "ct_commit")) {
> > -emit_ct(ctx, false, true);
> > -} else {
> > -action_syntax_error(ctx, "expecting action");
> > -}
> > -if (!lexer_match(ctx->lexer, LEX_T_SEMICOLON)) {
> > -action_syntax_error(ctx, "expecting ';'");
> > -}
> > -if (ctx->error) {
> > +if (!parse_action(ctx)) {
> > return;
> > }
> > }
> > -- 
> > 2.1.3
> > 
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 09/14] actions: Bundle action parsing parameters into a structure.

[Ben Pfaff]

On Tue, Dec 15, 2015 at 05:20:01PM -0800, Justin Pettit wrote:
> 
> > On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> > 
> > This will make it easier to add and change parameters, as done in an
> > upcoming commit.
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

Thanks, applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v3 RFC] ovs-lib: try to call exit before killing

[Ilya Maximets]

While killing OVS may not free all allocated resources.

Eample:
Socket for vhost-user port will stay in a system
after 'systemctl stop openvswitch' and opening
that port after restart will fail.

Signed-off-by: Ilya Maximets 
---

version 3:
* sleep after EXIT splitted.

version 2:
* added '-T 1'
* '-t $1' --> '-t $rundir/$1.$pid.ctl'

 utilities/ovs-lib.in | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index dd8a1e9..773efb3 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -202,11 +202,18 @@ start_daemon () {
 stop_daemon () {
 if test -e "$rundir/$1.pid"; then
 if pid=`cat "$rundir/$1.pid"`; then
-for action in TERM .1 .25 .65 1 1 1 1 KILL 1 1 1 2 10 15 30 FAIL; 
do
+for action in EXIT .1 .25 .65 1 \
+  TERM .1 .25 .65 1 1 1 1 \
+  KILL 1 1 1 2 10 15 30 \
+  FAIL; do
 if pid_exists "$pid" >/dev/null 2>&1; then :; else
 return 0
 fi
 case $action in
+EXIT)
+action "Exiting $1 ($pid)" \
+${bindir}/ovs-appctl -T 1 -t $rundir/$1.$pid.ctl 
exit
+;;
 TERM)
 action "Killing $1 ($pid)" kill $pid
 ;;
-- 
2.1.4

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

Re: [ovs-dev] [PATCH v3 RFC] ovs-lib: try to call exit before killing

[Ben Pfaff]

On Wed, Dec 16, 2015 at 03:32:21PM +0300, Ilya Maximets wrote:
> While killing OVS may not free all allocated resources.
> 
> Eample:
>   Socket for vhost-user port will stay in a system
>   after 'systemctl stop openvswitch' and opening
>   that port after restart will fail.
> 
> Signed-off-by: Ilya Maximets 

Applied to master.  Thank you!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 RFC] ovs-lib: try to call exit before killing

[Ilya Maximets]

On 16.12.2015 15:21, Ben Pfaff wrote:
> On Wed, Dec 16, 2015 at 01:31:03PM +0300, Ilya Maximets wrote:
>>
>>
>> On 16.12.2015 13:02, Ben Pfaff wrote:
>>> On Wed, Dec 16, 2015 at 12:07:23PM +0300, Ilya Maximets wrote:
 On 16.12.2015 11:27, Ben Pfaff wrote:
> On Wed, Dec 16, 2015 at 09:58:56AM +0300, Ilya Maximets wrote:
>>
>>
>> On 16.12.2015 09:53, Ben Pfaff wrote:
>>> On Wed, Dec 16, 2015 at 09:50:56AM +0300, Ilya Maximets wrote:
 While killing OVS may not free all allocated resources.

 Eample:
Socket for vhost-user port will stay in a system
after 'systemctl stop openvswitch' and opening
that port after restart will fail.

 Signed-off-by: Ilya Maximets 
 ---

 version 2:
* added '-T 1'
* '-t $1' --> '-t $rundir/$1.$pid.ctl'
>>>
>>> Thanks.
>>>
>>> Have you tested it?
>>>
>>
>> Yes. On my setup it works.
>> Only reason why it put RFC to that patch:
>> Is all daemons have unixctl exit command? Or we can just let ovs-appctl 
>> to fail here
>> in that case?
>
> I think that all of our current daemons do have an "exit" command, but
> it might be worth removing the "2" after EXIT.  The purpose of the
> delays in this chain is to wait a bit for a given signal to be processed
> through the operating system and the daemon, but EXIT should wait until
> the process has actually exited, or at least get very close to that.
> Maybe a short delay like .1 would be reasonable, to allow for a brief
> race window.

 I don't think so, because ovs-vswitchd replies on connection to ovs-appctl
 immediately, but actual exiting performed while stopping execution in
 main loop.
>>>
>>> Then that means that this patch will generally make it take about 4
>>> longer to stop OVS, since killing each daemon will sleep for 2 seconds.
>>
>> Ok. May be something like 'EXIT .1 .25 .65 1 1 TERM' will be better?
> 
> I'm more comfortable with that.

OK. New version posted.

Best regards, Ilya Maximets.

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

Re: [ovs-dev] [PATCH 2/5] debian: Add a package for OVN common components.

[Russell Bryant]

On 12/16/2015 02:03 AM, b...@ovn.org wrote:
> On Tue, Dec 15, 2015 at 09:52:03AM -0800, Gurucharan Shetty wrote:
>> Signed-off-by: Gurucharan Shetty 
> 
> I'm not sure about the dependency on openvswitch-switch.  One could run
> northd and the nb and sb database on a machine that does not have an
> openvswitch switch running; it might even be preferred in a large
> deployment.  What do you think?

ovn-ctl assumes ovsdb-server is running locally for ovn-northd right
now.  Is ovsdb-server in the -switch package?

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

Re: [ovs-dev] [PATCH 1/5] ovn-ctl: Add daemon status functions.

[Russell Bryant]

On 12/16/2015 02:00 AM, Ben Pfaff wrote:
> On Tue, Dec 15, 2015 at 10:59:58PM -0800, Ben Pfaff wrote:
>> On Tue, Dec 15, 2015 at 09:52:02AM -0800, Gurucharan Shetty wrote:
>>> Signed-off-by: Gurucharan Shetty 
>>> ---
>>>  ovn/utilities/ovn-ctl |6 ++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
>>> index 3e2ccf9..b171934 100755
>>> --- a/ovn/utilities/ovn-ctl
>>> +++ b/ovn/utilities/ovn-ctl
>>> @@ -225,6 +225,12 @@ case $command in
>>>  restart_controller)
>>>  restart_controller
>>>  ;;
>>> +status_northd)
>>> +daemon_status ovn-northd || exit 1
>>> +;;
>>> +status_controller)
>>> +daemon_status ovn-controller || exit 1
>>> +;;
>>
>> Do you think it's worth breaking out individual statuses this way?
>> "ovs-ctl status" shows all OVS daemons' status; maybe "ovn-ctl status"
>> should follow the same pattern.
> 
> Oh, but in case you disagree, I don't feel strongly, so have an ack if
> you want:
> Acked-by: Ben Pfaff 

It seems reasonable that you might want to check the status of only
ovn-controller on a hypervisor, for example.  We could have a third
"status" arg that checks all though.

Acked-by: Russell Bryant 

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

Re: [ovs-dev] [PATCH 1/5] ovn-ctl: Add daemon status functions.

[Guru Shetty]

>
>
> Do you think it's worth breaking out individual statuses this way?
> "ovs-ctl status" shows all OVS daemons' status; maybe "ovn-ctl status"
> should follow the same pattern.
>
The reason I chose to break it was because I was splitting ovn-northd and
ovn-controller into separate packages with separate startup scripts and
since the 'status' for each of those startup scripts should only get the
corresponding ones.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/5] debian: Add a package for OVN common components.

[Guru Shetty]

>
> I'm not sure about the dependency on openvswitch-switch.  One could run
> northd and the nb and sb database on a machine that does not have an
> openvswitch switch running; it might even be preferred in a large
> deployment.  What do you think?
>

openvswitch-switch has ovs-lib and ovn-ctl depends on it, hence the
dependency. Rationale for including ovn-ctl, ovn-sbctl and ovn-nbctl in
this package:

ovn-ctl: needed to start ovn-northd and ovn-controller
ovn-nbctl and ovn-sbctl: one can use them over tcp/ssl from hypervisors to
get information about central components.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 5/5] debian: Add a package for OVN docker drivers.

[Guru Shetty]

On 16 December 2015 at 01:55, Ben Pfaff  wrote:

> On Tue, Dec 15, 2015 at 09:52:06AM -0800, Gurucharan Shetty wrote:
> > Signed-off-by: Gurucharan Shetty 
>
> I think that this is architecture-independent (there's no actual
> binaries in it, just Python source), so that it should be Architecture:
> "all" instead of "linux-any".
>
I will correct it.


>
> Is there a reason someone wouldn't want to install this?  That is, is
> there a reason not to fold it into the -common package?
>
One reason I separated it out:
Eventually, a startup script to automatically start these drivers during
system startup.
If you feel that we are adding too many packages, I will reconsider the
approach.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/5] debian: Add a package for ovn-controller.

[Guru Shetty]

On 16 December 2015 at 01:39, Ben Pfaff  wrote:

> On Tue, Dec 15, 2015 at 09:52:04AM -0800, Gurucharan Shetty wrote:
> > Signed-off-by: Gurucharan Shetty 
>
> I wonder whether the name of this package is well chosen.  I know that
> the program that it starts is ovn-controller, and I know that that runs
> on every hypervisor in OVN, but still at first I assumed that any
> package with "controller" in the name would run centrally, that is, my
> first guess was that it would run ovn-northd or the nb or sb
> ovsdb-server.  So maybe it would confuse others too.  If you agree, then
> maybe we should name it openvswitch-ovn-hypervisor or -host or something
> of that sort.
>

Let me rename it to '-host'.


>
> On another naming note, I wonder whether we should drop the
> "openvswitch-" prefix from these package names.  I'm not sure it's
> helpful.
>
I think I gave it the name "openvswitch-" because I was using files from
/usr/share/openvswitch and installing files there too. I guess, I can
remove it and linitian won't complain. Let me try.


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

Re: [ovs-dev] Bug: ovsdb monitor2 - includes unknown object

[Han Zhou]

Justin, you are right. I am sorry it was my NTP out of sync when I sent the
first patch, so the message was shown up in the middle of the earlier
messages. Sorry for the confusion!

On Wed, Dec 16, 2015 at 12:08 AM, Justin Pettit  wrote:

>
> > On Dec 15, 2015, at 7:13 PM, Han Zhou  wrote:
> >
> > For some reason my patch was blocked by the mailinglist, so I just
> removed
> > the http link from commit message and resent. Now it is seen in the
> > mailinglist:
> >
> > http://openvswitch.org/pipermail/dev/2015-December/063461.html
>
> Ben is right that our spam filter isn't great, but I went through the
> logs, and it showed it allowed your messages.  I also received this
> message, which is also in the archives that appears to have the URL:
>
> http://openvswitch.org/pipermail/dev/2015-December/063459.html
>
> I'm happy to add you to the white-list, but I'm curious if there was
> actually a problem.
>
> --Justin
>
>
>


-- 
Best regards,
Han
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 11/14] ovn-controller: Add data structure for indexing lports, multicast groups.

[Numan Siddique]

On 12/09/2015 06:38 AM, Ben Pfaff wrote:
> This was more or less implemented inside lflow.c until now, but some
> upcoming code that shouldn't be in that file needs to use it too.
>
> This also adds a second index on lports, so that lports can be looked up
> based on the logical datapath tunnel key and the logical port tunnel key.
> An upcoming commit will add a user for this new index.
>
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/automake.mk  |   2 +
>  ovn/controller/lflow.c  | 165 
> +---
>  ovn/controller/lflow.h  |   7 +-
>  ovn/controller/lport.c  | 157 ++
>  ovn/controller/lport.h  |  69 +
>  ovn/controller/ovn-controller.c |  33 
>  6 files changed, 288 insertions(+), 145 deletions(-)
>  create mode 100644 ovn/controller/lport.c
>  create mode 100644 ovn/controller/lport.h
>
> diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
> index cadfa9c..cf57bbd 100644
> --- a/ovn/controller/automake.mk
> +++ b/ovn/controller/automake.mk
> @@ -8,6 +8,8 @@ ovn_controller_ovn_controller_SOURCES = \
>   ovn/controller/encaps.h \
>   ovn/controller/lflow.c \
>   ovn/controller/lflow.h \
> + ovn/controller/lport.c \
> + ovn/controller/lport.h \
>   ovn/controller/ofctrl.c \
>   ovn/controller/ofctrl.h \
>   ovn/controller/pinctrl.c \
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index dcabe2f..2bfc9e1 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -15,12 +15,13 @@
>  
>  #include 
>  #include "lflow.h"
> +#include "lport.h"
>  #include "dynamic-string.h"
>  #include "ofctrl.h"
>  #include "ofp-actions.h"
>  #include "ofpbuf.h"
>  #include "openvswitch/vlog.h"
> -#include "ovn/controller/ovn-controller.h"
> +#include "ovn-controller.h"
>  #include "ovn/lib/actions.h"
>  #include "ovn/lib/expr.h"
>  #include "ovn/lib/ovn-sb-idl.h"
> @@ -42,8 +43,8 @@ add_logical_register(struct shash *symtab, enum mf_field_id 
> id)
>  expr_symtab_add_field(symtab, name, id, NULL, false);
>  }
>  
> -static void
> -symtab_init(void)
> +void
> +lflow_init(void)
>  {
>  shash_init(&symtab);
>  
> @@ -147,149 +148,48 @@ symtab_init(void)
>  expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
>  }
>  
> -/* Logical datapaths and logical port numbers. */
> -
> -/* A logical datapath.
> - *
> - * 'ports' maps 'logical_port' names to 'tunnel_key' values in the OVN_SB
> - * Port_Binding table within the logical datapath. */
> -struct logical_datapath {
> -struct hmap_node hmap_node; /* Indexed on 'uuid'. */
> -struct uuid uuid;   /* UUID from Datapath_Binding row. */
> -uint32_t tunnel_key;/* 'tunnel_key' from Datapath_Binding row. */
> -struct simap ports; /* Logical port name to port number. */
> +struct lookup_port_aux {
> +const struct lport_index *lports;
> +const struct mcgroup_index *mcgroups;
> +const struct sbrec_datapath_binding *dp;
>  };
>  
> -/* Contains "struct logical_datapath"s. */
> -static struct hmap logical_datapaths = HMAP_INITIALIZER(&logical_datapaths);
> -
> -/* Finds and returns the logical_datapath for 'binding', or NULL if no such
> - * logical_datapath exists. */
> -static struct logical_datapath *
> -ldp_lookup(const struct sbrec_datapath_binding *binding)
> -{
> -struct logical_datapath *ldp;
> -HMAP_FOR_EACH_IN_BUCKET (ldp, hmap_node, 
> uuid_hash(&binding->header_.uuid),
> - &logical_datapaths) {
> -if (uuid_equals(&ldp->uuid, &binding->header_.uuid)) {
> -return ldp;
> -}
> -}
> -return NULL;
> -}
> -
> -/* Creates a new logical_datapath for the given 'binding'. */
> -static struct logical_datapath *
> -ldp_create(const struct sbrec_datapath_binding *binding)
> -{
> -struct logical_datapath *ldp;
> -
> -ldp = xmalloc(sizeof *ldp);
> -hmap_insert(&logical_datapaths, &ldp->hmap_node,
> -uuid_hash(&binding->header_.uuid));
> -ldp->uuid = binding->header_.uuid;
> -ldp->tunnel_key = binding->tunnel_key;
> -simap_init(&ldp->ports);
> -return ldp;
> -}
> -
> -static struct logical_datapath *
> -ldp_lookup_or_create(const struct sbrec_datapath_binding *binding)
> -{
> -struct logical_datapath *ldp = ldp_lookup(binding);
> -return ldp ? ldp : ldp_create(binding);
> -}
> -
> -static void
> -ldp_free(struct logical_datapath *ldp)
> -{
> -simap_destroy(&ldp->ports);
> -hmap_remove(&logical_datapaths, &ldp->hmap_node);
> -free(ldp);
> -}
> -
> -/* Iterates through all of the records in the Port_Binding table, updating 
> the
> - * table of logical_datapaths to match the values found in active
> - * Port_Bindings. */
> -static void
> -ldp_run(struct controller_ctx *ctx)
> +static bool
> +lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
>  {
> 

[ovs-dev] Your account has a debt and is past due

[Leroy Black]

Dear Customer,

Our records show that your account has a debt of $363.{rand(10,99)}}. Previous 
attempts of collecting this sum have failed.

Down below you can find an attached file with the information on your case.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/5] debian: Add a package for OVN common components.

[Guru Shetty]

>
>
> ovn-ctl assumes ovsdb-server is running locally for ovn-northd right
> now.  Is ovsdb-server in the -switch package?
>
> Yes.

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

Re: [ovs-dev] [PATCH] match: Add support for matching IGMP fields.

[Jarno Rajahalme]

> On Dec 15, 2015, at 5:48 PM, Jarno Rajahalme  wrote:
> 
> 
>> On Dec 15, 2015, at 5:47 PM, Ben Pfaff mailto:b...@ovn.org>> 
>> wrote:
>> 
>> On Mon, Dec 14, 2015 at 04:36:08PM -0800, Jarno Rajahalme wrote:
>>> 
 On Dec 14, 2015, at 3:12 AM, Ben Pfaff >>> > wrote:
 
 On Thu, Dec 10, 2015 at 01:42:41PM -0800, Jarno Rajahalme wrote:
> Complete the IGMP protocol support by making IGMP fields (type, code,
> and group) matchable via OpenFlow by the way of new Nicira extensions.
> 
> The new fields are: 8-bit NXM_NX_IGMP_TYPE (111), 8-bit
> NXM_NX_IGMP_CODE (112), and 32-bit NXM_NX_IGMP_GROUP (113).
> 
> VMware-BZ: #1558992
> Signed-off-by: Jarno Rajahalme mailto:ja...@ovn.org>>
 
 Is this something we're targeting to backport to OVS 2.5?  If not, then
 the meta-flow.h headers should say "since v2.6" instead of "since
 v2.5".
 
 This needs to add an item to NEWS and documentation to ovs-ofctl(8).
 
 What's here looks good, and I trust you to do a good job on the above,
 so:
 Acked-by: Ben Pfaff mailto:b...@ovn.org>>
>>> 
>>> While doing this I just realized that this may require further work,
>>> as the kernel module does not seem to parse the IGMP fields!
>> 
>> Userspace should be able to handle that.  It might require a little work
>> in odp-util.c to mark the received packet as ODP_FIT_TOO_LITTLE.
> 
> Justin suggested the same offline. I’ll have a look at this tomorrow,
> 

It seems to me that the users in upcall processing and revalidation only care 
about ODP_FIT_ERROR, i.e., ODP_FIT_TOO_LITTLE and ODP_FIT_TOO_MUCH get the same 
treatment as ODP_FIT_PERFECT. However, to work as intended, at least for IGMP 
matching, ODP_FIT_TOO_LITTLE flows should be slow-pathed (a new SLOW_MATCH is 
needed), and ODP_FIT_TOO_MUCH flows should be exact matched (i.e., 
non-mega-flow-ed). Making these changes and making sure we don’t introduce new 
revalidation bugs would take more time than I have now.

  Jarno

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

Re: [ovs-dev] [PATCH] match: Add support for matching IGMP fields.

[Jarno Rajahalme]

> On Dec 16, 2015, at 1:22 PM, Jarno Rajahalme  wrote:
> 
> 
>> On Dec 15, 2015, at 5:48 PM, Jarno Rajahalme > > wrote:
>> 
>> 
>>> On Dec 15, 2015, at 5:47 PM, Ben Pfaff mailto:b...@ovn.org>> 
>>> wrote:
>>> 
>>> On Mon, Dec 14, 2015 at 04:36:08PM -0800, Jarno Rajahalme wrote:
 
> On Dec 14, 2015, at 3:12 AM, Ben Pfaff  > wrote:
> 
> On Thu, Dec 10, 2015 at 01:42:41PM -0800, Jarno Rajahalme wrote:
>> Complete the IGMP protocol support by making IGMP fields (type, code,
>> and group) matchable via OpenFlow by the way of new Nicira extensions.
>> 
>> The new fields are: 8-bit NXM_NX_IGMP_TYPE (111), 8-bit
>> NXM_NX_IGMP_CODE (112), and 32-bit NXM_NX_IGMP_GROUP (113).
>> 
>> VMware-BZ: #1558992
>> Signed-off-by: Jarno Rajahalme mailto:ja...@ovn.org>>
> 
> Is this something we're targeting to backport to OVS 2.5?  If not, then
> the meta-flow.h headers should say "since v2.6" instead of "since
> v2.5".
> 
> This needs to add an item to NEWS and documentation to ovs-ofctl(8).
> 
> What's here looks good, and I trust you to do a good job on the above,
> so:
> Acked-by: Ben Pfaff mailto:b...@ovn.org>>
 
 While doing this I just realized that this may require further work,
 as the kernel module does not seem to parse the IGMP fields!
>>> 
>>> Userspace should be able to handle that.  It might require a little work
>>> in odp-util.c to mark the received packet as ODP_FIT_TOO_LITTLE.
>> 
>> Justin suggested the same offline. I’ll have a look at this tomorrow,
>> 
> 
> It seems to me that the users in upcall processing and revalidation only care 
> about ODP_FIT_ERROR, i.e., ODP_FIT_TOO_LITTLE and ODP_FIT_TOO_MUCH get the 
> same treatment as ODP_FIT_PERFECT. However, to work as intended, at least for 
> IGMP matching, ODP_FIT_TOO_LITTLE flows should be slow-pathed (a new 
> SLOW_MATCH is needed), and ODP_FIT_TOO_MUCH flows should be exact matched 
> (i.e., non-mega-flow-ed). Making these changes and making sure we don’t 
> introduce new revalidation bugs would take more time than I have now.
> 

I was too fast here, the ODP_FIT_TOO_MUCH is already treated correctly, as the 
unknown-to-userspace fields are left wildcarded in the megaflow. However, 
slowpathing ODP_FIT_TOO_LITTLE flow keys needs to be added in the case where 
the fields not understood by the datapath are used in the flow translation. 
Daniele promised to take a stab at it.

  Jarno

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

Re: [ovs-dev] [PATCH] ovsdb: separate json cache for different monitor versions

[Andy Zhou]

On Tue, Dec 15, 2015 at 8:24 PM, Ben Pfaff  wrote:

> On Tue, Dec 15, 2015 at 07:06:34PM -0800, Han Zhou wrote:
> > Cached json objects were reused when sending notifications to
> > clients. This created a problem when there were different versions
> > of monitors coexiting. E.g. clients expecting version2 notification
> > would receive messages with method == "update2" but payload in
> > version1 format, which end up failure of processing the updates.
> >
> > This patch fixes the issue by using dedicated cache for each version.
> >
> > Signed-off-by: Han Zhou 
>
> Looks good at a quick skim.
>
> Andy, will you review this?
>
Sure,  Han, thanks for the bug report and fix. I will take a closer look at
the patch.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] OVS/OVN: Interface with NAT

[Amitabha Biswas]

Hi Jarno,

As part of OVN integration in Openstack, I’ve been trying to setup OpenFlow NAT 
rules and run the system-test-suite 33: system-traffic.at:1396 conntrack - 
simple SNAT. Using the latest (as of 12/15/2015) ovs master branch on Ubuntu 
Linux 4.3 kernels (from kernel.ubuntu.com ) fail 
with the following errors:

> 2015-12-16T03:05:13.945Z|00037|connmgr|INFO|br0<->unix: sending 
> OFPBAC_BAD_TYPE error reply to OFPT_FLOW_MOD message
> 2015-12-16T03:05:13.945Z|00038|vconn|DBG|unix: sent (Success): OFPT_ERROR 
> (OF1.4) (xid=0x2): OFPBAC_BAD_TYPE

related to the flow
in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255)),2

Am I missing some conntrack kernel patches to get the OpenFlow NAT rules 
installed? If yes, how would one get those patches and which kernel images 
would contain them.

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

[ovs-dev] [PATCH v2 3/5] debian: Add a package for host components.

[Gurucharan Shetty]

Signed-off-by: Gurucharan Shetty 
---
v1-v2:
change the name of package from openvswitch-ovn-controller to ovn-host
---
 debian/.gitignore|1 +
 debian/automake.mk   |7 ++
 debian/control   |   16 ++
 debian/ovn-host.dirs |1 +
 debian/ovn-host.init |   53 ++
 debian/ovn-host.install  |1 +
 debian/ovn-host.manpages |1 +
 debian/ovn-host.postinst |   49 ++
 debian/ovn-host.postrm   |   44 ++
 debian/ovn-host.template |5 +
 debian/rules |3 +++
 11 files changed, 181 insertions(+)
 create mode 100644 debian/ovn-host.dirs
 create mode 100755 debian/ovn-host.init
 create mode 100644 debian/ovn-host.install
 create mode 100644 debian/ovn-host.manpages
 create mode 100755 debian/ovn-host.postinst
 create mode 100755 debian/ovn-host.postrm
 create mode 100644 debian/ovn-host.template

diff --git a/debian/.gitignore b/debian/.gitignore
index fdcf9b3..a7a2be8 100644
--- a/debian/.gitignore
+++ b/debian/.gitignore
@@ -19,5 +19,6 @@
 /openvswitch-testcontroller
 /openvswitch-vtep
 /ovn-common
+/ovn-host
 /python-openvswitch
 /tmp
diff --git a/debian/automake.mk b/debian/automake.mk
index 2b6f098..422cdc9 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -52,6 +52,13 @@ EXTRA_DIST += \
debian/openvswitch-vtep.manpages \
debian/ovn-common.install \
debian/ovn-common.manpages \
+   debian/ovn-host.dirs \
+   debian/ovn-host.init \
+   debian/ovn-host.install \
+   debian/ovn-host.manpages \
+   debian/ovn-host.postinst \
+   debian/ovn-host.postrm \
+   debian/ovn-host.template \
debian/ovs-monitor-ipsec \
debian/python-openvswitch.dirs \
debian/python-openvswitch.install \
diff --git a/debian/control b/debian/control
index 57285f1..725f552 100644
--- a/debian/control
+++ b/debian/control
@@ -112,6 +112,22 @@ Description: OVN common components
  .
  ovn-common provides components required by other OVN packages.
 
+Package: ovn-host
+Architecture: linux-any
+Depends: openvswitch-switch (= ${binary:Version}),
+ openvswitch-common (= ${binary:Version}),
+ ovn-common (= ${binary:Version}),
+ ${misc:Depends},
+ ${shlibs:Depends}
+Description: OVN host components
+ OVN, the Open Virtual Network, is a system to support virtual network
+ abstraction.  OVN complements the existing capabilities of OVS to add
+ native support for virtual network abstractions, such as virtual L2 and L3
+ overlays and security groups.
+ .
+ ovn-host provides the userspace components and utilities for
+ OVN that can be run on every host/hypervisor.
+
 Package: openvswitch-ipsec
 Architecture: linux-any
 Depends: ipsec-tools (>=0.8~alpha20101208),
diff --git a/debian/ovn-host.dirs b/debian/ovn-host.dirs
new file mode 100644
index 000..7d3c761
--- /dev/null
+++ b/debian/ovn-host.dirs
@@ -0,0 +1 @@
+/usr/share/ovn/host
diff --git a/debian/ovn-host.init b/debian/ovn-host.init
new file mode 100755
index 000..bddf628
--- /dev/null
+++ b/debian/ovn-host.init
@@ -0,0 +1,53 @@
+#! /bin/sh
+#
+### BEGIN INIT INFO
+# Provides:  ovn-host
+# Required-Start:openvswitch-switch $remote_fs $syslog
+# Required-Stop: $remote_fs
+# Default-Start: 2 3 4 5
+# Default-Stop:  0 1 6
+# Short-Description: OVN host components
+# Description:   ovn-host provides the userspace
+#components and utilities for OVN that can be run on
+#every host/hypervisor.
+### END INIT INFO
+
+test -x /usr/bin/ovn-controller  || exit 0
+test -x /usr/share/openvswitch/scripts/ovn-ctl || exit 0
+
+_SYSTEMCTL_SKIP_REDIRECT=yes
+
+. /usr/share/openvswitch/scripts/ovs-lib
+if [ -e /etc/default/ovn-host ]; then
+. /etc/default/ovn-host
+fi
+
+start () {
+set /usr/share/openvswitch/scripts/ovn-ctl ${1-start_controller}
+set "$@" $OVN_CTL_OPTS
+"$@" || exit $?
+}
+
+case $1 in
+start)
+start
+;;
+stop | force-stop)
+/usr/share/openvswitch/scripts/ovn-ctl stop_controller
+;;
+restart)
+start restart_controller
+;;
+status)
+/usr/share/openvswitch/scripts/ovn-ctl status_controller
+exit $?
+;;
+reload | force-reload)
+;;
+*)
+echo "Usage: $0 {start|stop|reload|force-reload|restart|status}" >&2
+exit 1
+;;
+esac
+
+exit 0
diff --git a/debian/ovn-host.install b/debian/ovn-host.install
new file mode 100644
index 000..d2de82f
--- /dev/null
+++ b/debian/ovn-host.install
@@ -0,0 +1 @@
+usr/bin/ovn-controller
diff --git a/debian/ovn-host.manpages b/debian/ovn-host.manpages
new file mode 100644
index 000..4f9e7bc
--- /dev/null
+++ b/debian/ovn-host.manpages
@@ -0,0 +1 @@
+ovn/controller/ovn-controller.8
diff --git a/debian/ovn-host.postinst b/debian/ovn-host.postinst
new fi

[ovs-dev] [PATCH v2 2/5] debian: Add a package for OVN common components.

[Gurucharan Shetty]

Signed-off-by: Gurucharan Shetty 
---
v1-v2:
change name of the package from openvswitch-ovn-common to ovn-common
---
 debian/.gitignore  |1 +
 debian/automake.mk |2 ++
 debian/control |   14 ++
 debian/ovn-common.install  |3 +++
 debian/ovn-common.manpages |4 
 5 files changed, 24 insertions(+)
 create mode 100644 debian/ovn-common.install
 create mode 100644 debian/ovn-common.manpages

diff --git a/debian/.gitignore b/debian/.gitignore
index e8d9d31..fdcf9b3 100644
--- a/debian/.gitignore
+++ b/debian/.gitignore
@@ -18,5 +18,6 @@
 /openvswitch-test
 /openvswitch-testcontroller
 /openvswitch-vtep
+/ovn-common
 /python-openvswitch
 /tmp
diff --git a/debian/automake.mk b/debian/automake.mk
index c29a560..2b6f098 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -50,6 +50,8 @@ EXTRA_DIST += \
debian/openvswitch-vtep.init \
debian/openvswitch-vtep.install \
debian/openvswitch-vtep.manpages \
+   debian/ovn-common.install \
+   debian/ovn-common.manpages \
debian/ovs-monitor-ipsec \
debian/python-openvswitch.dirs \
debian/python-openvswitch.install \
diff --git a/debian/control b/debian/control
index 49d6f5f..57285f1 100644
--- a/debian/control
+++ b/debian/control
@@ -98,6 +98,20 @@ Description: Open vSwitch switch implementations
  openvswitch-switch provides the userspace components and utilities for
  the Open vSwitch kernel-based switch.
 
+Package: ovn-common
+Architecture: linux-any
+Depends: openvswitch-switch (= ${binary:Version}),
+ openvswitch-common (= ${binary:Version}),
+ ${misc:Depends},
+ ${shlibs:Depends}
+Description: OVN common components
+ OVN, the Open Virtual Network, is a system to support virtual network
+ abstraction.  OVN complements the existing capabilities of OVS to add
+ native support for virtual network abstractions, such as virtual L2 and L3
+ overlays and security groups.
+ .
+ ovn-common provides components required by other OVN packages.
+
 Package: openvswitch-ipsec
 Architecture: linux-any
 Depends: ipsec-tools (>=0.8~alpha20101208),
diff --git a/debian/ovn-common.install b/debian/ovn-common.install
new file mode 100644
index 000..acb1dc9
--- /dev/null
+++ b/debian/ovn-common.install
@@ -0,0 +1,3 @@
+usr/bin/ovn-nbctl
+usr/bin/ovn-sbctl
+usr/share/openvswitch/scripts/ovn-ctl
diff --git a/debian/ovn-common.manpages b/debian/ovn-common.manpages
new file mode 100644
index 000..d48d801
--- /dev/null
+++ b/debian/ovn-common.manpages
@@ -0,0 +1,4 @@
+ovn/ovn-architecture.7
+ovn/utilities/ovn-ctl.8
+ovn/utilities/ovn-nbctl.8
+ovn/utilities/ovn-sbctl.8
-- 
1.7.9.5

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

[ovs-dev] [PATCH v2 4/5] debian: Add a package for OVN central components.

[Gurucharan Shetty]

Signed-off-by: Gurucharan Shetty 
---
v1-v2:
change name from openvswitch-ovn-central to ovn-central
---
 debian/.gitignore   |1 +
 debian/automake.mk  |7 ++
 debian/control  |   16 +
 debian/ovn-central.dirs |1 +
 debian/ovn-central.init |   53 +++
 debian/ovn-central.install  |3 +++
 debian/ovn-central.manpages |3 +++
 debian/ovn-central.postinst |   49 +++
 debian/ovn-central.postrm   |   48 +++
 debian/ovn-central.template |5 
 debian/rules|3 +++
 11 files changed, 189 insertions(+)
 create mode 100644 debian/ovn-central.dirs
 create mode 100755 debian/ovn-central.init
 create mode 100644 debian/ovn-central.install
 create mode 100644 debian/ovn-central.manpages
 create mode 100755 debian/ovn-central.postinst
 create mode 100755 debian/ovn-central.postrm
 create mode 100644 debian/ovn-central.template

diff --git a/debian/.gitignore b/debian/.gitignore
index a7a2be8..40fea69 100644
--- a/debian/.gitignore
+++ b/debian/.gitignore
@@ -20,5 +20,6 @@
 /openvswitch-vtep
 /ovn-common
 /ovn-host
+/ovn-central
 /python-openvswitch
 /tmp
diff --git a/debian/automake.mk b/debian/automake.mk
index 422cdc9..01697b6 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -50,6 +50,13 @@ EXTRA_DIST += \
debian/openvswitch-vtep.init \
debian/openvswitch-vtep.install \
debian/openvswitch-vtep.manpages \
+   debian/ovn-central.dirs \
+   debian/ovn-central.init \
+   debian/ovn-central.install \
+   debian/ovn-central.manpages \
+   debian/ovn-central.postinst \
+   debian/ovn-central.postrm \
+   debian/ovn-central.template \
debian/ovn-common.install \
debian/ovn-common.manpages \
debian/ovn-host.dirs \
diff --git a/debian/control b/debian/control
index 725f552..2e315e2 100644
--- a/debian/control
+++ b/debian/control
@@ -128,6 +128,22 @@ Description: OVN host components
  ovn-host provides the userspace components and utilities for
  OVN that can be run on every host/hypervisor.
 
+Package: ovn-central
+Architecture: linux-any
+Depends: openvswitch-switch (= ${binary:Version}),
+ openvswitch-common (= ${binary:Version}),
+ ovn-common (= ${binary:Version}),
+ ${misc:Depends},
+ ${shlibs:Depends}
+Description: OVN central components
+ OVN, the Open Virtual Network, is a system to support virtual network
+ abstraction.  OVN complements the existing capabilities of OVS to add
+ native support for virtual network abstractions, such as virtual L2 and L3
+ overlays and security groups.
+ .
+ ovn-central provides the userspace daemons, utilities and
+ databases for OVN that is run at a central location.
+
 Package: openvswitch-ipsec
 Architecture: linux-any
 Depends: ipsec-tools (>=0.8~alpha20101208),
diff --git a/debian/ovn-central.dirs b/debian/ovn-central.dirs
new file mode 100644
index 000..6394883
--- /dev/null
+++ b/debian/ovn-central.dirs
@@ -0,0 +1 @@
+/usr/share/ovn/central
diff --git a/debian/ovn-central.init b/debian/ovn-central.init
new file mode 100755
index 000..a75192f
--- /dev/null
+++ b/debian/ovn-central.init
@@ -0,0 +1,53 @@
+#! /bin/sh
+#
+### BEGIN INIT INFO
+# Provides:  ovn-central
+# Required-Start:openvswitch-switch $remote_fs $syslog
+# Required-Stop: $remote_fs
+# Default-Start: 2 3 4 5
+# Default-Stop:  0 1 6
+# Short-Description: OVN central components
+# Description:   ovn-central provides the userspace daemons,
+#utilities and databases for OVN that is run at a central
+#location.
+### END INIT INFO
+
+test -x /usr/bin/ovn-northd  || exit 0
+test -x /usr/share/openvswitch/scripts/ovn-ctl || exit 0
+
+_SYSTEMCTL_SKIP_REDIRECT=yes
+
+. /usr/share/openvswitch/scripts/ovs-lib
+if [ -e /etc/default/ovn-central ]; then
+. /etc/default/ovn-central
+fi
+
+start () {
+set /usr/share/openvswitch/scripts/ovn-ctl ${1-start_northd}
+set "$@" $OVN_CTL_OPTS
+"$@" || exit $?
+}
+
+case $1 in
+start)
+start
+;;
+stop)
+/usr/share/openvswitch/scripts/ovn-ctl stop_northd
+;;
+restart)
+start restart_northd
+;;
+reload | force-reload)
+;;
+status)
+/usr/share/openvswitch/scripts/ovn-ctl status_northd
+exit $?
+;;
+*)
+echo "Usage: $0 {start|stop|reload|force-reload|restart|status}" >&2
+exit 1
+;;
+esac
+
+exit 0
diff --git a/debian/ovn-central.install b/debian/ovn-central.install
new file mode 100644
index 000..733d3fc
--- /dev/null
+++ b/debian/ovn-central.install
@@ -0,0 +1,3 @@
+usr/bin/ovn-northd
+usr/share/openvswitch/ovn-nb.ovsschema
+usr/share/openvswitch/ovn-sb.ovsschema
diff --git a/debian/ovn-central.manpages b/debian/ovn-central.manpages
new file mode 100644
index

[ovs-dev] [PATCH v2 5/5] debian: Add a package for OVN docker drivers.

[Gurucharan Shetty]

Signed-off-by: Gurucharan Shetty 
---
v1-v2: change the name from openvswitch-ovn-docker to ovn-docker
---
 debian/.gitignore |1 +
 debian/automake.mk|1 +
 debian/control|   18 ++
 debian/ovn-docker.install |2 ++
 4 files changed, 22 insertions(+)
 create mode 100644 debian/ovn-docker.install

diff --git a/debian/.gitignore b/debian/.gitignore
index 40fea69..2a509ca 100644
--- a/debian/.gitignore
+++ b/debian/.gitignore
@@ -21,5 +21,6 @@
 /ovn-common
 /ovn-host
 /ovn-central
+/ovn-docker
 /python-openvswitch
 /tmp
diff --git a/debian/automake.mk b/debian/automake.mk
index 01697b6..daaf327 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -59,6 +59,7 @@ EXTRA_DIST += \
debian/ovn-central.template \
debian/ovn-common.install \
debian/ovn-common.manpages \
+debian/ovn-docker.install \
debian/ovn-host.dirs \
debian/ovn-host.init \
debian/ovn-host.install \
diff --git a/debian/control b/debian/control
index 2e315e2..53250dd 100644
--- a/debian/control
+++ b/debian/control
@@ -144,6 +144,24 @@ Description: OVN central components
  ovn-central provides the userspace daemons, utilities and
  databases for OVN that is run at a central location.
 
+Package: ovn-docker
+Architecture: any
+Depends: openvswitch-switch (= ${binary:Version}),
+ openvswitch-common (= ${binary:Version}),
+ python (>= 2.7),
+ python-openvswitch (= ${source:Version}),
+ ovn-common (= ${binary:Version}),
+ ${misc:Depends},
+ ${python:Depends},
+ ${shlibs:Depends}
+Description: OVN Docker drivers
+ OVN, the Open Virtual Network, is a system to support virtual network
+ abstraction.  OVN complements the existing capabilities of OVS to add
+ native support for virtual network abstractions, such as virtual L2 and L3
+ overlays and security groups.
+ .
+ ovn-docker provides the docker drivers for OVN.
+
 Package: openvswitch-ipsec
 Architecture: linux-any
 Depends: ipsec-tools (>=0.8~alpha20101208),
diff --git a/debian/ovn-docker.install b/debian/ovn-docker.install
new file mode 100644
index 000..5833067
--- /dev/null
+++ b/debian/ovn-docker.install
@@ -0,0 +1,2 @@
+usr/bin/ovn-docker-overlay-driver
+usr/bin/ovn-docker-underlay-driver
-- 
1.7.9.5

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

[ovs-dev] [PATCH v2 1/5] ovn-ctl: Add daemon status functions.

[Gurucharan Shetty]

Signed-off-by: Gurucharan Shetty 
Acked-by: Russell Bryant 
Acked-by: Ben Pfaff 
---
patch already applied.
---
 ovn/utilities/ovn-ctl |6 ++
 1 file changed, 6 insertions(+)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 3e2ccf9..b171934 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -225,6 +225,12 @@ case $command in
 restart_controller)
 restart_controller
 ;;
+status_northd)
+daemon_status ovn-northd || exit 1
+;;
+status_controller)
+daemon_status ovn-controller || exit 1
+;;
 help)
 usage
 ;;
-- 
1.7.9.5

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

[ovs-dev] [PATCH net-next v5 3/8] netfilter: Allow calling into nat helper without skb_dst.

[Jarno Rajahalme]

NAT checksum recalculation code assumes existence of skb_dst, which
becomes a problem for a later patch in the series ("openvswitch:
Interface with NAT.").  Simplify this by removing the check on
skb_dst, as the checksum will be dealt with later in the stack.

Suggested-by: Pravin Shelar 
Signed-off-by: Jarno Rajahalme 
---
 net/ipv4/netfilter/nf_nat_l3proto_ipv4.c | 30 --
 net/ipv6/netfilter/nf_nat_l3proto_ipv6.c | 30 --
 2 files changed, 16 insertions(+), 44 deletions(-)

diff --git a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c 
b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
index 61c7cc2..f8aad03 100644
--- a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
@@ -127,29 +127,15 @@ static void nf_nat_ipv4_csum_recalc(struct sk_buff *skb,
u8 proto, void *data, __sum16 *check,
int datalen, int oldlen)
 {
-   const struct iphdr *iph = ip_hdr(skb);
-   struct rtable *rt = skb_rtable(skb);
-
if (skb->ip_summed != CHECKSUM_PARTIAL) {
-   if (!(rt->rt_flags & RTCF_LOCAL) &&
-   (!skb->dev || skb->dev->features &
-(NETIF_F_IP_CSUM | NETIF_F_HW_CSUM))) {
-   skb->ip_summed = CHECKSUM_PARTIAL;
-   skb->csum_start = skb_headroom(skb) +
- skb_network_offset(skb) +
- ip_hdrlen(skb);
-   skb->csum_offset = (void *)check - data;
-   *check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-   datalen, proto, 0);
-   } else {
-   *check = 0;
-   *check = csum_tcpudp_magic(iph->saddr, iph->daddr,
-  datalen, proto,
-  csum_partial(data, datalen,
-   0));
-   if (proto == IPPROTO_UDP && !*check)
-   *check = CSUM_MANGLED_0;
-   }
+   const struct iphdr *iph = ip_hdr(skb);
+
+   skb->ip_summed = CHECKSUM_PARTIAL;
+   skb->csum_start = skb_headroom(skb) + skb_network_offset(skb) +
+   ip_hdrlen(skb);
+   skb->csum_offset = (void *)check - data;
+   *check = ~csum_tcpudp_magic(iph->saddr, iph->daddr, datalen,
+   proto, 0);
} else
inet_proto_csum_replace2(check, skb,
 htons(oldlen), htons(datalen), true);
diff --git a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c 
b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
index 6ce3099..e0be97e 100644
--- a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
@@ -131,29 +131,15 @@ static void nf_nat_ipv6_csum_recalc(struct sk_buff *skb,
u8 proto, void *data, __sum16 *check,
int datalen, int oldlen)
 {
-   const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
-   struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
-
if (skb->ip_summed != CHECKSUM_PARTIAL) {
-   if (!(rt->rt6i_flags & RTF_LOCAL) &&
-   (!skb->dev || skb->dev->features &
-(NETIF_F_IPV6_CSUM | NETIF_F_HW_CSUM))) {
-   skb->ip_summed = CHECKSUM_PARTIAL;
-   skb->csum_start = skb_headroom(skb) +
- skb_network_offset(skb) +
- (data - (void *)skb->data);
-   skb->csum_offset = (void *)check - data;
-   *check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
- datalen, proto, 0);
-   } else {
-   *check = 0;
-   *check = csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
-datalen, proto,
-csum_partial(data, datalen,
- 0));
-   if (proto == IPPROTO_UDP && !*check)
-   *check = CSUM_MANGLED_0;
-   }
+   const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+
+   skb->ip_summed = CHECKSUM_PARTIAL;
+   skb->csum_start = skb_headroom(skb) + skb_network_offset(skb) +
+   (data - (void *)skb->data);
+   skb->csum_offset = (void *)check - data;
+   *check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
+ datalen, proto, 0);
} else
 

[ovs-dev] [PATCH net-next v5 0/8] openvswitch: NAT support.

[Jarno Rajahalme]

This series adds NAT support to openvswitch kernel module.  A few
changes are needed to the netfilter code to facilitate this (patches
1-3/8).  Patches 4-7 make the openvswitch kernel module ready for the
patch 8 that adds the NAT support by calling into netfilter NAT code
from the openvswitch conntrack action.

This version addresses all the comments received on prior versions and
rebases to current net-next.

The OVS master now has the corresponding OVS userspace support to use
and test the NAT features.  Below if a walk through of a simple use
case.

In this case ports 1 and 2 are in different namespaces.  The OpenFlow
table below only allows IPv4 connections initiated from port 1, and
applies source NAT to those connections:

   in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255)),2
   in_port=2,ct_state=-trk,ip,action=ct(table=0,zone=1,nat)
   in_port=2,ct_state=+est,ct_zone=1,ip,action=1

This flow table matches all IPv4 traffic from port 1, runs them
through conntrack in zone 1 and NATs them.  The NAT is initialized to
do source IP mapping to the given range for the first packet of each
connection, after which the new connection is committed (confirmed).
For further packets of already tracked connections NAT is done
according to the connection state and the commit is a no-op.  Each
packet that is not flagged as a drop by the CT action is forwarded to
port 2.  The CT action does an implicit fragmentation reassembly, so
that only complete packets are run through conntrack.  Reassembled
packets are re-fragmented on output.

The IPv4 traffic coming from port 2 is first matched for the
non-tracked state (-trk), which means that the packet has not been
through a CT action yet.  Such traffic is run trough the conntrack in
zone 1 and all packets associated with a NATted connection are NATted
also in the return direction.  After the packet has been through
conntrack it is recirculated back to OpenFlow table 0 (which is the
default table, so all the rules above are in table 0).  The CT action
changes the 'trk' flag to being set, so the packets after
recirculation no longer match the second rule.  The third rule then
matches the recirculated packets that were marked as established by
conntrack (+est), and the packet is output on port 1.  Matching on
ct_zone is not strictly needed, but in this test case it verifies that
the ct_zone key attribute is properly set by the conntrack action.

A full test case requires rules for ARP handling not shown here.

The flow table above is an OpenFlow table, and the rules therein
are translated to kernel flow entries on-demand by ovs-vswitchd.

Jarno Rajahalme (8):
  netfilter: Remove IP_CT_NEW_REPLY definition.
  netfilter: Factor out nf_ct_get_info().
  netfilter: Allow calling into nat helper without skb_dst.
  openvswitch: Update the CT state key only after nf_conntrack_in().
  openvswitch: Find existing conntrack entry after upcall.
  openvswitch: Handle NF_REPEAT in conntrack action.
  openvswitch: Delay conntrack helper call for new connections.
  openvswitch: Interface with NAT.

 include/net/netfilter/nf_conntrack.h   |  15 +
 include/uapi/linux/netfilter/nf_conntrack_common.h |  12 +-
 include/uapi/linux/openvswitch.h   |  47 ++
 net/ipv4/netfilter/nf_nat_l3proto_ipv4.c   |  30 +-
 net/ipv6/netfilter/nf_nat_l3proto_ipv6.c   |  30 +-
 net/netfilter/nf_conntrack_core.c  |  28 +-
 net/openvswitch/conntrack.c| 630 +++--
 net/openvswitch/conntrack.h|   3 +-
 8 files changed, 690 insertions(+), 105 deletions(-)

-- 
2.1.4

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

[ovs-dev] [PATCH net-next v5 1/8] netfilter: Remove IP_CT_NEW_REPLY definition.

[Jarno Rajahalme]

Remove the definition of IP_CT_NEW_REPLY from the kernel as it does
not make sense.  This allows the definition of IP_CT_NUMBER to be
simplified as well.

Signed-off-by: Jarno Rajahalme 
---
 include/uapi/linux/netfilter/nf_conntrack_common.h | 12 +---
 net/openvswitch/conntrack.c|  2 --
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h 
b/include/uapi/linux/netfilter/nf_conntrack_common.h
index 319f471..6d074d1 100644
--- a/include/uapi/linux/netfilter/nf_conntrack_common.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_common.h
@@ -20,9 +20,15 @@ enum ip_conntrack_info {
 
IP_CT_ESTABLISHED_REPLY = IP_CT_ESTABLISHED + IP_CT_IS_REPLY,
IP_CT_RELATED_REPLY = IP_CT_RELATED + IP_CT_IS_REPLY,
-   IP_CT_NEW_REPLY = IP_CT_NEW + IP_CT_IS_REPLY,   
-   /* Number of distinct IP_CT types (no NEW in reply dirn). */
-   IP_CT_NUMBER = IP_CT_IS_REPLY * 2 - 1
+   /* No NEW in reply direction. */
+
+   /* Number of distinct IP_CT types. */
+   IP_CT_NUMBER,
+
+   /* only for userspace compatibility */
+#ifndef __KERNEL__
+   IP_CT_NEW_REPLY = IP_CT_NUMBER,
+#endif
 };
 
 #define NF_CT_STATE_INVALID_BIT(1 << 0)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index c2cc111..a28a819 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -73,7 +73,6 @@ static u8 ovs_ct_get_state(enum ip_conntrack_info ctinfo)
switch (ctinfo) {
case IP_CT_ESTABLISHED_REPLY:
case IP_CT_RELATED_REPLY:
-   case IP_CT_NEW_REPLY:
ct_state |= OVS_CS_F_REPLY_DIR;
break;
default:
@@ -90,7 +89,6 @@ static u8 ovs_ct_get_state(enum ip_conntrack_info ctinfo)
ct_state |= OVS_CS_F_RELATED;
break;
case IP_CT_NEW:
-   case IP_CT_NEW_REPLY:
ct_state |= OVS_CS_F_NEW;
break;
default:
-- 
2.1.4

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

[ovs-dev] [PATCH net-next v5 5/8] openvswitch: Find existing conntrack entry after upcall.

[Jarno Rajahalme]

Add a new function ovs_ct_find_existing() to find an existing
conntrack entry for which this packet was already applied to.  This is
only to be called when there is evidence that the packet was already
tracked and committed, but we lost the ct reference due to an
userspace upcall.

ovs_ct_find_existing() is called from skb_nfct_cached(), which can now
hide the fact that the ct reference may have been lost due to an
upcall.  This allows ovs_ct_commit() to be simplified.

This patch is needed by later "openvswitch: Interface with NAT" patch,
as we need to be able to pass the packet through NAT using the
original ct reference also after the reference is lost after an
upcall.

Signed-off-by: Jarno Rajahalme 
---
 net/openvswitch/conntrack.c | 95 ++---
 1 file changed, 82 insertions(+), 13 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 10f4a6e..0c371d0 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -359,16 +359,87 @@ ovs_ct_expect_find(struct net *net, const struct 
nf_conntrack_zone *zone,
return __nf_ct_expect_find(net, zone, &tuple);
 }
 
+/* Find an existing conntrack entry for which this packet was already applied
+ * to.  This is only called when there is evidence that the packet was already
+ * tracked and commited, but we lost the ct reference due to an userspace
+ * upcall. This means that on entry skb->nfct is NULL.
+ * On success, returns conntrack ptr, sets skb->nfct and ctinfo.
+ * Must be called rcu_read_lock()ed. */
+static struct nf_conn *
+ovs_ct_find_existing(struct net *net, const struct nf_conntrack_zone *zone,
+u_int8_t l3num, struct sk_buff *skb,
+enum ip_conntrack_info *ctinfo)
+{
+   struct nf_conntrack_l3proto *l3proto;
+   struct nf_conntrack_l4proto *l4proto;
+   struct nf_conntrack_tuple tuple;
+   struct nf_conntrack_tuple_hash *h;
+   struct nf_conn *ct;
+   unsigned int dataoff;
+   u_int8_t protonum;
+
+   BUG_ON(skb->nfct != NULL);
+
+   l3proto = __nf_ct_l3proto_find(l3num);
+   if (!l3proto) {
+   pr_debug("ovs_ct_find_existing: Can't get l3proto\n");
+   return NULL;
+   }
+   if (l3proto->get_l4proto(skb, skb_network_offset(skb), &dataoff,
+&protonum) <= 0) {
+   pr_debug("ovs_ct_find_existing: Can't get protonum\n");
+   return NULL;
+   }
+   l4proto = __nf_ct_l4proto_find(l3num, protonum);
+   if (!l4proto) {
+   pr_debug("ovs_ct_find_existing: Can't get l4proto\n");
+   return NULL;
+   }
+   if (!nf_ct_get_tuple(skb, skb_network_offset(skb), dataoff, l3num,
+protonum, net, &tuple, l3proto, l4proto)) {
+   pr_debug("ovs_ct_find_existing: Can't get tuple\n");
+   return NULL;
+   }
+
+   /* look for tuple match */
+   h = nf_conntrack_find_get(net, zone, &tuple);
+   if (!h)
+   return NULL;   /* Not found. */
+
+   ct = nf_ct_tuplehash_to_ctrack(h);
+
+   *ctinfo = nf_ct_get_info(h);
+   if (*ctinfo == IP_CT_NEW) {
+   /* This should not happen. */
+   WARN_ONCE(1, "ovs_ct_find_existing: new packet for %p\n", ct);
+   }
+   skb->nfct = &ct->ct_general;
+   skb->nfctinfo = *ctinfo;
+   return ct;
+}
+
 /* Determine whether skb->nfct is equal to the result of conntrack lookup. */
-static bool skb_nfct_cached(const struct net *net, const struct sk_buff *skb,
-   const struct ovs_conntrack_info *info)
+static bool skb_nfct_cached(struct net *net,
+   const struct sw_flow_key *key,
+   const struct ovs_conntrack_info *info,
+   struct sk_buff *skb)
 {
enum ip_conntrack_info ctinfo;
struct nf_conn *ct;
 
ct = nf_ct_get(skb, &ctinfo);
+   /* If no ct, check if we have evidence that an existing conntrack entry
+* might be found for this skb.  This happens when we lose a skb->nfct
+* due to an upcall.  If the connection was not confirmed, it is not
+* cached and needs to be run through conntrack again. */
+   if (!ct && key->ct.state & OVS_CS_F_TRACKED
+   && !(key->ct.state & OVS_CS_F_INVALID)
+   && key->ct.zone == info->zone.id)
+   ct = ovs_ct_find_existing(net, &info->zone, info->family, skb,
+ &ctinfo);
if (!ct)
return false;
+
if (!net_eq(net, read_pnet(&ct->ct_net)))
return false;
if (!nf_ct_zone_equal_any(info->ct, nf_ct_zone(ct)))
@@ -397,7 +468,7 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
 * actually run the packet through conntrack twice unless it's for a
 * different zone.
 */
-   

[ovs-dev] [PATCH net-next v5 4/8] openvswitch: Update the CT state key only after nf_conntrack_in().

[Jarno Rajahalme]

Only a successful nf_conntrack_in() call can effect a connection state
change, so if suffices to update the key only after the
nf_conntrack_in() returns.

This change is needed for the later NAT patches.

Signed-off-by: Jarno Rajahalme 
---
 net/openvswitch/conntrack.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index a28a819..10f4a6e 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -194,7 +194,6 @@ static int ovs_ct_set_mark(struct sk_buff *skb, struct 
sw_flow_key *key,
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, &ctinfo);
if (!ct)
@@ -385,6 +384,10 @@ static bool skb_nfct_cached(const struct net *net, const 
struct sk_buff *skb,
return true;
 }
 
+/* Pass 'skb' through conntrack in 'net', using zone configured in 'info', if
+ * not done already.  Update key with new CT state after passing the packet
+ * through conntrack.
+ */
 static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
   const struct ovs_conntrack_info *info,
   struct sk_buff *skb)
@@ -410,14 +413,14 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
skb) != NF_ACCEPT)
return -ENOENT;
 
+   ovs_ct_update_key(skb, key, true);
+
if (ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
WARN_ONCE(1, "helper rejected packet");
return -EINVAL;
}
}
 
-   ovs_ct_update_key(skb, key, true);
-
return 0;
 }
 
-- 
2.1.4

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

[ovs-dev] [PATCH net-next v5 2/8] netfilter: Factor out nf_ct_get_info().

[Jarno Rajahalme]

Define a new inline function to map conntrack status to enum
ip_conntrack_info.  This removes the need to otherwise duplicate this
code in a later patch ("openvswitch: Find existing conntrack entry
after upcall.").

Signed-off-by: Jarno Rajahalme 
---
 include/net/netfilter/nf_conntrack.h | 15 +++
 net/netfilter/nf_conntrack_core.c| 28 +---
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack.h 
b/include/net/netfilter/nf_conntrack.h
index fde4068..b3de10e 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -125,6 +125,21 @@ nf_ct_tuplehash_to_ctrack(const struct 
nf_conntrack_tuple_hash *hash)
tuplehash[hash->tuple.dst.dir]);
 }
 
+static inline enum ip_conntrack_info
+nf_ct_get_info(const struct nf_conntrack_tuple_hash *h)
+{
+   const struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);
+
+   if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
+   return IP_CT_ESTABLISHED_REPLY;
+   /* Once we've had two way comms, always ESTABLISHED. */
+   if (test_bit(IPS_SEEN_REPLY_BIT, &ct->status))
+   return IP_CT_ESTABLISHED;
+   if (test_bit(IPS_EXPECTED_BIT, &ct->status))
+   return IP_CT_RELATED;
+   return IP_CT_NEW;
+}
+
 static inline u_int16_t nf_ct_l3num(const struct nf_conn *ct)
 {
return ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.l3num;
diff --git a/net/netfilter/nf_conntrack_core.c 
b/net/netfilter/nf_conntrack_core.c
index 3cb3cb8..7546fc7 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1056,25 +1056,15 @@ resolve_normal_ct(struct net *net, struct nf_conn *tmpl,
ct = nf_ct_tuplehash_to_ctrack(h);
 
/* It exists; we have (non-exclusive) reference. */
-   if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY) {
-   *ctinfo = IP_CT_ESTABLISHED_REPLY;
-   /* Please set reply bit if this packet OK */
-   *set_reply = 1;
-   } else {
-   /* Once we've had two way comms, always ESTABLISHED. */
-   if (test_bit(IPS_SEEN_REPLY_BIT, &ct->status)) {
-   pr_debug("nf_conntrack_in: normal packet for %p\n", ct);
-   *ctinfo = IP_CT_ESTABLISHED;
-   } else if (test_bit(IPS_EXPECTED_BIT, &ct->status)) {
-   pr_debug("nf_conntrack_in: related packet for %p\n",
-ct);
-   *ctinfo = IP_CT_RELATED;
-   } else {
-   pr_debug("nf_conntrack_in: new packet for %p\n", ct);
-   *ctinfo = IP_CT_NEW;
-   }
-   *set_reply = 0;
-   }
+   *ctinfo = nf_ct_get_info(h);
+   if (*ctinfo == IP_CT_ESTABLISHED)
+   pr_debug("nf_conntrack_in: normal packet for %p\n", ct);
+   else if (*ctinfo == IP_CT_RELATED)
+   pr_debug("nf_conntrack_in: related packet for %p\n", ct);
+   else if (*ctinfo == IP_CT_NEW)
+   pr_debug("nf_conntrack_in: new packet for %p\n", ct);
+   *set_reply = NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY;
+
skb->nfct = &ct->ct_general;
skb->nfctinfo = *ctinfo;
return ct;
-- 
2.1.4

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

[ovs-dev] [PATCH net-next v5 6/8] openvswitch: Handle NF_REPEAT in conntrack action.

[Jarno Rajahalme]

Repeat the nf_conntrack_in() call when it returns NF_REPEAT.  This
avoids dropping a SYN packet re-opening an existing TCP connection.

Signed-off-by: Jarno Rajahalme 
---
 net/openvswitch/conntrack.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 0c371d0..7aa38fa 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -470,6 +470,7 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
 */
if (!skb_nfct_cached(net, key, info, skb)) {
struct nf_conn *tmpl = info->ct;
+   int err;
 
/* Associate skb with specified zone. */
if (tmpl) {
@@ -480,8 +481,13 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
skb->nfctinfo = IP_CT_NEW;
}
 
-   if (nf_conntrack_in(net, info->family, NF_INET_PRE_ROUTING,
-   skb) != NF_ACCEPT)
+   /* Repeat if requested, see nf_iterate(). */
+   do {
+   err = nf_conntrack_in(net, info->family,
+ NF_INET_PRE_ROUTING, skb);
+   } while (err == NF_REPEAT);
+
+   if (err != NF_ACCEPT)
return -ENOENT;
 
ovs_ct_update_key(skb, key, true);
-- 
2.1.4

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

[ovs-dev] [PATCH net-next v5 7/8] openvswitch: Delay conntrack helper call for new connections.

[Jarno Rajahalme]

There is no need to help connections that are not confirmed, so we can
delay helping new connections to the time when they are confirmed.
This change is needed for NAT support, and having this as a separate
patch will make the following NAT patch a bit easier to review.

Signed-off-by: Jarno Rajahalme 
---
 net/openvswitch/conntrack.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 7aa38fa..ba44287 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -458,6 +458,7 @@ static bool skb_nfct_cached(struct net *net,
 /* Pass 'skb' through conntrack in 'net', using zone configured in 'info', if
  * not done already.  Update key with new CT state after passing the packet
  * through conntrack.
+ * Note that invalid packets are accepted while the skb->nfct remains unset!
  */
 static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
   const struct ovs_conntrack_info *info,
@@ -468,7 +469,11 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
 * actually run the packet through conntrack twice unless it's for a
 * different zone.
 */
-   if (!skb_nfct_cached(net, key, info, skb)) {
+   bool cached = skb_nfct_cached(net, key, info, skb);
+   enum ip_conntrack_info ctinfo;
+   struct nf_conn *ct;
+
+   if (!cached) {
struct nf_conn *tmpl = info->ct;
int err;
 
@@ -491,11 +496,16 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
return -ENOENT;
 
ovs_ct_update_key(skb, key, true);
+   }
 
-   if (ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
-   WARN_ONCE(1, "helper rejected packet");
-   return -EINVAL;
-   }
+   /* Call the helper right after nf_conntrack_in() for confirmed
+* connections, but only when commiting for unconfirmed connections.
+*/
+   ct = nf_ct_get(skb, &ctinfo);
+   if (ct && (nf_ct_is_confirmed(ct) ? !cached : info->commit)
+   && ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
+   WARN_ONCE(1, "helper rejected packet");
+   return -EINVAL;
}
 
return 0;
-- 
2.1.4

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

[ovs-dev] [PATCH net-next v5 8/8] openvswitch: Interface with NAT.

[Jarno Rajahalme]

Extend OVS conntrack interface to cover NAT.  New nested
OVS_CT_ATTR_NAT attribute may be used to include NAT with a CT action.
A bare OVS_CT_ATTR_NAT only mangles existing and expected connections.
If OVS_NAT_ATTR_SRC or OVS_NAT_ATTR_DST is included within the nested
attributes, new (non-committed/non-confirmed) connections are mangled
according to the rest of the nested attributes.

The corresponding OVS userspace patch series includes test cases (in
tests/system-traffic.at) that also serve as example uses.

This work extends on a branch by Thomas Graf at
https://github.com/tgraf/ovs/tree/nat.

Signed-off-by: Jarno Rajahalme 
---
 include/uapi/linux/openvswitch.h |  47 
 net/openvswitch/conntrack.c  | 516 +--
 net/openvswitch/conntrack.h  |   3 +-
 3 files changed, 541 insertions(+), 25 deletions(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 28ccedd..1a4bbdc 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -454,6 +454,12 @@ struct ovs_key_ct_labels {
 #define OVS_CS_F_REPLY_DIR 0x08 /* Flow is in the reply direction. */
 #define OVS_CS_F_INVALID   0x10 /* Could not track connection. */
 #define OVS_CS_F_TRACKED   0x20 /* Conntrack has occurred. */
+#define OVS_CS_F_SRC_NAT   0x40 /* Packet's source address/port was
+  mangled by NAT. */
+#define OVS_CS_F_DST_NAT   0x80 /* Packet's destination address/port
+  was mangled by NAT. */
+
+#define OVS_CS_F_NAT_MASK (OVS_CS_F_SRC_NAT | OVS_CS_F_DST_NAT)
 
 /**
  * enum ovs_flow_attr - attributes for %OVS_FLOW_* commands.
@@ -632,6 +638,8 @@ struct ovs_action_hash {
  * mask. For each bit set in the mask, the corresponding bit in the value is
  * copied to the connection tracking label field in the connection.
  * @OVS_CT_ATTR_HELPER: variable length string defining conntrack ALG.
+ * @OVS_CT_ATTR_NAT: Nested OVS_NAT_ATTR_* for performing L3 network address
+ * translation (NAT) on the packet.
  */
 enum ovs_ct_attr {
OVS_CT_ATTR_UNSPEC,
@@ -641,12 +649,51 @@ enum ovs_ct_attr {
OVS_CT_ATTR_LABELS, /* labels to associate with this connection. */
OVS_CT_ATTR_HELPER, /* netlink helper to assist detection of
   related connections. */
+   OVS_CT_ATTR_NAT,/* Nested OVS_NAT_ATTR_* */
__OVS_CT_ATTR_MAX
 };
 
 #define OVS_CT_ATTR_MAX (__OVS_CT_ATTR_MAX - 1)
 
 /**
+ * enum ovs_nat_attr - Attributes for %OVS_CT_ATTR_NAT.
+ *
+ * @OVS_NAT_ATTR_SRC: Flag for Source NAT (mangle source address/port).
+ * @OVS_NAT_ATTR_DST: Flag for Destination NAT (mangle destination
+ * address/port).  Only one of (@OVS_NAT_ATTR_SRC, @OVS_NAT_ATTR_DST) may be
+ * specified.  Effective only for packets for ct_state NEW connections.
+ * Packets of committed connections are mangled by the NAT action according to
+ * the committed NAT type regardless of the flags specified.  As a corollary, a
+ * NAT action without a NAT type flag will only mangle packets of committed
+ * connections.  The following NAT attributes only apply for NEW
+ * (non-committed) connections, and they may be included only when the CT
+ * action has the @OVS_CT_ATTR_COMMIT flag and either @OVS_NAT_ATTR_SRC or
+ * @OVS_NAT_ATTR_DST is also included.
+ * @OVS_NAT_ATTR_IP_MIN: struct in_addr or struct in6_addr
+ * @OVS_NAT_ATTR_IP_MAX: struct in_addr or struct in6_addr
+ * @OVS_NAT_ATTR_PROTO_MIN: u16 L4 protocol specific lower boundary (port)
+ * @OVS_NAT_ATTR_PROTO_MAX: u16 L4 protocol specific upper boundary (port)
+ * @OVS_NAT_ATTR_PERSISTENT: Flag for persistent IP mapping across reboots
+ * @OVS_NAT_ATTR_PROTO_HASH: Flag for pseudo random L4 port mapping (MD5)
+ * @OVS_NAT_ATTR_PROTO_RANDOM: Flag for fully randomized L4 port mapping
+ */
+enum ovs_nat_attr {
+   OVS_NAT_ATTR_UNSPEC,
+   OVS_NAT_ATTR_SRC,
+   OVS_NAT_ATTR_DST,
+   OVS_NAT_ATTR_IP_MIN,
+   OVS_NAT_ATTR_IP_MAX,
+   OVS_NAT_ATTR_PROTO_MIN,
+   OVS_NAT_ATTR_PROTO_MAX,
+   OVS_NAT_ATTR_PERSISTENT,
+   OVS_NAT_ATTR_PROTO_HASH,
+   OVS_NAT_ATTR_PROTO_RANDOM,
+   __OVS_NAT_ATTR_MAX,
+};
+
+#define OVS_NAT_ATTR_MAX (__OVS_NAT_ATTR_MAX - 1)
+
+/**
  * enum ovs_action_attr - Action types.
  *
  * @OVS_ACTION_ATTR_OUTPUT: Output packet to port.
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index ba44287..096f5da 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -20,14 +20,24 @@
 #include 
 #include 
 
+#ifdef CONFIG_NF_NAT_NEEDED
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#endif
+
 #include "datapath.h"
 #include "conntrack.h"
 #include "flow.h"
 #include "flow_netlink.h"
 
 struct ovs_ct_len_tbl {
-   size_t maxlen;
-   size_t minlen;
+   int maxlen;
+   int minlen;
 };
 
 /* Metadata mark 

Re: [ovs-dev] OVS/OVN: Interface with NAT

[Jarno Rajahalme]

> On Dec 16, 2015, at 2:15 PM, Amitabha Biswas  wrote:
> 
> Hi Jarno,
> 
> As part of OVN integration in Openstack, I’ve been trying to setup OpenFlow 
> NAT rules and run the system-test-suite 33: system-traffic.at 
> :1396 conntrack - simple SNAT. Using the latest 
> (as of 12/15/2015) ovs master branch on Ubuntu Linux 4.3 kernels (from 
> kernel.ubuntu.com ) fail with the following errors:
> 
> > 2015-12-16T03:05:13.945Z|00037|connmgr|INFO|br0<->unix: sending 
> > OFPBAC_BAD_TYPE error reply to OFPT_FLOW_MOD message
> > 2015-12-16T03:05:13.945Z|00038|vconn|DBG|unix: sent (Success): OFPT_ERROR 
> > (OF1.4) (xid=0x2): OFPBAC_BAD_TYPE
> 
> related to the flow
> in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255)),2
> 
> Am I missing some conntrack kernel patches to get the OpenFlow NAT rules 
> installed? If yes, how would one get those patches and which kernel images 
> would contain them.


New OVS Linux datapath features are developed on top of the upstream net-next 
git repo. I have just addressed feedback on v4 and rebased to the now current 
net-next.

OVS tree kernel module will be updated with NAT support (and backports as far 
as feasible) only after the code has been merged in net-next.

If you want to test OVS NAT integration now you need to clone the net-next 
repo, apply the patches, compile a new kernel from there, and then run the OVS 
on top of the new kernel.

To make this a bit easier I have created a bundle for these patches in 
patchwork:

http://patchwork.ozlabs.org/bundle/jarno/OVS_NAT_v5/ 


You should be able to download the bundle and then apply it with “git am” to 
your local net-next repo.

Regards,

  Jarno

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

Re: [ovs-dev] [PATCH net-next v4 8/8] openvswitch: Interface with NAT.

[Jarno Rajahalme]

Thanks for review, I removed these in version 5.

  Jarno

> On Dec 10, 2015, at 11:10 AM, Pablo Neira Ayuso  wrote:
> 
> On Tue, Dec 08, 2015 at 05:01:10PM -0800, Jarno Rajahalme wrote:
>> -/* Call the helper right after nf_conntrack_in() for confirmed
>> - * connections, but only when commiting for unconfirmed connections.
>> - */
>>  ct = nf_ct_get(skb, &ctinfo);
>> -if (ct && (nf_ct_is_confirmed(ct) ? !cached : info->commit)
>> -&& ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
>> -WARN_ONCE(1, "helper rejected packet");
>> -return -EINVAL;
>> +if (ct) {
>> +#ifdef CONFIG_NF_NAT_NEEDED
>> +/* Packets starting a new connection must be NATted before the
>> + * helper, so that the helper knows about the NAT.  We enforce
>> + * this by delaying both NAT and helper calls for unconfirmed
>> + * connections until the commiting CT action.  For later
>> + * packets NAT and Helper may be called in either order.
>> + *
>> + * NAT will be done only if the CT action has NAT, and only
>> + * once per packet (per zone), as guarded by the NAT bits in
>> + * the key->ct.state.
>> + */
>> +if (info->nat && !(key->ct.state & OVS_CS_F_NAT_MASK) &&
>> +(nf_ct_is_confirmed(ct) || info->commit) &&
>> +ovs_ct_nat(net, key, info, skb, ct, ctinfo) != NF_ACCEPT) {
>> +WARN_ONCE(1, "NAT rejected packet");
> 
> NAT can drop packets, so this warn_on I don't think you need it.
> 
>> +return -EINVAL;
>> +}
>> +#endif
>> +/* Call the helper whenever nf_conntrack_in() was called for
>> + * confirmed connections, but only when commiting for
>> + * unconfirmed connections.
>> + */
>> +if ((nf_ct_is_confirmed(ct) ? !cached : info->commit)
>> +&& ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
>> +WARN_ONCE(1, "helper rejected packet");
> 
> Same thing may happen with helpers.

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

Re: [ovs-dev] [PATCH] flow: pass last field to miniflow_pad_to_64

[Jarno Rajahalme]

Sorry Ben,

I forgot about this and now I ran out of time.

  Jarno

> On Dec 1, 2015, at 10:29 AM, Ben Pfaff  wrote:
> 
> On Tue, Dec 01, 2015 at 03:03:16PM +0900, Simon Horman wrote:
>> Make miniflow_pad_to_64() a little more robust with regards to updates to
>> struct flow by passing the last field, whose end should be considered for
>> padding, rather than the next field, whose start should be considered.
>> 
>> Signed-off-by: Simon Horman 
> 
> Jarno, will you review this?
> 
> Thanks,
> 
> Ben.

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

Re: [ovs-dev] OVS/OVN: Interface with NAT

[Amitabha Biswas]

> On Dec 16, 2015, at 4:42 PM, Jarno Rajahalme  wrote:
> 
> 
>> On Dec 16, 2015, at 2:15 PM, Amitabha Biswas > > wrote:
>> 
>> Hi Jarno,
>> 
>> As part of OVN integration in Openstack, I’ve been trying to setup OpenFlow 
>> NAT rules and run the system-test-suite 33: system-traffic.at 
>> :1396 conntrack - simple SNAT. Using the latest 
>> (as of 12/15/2015) ovs master branch on Ubuntu Linux 4.3 kernels (from 
>> kernel.ubuntu.com ) fail with the following 
>> errors:
>> 
>> > 2015-12-16T03:05:13.945Z|00037|connmgr|INFO|br0<->unix: sending 
>> > OFPBAC_BAD_TYPE error reply to OFPT_FLOW_MOD message
>> > 2015-12-16T03:05:13.945Z|00038|vconn|DBG|unix: sent (Success): OFPT_ERROR 
>> > (OF1.4) (xid=0x2): OFPBAC_BAD_TYPE
>> 
>> related to the flow
>> in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255)),2
>> 
>> Am I missing some conntrack kernel patches to get the OpenFlow NAT rules 
>> installed? If yes, how would one get those patches and which kernel images 
>> would contain them.
> 
> 
> New OVS Linux datapath features are developed on top of the upstream net-next 
> git repo. I have just addressed feedback on v4 and rebased to the now current 
> net-next.
> 
> OVS tree kernel module will be updated with NAT support (and backports as far 
> as feasible) only after the code has been merged in net-next.
> 
> If you want to test OVS NAT integration now you need to clone the net-next 
> repo, apply the patches, compile a new kernel from there, and then run the 
> OVS on top of the new kernel.
> 
> To make this a bit easier I have created a bundle for these patches in 
> patchwork:
> 
> http://patchwork.ozlabs.org/bundle/jarno/OVS_NAT_v5/ 
> 
> 
> You should be able to download the bundle and then apply it with “git am” to 
> your local net-next repo.
> 
> Regards,
> 
>   Jarno
> 


Thanks for the tip Jarno.

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

[ovs-dev] Returned mail: Data format error

[julieday]

Dear user of openvswitch.org,

Your account has been used to send a large amount of junk email messages during 
this week.
We suspect that your computer was compromised and now runs a trojaned proxy 
server.

Please follow our instructions in order to keep your computer safe.

Best wishes,
openvswitch.org support team.

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