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

[Ben Pfaff]

On Sat, Jul 15, 2017 at 11:23:04AM +0900, Takashi YAMAMOTO wrote:
> Hi,
> 
> 2017/07/15 6:55 "Ben Pfaff" :
> 
> On Sat, Jul 15, 2017 at 02:39:44AM +0900, YAMAMOTO Takashi wrote:
> > SUS says:
> > When jobs reports the termination status of a job,
> > the shell removes its process ID from the list of those
> > "known in the current shell execution environment";
> >
> > With NetBSD /bin/sh, the list involves zombie processes and
> > ends up with "can not fork" during test runs.
> 
> Can you squash the new patch into the existing one?  And then we can
> apply it unconditionally?  It isn't great to have conditional code here,
> especially since the testsuite is distributed.
> 
> 
> I thought the existing windows patch was conditional for some reasons. Do
> you remember the reason?

Thanks for reminding me of that.

It's because the patch only works with some Autoconf versions:

commit 83e09b5dfa35b95e9995713bdfcb9a27f9b4ed7f
Author: Gurucharan Shetty   Thu Apr 23 07:13:04 2015
Committer:  Gurucharan Shetty   Thu Apr 23 10:49:13 2015

testsuite: Don't apply the testsuite.patch on non-Windows platforms.

On CentOS machines which use autoconf version 2.63, the patch
application would fail.

Reported-by: Ian Stokes 
Tested-by: Ian Stokes 
Signed-off-by: Gurucharan Shetty 

This is probably true of the new patch that you are providing, too.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] incorrect revalidated action for igmp

[Huanle Han]

This patch can slove the problem.

But I think wc->masks.tp_src should not be masked in userspace as igmp type
is not supported in datapath. Otherwise, flow_wildcards_has_extra() in
revalidate_ukey__ always return true for igmp megaflow, and igmp flow is
never kept in datapath.

Thanks,
Huanle

On Wednesday, July 12, 2017, Ben Pfaff  wrote:

> On Thu, Jun 22, 2017 at 01:04:48AM +0800, Huanle Han wrote:
> > In "Normal" action, igmp report packet is expected to processed in slow
> > path.
> > However, the igmp_type(flow->tp_src) is not supported to be masked in
> > datapath.
> > Then ovs-vswitchd revalidate the flow with igmp_type(flow->tp_src) == 0.
> It
> > leads to a "multicast traffic, flooding" action and overwrites the
> > "userspace()" in datapath.
> >
> > This is code segment from function "xlate_normal":
> >
> > if (is_igmp(flow, wc)) {
> > memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
> > if (mcast_snooping_is_membership(flow->tp_src) ||
> > mcast_snooping_is_query(flow->tp_src)) { <
> > flow->tp_src is 0
> > if (ctx->xin->allow_side_effects && ctx->xin->packet) {
> > update_mcast_snooping_table(ctx, flow, vlan,
> > in_xbundle,
> > ctx->xin->packet);
> > }
> > /*
> >  * IGMP packets need to take the slow path, in order to
> be
> >  * processed for mdb updates. That will prevent expires
> >  * firing off even after hosts have sent reports.
> >  */
> > ctx->xout->slow |= SLOW_ACTION;
> > }
> > ...
> > }
> >
> > To reproduce this bug, you need to ovs-appctl upcall/disable-megaflows
>
> Thanks for the bug report.
>
> Does the following patch fix the problem?
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 089c7f170d18..5030dafd9f42 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2720,6 +2720,13 @@ xlate_normal(struct xlate_ctx *ctx)
>  struct mcast_group *grp = NULL;
>
>  if (is_igmp(flow, wc)) {
> +/*
> + * IGMP packets need to take the slow path, in order to be
> + * processed for mdb updates. That will prevent expires
> + * firing off even after hosts have sent reports.
> + */
> +ctx->xout->slow |= SLOW_ACTION;
> +
>  memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
>  if (mcast_snooping_is_membership(flow->tp_src) ||
>  mcast_snooping_is_query(flow->tp_src)) {
> @@ -2727,12 +2734,6 @@ xlate_normal(struct xlate_ctx *ctx)
>  update_mcast_snooping_table(ctx, flow, vlan,
>  in_xbundle,
> ctx->xin->packet);
>  }
> -/*
> - * IGMP packets need to take the slow path, in order to be
> - * processed for mdb updates. That will prevent expires
> - * firing off even after hosts have sent reports.
> - */
> -ctx->xout->slow |= SLOW_ACTION;
>  }
>
>  if (mcast_snooping_is_membership(flow->tp_src)) {
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Your Consignment Boxes Delivery.

[Mr. Grant Watson]

MR GRANT WATSON
UNITED NATION INSPECTION AGENCY
HARTS FIELD-JACKSON INTERNATIONAL AIRPORT.
ATLANTA, GEORGIA.



I am GRANT WATSON Inspection Agency in Harts field-Jackson International
Airport Atlanta, Georgia. During our investigation, I discovered an
abandoned shipment through a Diplomat from UNITED KINGDOM (UK)
which was transferred from Cardiff International Airport. To our facility
here in Atlanta, and when scanned it revealed an undisclosed sum of money
in 2 Metal Trunk Boxes. The consignment was abandoned because the Content
was not properly declared by the consignee as money rather it was declared
as personal Effect/classified document to either avoid diversion by the
Shipping Agent or confiscation by the relevant authorities.



The diplomat's inability to pay for Non Inspection fees among other things
are the reason why the consignment is delayed and abandoned. By my
assessment, each of the boxes contains about $4M or more. They are still
left in the airport storage facility till today. The Consignments like I
said are two metal trunk boxes, the details of the consignment including
your name and email on the official document from United Nations' office
in UNITED KINGDOM AND NIGERIA where the shipment was tagged as personal
effects/classified document is still available with us.  As it stands
now, you have to reconfirm your Full name, Phone Number, full address so
I can cross-check and see if it corresponds with the one on the official
documents.



It is now left to you to decide if you are the beneficiary and still need
the consignment or allow us repatriate it back to NIGERIA (place of
origin) as we were instructed. Like I did say again, the shipper abandoned
it and ran away most importantly because he gave a false declaration, he
could not pay for the yellow tag, he could not secure a valid non
inspection document(s), etc. I am ready to assist you in any way I can for
you to get back this packages if only you are willing to work with me with
trust. You can either come in person, or you engage the services of a
secure shipping/delivery Company/agent that will provide the necessary
security. That is required to deliver the package to your doorstep or the
destination of your choice. I need the entire guarantee that I can get
from you before I can get involved in this project.



Best Regards,
MR GRANT WATSON
INSPECTION OFFICER.
Email :{grantwatson2...@gmail.com}

---
This email has been checked for viruses by AVG.
http://www.avg.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Takashi YAMAMOTO]

Hi,

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

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

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


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

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


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


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

Re: [ovs-dev] Fix up to the ovn-northd sync

[Russell Bryant]

On Thu, Jul 13, 2017 at 9:13 AM, Miguel Angel Ajo  wrote:
> This is a fixup and regression test I've written for the l3ha series
> for an issue I found while restarting ovn-controllers around.
>
> I hope it looks good, it can be concatenated to the series, or I can
> squash it to the ovn-northd patch.

I replied to the patch itself before seeing this message.  I think it
should be squashed into the ovn-northd patch.

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

Re: [ovs-dev] [PATCH v5 1/8] ovn: l3ha, add extra check on distributed gateway ports

[Russell Bryant]

On Thu, Jul 13, 2017 at 1:02 PM, Miguel Angel Ajo  wrote:
> Check that removing the options:redirect-chassis in NBDB is
> going to remove the cr-${port} in the SBDB.
>
> This is introduced to avoid any regression of this behaviour
> on the l3ha series.
>
> Signed-off-by: Miguel Angel Ajo 
> ---
>  tests/ovn.at | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index efcbd91..f3e6b4b 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -7142,6 +7142,14 @@ 
> expected=${dst_mac}${src_mac}0800451c3f110100${src_ip}${dst_ip}00351
>  echo $expected >> hv2-vif1.expected
>  OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [hv2-vif1.expected])
>
> +AT_CHECK([ovn-sbctl --bare --columns _uuid find Port_Binding 
> logical_port=cr-alice | wc --lines], [0], [1
> +])
> +
> +ovn-nbctl remove Logical_Router_Port alice options redirect-chassis

I think we should  add "--sync=sb" here to ensure ovn-northd processes
our change before doing the next check.

I've added it.

> +
> +AT_CHECK([ovn-sbctl find Port_Binding logical_port=cr-alice | wc --lines], 
> [0], [0
> +])
> +
>  OVN_CLEANUP([hv1],[hv2],[hv3])
>
>  AT_CLEANUP

I tweaked the subject line to remove "l3ha" since this patch isn't
really l3ha code.

I made the above changes and applied this to master.

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

Re: [ovs-dev] [PATCH v5 1/1] ovn: l3ha ensure no master bouncing when ovn-controller is restarted

[Russell Bryant]

On Thu, Jul 13, 2017 at 9:13 AM, Miguel Angel Ajo  wrote:
> When ovn-controller is restarted, ovn-controller removes the old
> Chassis entry from the SBDB and a new one is inserted.
>
> This cleared the Gateway_Chassis chassis column in the SBDB and then
> ovn-northd removed the empty-column Gateway_Chassis entry.
> Such event made the other (non-restarted and master gateway chassis)
> believe that he was a single (non-HA) gateway, turning off BFD and
> releasing the port for a tiny time frame causing unnecesary downtime.
>
> Signed-off-by: Miguel Angel Ajo 
> ---
>  ovn/northd/ovn-northd.c | 34 +++-
>  tests/ovn.at| 82 
> +
>  2 files changed, 102 insertions(+), 14 deletions(-)

This just looks like a bug fix for earlier ovn-northd changes.  How
about we just squash this into the ovn-northd patch?

>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 9a1e6c1..62a73f3 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1684,11 +1684,22 @@ gateway_chassis_equal(const struct 
> nbrec_gateway_chassis *nb_gwc,
>const struct sbrec_chassis *nb_gwc_c,
>const struct sbrec_gateway_chassis *sb_gwc)
>  {
> -return !strcmp(nb_gwc->name, sb_gwc->name)
> -   && !strcmp(nb_gwc_c->name, sb_gwc->chassis->name)
> -   && nb_gwc->priority == sb_gwc->priority
> -   && smap_equal(&nb_gwc->options, &sb_gwc->options)
> -   && smap_equal(&nb_gwc->external_ids, &sb_gwc->external_ids);
> +bool equal = !strcmp(nb_gwc->name, sb_gwc->name)
> + && nb_gwc->priority == sb_gwc->priority
> + && smap_equal(&nb_gwc->options, &sb_gwc->options)
> + && smap_equal(&nb_gwc->external_ids, &sb_gwc->external_ids);
> +
> +if (!equal) {
> +return false;
> +}
> +
> +/* If everything else matched and we were unable to find the SBDB
> + * Chassis entry at this time, assume a match and return true.
> + * This happens when an ovn-controller is restarting and the Chassis
> + * entry is gone away momentarily */
> +return !nb_gwc_c
> +   || (sb_gwc->chassis && !strcmp(nb_gwc_c->name,
> +  sb_gwc->chassis->name));
>  }
>
>  static bool
> @@ -1723,11 +1734,10 @@ sbpb_gw_chassis_needs_update(
>  chassis_lookup_by_name(chassis_index,
> lrp->gateway_chassis[n]->chassis_name);
>
> -if (chassis) {
> -lrp_gwc_c[lrp_n_gateway_chassis] = chassis;
> -lrp_gwc[lrp_n_gateway_chassis] = lrp->gateway_chassis[n];
> -lrp_n_gateway_chassis++;
> -} else {
> +lrp_gwc_c[lrp_n_gateway_chassis] = chassis;
> +lrp_gwc[lrp_n_gateway_chassis] = lrp->gateway_chassis[n];
> +lrp_n_gateway_chassis++;
> +if (!chassis) {
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
>  VLOG_WARN_RL(
>  &rl, "Chassis name %s referenced in NBDB via Gateway_Chassis 
> "
> @@ -1807,10 +1817,6 @@ copy_gw_chassis_from_nbrp_to_sbpb(
>  const struct sbrec_chassis *chassis =
>  chassis_lookup_by_name(chassis_index, lrp_gwc->chassis_name);
>
> -if (!chassis) {
> -continue;
> -}
> -
>  gw_chassis = xrealloc(gw_chassis, (n_gwc + 1) * sizeof *gw_chassis);
>
>  struct sbrec_gateway_chassis *pb_gwc =
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 5a0b761..07b822d 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -8080,3 +8080,85 @@ AT_CHECK([grep $garp hv2_br_phys_tx | sort], [0], [])
>  OVN_CLEANUP([hv1],[hv2],[hv3])
>
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- ensure one gw controller restart in HA doesn't bounce the 
> master])
> +AT_SKIP_IF([test $HAVE_PYTHON = no])
> +ovn_start
> +
> +net_add n1
> +
> +# create two gateways with external network connectivity
> +for i in 1 2; do
> +sim_add gw$i
> +as gw$i
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.$i
> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> +done
> +
> +ovn-nbctl ls-add inside
> +ovn-nbctl ls-add outside
> +
> +# create one hypervisors with a vif port the internal network
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.11
> +ovs-vsctl -- add-port br-int hv1-vif1 -- \
> +set interface hv1-vif1 external-ids:iface-id=inside1 \
> +options:tx_pcap=hv1/vif1-tx.pcap \
> +options:rxq_pcap=hv1/vif1-rx.pcap \
> +ofport-request=1
> +
> +ovn-nbctl lsp-add inside inside1 \
> +-- lsp-set-addresses inside1 "f0:00:00:01:22:01 192.168.1.101"
> +
> +
> +ovn_populate_arp
> +
> +ovn-nbctl create Logical_Router name=R1
> +
> +# Connect inside to R1
> +ovn-nbctl lrp-add R1 inside 00:00:01:01:02:03 192.168.1.1/24
> +ovn-nbctl lsp-add inside rp-inside -- set Logical_Switc

Re: [ovs-dev] [PATCH] tests: Disable no-format-truncation warnings

[Russell Bryant]

On Fri, Jul 14, 2017 at 4:46 AM, Timothy M. Redaelli
 wrote:
> On 07/13/2017 07:21 PM, Ben Pfaff wrote:
>> On Thu, Jul 13, 2017 at 04:29:33PM +0200, Timothy Redaelli wrote:
>>> test_snprintf function (tests/test-util.c) tests snprintf with shorter 
>>> length,
>>> but this emit a warning on GCC 7.0 or later.
>>>
>>> This commit disables that warning on tests only.
>>>
>>> Signed-off-by: Timothy Redaelli 
>>
>> How about disabling it just for those lines of code?
>
> Good idea, it's surely better to limit the portion of disabled warning
> in order to maximize the effectiveness of the warning itself.
>
> LGTM

Oops, I applied the original version of this patch from patchwork
before seeing Ben's alternative.

Patchwork turned Ben's reply into a patch on patchwork instead of
associating it with the original, so I missed it.

I'll squash a revert of the original into Ben's patch here and apply that, too.

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

Re: [ovs-dev] [PATCH v2 4/4] tunneling: Avoid datapath-recirc by combining recirc actions at xlate.

[Joe Stringer]

On 4 July 2017 at 02:46, Sugesh Chandran  wrote:
> This patch set removes the recirculation of encapsulated tunnel packets
> if possible. It is done by computing the post tunnel actions at the time of
> translation. The combined nested action set are programmed in the datapath
> using CLONE action.
>
> Signed-off-by: Sugesh Chandran 
> Signed-off-by: Zoltán Balogh 
> Co-authored-by: Zoltán Balogh 
> ---

Hi Sugesh, Zoltán,

Thanks for working on this, it's subtle code but it seems like you've
found a useful optimization here. Hopefully we can move this forward
soon.

Would you be able to put your performance numbers into the commit
message here? They could be useful to refer back to in the future.

This patch needs rebasing.

More feedback below.

>  lib/dpif-netdev.c  |  18 +--
>  ofproto/ofproto-dpif-xlate-cache.c |  14 +-
>  ofproto/ofproto-dpif-xlate-cache.h |  12 +-
>  ofproto/ofproto-dpif-xlate.c   | 295 
> -
>  ofproto/ofproto-dpif-xlate.h   |   1 +
>  ofproto/ofproto-dpif.c |   3 +-
>  tests/packet-type-aware.at |  24 +--
>  7 files changed, 324 insertions(+), 43 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 4e29085..4d996c1 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -5028,24 +5028,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
> *packets_,
>
>  case OVS_ACTION_ATTR_TUNNEL_PUSH:
>  if (*depth < MAX_RECIRC_DEPTH) {
> -struct dp_packet_batch tnl_pkt;
> -struct dp_packet_batch *orig_packets_ = packets_;
> -int err;
> -
> -if (!may_steal) {
> -dp_packet_batch_clone(&tnl_pkt, packets_);
> -packets_ = &tnl_pkt;
> -dp_packet_batch_reset_cutlen(orig_packets_);
> -}
> -
>  dp_packet_batch_apply_cutlen(packets_);
> -
> -err = push_tnl_action(pmd, a, packets_);
> -if (!err) {
> -(*depth)++;
> -dp_netdev_recirculate(pmd, packets_);
> -(*depth)--;
> -}
> +push_tnl_action(pmd, a, packets_);

If I follow, this was the previous datapath-level code to ensure that
when packets are output to a tunnel, only the copy of the packet that
goes to the tunnel gets the cutlen applied. With the new version of
the code, we replace this by making sure that the ofproto layer
generates a clone to wrap the push_tunnel and all subsequent bridge
translation, so then it results in the equivalent behaviour: The
cutlen is only ever applied to a clone of the packets. Is that right?



> +/* Validate if the transalated combined actions are OK to proceed.
> + * If actions consist of TRUNC action, it is not allowed to do the
> + * tunnel_push combine as it cannot update stats correctly.
> + */
> +static bool
> +is_tunnel_actions_clone_ready(struct ofpbuf *actions)
> +{
> +struct nlattr *tnl_actions;
> +const struct nlattr *a;
> +unsigned int left;
> +size_t actions_len;
> +
> +if (!actions) {
> +/* No actions, no harm in doing combine. */
> +return true;
> +}
> +actions_len = actions->size;
> +
> +tnl_actions =(struct nlattr *)(actions->data);
> +NL_ATTR_FOR_EACH_UNSAFE (a, left, tnl_actions, actions_len) {
> +int type = nl_attr_type(a);
> +if (type == OVS_ACTION_ATTR_TRUNC) {
> +VLOG_DBG("Cannot do tunnel action-combine on trunc action");
> +return false;
> +break;
> +}
> +}
> +return true;
> +}
> +
> +/*
> + * Copy the xc entry and update the references accordingly.
> + */
> +static void
> +copy_xc_entry(struct xc_entry *xc_dst, struct xc_entry *xc_src)

I think that this whole function should probably reside in
ofproto/ofproto-dpif-xlate-cache.c. However, I have an alternative
proposal detailed below that would not require us to also take
expensive refcounts due to this approach.

> +{
> +memcpy(xc_dst, xc_src, sizeof *xc_dst);
> +switch (xc_src->type) {
> +case XC_RULE:
> +ofproto_rule_ref(&xc_dst->rule->up);
> +break;
> +case XC_BOND:
> +bond_ref(xc_dst->bond.bond);
> +xc_dst->bond.flow = xmemdup(xc_src->bond.flow,
> +sizeof *xc_src->bond.flow);
> +break;
> +case XC_NETDEV:
> +if (xc_src->dev.tx) {
> +netdev_ref(xc_dst->dev.tx);
> +}
> +if (xc_src->dev.rx) {
> +netdev_ref(xc_dst->dev.rx);
> +}
> +if (xc_src->dev.bfd) {
> +bfd_ref(xc_dst->dev.bfd);
> +}
> +break;
> +case XC_NETFLOW:
> +netflow_ref(xc_dst->nf.netflow);
> +xc_dst->nf.flow = xmemdup(xc_src->nf.flow, sizeof *xc_src->nf.flow);
> +break;
> +case XC_MIRROR:
> +mbridge_ref(xc_dst->mirror.mbridge);
> +break;
> +case XC_LEARN:
> +case XC_TABLE:
> +case XC_NORMAL:
> 

Re: [ovs-dev] [PATCH v2 2/4] xlate: Clear tunnel mask along with other fields while combine actions.

[Joe Stringer]

On 4 July 2017 at 02:46, Sugesh Chandran  wrote:
> The tunnel mask in the translation state should be cleared along with other
> context fields. It is necessary in 'apply_nested_clone_actions' as it will be
> used to combine post tunnel output actions with tunnel push. This will assure
> right openflow state while executing the translation.
>
> Signed-off-by: Sugesh Chandran 
> Signed-off-by: Zoltán Balogh 
> Co-authored-by: Zoltán Balogh 
> ---

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

Re: [ovs-dev] [PATCH v2 3/4] tunneling: Calculate and update packet l4_offset in tunnel push.

[Joe Stringer]

On 4 July 2017 at 02:46, Sugesh Chandran  wrote:
> The following tunnel combine patch series avoids the packets recirculation
> after the tunnel push. So it is necessary to populate all relevant packet meta
> data fields for the following combined action-set.
>
> Consider a chained tunnel test case shown below,
>
> PKT-IN --> TUNNEL_PUSH --> MOD_PKT_HDR --> TUNNEL_POP
>
> In this eg: the last tunnel_pop operation uses the l4_offset in the packet to
> validate the packets. So it must be calculated and updated in the packet 
> before
> executing the action. Since there is no recirculation now on, this calculation
> is doing as part of tunnel_push.
>
> Signed-off-by: Sugesh Chandran 
> Signed-off-by: Zoltán Balogh 
> Co-authored-by: Zoltán Balogh 
> ---

Could this be done inside netdev_tnl_push_ip_header() instead, for a
generic one-liner that applies to all IP tunnels?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/4] xlate: Refactor translation of patch port output actions.

[Joe Stringer]

On 4 July 2017 at 02:46, Sugesh Chandran  wrote:
> Outputting to a patch port is translated by its peer patch port actions.
> Refactoring the translation part to use later in the patch series for the
> tunnel push.
>
> Signed-off-by: Sugesh Chandran 
> Signed-off-by: Zoltán Balogh 
> Co-authored-by: Zoltán Balogh 
> ---

Thanks for the patch, and sorry about the delay while I was traveling.

I'll apply this soon with the following incremental:

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index c4dd3a729e6e..004585cef060 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3302,9 +3302,8 @@ apply_nested_clone_actions(struct xlate_ctx
*ctx, const struct xport *in_dev,
memset(flow->regs, 0, sizeof flow->regs);
flow->actset_output = OFPP_UNSET;
clear_conntrack(ctx);
-ctx->xin->trace = xlate_report(ctx, OFT_BRIDGE,
-   "bridge(\"%s\")",
-   out_dev->xbridge->name);
+ctx->xin->trace = xlate_report(ctx, OFT_BRIDGE, "bridge(\"%s\")",
+   out_dev->xbridge->name);
mirror_mask_t old_mirrors = ctx->mirrors;
bool independent_mirrors = out_dev->xbridge != ctx->xbridge;
if (independent_mirrors) {
@@ -3388,6 +3387,7 @@ apply_nested_clone_actions(struct xlate_ctx
*ctx, const struct xport *in_dev,
}
if (ctx->xin->xcache) {
struct xc_entry *entry;
+
entry = xlate_cache_add_entry(ctx->xin->xcache, XC_NETDEV);
entry->dev.tx = netdev_ref(in_dev->netdev);
entry->dev.rx = netdev_ref(out_dev->netdev);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/3] ofproto-dpif-xlate: drop L3 packets on L2 legacy port

[Ben Pfaff]

On Fri, Jul 14, 2017 at 09:24:29PM +0200, Zoltán Balogh wrote:
> From: Zoltán Balogh 
> 
> This commit drops packet during xlate if it is a L3 packet and output
> port packet_type is legacy_l2. New PTAP unit test is added.
> 
> Signed-off-by: Zoltán Balogh 

Applied to master, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] sparse: Add missing protoype for sendmmsg.

[Ben Pfaff]

On Fri, Jul 14, 2017 at 03:04:47PM -0700, Joe Stringer wrote:
> On 14 July 2017 at 14:20, Ben Pfaff  wrote:
> > Reported-by: Joe Stringer 
> > Signed-off-by: Ben Pfaff 
> > Fixes: 00f5565c7eed ("socket-util: Fix recursion issue in sendmmsg")
> > ---
> 
> Thanks for the quick response!
> 
> Acked-by: Joe Stringer 

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

Re: [ovs-dev] [PATCH] sparse: Add missing protoype for sendmmsg.

[Joe Stringer]

On 14 July 2017 at 14:20, Ben Pfaff  wrote:
> Reported-by: Joe Stringer 
> Signed-off-by: Ben Pfaff 
> Fixes: 00f5565c7eed ("socket-util: Fix recursion issue in sendmmsg")
> ---

Thanks for the quick response!

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

Re: [ovs-dev] [PATCH v5 3/4] ovsdb-idl: idl compound indexes implementation

[Lance Richardson]

> From: "Rodriguez Betancourt, Esteban" 
> To: "Lance Richardson" 
> Cc: "Ben Pfaff" , d...@openvswitch.org, "Javier Albornoz" 
> , "Arnoldo Lutz"
> 
> Sent: Friday, 14 July, 2017 5:27:44 PM
> Subject: RE: [ovs-dev] [PATCH v5 3/4] ovsdb-idl: idl compound indexes 
> implementation
> 
> I think that is ok to remove that last pointer check in the generic
> comparison function. UUID is more than enough.
> Regards,
> Esteban
> 

Thanks, Esteban. I think this will be a very useful facility, many thanks to
you and the others who have worked on it.

I've posted a v7 of the patch series a little while ago, in patchwork here 
(removing
the pointer check in the comparison function will be a v8 item):

   https://patchwork.ozlabs.org/project/openvswitch/list/?submitter=67850

Regards,

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

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

[Ben Pfaff]

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

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

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

[Ben Pfaff]

On Sat, Jul 15, 2017 at 02:39:45AM +0900, YAMAMOTO Takashi wrote:
> == is a GNU extension.
> 
> Signed-off-by: YAMAMOTO Takashi 

I wish this didn't exist in the GNU version.  It's a useless extension.

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

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

[Ben Pfaff]

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

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

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

[Ben Pfaff]

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

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

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

[Ben Pfaff]

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

Thank you!

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

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

[Ben Pfaff]

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

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

Did you report this problem to the Autoconf maintainers?  It would be
best to fix it in upstream Autoconf.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] checkpatch: Check for trailing operators.

[Joe Stringer]

On 14 July 2017 at 14:29, Ben Pfaff  wrote:
> On Fri, Jul 14, 2017 at 02:22:16PM -0700, Joe Stringer wrote:
>> On 14 July 2017 at 14:03, Ben Pfaff  wrote:
>> > On Fri, Jul 14, 2017 at 01:59:06PM -0700, Joe Stringer wrote:
>> >> The style guide states that lines should not end with '&&', '||' '&'
>> >> '|', '?', ':'. Check for this and report an error.
>> >>
>> >> Signed-off-by: Joe Stringer 
>> >
>> > Hmm, is that right?  It's pretty explicit about ?:, but I don't know
>> > about the others.
>>
>> Ah, right you are. It's not explicit about the others. That said,
>> following the explicit description for ?: there's a few examples that
>> apply this style also to &&,||,&,|. I can respin this patch if you
>> think this is too strict.
>
> I personally like to treat && || & | on a case-by-case basis.  Sometimes
> stuff lines up nicer one way, sometimes the other.

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

Re: [ovs-dev] [PATCH] Free port bindings when deleting cached ports.

[Ben Pfaff]

On Fri, Jul 14, 2017 at 02:27:24PM -0500, Mark Michelson wrote:
> Running test "ovn-controller-vtep binding 2" with address sanitizer
> enabled resulted in a failure due to a memory leak. The cached switch
> port's bindings were not being freed when the port was freed. The
> fix is to destroy the bindings hash table when the switch port is
> freed.
> 
> Signed-off-by: Mark Michelson 
> Reported-by: Lance Richardson 

Applied to master, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Support IPv6 link-local address scopes on Linux.

[Ben Pfaff]

On Wed, Jul 12, 2017 at 07:11:31AM +, Darrell Ball wrote:
> 
> 
> On 7/11/17, 9:23 PM, "Ben Pfaff"  wrote:
> 
> Thanks for the review.
> 
> I'm very new to the concept of IPv6 link-local address scoping.  What
> does this subset out of the larger feature?
> 
> There is lots of flexibility here, mostly not relevant to ovs; for example 
> one basic
> zone mapping option is the ifindex directly.
> It can be used for direct mapping to zone id.
> If this option of encoding were to be additionally supported, it would mean
> something like 
> 
> sudo ovs-vsctl set-controller s1 'tcp:[fe80::41f3:ab56:bbab:a528%1]'
> where ‘1’ is the ifindex
> is another option.
> 
> In this case, it means the scope string converted to int would be the
> ifindex directly, something like
> 
> > +char *scope = strsep(&host_s, "%");
> > +if (scope && *scope) {
> > +sin6->sin6_scope_id = str_to_int(scope);
> 
> I did not elaborate earlier since ifindex is mostly secondary in ovs

OK, I see.

I realized that this patch was incomplete: it accepted scope
identifiers, but it dropped them if they needed to be formatted later.
But for formatting, it's unsafe to just randomly put in any netdev name
(it might contain confusing punctuation), so in a case like that one
needs to use the ifindex.  And once that's in there, for consistency one
should accept ifindex values also.

Anyway, to fix all of that, I respun this as a 3-patch series, starting here:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335663.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 3/3] Support IPv6 link-local address scopes on Linux.

[Ben Pfaff]

I hadn't even heard of this feature before, but it seems to be at least
semi-standard to support Linux link-local address scopes via a % suffix,
e.g. fe80::1234%eth0 for a link-local address scoped to eth0.  This commit
adds support.

I'd appreciate feedback from folks who understand this feature better than
me.

Reported-by: Ali Volkan Atli 
Signed-off-by: Ben Pfaff 
---
 NEWS |  2 ++
 configure.ac |  3 +++
 lib/socket-util.c| 65 +---
 lib/vconn-active.man |  7 --
 lib/vconn-passive.man| 10 +---
 ovsdb/remote-active.man  | 20 +++
 ovsdb/remote-passive.man | 29 -
 7 files changed, 92 insertions(+), 44 deletions(-)

diff --git a/NEWS b/NEWS
index f2e453a61e98..d61fc5f7bd4a 100644
--- a/NEWS
+++ b/NEWS
@@ -69,6 +69,8 @@ Post-v2.7.0
- Add experimental support for hardware offloading
  * HW offloading is disabled by default.
  * HW offloading is done through the TC interface.
+   - IPv6 link local addresses are now supported on Linux.  Use % to designate
+ the scope device.
 
 v2.7.0 - 21 Feb 2017
 -
diff --git a/configure.ac b/configure.ac
index 23afe4c7129d..194c4b92ee34 100644
--- a/configure.ac
+++ b/configure.ac
@@ -107,6 +107,9 @@ AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct 
stat.st_mtimensec],
   [], [], [[#include ]])
 AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include ]])
 AC_CHECK_MEMBERS([struct mmsghdr.msg_len], [], [], [[#include ]])
+AC_CHECK_MEMBERS([struct sockaddr_in6.sin6_scope_id], [], [],
+  [[#include 
+#include ]])
 AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r sendmmsg])
 AC_CHECK_HEADERS([mntent.h sys/statvfs.h linux/types.h linux/if_ether.h 
stdatomic.h])
 AC_CHECK_HEADERS([net/if_mib.h], [], [], [[#include 
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 82975aa62981..85ed54c058d6 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -17,6 +17,7 @@
 #include 
 #include "socket-util.h"
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -365,7 +366,7 @@ parse_bracketed_token(char **pp)
 
 static bool
 parse_sockaddr_components(struct sockaddr_storage *ss,
-  const char *host_s,
+  char *host_s,
   const char *port_s, uint16_t default_port,
   const char *s)
 {
@@ -382,20 +383,38 @@ parse_sockaddr_components(struct sockaddr_storage *ss,
 }
 
 memset(ss, 0, sizeof *ss);
-if (strchr(host_s, ':')) {
+if (host_s && strchr(host_s, ':')) {
 struct sockaddr_in6 *sin6
 = ALIGNED_CAST(struct sockaddr_in6 *, ss);
 
+char *addr = strsep(&host_s, "%");
+
 sin6->sin6_family = AF_INET6;
 sin6->sin6_port = htons(port);
-if (!ipv6_parse(host_s, &sin6->sin6_addr)) {
-VLOG_ERR("%s: bad IPv6 address \"%s\"", s, host_s);
+if (!addr || !*addr || !ipv6_parse(addr, &sin6->sin6_addr)) {
+VLOG_ERR("%s: bad IPv6 address \"%s\"", s, addr ? addr : "");
 goto exit;
 }
+
+#ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID
+char *scope = strsep(&host_s, "%");
+if (scope && *scope) {
+if (!scope[strspn(scope, "0123456789")]) {
+sin6->sin6_scope_id = atoi(scope);
+} else {
+sin6->sin6_scope_id = if_nametoindex(scope);
+if (!sin6->sin6_scope_id) {
+VLOG_ERR("%s: bad IPv6 scope \"%s\" (%s)",
+ s, scope, ovs_strerror(errno));
+goto exit;
+}
+}
+}
+#endif
 } else {
 sin->sin_family = AF_INET;
 sin->sin_port = htons(port);
-if (!ip_parse(host_s, &sin->sin_addr.s_addr)) {
+if (host_s && !ip_parse(host_s, &sin->sin_addr.s_addr)) {
 VLOG_ERR("%s: bad IPv4 address \"%s\"", s, host_s);
 goto exit;
 }
@@ -421,7 +440,7 @@ inet_parse_active(const char *target_, uint16_t 
default_port,
 {
 char *target = xstrdup(target_);
 const char *port;
-const char *host;
+char *host;
 char *p;
 bool ok;
 
@@ -548,7 +567,7 @@ inet_parse_passive(const char *target_, int default_port,
 {
 char *target = xstrdup(target_);
 const char *port;
-const char *host;
+char *host;
 char *p;
 bool ok;
 
@@ -559,8 +578,7 @@ inet_parse_passive(const char *target_, int default_port,
 VLOG_ERR("%s: port must be specified", target_);
 ok = false;
 } else {
-ok = parse_sockaddr_components(ss, host ? host : "0.0.0.0",
-   port, default_port, target_);
+ok = parse_sockaddr_components(ss, host, port, default_port, target_);
 }
 if (!ok) {
 memset(ss, 0, sizeof *ss);
@@ -958,6 +976,21 @@ ss_get_port(const struct sockaddr_storage *ss)
 }
 }
 
+

[ovs-dev] [PATCH v2 2/3] socket-util: Change ss_format_address() to take a dynamic string.

[Ben Pfaff]

It's occasionally convenient to format into a fixed-size buffer, but
as the use cases, and the text to be formatted, get more sophisticated,
it becomes easier to deal with "struct ds *" than a buffer pointer and
length pair.  An upcoming commit will make ss_format_address() do more
work, and I think that this is the point at which it becomes easier to
take a dynamic string.  This commit makes the parameter type change
without yet changing what is formatted.

Signed-off-by: Ben Pfaff 
---
 lib/socket-util.c | 36 
 lib/socket-util.h |  6 +++---
 lib/stream-ssl.c  | 25 +
 lib/stream-tcp.c  | 30 +-
 4 files changed, 45 insertions(+), 52 deletions(-)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 04401d49de27..82975aa62981 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -808,11 +808,8 @@ describe_sockaddr(struct ds *string, int fd,
 
 if (!getaddr(fd, (struct sockaddr *) &ss, &len)) {
 if (ss.ss_family == AF_INET || ss.ss_family == AF_INET6) {
-char addrbuf[SS_NTOP_BUFSIZE];
-
-ds_put_format(string, "%s:%"PRIu16,
-  ss_format_address(&ss, addrbuf, sizeof addrbuf),
-  ss_get_port(&ss));
+ss_format_address(&ss, string);
+ds_put_format(string, ":%"PRIu16, ss_get_port(&ss));
 #ifndef _WIN32
 } else if (ss.ss_family == AF_UNIX) {
 struct sockaddr_un sun;
@@ -961,33 +958,32 @@ ss_get_port(const struct sockaddr_storage *ss)
 }
 }
 
-/* Formats the IPv4 or IPv6 address in 'ss' into the 'bufsize' bytes in 'buf'.
- * If 'ss' is an IPv6 address, puts square brackets around the address.
- * 'bufsize' should be at least SS_NTOP_BUFSIZE.
- *
- * Returns 'buf'. */
-char *
-ss_format_address(const struct sockaddr_storage *ss,
-  char *buf, size_t bufsize)
+
+/* Formats the IPv4 or IPv6 address in 'ss' into 's'.  If 'ss' is an IPv6
+ * address, puts square brackets around the address.  'bufsize' should be at
+ * least SS_NTOP_BUFSIZE. */
+void
+ss_format_address(const struct sockaddr_storage *ss, struct ds *s)
 {
-ovs_assert(bufsize >= SS_NTOP_BUFSIZE);
 if (ss->ss_family == AF_INET) {
 const struct sockaddr_in *sin
 = ALIGNED_CAST(const struct sockaddr_in *, ss);
 
-snprintf(buf, bufsize, IP_FMT, IP_ARGS(sin->sin_addr.s_addr));
+ds_put_format(s, IP_FMT, IP_ARGS(sin->sin_addr.s_addr));
 } else if (ss->ss_family == AF_INET6) {
 const struct sockaddr_in6 *sin6
 = ALIGNED_CAST(const struct sockaddr_in6 *, ss);
 
-buf[0] = '[';
-inet_ntop(AF_INET6, sin6->sin6_addr.s6_addr, buf + 1, bufsize - 1);
-strcpy(strchr(buf, '\0'), "]");
+ds_put_char(s, '[');
+ds_reserve(s, s->length + INET6_ADDRSTRLEN);
+char *tail = &s->string[s->length];
+inet_ntop(AF_INET6, sin6->sin6_addr.s6_addr, tail, INET6_ADDRSTRLEN);
+s->length += strlen(tail);
+
+ds_put_char(s, ']');
 } else {
 OVS_NOT_REACHED();
 }
-
-return buf;
 }
 
 size_t
diff --git a/lib/socket-util.h b/lib/socket-util.h
index ef316cb8cc72..1782745fa2c0 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -28,6 +28,8 @@
 #include 
 #include 
 
+struct ds;
+
 int set_nonblocking(int fd);
 void xset_nonblocking(int fd);
 void setsockopt_tcp_nodelay(int fd);
@@ -71,9 +73,7 @@ char *describe_fd(int fd);
 /* Functions for working with sockaddr_storage that might contain an IPv4 or
  * IPv6 address. */
 uint16_t ss_get_port(const struct sockaddr_storage *);
-#define SS_NTOP_BUFSIZE (1 + INET6_ADDRSTRLEN + 1)
-char *ss_format_address(const struct sockaddr_storage *,
-char *buf, size_t bufsize);
+void ss_format_address(const struct sockaddr_storage *, struct ds *);
 size_t ss_length(const struct sockaddr_storage *);
 const char *sock_strerror(int error);
 
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 6dc6f25fddf9..a198d6783dbb 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -825,8 +825,6 @@ static int
 pssl_open(const char *name OVS_UNUSED, char *suffix, struct pstream **pstreamp,
   uint8_t dscp)
 {
-char bound_name[SS_NTOP_BUFSIZE + 16];
-char addrbuf[SS_NTOP_BUFSIZE];
 struct sockaddr_storage ss;
 struct pssl_pstream *pssl;
 uint16_t port;
@@ -844,14 +842,18 @@ pssl_open(const char *name OVS_UNUSED, char *suffix, 
struct pstream **pstreamp,
 }
 
 port = ss_get_port(&ss);
-snprintf(bound_name, sizeof bound_name, "pssl:%"PRIu16":%s",
- port, ss_format_address(&ss, addrbuf, sizeof addrbuf));
+
+struct ds bound_name = DS_EMPTY_INITIALIZER;
+ds_put_format(&bound_name, "pssl:%"PRIu16":", port);
+ss_format_address(&ss, &bound_name);
 
 pssl = xmalloc(sizeof *pssl);
-pstream_init(&pssl->pstream, &pssl_pstream_class, xstrdup(bound_name));
+pstream_init(&pssl->pstream, &pssl_p

[ovs-dev] [PATCH v2 1/3] stream: Make [p]stream_init() take ownership of 'name' parameter.

[Ben Pfaff]

This will be a more sensible interface in an upcoming commit where many of
the callers are assembling dynamic name strings anyway.

Signed-off-by: Ben Pfaff 
---
 lib/stream-fd.c   |  8 ++--
 lib/stream-fd.h   |  4 ++--
 lib/stream-provider.h |  4 ++--
 lib/stream-ssl.c  |  9 +
 lib/stream-tcp.c  | 11 ++-
 lib/stream-unix.c | 17 +++--
 lib/stream-windows.c  |  6 +++---
 lib/stream.c  | 11 ++-
 8 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/lib/stream-fd.c b/lib/stream-fd.c
index 31bfc6ed9549..95373db11d7e 100644
--- a/lib/stream-fd.c
+++ b/lib/stream-fd.c
@@ -53,10 +53,12 @@ static void maybe_unlink_and_free(char *path);
  * 'connect_status' is interpreted as described for stream_init(). 'fd_type'
  * tells whether the socket is TCP or Unix domain socket.
  *
+ * Takes ownership of 'name'.
+ *
  * Returns 0 if successful, otherwise a positive errno value.  (The current
  * implementation never fails.) */
 int
-new_fd_stream(const char *name, int fd, int connect_status, int fd_type,
+new_fd_stream(char *name, int fd, int connect_status, int fd_type,
   struct stream **streamp)
 {
 struct stream_fd *s;
@@ -205,10 +207,12 @@ fd_pstream_cast(struct pstream *pstream)
  * When '*pstreamp' is closed, then 'unlink_path' (if nonnull) will be passed
  * to fatal_signal_unlink_file_now() and freed with free().
  *
+ * Takes ownership of 'name'.
+ *
  * Returns 0 if successful, otherwise a positive errno value.  (The current
  * implementation never fails.) */
 int
-new_fd_pstream(const char *name, int fd,
+new_fd_pstream(char *name, int fd,
int (*accept_cb)(int fd, const struct sockaddr_storage *ss,
 size_t ss_len, struct stream **streamp),
char *unlink_path, struct pstream **pstreamp)
diff --git a/lib/stream-fd.h b/lib/stream-fd.h
index 12452dd619c1..24639900b63e 100644
--- a/lib/stream-fd.h
+++ b/lib/stream-fd.h
@@ -28,9 +28,9 @@ struct stream;
 struct pstream;
 struct sockaddr_storage;
 
-int new_fd_stream(const char *name, int fd, int connect_status,
+int new_fd_stream(char *name, int fd, int connect_status,
   int fd_type, struct stream **streamp);
-int new_fd_pstream(const char *name, int fd,
+int new_fd_pstream(char *name, int fd,
int (*accept_cb)(int fd, const struct sockaddr_storage *ss,
 size_t ss_len, struct stream **),
char *unlink_path,
diff --git a/lib/stream-provider.h b/lib/stream-provider.h
index ce88785ca885..75f4f059b0a1 100644
--- a/lib/stream-provider.h
+++ b/lib/stream-provider.h
@@ -34,7 +34,7 @@ struct stream {
 };
 
 void stream_init(struct stream *, const struct stream_class *,
- int connect_status, const char *name);
+ int connect_status, char *name);
 static inline void stream_assert_class(const struct stream *stream,
const struct stream_class *class)
 {
@@ -135,7 +135,7 @@ struct pstream {
 ovs_be16 bound_port;
 };
 
-void pstream_init(struct pstream *, const struct pstream_class *, const char 
*name);
+void pstream_init(struct pstream *, const struct pstream_class *, char *name);
 void pstream_set_bound_port(struct pstream *, ovs_be16 bound_port);
 static inline void pstream_assert_class(const struct pstream *pstream,
 const struct pstream_class *class)
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 97d6e7464bf5..6dc6f25fddf9 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -220,8 +220,9 @@ want_to_poll_events(int want)
 }
 }
 
+/* Takes ownership of 'name'. */
 static int
-new_ssl_stream(const char *name, int fd, enum session_type type,
+new_ssl_stream(char *name, int fd, enum session_type type,
enum ssl_state state, struct stream **streamp)
 {
 struct ssl_stream *sslv;
@@ -323,7 +324,7 @@ ssl_open(const char *name, char *suffix, struct stream 
**streamp, uint8_t dscp)
  dscp);
 if (fd >= 0) {
 int state = error ? STATE_TCP_CONNECTING : STATE_SSL_CONNECTING;
-return new_ssl_stream(name, fd, CLIENT, state, streamp);
+return new_ssl_stream(xstrdup(name), fd, CLIENT, state, streamp);
 } else {
 VLOG_ERR("%s: connect: %s", name, ovs_strerror(error));
 return error;
@@ -847,7 +848,7 @@ pssl_open(const char *name OVS_UNUSED, char *suffix, struct 
pstream **pstreamp,
  port, ss_format_address(&ss, addrbuf, sizeof addrbuf));
 
 pssl = xmalloc(sizeof *pssl);
-pstream_init(&pssl->pstream, &pssl_pstream_class, bound_name);
+pstream_init(&pssl->pstream, &pssl_pstream_class, xstrdup(bound_name));
 pstream_set_bound_port(&pssl->pstream, htons(port));
 pssl->fd = fd;
 *pstreamp = &pssl->pstream;
@@ -896,7 +897,7 @@ pssl_accept(struct pstream *pstream, struct stream 
**new_streamp)
 

[ovs-dev] [PATCH v2 0/3] Support IPv6 link-local address scopes on Linux

[Ben Pfaff]

v1->v2:
  * Patches 1 and 2 are new.
  * Patch 3 now formats addresses with scope, instead of just parsing them.
Patch 3 accepts ifindexes instead of just interface names.

Ben Pfaff (3):
  stream: Make [p]stream_init() take ownership of 'name' parameter.
  socket-util: Change ss_format_address() to take a dynamic string.
  Support IPv6 link-local address scopes on Linux.

 NEWS |   2 +
 configure.ac |   3 ++
 lib/socket-util.c| 101 +--
 lib/socket-util.h|   6 +--
 lib/stream-fd.c  |   8 +++-
 lib/stream-fd.h  |   4 +-
 lib/stream-provider.h|   4 +-
 lib/stream-ssl.c |  30 +++---
 lib/stream-tcp.c |  37 -
 lib/stream-unix.c|  17 
 lib/stream-windows.c |   6 +--
 lib/stream.c |  11 +++---
 lib/vconn-active.man |   7 +++-
 lib/vconn-passive.man|  10 +++--
 ovsdb/remote-active.man  |  20 +-
 ovsdb/remote-passive.man |  29 ++
 16 files changed, 170 insertions(+), 125 deletions(-)

-- 
2.10.2

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

Re: [ovs-dev] [PATCH] checkpatch: Check for trailing operators.

[Ben Pfaff]

On Fri, Jul 14, 2017 at 02:22:16PM -0700, Joe Stringer wrote:
> On 14 July 2017 at 14:03, Ben Pfaff  wrote:
> > On Fri, Jul 14, 2017 at 01:59:06PM -0700, Joe Stringer wrote:
> >> The style guide states that lines should not end with '&&', '||' '&'
> >> '|', '?', ':'. Check for this and report an error.
> >>
> >> Signed-off-by: Joe Stringer 
> >
> > Hmm, is that right?  It's pretty explicit about ?:, but I don't know
> > about the others.
> 
> Ah, right you are. It's not explicit about the others. That said,
> following the explicit description for ?: there's a few examples that
> apply this style also to &&,||,&,|. I can respin this patch if you
> think this is too strict.

I personally like to treat && || & | on a case-by-case basis.  Sometimes
stuff lines up nicer one way, sometimes the other.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 3/4] ovsdb-idl: idl compound indexes implementation

[Rodriguez Betancourt, Esteban]

I think that is ok to remove that last pointer check in the generic comparison 
function. UUID is more than enough.
Regards,
Esteban

> -Original Message-
> From: Lance Richardson [mailto:lrich...@redhat.com]
> Sent: jueves, 13 de julio de 2017 16:48
> To: Rodriguez Betancourt, Esteban 
> Cc: Ben Pfaff ; d...@openvswitch.org; Albornoz, Javier
> ; Lutz, Arnoldo
> 
> Subject: Re: [ovs-dev] [PATCH v5 3/4] ovsdb-idl: idl compound indexes
> implementation
> 
> > From: "Rodriguez Betancourt, Esteban" 
> > To: "Lance Richardson" , "Ben Pfaff"
> > 
> > Cc: d...@openvswitch.org, "Javier Albornoz" ,
> > "Arnoldo Lutz" 
> > Sent: Thursday, 13 July, 2017 5:22:42 PM
> > Subject: RE: [ovs-dev] [PATCH v5 3/4] ovsdb-idl: idl compound indexes
> > implementation
> >
> > Hello,
> > The UUID comparison was added to guarantee that always rows with the
> > same keys are sorted in the same way (between executions of a
> command,
> > for example). Before that we  used the pointer comparison (which gives
> > us some randomness in sorting) but  we kept it to be really really
> > sure that we are deleting/inserting the correct row.
> >
> > The row sync effectively was intended for finding the correct row to
> > delete when there are duplicated "index keys" and there are
> > deletions/updates/inserts (note that the updates are really handled as
> > a delete-and-reinsert).
> 
> Hi Esteban,
> 
> Thanks, I think all of that makes sense. I do wonder, though, since UUIDs
> should be unique whether it would make just as much to assert that the
> addresses are equal if the UUIDs are equal.
> 
> >
> > We copied ovsdb_idl_index_read from ovsdb_idl_read because we were
> > concerned of what would happen if somebody access the indexes when a
> > transaction is being made. If the index read the new values instead of
> > the old one then the rows could be lost/wrongly sorted, etc. It is the
> > same story with
> > ovsdb_idl_index_write_() and index_set: we wanted the indexes to
> > behave correctly during a transaction with new changes.
> >
> 
> Comparing the v4 version of ovsdb_idl_index_read() against the current
> ovsdb_idl_read(), the only difference between the two that I can find is that
> ovsdb_idl_index_read() takes the table class as a function parameter while
> ovsdb_idl_read() expects row->table to be non-NULL and takes the table
> class from row->table->class.  Since ovsdb_idl_index_init_row() initializes
> row->table appropriately, ovsdb_idl_read() should produce exactly the same
> result as ovsdb_idl_index_read(), whether a transaction on that row is in
> progress or not. So it seems to me we should be able to get rid of
> ovsdb_idl_index_read().
> 
> > ovsdb_idl_index_{find,forward_to} are used inside the autogenerated
> > functions for iterating over the indexes.
> > We thought that it would be nice to be able to say things like:
> >
> > OVSREC_EXAMPLE_FOR_EACH_RANGE(row, cursor, start, end) /* [start,
> end]
> > */ OVSREC_EXAMPLE_FOR_EACH_RANGE(row, cursor, NULL, end) /* [0,
> end]
> > */ OVSREC_EXAMPLE_FOR_EACH_RANGE(row, cursor, start, NULL) /*
> [start,
> > "+infty"] */ OVSREC_EXAMPLE_FOR_EACH_RANGE(row, cursor, NULL,
> NULL) /*
> > everything */
> >
> > Regards,
> > Esteban
> 
> Thanks, that makes sense.  I think it would be good to add tests for these
> cases.
> 
> Thanks again!
> 
>Lance
> 
> >
> > -Original Message-
> > From: Lance Richardson [mailto:lrich...@redhat.com]
> > Sent: jueves, 13 de julio de 2017 14:29
> > To: Ben Pfaff 
> > Cc: d...@openvswitch.org; Rodriguez Betancourt, Esteban
> > ; Albornoz, Javier ; jorge
> > sauma ; Lutz, Arnoldo
> > 
> > Subject: Re: [ovs-dev] [PATCH v5 3/4] ovsdb-idl: idl compound indexes
> > implementation
> >
> >
> >
> > - Original Message -
> > > From: "Ben Pfaff" 
> > > To: "Lance Richardson" 
> > > Cc: d...@openvswitch.org, esteb...@hpe.com, "javier albornoz"
> > > , "jorge sauma"
> > > , "arnoldo lutz guevara"
> > > 
> > > Sent: Tuesday, 11 July, 2017 5:05:32 PM
> > > Subject: Re: [ovs-dev] [PATCH v5 3/4] ovsdb-idl: idl compound
> > > indexes implementation
> > >
> > > On Sat, Jun 24, 2017 at 05:01:51PM -0400, Lance Richardson wrote:
> > > > This patch adds support for the creation of multicolumn indexes in
> > > > the C IDL to enable for efficient search and retrieval of database
> > > > rows by key.
> > > >
> > > > Signed-off-by: Esteban Rodriguez Betancourt 
> > > > Co-authored-by: Lance Richardson 
> > > > Signed-off-by: Lance Richardson 
> > > > ---
> > > >   v5: - Coding style fixes (checkpatch.py)
> > > >   - Fixed memory leak (missing ovsdb_datum_destroy() in
> > > > ovsdb_idl_index_destroy_row__()).
> > > >   - Some polishing of comment and log message text.
> > >
> > > Thanks for reviving this series.
> > >
> > > I don't understand ovsdb_idl_index_read().  It is almost the same as
> > > ovsdb_idl_read().  It looks like ovsdb_idl_read() could be
> > > implemented as a wrapper around it, but I'm also not sure why
> > > ovsdb_idl_

Re: [ovs-dev] [PATCH] checkpatch: Check for trailing operators.

[Joe Stringer]

On 14 July 2017 at 14:03, Ben Pfaff  wrote:
> On Fri, Jul 14, 2017 at 01:59:06PM -0700, Joe Stringer wrote:
>> The style guide states that lines should not end with '&&', '||' '&'
>> '|', '?', ':'. Check for this and report an error.
>>
>> Signed-off-by: Joe Stringer 
>
> Hmm, is that right?  It's pretty explicit about ?:, but I don't know
> about the others.

Ah, right you are. It's not explicit about the others. That said,
following the explicit description for ?: there's a few examples that
apply this style also to &&,||,&,|. I can respin this patch if you
think this is too strict.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1] socket-util: Fix recursion issue in sendmmsg

[Ben Pfaff]

On Fri, Jul 14, 2017 at 01:11:01PM -0700, Joe Stringer wrote:
> On 14 July 2017 at 08:41, Ben Pfaff  wrote:
> > On Fri, Jul 14, 2017 at 03:06:50PM +, Zhenyu Gao wrote:
> >> The wrap_sendmmsg has infinite recursion issue.
> >> Fix it by undef sendmmsg.
> >>
> >> Signed-off-by: Zhenyu Gao 
> >
> > Thanks!  I applied this to master.
> 
> It looks like this breaks Travis:
> https://travis-ci.org/openvswitch/ovs/builds/253660557
> 
> Can we look into this or revert the change?

Oops.

I sent a fix:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335657.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] sparse: Add missing protoype for sendmmsg.

[Ben Pfaff]

Reported-by: Joe Stringer 
Signed-off-by: Ben Pfaff 
Fixes: 00f5565c7eed ("socket-util: Fix recursion issue in sendmmsg")
---
 include/sparse/sys/socket.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/sparse/sys/socket.h b/include/sparse/sys/socket.h
index 88a5387e7f9b..4178f57e2bda 100644
--- a/include/sparse/sys/socket.h
+++ b/include/sparse/sys/socket.h
@@ -162,6 +162,7 @@ ssize_t recvfrom(int, void *, size_t, int, struct sockaddr 
*, socklen_t *);
 ssize_t recvmsg(int, struct msghdr *, int);
 ssize_t send(int, const void *, size_t, int);
 ssize_t sendmsg(int, const struct msghdr *, int);
+int sendmmsg(int, struct mmsghdr *, unsigned int, unsigned int);
 ssize_t sendto(int, const void *, size_t, int, const struct sockaddr *,
socklen_t);
 int setsockopt(int, int, int, const void *, socklen_t);
-- 
2.10.2

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

Re: [ovs-dev] [PATCH] checkpatch: Check for trailing operators.

[Ben Pfaff]

On Fri, Jul 14, 2017 at 01:59:06PM -0700, Joe Stringer wrote:
> The style guide states that lines should not end with '&&', '||' '&'
> '|', '?', ':'. Check for this and report an error.
> 
> Signed-off-by: Joe Stringer 

Hmm, is that right?  It's pretty explicit about ?:, but I don't know
about the others.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] checkpatch: Check for trailing operators.

[Joe Stringer]

The style guide states that lines should not end with '&&', '||' '&'
'|', '?', ':'. Check for this and report an error.

Signed-off-by: Joe Stringer 
---
 utilities/checkpatch.py | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 65d188d607f1..543762c22bd9 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -87,6 +87,7 @@ __regex_ends_with_bracket = \
 re.compile(r'[^\s]\) {(\s+/\*[\s\Sa-zA-Z0-9\.,\?\*/+-]*)?$')
 __regex_ptr_declaration_missing_whitespace = re.compile(r'[a-zA-Z0-9]\*[^*]')
 __regex_is_comment_line = re.compile(r'^\s*(/\*|\*\s)')
+__regex_trailing_operator = re.compile(r'.*[&|]?[?:&|]$')
 
 skip_leading_whitespace_check = False
 skip_trailing_whitespace_check = False
@@ -199,6 +200,11 @@ def is_comment_line(line):
 return __regex_is_comment_line.match(line) is not None
 
 
+def trailing_operator(line):
+"""Returns TRUE if the current line ends with an operator, eg &&"""
+return __regex_trailing_operator.match(line) is not None
+
+
 checks = [
 {'regex': None,
  'match_name':
@@ -230,7 +236,12 @@ checks = [
  'prereq': lambda x: not is_comment_line(x),
  'check': lambda x: pointer_whitespace_check(x),
  'print':
- lambda: print_error("Inappropriate spacing in pointer declaration")}
+ lambda: print_error("Inappropriate spacing in pointer declaration")},
+
+{'regex': '(.c|.h)(.in)?$', 'match_name': None,
+ 'prereq': lambda x: not is_comment_line(x),
+ 'check': lambda x: trailing_operator(x),
+ 'print': lambda: print_error("Line has operator at end of line")},
 ]
 
 
-- 
2.11.1

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

Re: [ovs-dev] [PATCH] dp-packet: Remove misleading comment for refill init function.

[Andy Zhou]

On Fri, Jul 14, 2017 at 1:11 AM, Ilya Maximets  wrote:
> Function 'dp_packet_batch_refill_init' doesn't return anything.
> Looks like this comment came from one of the intermediate versions
> of the API enhancement patch. Additionally comment style changed
> to be consistent with other comments in the same file.
>
> CC: Andy Zhou 
> Fixes: 72c84bc2db23 ("dp-packet: Enhance packet batch APIs.")
> Signed-off-by: Ilya Maximets 

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

Re: [ovs-dev] [PATCH v1] socket-util: Fix recursion issue in sendmmsg

[Joe Stringer]

On 14 July 2017 at 08:41, Ben Pfaff  wrote:
> On Fri, Jul 14, 2017 at 03:06:50PM +, Zhenyu Gao wrote:
>> The wrap_sendmmsg has infinite recursion issue.
>> Fix it by undef sendmmsg.
>>
>> Signed-off-by: Zhenyu Gao 
>
> Thanks!  I applied this to master.

It looks like this breaks Travis:
https://travis-ci.org/openvswitch/ovs/builds/253660557

Can we look into this or revert the change?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto.at: Fix minor typo in test.

[Ben Pfaff]

On Sun, Jul 09, 2017 at 11:51:40PM -0700, Justin Pettit wrote:
> Signed-off-by: Justin Pettit 
> ---
>  tests/ofproto.at | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 9e6acfad653d..c7ea31a77ce9 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -3430,7 +3430,7 @@ 
> udp,vlan_tci=0x,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172
>  ovs-ofctl -O OpenFlow13 add-flow br0 send_flow_rem,actions=group:1234
>  ovs-ofctl -O OpenFlow13 --strict del-groups br0 group_id=1234
>  if test X"$1" = X"OFPRR_DELETE"; then shift;
> -echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=gropu_delete 
> table_id=0"
> +echo >>expout "OFPT_FLOW_REMOVED (OF1.3):  reason=group_delete 
> table_id=0"

Why didn't this cause a test failure?  (Is this code never actually
executed?)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] vlog: Make "vlog/set" without args set everything to "dbg".

[Ben Pfaff]

On Sun, Jul 09, 2017 at 11:51:41PM -0700, Justin Pettit wrote:
> The documentation stated that "ovs-appctl vlog/set" without any
> arguments should set every module and destination to debug level.
> 
> Signed-off-by: Justin Pettit 

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

Re: [ovs-dev] [PATCH] dpif-netdev: Initialize 'tun_md' member of match.

[Ben Pfaff]

On Sun, Jul 09, 2017 at 11:51:38PM -0700, Justin Pettit wrote:
> Found by valgrind.

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

[ovs-dev] [PATCH] ofp-util: Update OpenFlow 1.6 port support to track latest proposal.

[Ben Pfaff]

The latest updates to the OpenFlow 1.6 proposal removes the hw_addr_type
fields from ofp_port and ofp_port_mod.  This commit updates the OVS
prototype to match the updated proposal.

ONF-JIRA: EXT-566
Signed-off-by: Ben Pfaff 
---
 include/openflow/openflow-1.6.h | 13 +++--
 lib/ofp-util.c  | 35 +++
 2 files changed, 10 insertions(+), 38 deletions(-)

diff --git a/include/openflow/openflow-1.6.h b/include/openflow/openflow-1.6.h
index 1ba3cbd6fb3d..13c0b7bd5037 100644
--- a/include/openflow/openflow-1.6.h
+++ b/include/openflow/openflow-1.6.h
@@ -55,18 +55,12 @@
 
 #define OFP16_MAX_PORT_NAME_LEN  64
 
-/* Bitmap of hardware address types supported by an OpenFlow port. */
-enum ofp16_hardware_address_type {
-OFPPHAT16_EUI48 = 1 << 0,   /* 48-bit Ethernet address. */
-OFPPHAT16_EUI64 = 1 << 1,   /* 64-bit Ethernet address. */
-};
-
 struct ofp16_port {
 ovs_be32 port_no;
 ovs_be16 length;
-ovs_be16 hw_addr_type;/* Zero or more OFPPHAT16_*. */
-struct eth_addr hw_addr;  /* EUI-48 hardware address. */
 uint8_t pad[2];   /* Align to 64 bits. */
+struct eth_addr hw_addr;  /* EUI-48 hardware address. */
+uint8_t pad2[2];  /* Align to 64 bits. */
 struct eth_addr64 hw_addr64;  /* EUI-64 hardware address */
 char name[OFP16_MAX_PORT_NAME_LEN]; /* Null-terminated */
 
@@ -80,8 +74,7 @@ OFP_ASSERT(sizeof(struct ofp16_port) == 96);
 
 struct ofp16_port_mod {
 ovs_be32 port_no;
-ovs_be16 hw_addr_type;   /* Zero or more OFPPHAT16_*. */
-uint8_t pad[2];  /* Align to 64 bits. */
+uint8_t pad[4];  /* Align to 64 bits. */
 struct eth_addr hw_addr;
 uint8_t pad2[2];
 struct eth_addr64 hw_addr64; /* EUI-64 hardware address */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6052d3cc5f56..3e1ace1314c1 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -4517,12 +4517,8 @@ ofputil_pull_ofp16_port(struct ofputil_phy_port *pp, 
struct ofpbuf *msg)
 if (error) {
 return error;
 }
-if (op->hw_addr_type & htons(OFPPHAT16_EUI48)) {
-pp->hw_addr = op->hw_addr;
-}
-if (op->hw_addr_type & htons(OFPPHAT16_EUI64)) {
-pp->hw_addr64 = op->hw_addr64;
-}
+pp->hw_addr = op->hw_addr;
+pp->hw_addr64 = op->hw_addr64;
 ovs_strlcpy_arrays(pp->name, op->name);
 
 pp->config = ntohl(op->config) & OFPPC11_ALL;
@@ -4616,14 +4612,8 @@ ofputil_put_ofp16_port(const struct ofputil_phy_port 
*pp, struct ofpbuf *b)
 op = ofpbuf_put_zeros(b, sizeof *op);
 op->port_no = ofputil_port_to_ofp11(pp->port_no);
 op->length = htons(sizeof *op + sizeof *eth);
-if (!eth_addr_is_zero(pp->hw_addr)) {
-op->hw_addr_type |= htons(OFPPHAT16_EUI48);
-op->hw_addr = pp->hw_addr;
-}
-if (!eth_addr64_is_zero(pp->hw_addr64)) {
-op->hw_addr_type |= htons(OFPPHAT16_EUI64);
-op->hw_addr64 = pp->hw_addr64;
-}
+op->hw_addr = pp->hw_addr;
+op->hw_addr64 = pp->hw_addr64;
 ovs_strlcpy_arrays(op->name, pp->name);
 op->config = htonl(pp->config & OFPPC11_ALL);
 op->state = htonl(pp->state & OFPPS11_ALL);
@@ -5180,13 +5170,8 @@ ofputil_decode_ofp16_port_mod(struct ofpbuf *b, bool 
loose,
 return error;
 }
 
-if (opm->hw_addr_type & htons(OFPPHAT16_EUI48)) {
-pm->hw_addr = opm->hw_addr;
-}
-if (opm->hw_addr_type & htons(OFPPHAT16_EUI64)) {
-pm->hw_addr64 = opm->hw_addr64;
-}
 pm->hw_addr = opm->hw_addr;
+pm->hw_addr64 = opm->hw_addr64;
 pm->config = ntohl(opm->config) & OFPPC11_ALL;
 pm->mask = ntohl(opm->mask) & OFPPC11_ALL;
 
@@ -5282,14 +5267,8 @@ ofputil_encode_port_mod(const struct ofputil_port_mod 
*pm,
 b = ofpraw_alloc(OFPRAW_OFPT16_PORT_MOD, ofp_version, 0);
 opm = ofpbuf_put_zeros(b, sizeof *opm);
 opm->port_no = ofputil_port_to_ofp11(pm->port_no);
-if (!eth_addr_is_zero(pm->hw_addr)) {
-opm->hw_addr_type |= htons(OFPPHAT16_EUI48);
-opm->hw_addr = pm->hw_addr;
-}
-if (!eth_addr64_is_zero(pm->hw_addr64)) {
-opm->hw_addr_type |= htons(OFPPHAT16_EUI64);
-opm->hw_addr64 = pm->hw_addr64;
-}
+opm->hw_addr = pm->hw_addr;
+opm->hw_addr64 = pm->hw_addr64;
 opm->config = htonl(pm->config & OFPPC11_ALL);
 opm->mask = htonl(pm->mask & OFPPC11_ALL);
 
-- 
2.10.2

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

[ovs-dev] [PATCH V3 net] openvswitch: Fix for force/commit action failures

[Greg Rose]

When there is an established connection in direction A->B, it is
possible to receive a packet on port B which then executes
ct(commit,force) without first performing ct() - ie, a lookup.
In this case, we would expect that this packet can delete the existing
entry so that we can commit a connection with direction B->A. However,
currently we only perform a check in skb_nfct_cached() for whether
OVS_CS_F_TRACKED is set and OVS_CS_F_INVALID is not set, ie that a
lookup previously occurred. In the above scenario, a lookup has not
occurred but we should still be able to statelessly look up the
existing entry and potentially delete the entry if it is in the
opposite direction.

This patch extends the check to also hint that if the action has the
force flag set, then we will lookup the existing entry so that the
force check at the end of skb_nfct_cached has the ability to delete
the connection.

Fixes: dd41d330b03 ("openvswitch: Add force commit.")
CC: Pravin Shelar 
CC: d...@openvswitch.org
Signed-off-by: Joe Stringer 
Signed-off-by: Greg Rose 
---

V2: Make sure nf_conntrack_in() is called for force case
V3: Fix compiler warning for newer compilers
---
 net/openvswitch/conntrack.c | 51 -
 1 file changed, 36 insertions(+), 15 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 08679eb..e3c4c6c 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -629,6 +629,34 @@ static int handle_fragments(struct net *net, struct 
sw_flow_key *key,
return ct;
 }
 
+static
+struct nf_conn *ovs_ct_executed(struct net *net,
+   const struct sw_flow_key *key,
+   const struct ovs_conntrack_info *info,
+   struct sk_buff *skb,
+   bool *ct_executed)
+{
+   struct nf_conn *ct = NULL;
+
+   /* 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, or if the direction is being forced.  If the
+* connection was not confirmed, it is not cached and needs to be run
+* through conntrack again.
+*/
+   *ct_executed = (key->ct_state & OVS_CS_F_TRACKED) &&
+  !(key->ct_state & OVS_CS_F_INVALID) &&
+  (key->ct_zone == info->zone.id);
+
+   if (*ct_executed || (!key->ct_state && info->force)) {
+   ct = ovs_ct_find_existing(net, &info->zone, info->family, skb,
+ !!(key->ct_state &
+ OVS_CS_F_NAT_MASK));
+   }
+
+   return ct;
+}
+
 /* Determine whether skb->_nfct is equal to the result of conntrack lookup. */
 static bool skb_nfct_cached(struct net *net,
const struct sw_flow_key *key,
@@ -637,24 +665,17 @@ static bool skb_nfct_cached(struct net *net,
 {
enum ip_conntrack_info ctinfo;
struct nf_conn *ct;
+   bool ct_executed = true;
 
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,
- !!(key->ct_state
-& OVS_CS_F_NAT_MASK));
-   if (ct)
-   nf_ct_get(skb, &ctinfo);
-   }
if (!ct)
+   ct = ovs_ct_executed(net, key, info, skb, &ct_executed);
+
+   if (ct)
+   nf_ct_get(skb, &ctinfo);
+   else
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)))
@@ -679,7 +700,7 @@ static bool skb_nfct_cached(struct net *net,
return false;
}
 
-   return true;
+   return ct_executed;
 }
 
 #ifdef CONFIG_NF_NAT_NEEDED
-- 
1.8.3.1

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

Re: [ovs-dev] [patch_v4 4/8] Userspace Datapath: Add ALG infra and FTP.

[Ben Pfaff]

Wow, that was fast.

I'll look forward to v5.

On Fri, Jul 14, 2017 at 05:37:43PM +, Darrell Ball wrote:
> Thanks a lot for the review Ben
> 
> On 7/13/17, 3:15 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
>  wrote:
> 
> On Wed, Jul 05, 2017 at 09:32:22PM -0700, Darrell Ball wrote:
> > ALG infra and FTP (both V4 and V6) support is added to the userspace
> > datapath.  Also, NAT support is included.
> > 
> > Signed-off-by: Darrell Ball 
> 
> Thanks a lot for working on this.  It will be a valuable feature that
> brings the userspace datapath closer to the kernel datapath features.
> 
> I have some comments.  I'm not too familiar with this area, so a lot of
> my comments are ones that should help to make the code easier to
> understand not just for me but for other newbies to ALG implementation.
> 
> The data structures introduced or modified in this patch are almost
> comment-free.  The reader is left to guess important information like
> the purpose of the structure, as well as per-member info like what lists
> or hmaps a structure belongs to, what a "master" is, and so on.  Please
> add some comments.
> 
> Done
> 
> The same seems warranted for the collection of macros that this
> defines.  For example, what does FTP_MAX_PORT mean?  (If it's just the
> maximum TCP port, what makes it FTP specific? etc.)
> 
> Done
> 
> In the name "alg_exp_node", I don't know what an exp is.
> 
> Done
> 
> I don't know what "delinate" means.
> 
> Name changed
> 
> What OS X problem does this allude to?
> +/* CT_IPPORT_FTP is used in lieu of IPPORT_FTP as a workaround
> + * to handle OSX. */
> +#define CT_IPPORT_FTP 21
> 
> I added comment that it was build related.
> 
> Usually we put the conversion on the constant side in comparisons like
> the following, because compilers are better at optimizing it:
> +return (ip_proto == IPPROTO_TCP &&
> +(ntohs(th->tcp_src) == CT_IPPORT_FTP ||
> + ntohs(th->tcp_dst) == CT_IPPORT_FTP));
> 
> Got it
> 
> I guess that not everyone knows what "tuple space" is.  I had to think
> about it for a while.  I think you basically mean the ports available
> for NAT.  Maybe a more user friendly term could be used (at least in
> user messages)?  Why is exhaustion "likely a user error"?  (I would have
> guessed that it is more likely from some kind of DoS or port scan or
> equivalent.)  How should a user respond?
> 
> Sure, DoS is valid too as the system may be unprotected. I expanded the
> comments and added recommendations.
> 
> 
> process_one() has a variable alg_nat_repl_addr that it zeros and then
> appears to never use again.
> 
> I deprecated that variable use and I forgot to cleanup.
> 
> Also in process_one(), I think that this memcpy:
> memcpy(&alg_exp_entry, alg_exp, sizeof alg_exp_entry);
> can be written as just:
> alg_exp_entry = *alg_exp;
> although I don't know whether you have some expectation for padding etc.
> 
> structure copy is fine here; I thought I had caught all of these.
> 
> 
> process_one() uses the expression "conn && is_ftp_ctl(pkt)" twice, and
> the latter function is nontrivial.  Can it be evaluated just once?
> 
> Yes, thanks for pointing that out.
> 
> In conntrack_execute(), it seems odd to move the loop index declaration
> out of the for statement.
> 
> This is day one conntrack code, but I can fix it here.
> 
> conn_to_ct_dpif_entry() does an xstrdup but I wasn't able to spot where
> the string gets freed.
> 
> This is an existing API that I just used; the caller (in dpctl) frees the 
> string.
> I added a comment that the caller frees it.
> 
> I can't see how get_v4_byte_be() works properly on both big-endian and
> little-endian systems.
> 
> get_v4_byte_be() shouldn't need the "& 0xff", since it returns a
> uint8_t.
> 
> right, mask is not needed
> 
> In replace_substring(), it seems odd to use 8-bit quantities for sizes.
> Also, it looks like 'delta' can be negative, in which case it should not
> be plain char (which can be signed or unsigned given an ASCII character
> set): if you really want 8-bit, use "signed char" or int8_t.
> 
> Thanks for catching this; this is a real potential bug depending on the 
> compiler 
> and settings. At one point, I checked and converted all signed values to 
> either
> int or int64_t; this looks like the only one left over.
> 
> The expression "remain_substring + delta" in replace_substring() kind of
> threw me for a loop.  If you expand this based on variable definitions,
> you get:
> 
> remain_substring + delta
>  == (substr + substr_size) + (rep_str_size - substr_size)
>  == substr + rep_str_size
> 
> In oth

[ovs-dev] [PATCH] Free port bindings when deleting cached ports.

[Mark Michelson]

Running test "ovn-controller-vtep binding 2" with address sanitizer
enabled resulted in a failure due to a memory leak. The cached switch
port's bindings were not being freed when the port was freed. The
fix is to destroy the bindings hash table when the switch port is
freed.

Signed-off-by: Mark Michelson 
Reported-by: Lance Richardson 
---
 vtep/vtep-ctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index bbdf57fad..17afa0c59 100644
--- a/vtep/vtep-ctl.c
+++ b/vtep/vtep-ctl.c
@@ -542,6 +542,7 @@ del_cached_port(struct vtep_ctl_context *vtepctl_ctx,
 ovs_list_remove(&port->ports_node);
 shash_find_and_delete(&vtepctl_ctx->ports, cache_name);
 vteprec_physical_port_delete(port->port_cfg);
+shash_destroy(&port->bindings);
 free(cache_name);
 free(port);
 }
-- 
2.13.3

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

[ovs-dev] [PATCH v2 3/3] tests: Extend PTAP unit tests with decap action

[Zoltán Balogh]

From: Zoltán Balogh 

  - Checking decap() prerequisits.
  - Send L3 packet over patch port.
  - Output L2/L3 packet to ports with different packet_type properties.

Signed-off-by: Zoltán Balogh 
Suggested-by: Jan Scheurich 
---
 tests/packet-type-aware.at | 333 +
 1 file changed, 333 insertions(+)

diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
index c335b88ca..e90ba0d4a 100644
--- a/tests/packet-type-aware.at
+++ b/tests/packet-type-aware.at
@@ -519,3 +519,336 @@ 
tunnel(src=20.0.0.3,dst=20.0.0.2,flags(-df-csum)),recirc_id(0),in_port(gre_sys),
 
 OVS_VSWITCHD_STOP(["/The Open vSwitch kernel module is probably not loaded/d"])
 AT_CLEANUP
+
+
+AT_SETUP([ptap - check decap() prerequisits])
+OVS_VSWITCHD_START
+
+# Decap IP header, then set IP destination address. This should fail.
+AT_CHECK([
+ovs-ofctl add-flow br0 
"in_port=1,packet_type=(1,0x800),actions=decap(),set_field:1.1.1.1->nw_dst"
+], [1], [stdout], [stderr])
+
+AT_CHECK([
+cat stderr | cut -d '|' -f 3-
+], [0], [dnl
+ofp_actions|WARN|set_field ip_dst lacks correct prerequisites
+ovs-ofctl: actions are invalid with specified match (OFPBAC_MATCH_INCONSISTENT)
+])
+
+# Decap Ethernet header, then set IP destination address. This should work.
+AT_CHECK([
+ovs-ofctl add-flow br0 -OOpenFlow13 
"in_port=1,ip,actions=decap(),set_field:1.1.1.1->nw_dst"
+], [0])
+
+# Decap IP header, then set metadata. This should work.
+AT_CHECK([
+ovs-ofctl add-flow br0 -OOpenFlow13 
"in_port=1,packet_type=(1,0x800),actions=decap(),set_field:1234->metadata"
+], [0])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
+AT_SETUP([ptap - L3 over patch port])
+
+
+# L3 over patch port
+#
+# (192.168.10.10) (192.168.10.30)
+#n0  n1
+# |   |
+#  +--o--+ +--o--+
+#  |   br0   | |   br1   |
+#  +--o--+ +--o---o--+
+#  p0 |p1 |   gre1 (ptap)
+# +---+   10.0.0.1
+#
+# LOCAL
+#  +--o--+
+#  |   br2   |
+#  +--o--+
+# |
+#n2
+# 10.0.0.2
+
+HWADDR_BRP2=aa:55:00:00:00:02
+
+OVS_VSWITCHD_START([dnl
+-- add-br br1 \
+-- set bridge br1 datapath_type=dummy fail-mode=secure \
+-- add-br br2 \
+-- set bridge br2 datapath_type=dummy fail-mode=secure \
+   other_config:hwaddr=\"$HWADDR_BRP2\" \
+-- add-port br0 p0 \
+-- set interface p0 type=patch options:peer=p1 ofport_request=10 \
+-- add-port br1 p1 \
+-- set interface p1 type=patch options:peer=p0 ofport_request=20 \
+-- add-port br0 n0 \
+-- set interface n0 type=dummy ofport_request=30 \
+-- add-port br1 n1 \
+-- set interface n1 type=dummy options:tx_pcap=n1.pcap ofport_request=40 \
+-- add-port br2 n2 \
+-- set interface n2 type=dummy options:tx_pcap=n2.pcap ofport_request=50 \
+-- add-port br1 gre1 \
+-- set interface gre1 type=gre options:remote_ip=10.0.0.2 \
+   options:packet_type=ptap ofport_request=100
+])
+
+### Verify datapath configuration
+AT_CHECK([
+ovs-appctl dpif/show | grep -v hit | sed 's/\t//g' | sed 
's./[[0-9]]\{1,\}..'
+], [0], [dnl
+br0:
+br0 65534: (dummy-internal)
+n0 30: (dummy)
+p0 10/none: (patch: peer=p1)
+br1:
+br1 65534: (dummy-internal)
+gre1 100: (gre: packet_type=ptap, remote_ip=10.0.0.2)
+n1 40: (dummy)
+p1 20/none: (patch: peer=p0)
+br2:
+br2 65534: (dummy-internal)
+n2 50: (dummy)
+])
+
+AT_CHECK([
+ovs-appctl netdev-dummy/ip4addr br2 10.0.0.1/24 &&
+ovs-appctl ovs/route/add 10.0.0.0/24 br2 &&
+ovs-appctl tnl/arp/set br2 10.0.0.2 de:af:be:ef:ba:be
+], [0], [ignore])
+
+AT_CHECK([
+ovs-appctl ovs/route/show | grep User:
+], [0], [dnl
+User: 10.0.0.0/24 dev br2 SRC 10.0.0.1
+])
+
+
+AT_CHECK([
+ovs-ofctl del-flows br0 &&
+ovs-ofctl del-flows br1 &&
+ovs-ofctl del-flows br2 &&
+ovs-ofctl add-flow br0 in_port=n0,actions=decap,output=p0 -OOpenFlow13 &&
+ovs-ofctl add-flow br1 in_port=p1,actions=output=gre1 &&
+ovs-ofctl add-flow br2 in_port=LOCAL,actions=output=n2
+], [0])
+
+AT_CHECK([ovs-ofctl -OOpenFlow13 dump-flows br0 | ofctl_strip | grep actions],
+[0], [dnl
+ in_port=30 actions=decap(),output:10
+])
+
+AT_CHECK([ovs-ofctl -OOpenFlow13 dump-flows br1 | ofctl_strip | grep actions],
+[0], [dnl
+ reset_counts in_port=20 actions=output:100
+])
+
+AT_CHECK([ovs-ofctl -OOpenFlow13 dump-flows br2 | ofctl_strip | grep actions],
+[0], [dnl
+ reset_counts in_port=LOCAL actions=output:50
+])
+
+AT_CHECK([
+ovs-appctl netdev-dummy/receive n0 
1e2ce92a669e3a6dd2099cab080045548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a5827150200101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f30313233343

[ovs-dev] [PATCH v2 2/3] OF support and translation of generic encap and decap

[Zoltán Balogh]

From: Jan Scheurich 

This commit adds support for the OpenFlow actions generic encap
and decap (as specified in ONF EXT-382) to the OVS control plane.

CLI syntax for encap action with properties:
  encap(hdr=)
  encap(hdr=,
prop(class=,type=,val=),
prop(class=,type=,val()))

CLI syntax for decap action:
  decap()
  decap(packet_type(ns=,type=))

The first header supported for encap and decap is "ethernet" to convert
packets between packet_type (1,Ethertype) and (0,0).

This commit also implements a skeleton for the translation of generic
encap and decap actions in ofproto-dpif and adds support to encap and
decap an Ethernet header.

In general translation of encap commits pending actions and then rewrites
struct flow in accordance with the new packet type and header. In the
case of encap(ethernet) it suffices to change the packet type from
(1, Ethertype) to (0,0) and set the dl_type accordingly. A new
pending_encap flag in xlate ctx is set to mark that an corresponding
datapath encap action must be triggered at the next commit. In the
case of encap(ethernet) ofproto generetas a push_eth action.

The general case for translation of decap() is to emit a datapath action
to decap the current outermost header and then recirculate the packet
to reparse the inner headers. In the special case of an Ethernet packet,
decap() just changes the packet type from (0,0) to (1, dl_type) without
a need to recirculate. The emission of the pop_eth action for the
datapath is postponed to the next commit.

Hence encap(ethernet) and decap() on an Ethernet packet are OF octions
that only incur a cost in the dataplane when a modifed packet is
actually committed, e.g. because it is sent out. They can freely be
used for normalizing the packet type in the OF pipeline without
degrading performance.

Signed-off-by: Jan Scheurich 
Signed-off-by: Yi Yang 
Signed-off-by: Zoltán Balogh 
Co-authored-by: Zoltán Balogh 
---
 include/openflow/openflow-common.h |   1 +
 include/openvswitch/automake.mk|   1 +
 include/openvswitch/ofp-actions.h  |  31 +++
 include/openvswitch/ofp-ed-props.h |  69 +++
 include/openvswitch/ofp-errors.h   |   9 +
 lib/automake.mk|   1 +
 lib/odp-util.c |  84 ++---
 lib/odp-util.h |   3 +-
 lib/ofp-actions.c  | 378 -
 lib/ofp-ed-props.c | 150 +++
 lib/packets.h  |   3 +-
 ofproto/ofproto-dpif-xlate.c   | 108 ++-
 12 files changed, 796 insertions(+), 42 deletions(-)
 create mode 100644 include/openvswitch/ofp-ed-props.h
 create mode 100644 lib/ofp-ed-props.c

diff --git a/include/openflow/openflow-common.h 
b/include/openflow/openflow-common.h
index 3a0a57550..ff19cf5ca 100644
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -465,6 +465,7 @@ enum ofp_header_type_namespaces {
 OFPHTN_IP_PROTO = 2,/* ns_type is a IP protocol number. */
 OFPHTN_UDP_TCP_PORT = 3,/* ns_type is a TCP or UDP port. */
 OFPHTN_IPV4_OPTION = 4, /* ns_type is an IPv4 option number. */
+OFPHTN_N_TYPES
 };
 
 #endif /* openflow/openflow-common.h */
diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index 699d9d74e..c8e67a236 100644
--- a/include/openvswitch/automake.mk
+++ b/include/openvswitch/automake.mk
@@ -12,6 +12,7 @@ openvswitchinclude_HEADERS = \
include/openvswitch/meta-flow.h \
include/openvswitch/ofpbuf.h \
include/openvswitch/ofp-actions.h \
+   include/openvswitch/ofp-ed-props.h \
include/openvswitch/ofp-errors.h \
include/openvswitch/ofp-msgs.h \
include/openvswitch/ofp-parse.h \
diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index 7b4aa9201..7b9f6c199 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -25,6 +25,7 @@
 #include "openvswitch/ofp-util.h"
 #include "openvswitch/ofp-errors.h"
 #include "openvswitch/types.h"
+#include "openvswitch/ofp-ed-props.h"
 
 struct vl_mff_map;
 
@@ -89,6 +90,10 @@ struct vl_mff_map;
 OFPACT(PUSH_MPLS,   ofpact_push_mpls,   ofpact, "push_mpls")\
 OFPACT(POP_MPLS,ofpact_pop_mpls,ofpact, "pop_mpls") \
 \
+/* Generic encap & decap */ \
+OFPACT(ENCAP,   ofpact_encap,   props, "encap") \
+OFPACT(DECAP,   ofpact_decap,   ofpact, "decap")\
+\
 /* Metadata. */ \
 OFPACT(SET_TUNNEL,  ofpact_tunnel,  ofpact, "set_tunnel")   \
 OFPACT(SET_QUEUE,   ofpact_queue,   ofpact, "set_queue")\
@@ -969,6 +974,32 @@ struct ofpact_unroll_xlate {
 ovs_be64

[ovs-dev] [PATCH v2 1/3] ofproto-dpif-xlate: drop L3 packets on L2 legacy port

[Zoltán Balogh]

From: Zoltán Balogh 

This commit drops packet during xlate if it is a L3 packet and output
port packet_type is legacy_l2. New PTAP unit test is added.

Signed-off-by: Zoltán Balogh 
---
 ofproto/ofproto-dpif-xlate.c | 23 +++-
 tests/packet-type-aware.at   | 64 +---
 2 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 089c7f170..08dd9fe6d 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3312,6 +3312,14 @@ check_output_prerequisites(struct xlate_ctx *ctx,
 return false;
 }
 }
+
+if (xport->pt_mode == NETDEV_PT_LEGACY_L2 &&
+flow->packet_type != htonl(PT_ETH)) {
+xlate_report(ctx, OFT_WARN, "Trying to send non-Ethernet packet "
+ "through legacy L2 port. Dropping packet.");
+return false;
+}
+
 return true;
 }
 
@@ -3345,6 +3353,10 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
 odp_port_t out_port, odp_port, odp_tnl_port;
 bool is_native_tunnel = false;
 uint8_t dscp;
+struct eth_addr flow_dl_dst = flow->dl_dst;
+struct eth_addr flow_dl_src = flow->dl_src;
+ovs_be32 flow_packet_type = flow->packet_type;
+ovs_be16 flow_dl_type = flow->dl_type;
 
 /* If 'struct flow' gets additional metadata, we'll need to zero it out
  * before traversing a patch port. */
@@ -3361,13 +3373,6 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
 flow->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE,
ntohs(flow->dl_type));
 }
-} else {
-/* Add dummy Ethernet header for legacy L2 port. */
-if (xport->pt_mode == NETDEV_PT_LEGACY_L2) {
-flow->packet_type = htonl(PT_ETH);
-flow->dl_dst = eth_addr_zero;
-flow->dl_src = eth_addr_zero;
-}
 }
 
 if (xport->peer) {
@@ -3627,6 +3632,10 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
 /* Restore flow */
 memcpy(flow->vlans, flow_vlans, sizeof flow->vlans);
 flow->nw_tos = flow_nw_tos;
+flow->dl_dst = flow_dl_dst;
+flow->dl_src = flow_dl_src;
+flow->packet_type = flow_packet_type;
+flow->dl_type = flow_dl_type;
 }
 
 static void
diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
index 110407857..c335b88ca 100644
--- a/tests/packet-type-aware.at
+++ b/tests/packet-type-aware.at
@@ -64,9 +64,9 @@ AT_SETUP([ptap - triangle bridge setup with L2 and L3 GRE 
tunnels])
 #  1030   br-in1  gre-13  legacy-l2   br-in3 3010 (l2)
 #  2010   br-in2  gre-21  ptapbr-in1 1020 (l2), 1021 (l3)
 #  2030   br-in2  gre-23  ptapbr-in3 3020 (l2), 3021 (l3)
-#  3010   br-in1  gre-31  legacy-l2   br-in1 1030 (l2)
-#  3020   br-in1  gre-32  legacy-l2   br-in2 2010 (ptap)
-#  3021   br-in1  gre-32_l3   legacy-l3 same
+#  3010   br-in3  gre-31  legacy-l2   br-in1 1030 (l2)
+#  3020   br-in3  gre-32  legacy-l2   br-in2 2010 (ptap)
+#  3021   br-in3  gre-32_l3   legacy-l3 same
 
 HWADDR_BRP1=aa:55:00:00:00:01
 HWADDR_BRP2=aa:55:00:00:00:02
@@ -459,5 +459,63 @@ 
aa55aa550003461e7d1a95a108004554f7d440004001ad51c0a80a14c0a80a1e08000e760c1e
 
aa55aa550003461e7d1a95a108004554f89540004001ac90c0a80a14c0a80a1e0800736f0c1e000232519a58e1f30b00101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637
 ])
 
+
+# N3 to N2, from L3 GRE to PTAP port between br-in3 and br-in2. Dropping L3 
packet on L2 dummy port in br-in2.
+
+# Strips 'n_packets=...' from ovs-ofctl output.
+strip_n_packets () {
+sed 's/n_packets=[[0-9]]*, //'
+}
+
+# Strips 'n_bytes=...' from ovs-ofctl output.
+strip_n_bytes () {
+sed 's/n_bytes=[[0-9]]*, //'
+}
+
+# Modify flow rules to receive L3 packet in br-in2.
+AT_CHECK([
+ovs-ofctl add-flow br-in2 
packet_type=\(1,0x800\),nw_dst=$N2_IP,actions=$N2_OFPORT # Route L3 packet to 
N2 in br-in2
+ovs-ofctl add-flow br-in3 ip,nw_dst=$N2_IP,actions=3021 # Route to N2 via 
the L3 tunnel
+], [0])
+
+AT_CHECK([
+ovs-ofctl dump-flows br-in2 | ofctl_strip | strip_n_bytes | 
strip_n_packets | sort | grep actions
+], [0], [dnl
+ ip,nw_dst=192.168.10.10 actions=output:2010
+ ip,nw_dst=192.168.10.20 actions=mod_dl_dst:aa:55:aa:55:00:02,output:20
+ ip,nw_dst=192.168.10.30 actions=output:2010
+ packet_type=(1,0x800),nw_dst=192.168.10.10 actions=output:2010
+ packet_type=(1,0x800),nw_dst=192.168.10.20 actions=output:20
+ packet_type=(1,0x800),nw_dst=192.168.10.30 actions=output:2030
+])
+
+AT_CHECK([
+ovs-ofctl dump-flows br-in3 | ofctl_strip | strip_n_bytes | 
strip_n_packets | sort | grep actions
+], [0], [dnl
+ ip,nw_dst=192.168.10.10 actions=output:3021
+ ip,nw_dst=192.168.10.20 actions=output:3021
+ ip,nw_

[ovs-dev] [PATCH v2 0/3] basic encap/decap

[Zoltán Balogh]

From: Zoltán Balogh 


This series is a continuation of other patch series initiated by Jan Scheurich 
before. These were already applied to the master branch:
 - userspace: Support for L3 tunneling
   https://mail.openvswitch.org/pipermail/ovs-dev/2017-June/87.html
 - Packet type aware pipeline
   https://mail.openvswitch.org/pipermail/ovs-dev/2017-June/334512.html

The main purpose of this series is to add support for the OpenFlow actions 
generic encap and decap (ONF EXT-382) to the OVS control plane. It implements
a skeleton for translation of generic encap and decap actions in ofproto-dpif
and provides support to encap and decap an Ethernet header. 

v1->v2
 - Squash 1/4 and 2/4 commits of v1.
 - Put unit tests in a separate commit.
 - Use aligned cast.
 - Nicira extension numbers for encap/decap action numbers and error codes.
 - Small fixes according to comments.

Jan Scheurich (1):
  OF support and translation of generic encap and decap

Zoltán Balogh  (2):
  ofproto-dpif-xlate: drop L3 packets on L2 legacy port
  tests: Extend PTAP unit tests with decap action

 include/openflow/openflow-common.h |   1 +
 include/openvswitch/automake.mk|   1 +
 include/openvswitch/ofp-actions.h  |  31 +++
 include/openvswitch/ofp-ed-props.h |  69 +++
 include/openvswitch/ofp-errors.h   |   9 +
 lib/automake.mk|   1 +
 lib/odp-util.c |  84 +---
 lib/odp-util.h |   3 +-
 lib/ofp-actions.c  | 378 ++-
 lib/ofp-ed-props.c | 150 ++
 lib/packets.h  |   3 +-
 ofproto/ofproto-dpif-xlate.c   | 131 +++-
 tests/packet-type-aware.at | 397 -
 13 files changed, 1206 insertions(+), 52 deletions(-)
 create mode 100644 include/openvswitch/ofp-ed-props.h
 create mode 100644 lib/ofp-ed-props.c

-- 
2.11.0

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

[ovs-dev] [PATCH v7 4/7] ovsdb-idl: Autogenerated functions for compound indexes

[Lance Richardson]

Generates and fills in the default comparators for columns with
type int, real, string. Also creates the macros that allow
iteration over the contents of the index, and perform
queries.

Signed-off-by: Arnoldo Lutz Guevara 
Signed-off-by: Esteban Rodriguez Betancourt 
Co-authored-by: Arnoldo Lutz Guevara 
Co-authored-by: Esteban Rodriguez Betancourt 
Signed-off-by: Lance Richardson 
---
  v7: - Rebased and made needed changes in ovsdb-idlc.in for Python 3
compatibility.
  - Added a test case for *_FOR_EACH_RANGE (row, cursor, NULL, NULL)
(from=NULL is interpreted as -infinity, to=NULL is interpreted
 as +infinity).
  - Replaced assert() with ovs_assert() in test-ovsdb.c (this included
a few that were unrelated to this patch, mea culpa).
  v6: - No longer need to dynamically allocate strings passed to
ovsdb_idl_index_write().
  v5: - Coding style fixes, including in generated idl code.
  - Fixed memory leak of xmalloc()-ed struct ovsdb_datum in generated code.
  - Changed tests to dynamically allocate string values when building a row
to be matched; otherwise the ovsdb_datum_destroy() call added in this
version of the patch series attempts to free a string literal.

 ovsdb/ovsdb-idlc.in | 258 +++
 tests/ovsdb-idl.at  | 282 +++
 tests/test-ovsdb.c  | 286 +++-
 3 files changed, 821 insertions(+), 5 deletions(-)

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 7cbcbf5ab..f065ef1c6 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -9,6 +9,7 @@ import sys
 import ovs.json
 import ovs.db.error
 import ovs.db.schema
+from ovs.db.types import StringType, IntegerType, RealType
 
 argv0 = sys.argv[0]
 
@@ -202,6 +203,26 @@ static inline bool %(s)s_is_deleted(const struct %(s)s 
*row)
 return %(s)s_row_get_seqno(row, OVSDB_IDL_CHANGE_DELETE) > 0;
 }
 
+void %(s)s_index_destroy_row(const struct %(s)s *);
+int %(s)s_index_compare(struct ovsdb_idl_index_cursor *, const struct %(s)s *, 
const struct %(s)s *);
+const struct %(s)s *%(s)s_index_first(struct ovsdb_idl_index_cursor *);
+const struct %(s)s *%(s)s_index_next(struct ovsdb_idl_index_cursor *);
+const struct %(s)s *%(s)s_index_find(struct ovsdb_idl_index_cursor *, const 
struct %(s)s *);
+const struct %(s)s *%(s)s_index_forward_to(struct ovsdb_idl_index_cursor *, 
const struct %(s)s *);
+const struct %(s)s *%(s)s_index_get_data(const struct ovsdb_idl_index_cursor 
*);
+#define %(S)s_FOR_EACH_RANGE(ROW, CURSOR, FROM, TO) \\
+for ((ROW) = %(s)s_index_forward_to(CURSOR, FROM); \\
+ ((ROW) && %(s)s_index_compare(CURSOR, ROW, TO) <= 0); \\
+ (ROW) = %(s)s_index_next(CURSOR))
+#define %(S)s_FOR_EACH_EQUAL(ROW, CURSOR, KEY) \\
+for ((ROW) = %(s)s_index_find(CURSOR, KEY); \\
+ ((ROW) && %(s)s_index_compare(CURSOR, ROW, KEY) == 0); \\
+ (ROW) = %(s)s_index_next(CURSOR))
+#define %(S)s_FOR_EACH_BYINDEX(ROW, CURSOR) \\
+for ((ROW) = %(s)s_index_first(CURSOR); \\
+ (ROW); \\
+ (ROW) = %(s)s_index_next(CURSOR))
+
 void %(s)s_init(struct %(s)s *);
 void %(s)s_delete(const struct %(s)s *);
 struct %(s)s *%(s)s_insert(struct ovsdb_idl_txn *);
@@ -258,6 +279,19 @@ bool %(s)s_is_updated(const struct %(s)s *, enum 
%(s)s_column_id);
 print("")
 
 # Table indexes.
+print("struct %(s)s * %(s)s_index_init_row(struct ovsdb_idl *, const 
struct ovsdb_idl_table_class *);" % {'s': structName})
+print
+for columnName, column in sorted(table.columns.iteritems()):
+print('void %(s)s_index_set_%(c)s(const struct %(s)s *,' % {'s': 
structName, 'c': columnName})
+if column.type.is_smap():
+args = ['const struct smap *']
+else:
+comment, members = cMembers(prefix, tableName, columnName,
+column, True)
+args = ['%(type)s%(name)s' % member for member in members]
+print('%s);' % ', '.join(args))
+
+print
 printEnum("%stable_id" % prefix.lower(), ["%sTABLE_%s" % (prefix.upper(), 
tableName.upper()) for tableName in sorted(schema.tables)] + ["%sN_TABLES" % 
prefix.upper()])
 print("")
 for tableName in schema.tables:
@@ -980,6 +1014,230 @@ void
 print("free(%s);" % var)
 print("}")
 
+# Index table related functions
+print("""
+/* Destroy 'row' of kind "%(t)s". The row must have been
+ * created with ovsdb_idl_index_init_row.
+ */
+void
+%(s)s_index_destroy_row(const struct %(s)s *row)
+{
+ovsdb_idl_index_destroy_row__(&row->header_);
+}
+""" % { 's' : structName, 't': tableName })
+print("""
+/* Creates a new row of kind "%(t)s". */
+struct %(s)s *
+%(s)s_index_init_row(struct ovsdb_idl *idl, const struct ovsdb_idl_tabl

[ovs-dev] [PATCH v7 6/7] ovn-controller: use idl indexes for logical port table

[Lance Richardson]

Use IDL index for logical port table lookups, avoiding the overhead
of creating/destroying an index hmap for each iteration of the
ovn-controller main loop.

Signed-off-by: Lance Richardson 
---
 v7: New patch.

 ovn/controller/binding.c|  27 ---
 ovn/controller/binding.h|   3 +-
 ovn/controller/lflow.c  |  31 
 ovn/controller/lflow.h  |   2 -
 ovn/controller/lport.c  | 154 +---
 ovn/controller/lport.h  |  18 +
 ovn/controller/ovn-controller.c |  31 ++--
 ovn/controller/physical.c   |  14 ++--
 ovn/controller/physical.h   |   2 +-
 ovn/controller/pinctrl.c|  72 +--
 ovn/controller/pinctrl.h|   2 +-
 11 files changed, 178 insertions(+), 178 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 11145dd4c..0195fb314 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -106,8 +106,8 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
 }
 
 static void
-add_local_datapath__(const struct ldatapath_index *ldatapaths,
- const struct lport_index *lports,
+add_local_datapath__(struct controller_ctx *ctx,
+ const struct ldatapath_index *ldatapaths,
  const struct sbrec_datapath_binding *datapath,
  bool has_local_l3gateway, int depth,
  struct hmap *local_datapaths)
@@ -143,9 +143,9 @@ add_local_datapath__(const struct ldatapath_index 
*ldatapaths,
 const char *peer_name = smap_get(&pb->options, "peer");
 if (peer_name) {
 const struct sbrec_port_binding *peer = lport_lookup_by_name(
-lports, peer_name);
+ctx->ovnsb_idl, peer_name);
 if (peer && peer->datapath) {
-add_local_datapath__(ldatapaths, lports, peer->datapath,
+add_local_datapath__(ctx, ldatapaths, peer->datapath,
  false, depth + 1, local_datapaths);
 }
 }
@@ -154,12 +154,12 @@ add_local_datapath__(const struct ldatapath_index 
*ldatapaths,
 }
 
 static void
-add_local_datapath(const struct ldatapath_index *ldatapaths,
-   const struct lport_index *lports,
+add_local_datapath(struct controller_ctx *ctx,
+   const struct ldatapath_index *ldatapaths,
const struct sbrec_datapath_binding *datapath,
bool has_local_l3gateway, struct hmap *local_datapaths)
 {
-add_local_datapath__(ldatapaths, lports, datapath, has_local_l3gateway, 0,
+add_local_datapath__(ctx, ldatapaths, datapath, has_local_l3gateway, 0,
  local_datapaths);
 }
 
@@ -356,7 +356,6 @@ setup_qos(const char *egress_iface, struct hmap *queue_map)
 static void
 consider_local_datapath(struct controller_ctx *ctx,
 const struct ldatapath_index *ldatapaths,
-const struct lport_index *lports,
 const struct sbrec_chassis *chassis_rec,
 const struct sbrec_port_binding *binding_rec,
 struct hmap *qos_map,
@@ -375,7 +374,7 @@ consider_local_datapath(struct controller_ctx *ctx,
 /* Add child logical port to the set of all local ports. */
 sset_add(local_lports, binding_rec->logical_port);
 }
-add_local_datapath(ldatapaths, lports, binding_rec->datapath,
+add_local_datapath(ctx, ldatapaths, binding_rec->datapath,
false, local_datapaths);
 if (iface_rec && qos_map && ctx->ovs_idl_txn) {
 get_qos_params(binding_rec, qos_map);
@@ -390,7 +389,7 @@ consider_local_datapath(struct controller_ctx *ctx,
 our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
 if (our_chassis) {
 sset_add(local_lports, binding_rec->logical_port);
-add_local_datapath(ldatapaths, lports, binding_rec->datapath,
+add_local_datapath(ctx, ldatapaths, binding_rec->datapath,
false, local_datapaths);
 }
 } else if (!strcmp(binding_rec->type, "chassisredirect")) {
@@ -398,7 +397,7 @@ consider_local_datapath(struct controller_ctx *ctx,
   "redirect-chassis");
 our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
 if (our_chassis) {
-add_local_datapath(ldatapaths, lports, binding_rec->datapath,
+add_local_datapath(ctx, ldatapaths, binding_rec->datapath,
false, local_datapaths);
 }
 } else if (!strcmp(binding_rec->type, "l3gateway")) {
@@ -406,7 +405,7 @@ consider_local_datapath(struct controller_ctx *ctx,
   "l3gateway-chassis");
 our_chassis = chassi

[ovs-dev] [PATCH v7 7/7] ovn-controller: use idl indexes for logical datapath

[Lance Richardson]

Use IDL index to iterate over all logical ports in a given logical
datapath, avoiding the overhead of creating/destroying an indexing
data structure in each iteration of the ovn-controller main loop.

Signed-off-by: Lance Richardson 
---
 v7: New patch.

 ovn/controller/binding.c| 42 +++-
 ovn/controller/binding.h|  3 +-
 ovn/controller/lport.c  | 61 -
 ovn/controller/lport.h  | 21 --
 ovn/controller/ovn-controller.c | 13 +
 ovn/controller/pinctrl.c| 15 --
 6 files changed, 44 insertions(+), 111 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 0195fb314..4d8b737f4 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -107,12 +107,14 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
 
 static void
 add_local_datapath__(struct controller_ctx *ctx,
- const struct ldatapath_index *ldatapaths,
  const struct sbrec_datapath_binding *datapath,
  bool has_local_l3gateway, int depth,
  struct hmap *local_datapaths)
 {
 uint32_t dp_key = datapath->tunnel_key;
+const struct sbrec_port_binding *pb;
+struct ovsdb_idl_index_cursor cursor;
+struct sbrec_port_binding *lpval;
 
 struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key);
 if (ld) {
@@ -125,8 +127,6 @@ add_local_datapath__(struct controller_ctx *ctx,
 ld = xzalloc(sizeof *ld);
 hmap_insert(local_datapaths, &ld->hmap_node, dp_key);
 ld->datapath = datapath;
-ld->ldatapath = ldatapath_lookup_by_key(ldatapaths, dp_key);
-ovs_assert(ld->ldatapath);
 ld->localnet_port = NULL;
 ld->has_local_l3gateway = has_local_l3gateway;
 
@@ -136,30 +136,36 @@ add_local_datapath__(struct controller_ctx *ctx,
 return;
 }
 
+lpval = sbrec_port_binding_index_init_row(ctx->ovnsb_idl,
+  &sbrec_table_port_binding);
+sbrec_port_binding_index_set_datapath(lpval, datapath);
+ovsdb_idl_initialize_cursor(ctx->ovnsb_idl, &sbrec_table_port_binding,
+"lport-by-datapath", &cursor);
+
 /* Recursively add logical datapaths to which this one patches. */
-for (size_t i = 0; i < ld->ldatapath->n_lports; i++) {
-const struct sbrec_port_binding *pb = ld->ldatapath->lports[i];
+SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, &cursor, lpval) {
 if (!strcmp(pb->type, "patch")) {
 const char *peer_name = smap_get(&pb->options, "peer");
 if (peer_name) {
-const struct sbrec_port_binding *peer = lport_lookup_by_name(
-ctx->ovnsb_idl, peer_name);
+const struct sbrec_port_binding *peer;
+
+peer = lport_lookup_by_name( ctx->ovnsb_idl, peer_name);
 if (peer && peer->datapath) {
-add_local_datapath__(ctx, ldatapaths, peer->datapath,
- false, depth + 1, local_datapaths);
+add_local_datapath__(ctx, peer->datapath, false,
+ depth + 1, local_datapaths);
 }
 }
 }
 }
+sbrec_port_binding_index_destroy_row(lpval);
 }
 
 static void
 add_local_datapath(struct controller_ctx *ctx,
-   const struct ldatapath_index *ldatapaths,
const struct sbrec_datapath_binding *datapath,
bool has_local_l3gateway, struct hmap *local_datapaths)
 {
-add_local_datapath__(ctx, ldatapaths, datapath, has_local_l3gateway, 0,
+add_local_datapath__(ctx, datapath, has_local_l3gateway, 0,
  local_datapaths);
 }
 
@@ -355,7 +361,6 @@ setup_qos(const char *egress_iface, struct hmap *queue_map)
 
 static void
 consider_local_datapath(struct controller_ctx *ctx,
-const struct ldatapath_index *ldatapaths,
 const struct sbrec_chassis *chassis_rec,
 const struct sbrec_port_binding *binding_rec,
 struct hmap *qos_map,
@@ -374,13 +379,13 @@ consider_local_datapath(struct controller_ctx *ctx,
 /* Add child logical port to the set of all local ports. */
 sset_add(local_lports, binding_rec->logical_port);
 }
-add_local_datapath(ctx, ldatapaths, binding_rec->datapath,
+add_local_datapath(ctx, binding_rec->datapath,
false, local_datapaths);
 if (iface_rec && qos_map && ctx->ovs_idl_txn) {
 get_qos_params(binding_rec, qos_map);
 }
 /* This port is in our chassis unless it is a localport. */
-   if (strcmp(binding_rec->type, "localport")) {
+   if (strcmp(binding_rec->type, "localport")) {
 our_chassis = true;
 }
 } e

[ovs-dev] [PATCH v7 5/7] ovn-controller: use idl index for multicast group table

[Lance Richardson]

Use IDL index for multicast group table lookups, avoiding the overhead
of creating/destroying an index hmap for each iteration of the
ovn-controller main loop.

Signed-off-by: Lance Richardson 
---
 v7: New patch.

 ovn/controller/lflow.c  | 20 +-
 ovn/controller/lflow.h  |  2 -
 ovn/controller/lport.c  | 85 -
 ovn/controller/lport.h  | 20 ++
 ovn/controller/ovn-controller.c | 26 ++---
 5 files changed, 65 insertions(+), 88 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index b1b4b2372..56a7c02c2 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -45,8 +45,8 @@ lflow_init(void)
 }
 
 struct lookup_port_aux {
+struct ovsdb_idl *ovnsb_idl;
 const struct lport_index *lports;
-const struct mcgroup_index *mcgroups;
 const struct sbrec_datapath_binding *dp;
 };
 
@@ -55,8 +55,8 @@ struct condition_aux {
 const struct sbrec_chassis *chassis;
 };
 
-static void consider_logical_flow(const struct lport_index *lports,
-  const struct mcgroup_index *mcgroups,
+static void consider_logical_flow(struct controller_ctx *ctx,
+  const struct lport_index *lports,
   const struct sbrec_logical_flow *lflow,
   const struct hmap *local_datapaths,
   struct group_table *group_table,
@@ -80,7 +80,7 @@ lookup_port_cb(const void *aux_, const char *port_name, 
unsigned int *portp)
 }
 
 const struct sbrec_multicast_group *mg
-= mcgroup_lookup_by_dp_name(aux->mcgroups, aux->dp, port_name);
+= mcgroup_lookup_by_dp_name(aux->ovnsb_idl, aux->dp, port_name);
 if (mg) {
 *portp = mg->tunnel_key;
 return true;
@@ -118,7 +118,6 @@ is_gateway_router(const struct sbrec_datapath_binding *ldp,
 /* Adds the logical flows from the Logical_Flow table to flow tables. */
 static void
 add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
-  const struct mcgroup_index *mcgroups,
   const struct hmap *local_datapaths,
   struct group_table *group_table,
   const struct sbrec_chassis *chassis,
@@ -144,7 +143,7 @@ add_logical_flows(struct controller_ctx *ctx, const struct 
lport_index *lports,
 }
 
 SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
-consider_logical_flow(lports, mcgroups, lflow, local_datapaths,
+consider_logical_flow(ctx, lports, lflow, local_datapaths,
   group_table, chassis,
   &dhcp_opts, &dhcpv6_opts, &conj_id_ofs,
   addr_sets, flow_table);
@@ -155,8 +154,8 @@ add_logical_flows(struct controller_ctx *ctx, const struct 
lport_index *lports,
 }
 
 static void
-consider_logical_flow(const struct lport_index *lports,
-  const struct mcgroup_index *mcgroups,
+consider_logical_flow(struct controller_ctx *ctx,
+  const struct lport_index *lports,
   const struct sbrec_logical_flow *lflow,
   const struct hmap *local_datapaths,
   struct group_table *group_table,
@@ -220,7 +219,7 @@ consider_logical_flow(const struct lport_index *lports,
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
 struct lookup_port_aux aux = {
 .lports = lports,
-.mcgroups = mcgroups,
+.ovnsb_idl = ctx->ovnsb_idl,
 .dp = lflow->logical_datapath
 };
 struct ovnact_encode_params ep = {
@@ -387,13 +386,12 @@ void
 lflow_run(struct controller_ctx *ctx,
   const struct sbrec_chassis *chassis,
   const struct lport_index *lports,
-  const struct mcgroup_index *mcgroups,
   const struct hmap *local_datapaths,
   struct group_table *group_table,
   const struct shash *addr_sets,
   struct hmap *flow_table)
 {
-add_logical_flows(ctx, lports, mcgroups, local_datapaths, group_table,
+add_logical_flows(ctx, lports, local_datapaths, group_table,
   chassis, addr_sets, flow_table);
 add_neighbor_flows(ctx, lports, flow_table);
 }
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index a23cde099..60d86cdc8 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -39,7 +39,6 @@ struct controller_ctx;
 struct group_table;
 struct hmap;
 struct lport_index;
-struct mcgroup_index;
 struct sbrec_chassis;
 struct simap;
 struct uuid;
@@ -65,7 +64,6 @@ void lflow_init(void);
 void lflow_run(struct controller_ctx *,
const struct sbrec_chassis *chassis,
const struct lport_index *,
-   const struct mcgroup_index *,
const struct hmap *local_datapaths,
struct group_table *group_table,

[ovs-dev] [PATCH v7 3/7] ovsdb-idl: idl compound indexes implementation

[Lance Richardson]

This patch adds support for the creation of multicolumn indexes
in the C IDL to enable for efficient search and retrieval of database
rows by key.

Signed-off-by: Esteban Rodriguez Betancourt 
Co-authored-by: Lance Richardson 
Signed-off-by: Lance Richardson 
---
  v7: - Reworked "row_sync" comments and changed field name to "ins_del" to
(hopefully) better explain why this mechanism exists.
  - Removed ovsdb_idl_index_read() since ovsdb_idl_read() is equivalent
(suggested by Ben Pfaff).
  - Removed several declared-but-not-defined functions from ovsdb-idl.h,
also re-ordered indexing-related items in this file to have macros
followed by data type definitions and finally function declarations.
  v6: - Reworked v5 memory leak fix to avoid need for dynamically allocating
strings passed to ovsdb_idl_index_write().
  - Fixed null pointer dereference in ovsdb_idl_initialize_cursor()
when called with non-existent index name.
  v5: - Coding style fixes (checkpatch.py)
  - Fixed memory leak (missing ovsdb_datum_destroy() in
ovsdb_idl_index_destroy_row__()).
  - Some polishing of comment and log message text.

 lib/ovsdb-idl-provider.h |  29 
 lib/ovsdb-idl.c  | 401 +++
 lib/ovsdb-idl.h  |  51 ++
 3 files changed, 481 insertions(+)

diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index 8cfbb95aa..59fb24015 100644
--- a/lib/ovsdb-idl-provider.h
+++ b/lib/ovsdb-idl-provider.h
@@ -1,4 +1,5 @@
 /* Copyright (c) 2009, 2010, 2011, 2012, 2016 Nicira, Inc.
+ * Copyright (C) 2016 Hewlett Packard Enterprise Development LP
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -111,6 +112,7 @@ struct ovsdb_idl_table {
 struct hmap rows;/* Contains "struct ovsdb_idl_row"s. */
 struct ovsdb_idl *idl;   /* Containing idl. */
 unsigned int change_seqno[OVSDB_IDL_CHANGE_MAX];
+struct shash indexes;/* Contains "struct ovsdb_idl_index"s */
 struct ovs_list track_list; /* Tracked rows (ovsdb_idl_row.track_node). */
 struct ovsdb_idl_condition condition;
 bool cond_changed;
@@ -122,6 +124,33 @@ struct ovsdb_idl_class {
 size_t n_tables;
 };
 
+/*
+ * Structure containing the per-column configuration of the index.
+ */
+struct ovsdb_idl_index_column {
+const struct ovsdb_idl_column *column; /* Column used for index key. */
+column_comparator *comparer; /* Column comparison function. */
+int sorting_order; /* Sorting order (ascending or descending). */
+};
+
+/*
+ * Defines a IDL compound index
+ */
+struct ovsdb_idl_index {
+struct skiplist *skiplist;/* Skiplist with pointers to rows. */
+struct ovsdb_idl_index_column *columns; /* Columns configuration */
+size_t n_columns; /* Number of columns in index. */
+size_t alloc_columns; /* Size allocated memory for columns,
+ comparers and sorting order. */
+bool ins_del; /* True if a row in the index is being
+ inserted or deleted; if true, the
+ search key is augmented with the
+ UUID and address in order to discriminate
+ between entries with identical keys. */
+const struct ovsdb_idl_table *table; /* Table that owns this index */
+const char *index_name;   /* The name of this index. */
+};
+
 struct ovsdb_idl_row *ovsdb_idl_get_row_arc(
 struct ovsdb_idl_row *src,
 const struct ovsdb_idl_table_class *dst_table,
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 893143c55..5753c1dc6 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1,4 +1,5 @@
 /* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, 
Inc.
+ * Copyright (C) 2016 Hewlett Packard Enterprise Development LP
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -37,8 +38,10 @@
 #include "ovsdb-parser.h"
 #include "poll-loop.h"
 #include "openvswitch/shash.h"
+#include "skiplist.h"
 #include "sset.h"
 #include "util.h"
+#include "uuid.h"
 #include "openvswitch/vlog.h"
 
 VLOG_DEFINE_THIS_MODULE(ovsdb_idl);
@@ -228,6 +231,14 @@ ovsdb_idl_table_from_class(const struct ovsdb_idl *,
 static bool ovsdb_idl_track_is_set(struct ovsdb_idl_table *table);
 static void ovsdb_idl_send_cond_change(struct ovsdb_idl *idl);
 
+static struct ovsdb_idl_index *ovsdb_idl_create_index_(const struct
+   ovsdb_idl_table *table,
+   size_t allocated_cols);
+static void
+ ovsdb_idl_destroy_indexes(struct ovsdb_idl_table *table);
+static void ovsdb_idl_add_to_indexes(const struct ovs

[ovs-dev] [PATCH v7 1/7] ovsdb-idl: compound indexes design document

[Lance Richardson]

In the work made in our projects, it was found the need to have a faster
access to the rows contained in tables in the replica, as well to have
the possibility to loop over a subset of rows that meet some specified
criteria.
Those needs lead us to design and implement a functionality that
satisfies those requirements, so an implementation of special indexes were
done.
In order to keep the OVSDB server implementation unmodified and avoid
extra load of processing, the indexes are created as part of the IDL.
The indexes are created as part of the initialization of the replica request
and are maintained automatically when there are changes in the replica.

This document explains the design rationale of the compound indexes feature.

Signed-off-by: Javier Albornoz 
Signed-off-by: Esteban Rodriguez Betancourt 
Signed-off-by: Jorge Arturo Sauma Vargas 
Co-authored-by: Javier Albornoz 
Co-authored-by: Esteban Rodriguez Betancourt 
Co-authored-by: Jorge Arturo Sauma Vargas 
Co-aughored-by: Lance Richardson 
Signed-off-by: Lance Richardson 
---
 v7: - Fixed whitespace complaint from "git am".
 v6: - Revised design document text for clarity, minor edits to code
   examples.
 v5: - Added document to table of contents.
 - Noted that uuid and boolean columns can also be used with
   default comparators.
 - Changed code examples to better conform with OVS coding style.
 - Minor edits.

 Documentation/automake.mk |   1 +
 Documentation/topics/idl-compound-indexes.rst | 302 ++
 Documentation/topics/index.rst|   1 +
 3 files changed, 304 insertions(+)
 create mode 100644 Documentation/topics/idl-compound-indexes.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 9c9b42d34..24fe63d87 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -28,6 +28,7 @@ DOC_SOURCE = \
Documentation/tutorials/ovn-sandbox.rst \
Documentation/topics/index.rst \
Documentation/topics/bonding.rst \
+   Documentation/topics/idl-compound-indexes.rst \
Documentation/topics/datapath.rst \
Documentation/topics/design.rst \
Documentation/topics/dpdk/index.rst \
diff --git a/Documentation/topics/idl-compound-indexes.rst 
b/Documentation/topics/idl-compound-indexes.rst
new file mode 100644
index 0..44c626b31
--- /dev/null
+++ b/Documentation/topics/idl-compound-indexes.rst
@@ -0,0 +1,302 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+==
+C IDL Compound Indexes
+==
+
+Introduction
+
+
+This document describes the design and usage of the C IDL Compound
+Indexes feature, which allows OVSDB client applications to efficiently
+search table contents using arbitrary sets of column values in a generic
+way.
+
+This feature is implemented entirely in the client IDL, requiring no changes
+to the OVSDB Server, OVSDB Protocol (OVSDB RFC (RFC 7047)) or additional
+interaction with the OVSDB server.
+
+Please note that in this document, the term "index" refers to the common
+database term defined as "a data structure that facilitates data
+retrieval". Unless stated otherwise, the definition for index from the
+OVSDB RFC (RFC 7047) is not used.
+
+Typical Use Cases
+-
+
+Fast lookups
+
+
+Depending on the topology, the route table of a network device could
+manage thousands of routes. Commands such as "show ip route <*specific
+route*>" would need to do a sequential lookup of the routing table to
+find the specific route. With an index created, the lookup time could be
+faster.
+
+This same scenario could be applied to other features such as Access
+List rules and even interfaces lists.
+
+Lexicographic order
+~~~
+
+There are a number of cases in which retrieving data in a particular
+lexicographic order is needed. For example, SNMP. When an administrator
+or even a NMS would like to retrieve data from a specific device, it's
+possible that they will request data from full tables instead of just
+specific values.  Also, they would like to have this inform

[ovs-dev] [PATCH v7 2/7] lib: skiplist implementation

[Lance Richardson]

Skiplist implementation intended for use in the IDL compound indexes
feature.

Signed-off-by: Esteban Rodriguez Betancourt 
Co-authored-by: Lance Richardson 
Signed-off-by: Lance Richardson 
---
 v7: - More coding style fixes, provided by Ben Pfaff.
 - Remove expensive and unnecessary local variable initialization,
   suggested by Ben Pfaff.
 - Replaced assert() with ovs_assert() in test-skiplist.c.
 v6: - Coding style edits.
 v5: - Changed skiplist_node and skiplist structs to use smaller integer
   types than 64-bit as appropriate.
 - Removed free_func member from struct skiplist, as suggested in v4
   review.
 - Fixed max_levels at 32 as suggested in v4 review.
 - Changed skiplist_determine_levels() to use clz32(random_uint32()) for
   node height distribution. Also changed function to ensure that the
   maximum node height is increased by no more than one level, this
   is suggested in the original Pugh paper and seems to have been intended
   here according to comments.
 - Verified node distribution as expected (1M node list has about 500K
   level 0 nodes, 250K level 1 nodes, 125K level 2 nodes, etc.)
 - Coding style fixes (with checkpatch.py assistance).

 lib/automake.mk   |   2 +
 lib/skiplist.c| 261 ++
 lib/skiplist.h|  49 ++
 tests/.gitignore  |   1 +
 tests/automake.mk |   2 +
 tests/library.at  |  11 +++
 tests/test-skiplist.c | 211 
 7 files changed, 537 insertions(+)
 create mode 100644 lib/skiplist.c
 create mode 100644 lib/skiplist.h
 create mode 100644 tests/test-skiplist.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 54a103234..ec03a0caf 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -229,6 +229,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/shash.c \
lib/simap.c \
lib/simap.h \
+   lib/skiplist.c \
+   lib/skiplist.h \
lib/smap.c \
lib/smap.h \
lib/socket-util.c \
diff --git a/lib/skiplist.c b/lib/skiplist.c
new file mode 100644
index 0..2cea6d8df
--- /dev/null
+++ b/lib/skiplist.c
@@ -0,0 +1,261 @@
+/* Copyright (C) 2016 Hewlett Packard Enterprise Development LP
+ * All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License. You may obtain
+ * a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+/* Skiplist implementation based on:
+ * "Skip List: A Probabilistic Alternative to Balanced Trees",
+ * by William Pugh. */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "skiplist.h"
+#include "random.h"
+#include "util.h"
+
+/*
+ * A maximum height level of 32 should be more than sufficient for
+ * anticipated use cases, delivering good expected performance with
+ * up to 2**32 list nodes. Changes to this limit will also require
+ * changes in skiplist_determine_level().
+ */
+#define SKIPLIST_MAX_LEVELS 32
+
+/* Skiplist node container */
+struct skiplist_node {
+const void *data; /* Pointer to saved data. */
+int height;   /* Height of this node. */
+struct skiplist_node *forward[];  /* Links to the next nodes. */
+};
+
+/* Skiplist container */
+struct skiplist {
+struct skiplist_node *header; /* Pointer to head node (not first
+   * data node). */
+skiplist_comparator *cmp; /* Pointer to the skiplist's comparison
+   * function. */
+void *cfg;/* Pointer to optional comparison
+   * configuration, used by the comparator. */
+int level;/* Maximum level currently in use. */
+uint32_t size;/* Current number of nodes in skiplist. */
+};
+
+/* Create a new skiplist_node with given level and data. */
+static struct skiplist_node *
+skiplist_create_node(int level, const void *object)
+{
+struct skiplist_node *new_node;
+size_t alloc_size = sizeof *new_node +
+(level + 1) * sizeof new_node->forward[0];
+
+new_node = xmalloc(alloc_size);
+new_node->data = object;
+new_node->height = level;
+memset(new_node->forward, 0,
+   (level + 1) * sizeof new_node->forward[0]);
+
+return new_node;
+}
+
+/*
+ * Create a new skiplist, configured with given data comparison function
+ * and configuration.
+ */
+struct skiplist *
+skiplist_create(skiplist_comparator object_comparator, v

[ovs-dev] [PATCH v7 0/7] ovsdb-idl: ovsdb client index support

[Lance Richardson]

This set of patches is based on a series authored by Esteban Rodriguez
Betancourt, Jorge Arturo Sauma Vargas, Javier Albornoz, and Arnoldo Lutz
Guevara that has been inactive for a number of months.

v7:
  - Added patches to make use of client idl indexing in ovn-controller
(these were previously in a separate RFC series).
  - Incorporated feedback from Ben Pfaff.
  - Rebased.
  - Added a test case for NULL range endpoints.
  - Fixed "git am" whitespace issue.
  - Replaced assert() with ovs_assert() (checkpatch.py)
v6:
  - More coding style fixes, text polishing.
  - Fixed null pointer dereference when initializing cursor with
non-existent index name.
  - Reworked v5 memory leak fix to avoid having to dynamically allocate
strings passed to *_index_set_*() functions.
  - Simplified ovsdb_idl_index_init_row() and
ovsdb_idl_index_destroy_row__() somewhat.
v5:
  - Rebased on ovs master.
  - Implemented changes suggestion in review of v4.
  - Coding style fixes, some text polishing.
  - Testing by using this feature to eliminate a number of ad-hoc
indexing structures used in ovn-controller.
  - Fixes for memory leaks found in testing.

Lance Richardson (7):
  ovsdb-idl: compound indexes design document
  lib: skiplist implementation
  ovsdb-idl: idl compound indexes implementation
  ovsdb-idl: Autogenerated functions for compound indexes
  ovn-controller: use idl index for multicast group table
  ovn-controller: use idl indexes for logical port table
  ovn-controller: use idl indexes for logical datapath

 Documentation/automake.mk |   1 +
 Documentation/topics/idl-compound-indexes.rst | 302 +++
 Documentation/topics/index.rst|   1 +
 lib/automake.mk   |   2 +
 lib/ovsdb-idl-provider.h  |  29 ++
 lib/ovsdb-idl.c   | 401 ++
 lib/ovsdb-idl.h   |  51 
 lib/skiplist.c| 261 +
 lib/skiplist.h|  49 
 ovn/controller/binding.c  |  49 ++--
 ovn/controller/binding.h  |   6 +-
 ovn/controller/lflow.c|  43 ++-
 ovn/controller/lflow.h|   4 -
 ovn/controller/lport.c| 302 ---
 ovn/controller/lport.h|  59 +---
 ovn/controller/ovn-controller.c   |  66 +++--
 ovn/controller/physical.c |  14 +-
 ovn/controller/physical.h |   2 +-
 ovn/controller/pinctrl.c  |  87 +++---
 ovn/controller/pinctrl.h  |   2 +-
 ovsdb/ovsdb-idlc.in   | 258 +
 tests/.gitignore  |   1 +
 tests/automake.mk |   2 +
 tests/library.at  |  11 +
 tests/ovsdb-idl.at| 282 ++
 tests/test-ovsdb.c| 286 +-
 tests/test-skiplist.c | 211 ++
 27 files changed, 2415 insertions(+), 367 deletions(-)
 create mode 100644 Documentation/topics/idl-compound-indexes.rst
 create mode 100644 lib/skiplist.c
 create mode 100644 lib/skiplist.h
 create mode 100644 tests/test-skiplist.c

-- 
2.13.0

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

Re: [ovs-dev] [patch_v4 4/8] Userspace Datapath: Add ALG infra and FTP.

[Darrell Ball]

Correction inline 

On 7/14/17, 10:37 AM, "ovs-dev-boun...@openvswitch.org on behalf of Darrell 
Ball"  wrote:

Thanks a lot for the review Ben



On 7/13/17, 3:15 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben 
Pfaff"  wrote:



On Wed, Jul 05, 2017 at 09:32:22PM -0700, Darrell Ball wrote:

> ALG infra and FTP (both V4 and V6) support is added to the userspace

> datapath.  Also, NAT support is included.

> 

> Signed-off-by: Darrell Ball 



Thanks a lot for working on this.  It will be a valuable feature that

brings the userspace datapath closer to the kernel datapath features.



I have some comments.  I'm not too familiar with this area, so a lot of

my comments are ones that should help to make the code easier to

understand not just for me but for other newbies to ALG implementation.



The data structures introduced or modified in this patch are almost

comment-free.  The reader is left to guess important information like

the purpose of the structure, as well as per-member info like what lists

or hmaps a structure belongs to, what a "master" is, and so on.  Please

add some comments.



Done



The same seems warranted for the collection of macros that this

defines.  For example, what does FTP_MAX_PORT mean?  (If it's just the

maximum TCP port, what makes it FTP specific? etc.)



Done



In the name "alg_exp_node", I don't know what an exp is.



Done



I don't know what "delinate" means.



Name changed



What OS X problem does this allude to?

+/* CT_IPPORT_FTP is used in lieu of IPPORT_FTP as a workaround

+ * to handle OSX. */

+#define CT_IPPORT_FTP 21



I added comment that it was build related.



Usually we put the conversion on the constant side in comparisons like

the following, because compilers are better at optimizing it:

+return (ip_proto == IPPROTO_TCP &&

+(ntohs(th->tcp_src) == CT_IPPORT_FTP ||

+ ntohs(th->tcp_dst) == CT_IPPORT_FTP));



Got it



I guess that not everyone knows what "tuple space" is.  I had to think

about it for a while.  I think you basically mean the ports available

for NAT.  Maybe a more user friendly term could be used (at least in

user messages)?  Why is exhaustion "likely a user error"?  (I would have

guessed that it is more likely from some kind of DoS or port scan or

equivalent.)  How should a user respond?



Sure, DoS is valid too as the system may be unprotected. I expanded the

comments and added recommendations.





process_one() has a variable alg_nat_repl_addr that it zeros and then

appears to never use again.



I deprecated that variable use and I forgot to cleanup.



Also in process_one(), I think that this memcpy:

memcpy(&alg_exp_entry, alg_exp, sizeof alg_exp_entry);

can be written as just:

alg_exp_entry = *alg_exp;

although I don't know whether you have some expectation for padding etc.



structure copy is fine here; I thought I had caught all of these.





process_one() uses the expression "conn && is_ftp_ctl(pkt)" twice, and

the latter function is nontrivial.  Can it be evaluated just once?



Yes, thanks for pointing that out.



In conntrack_execute(), it seems odd to move the loop index declaration

out of the for statement.



This is day one conntrack code, but I can fix it here.



conn_to_ct_dpif_entry() does an xstrdup but I wasn't able to spot where

the string gets freed.



This is an existing API that I just used; the caller (in dpctl) frees the 
string.

I added a comment that the caller frees it.



I can't see how get_v4_byte_be() works properly on both big-endian and

little-endian systems.



get_v4_byte_be() shouldn't need the "& 0xff", since it returns a

uint8_t.



right, mask is not needed



In replace_substring(), it seems odd to use 8-bit quantities for sizes.

Also, it looks like 'delta' can be negative, in which case it should n

Re: [ovs-dev] [PATCHv3] ofproto-dpif: Detect support for ct_tuple6.

[Joe Stringer]

On 6 July 2017 at 16:58, Ben Pfaff  wrote:
> On Fri, Jun 02, 2017 at 09:38:47AM -0700, Joe Stringer wrote:
>> Support for extracting original direction 5 tuple fields from the
>> connection tracking module may differ on some platforms between the IPv4
>> original tuple fields vs. IPv6. Detect IPv6 original tuple support
>> separately and reflect this support up to the OpenFlow layer.
>>
>> Signed-off-by: Joe Stringer 
>> ---
>> CC: Sairam Venugopal 
>>
>> v3: Only serialize IPv6 orig tuple in ODP if datapath support exists.
>> v2: Fix setting of support.ct_orig_tuple6 field prior to probe.
>
> I didn't test it, but the principles all seem solid.
>
> Acked-by: Ben Pfaff 

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

Re: [ovs-dev] [PATCH net] openvswitch: Fix for force/commit action failures

[Greg Rose]

On 07/14/2017 11:42 AM, Joe Stringer wrote:

On 14 July 2017 at 09:10, Greg Rose  wrote:
> When there is an established connection in direction A->B, it is
> possible to receive a packet on port B which then executes
> ct(commit,force) without first performing ct() - ie, a lookup.
> In this case, we would expect that this packet can delete the existing
> entry so that we can commit a connection with direction B->A. However,
> currently we only perform a check in skb_nfct_cached() for whether
> OVS_CS_F_TRACKED is set and OVS_CS_F_INVALID is not set, ie that a
> lookup previously occurred. In the above scenario, a lookup has not
> occurred but we should still be able to statelessly look up the
> existing entry and potentially delete the entry if it is in the
> opposite direction.
>
> This patch extends the check to also hint that if the action has the
> force flag set, then we will lookup the existing entry so that the
> force check at the end of skb_nfct_cached has the ability to delete
> the connection.
>
> Fixes: dd41d330b03 ("openvswitch: Add force commit.")
> CC: Pravin Shelar 
> CC: d...@openvswitch.org
> Signed-off-by: Joe Stringer 
> Signed-off-by: Greg Rose 
> ---
>   net/openvswitch/conntrack.c | 50 
+++--
>   1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 08679eb..1260f2b 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -629,6 +629,33 @@ static int handle_fragments(struct net *net, struct 
sw_flow_key *key,
>  return ct;
>   }
>
> +struct nf_conn *ovs_ct_executed(struct net *net,
> +   const struct sw_flow_key *key,
> +   const struct ovs_conntrack_info *info,
> +   struct sk_buff *skb,
> +   bool *ct_executed)

Actually, some compilers will report this new warning:
net/openvswitch/conntrack.c:632:16: warning: symbol 'ovs_ct_executed'
was not declared. Should it be static?

Could you make this function static and repost?

Thanks.


Yes, I just saw that too.  Need to update my compiler for my primary build 
machine.  I'll send a V2...

Thanks!

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

Re: [ovs-dev] [PATCH net] openvswitch: Fix for force/commit action failures

[Joe Stringer]

On 14 July 2017 at 09:10, Greg Rose  wrote:
> When there is an established connection in direction A->B, it is
> possible to receive a packet on port B which then executes
> ct(commit,force) without first performing ct() - ie, a lookup.
> In this case, we would expect that this packet can delete the existing
> entry so that we can commit a connection with direction B->A. However,
> currently we only perform a check in skb_nfct_cached() for whether
> OVS_CS_F_TRACKED is set and OVS_CS_F_INVALID is not set, ie that a
> lookup previously occurred. In the above scenario, a lookup has not
> occurred but we should still be able to statelessly look up the
> existing entry and potentially delete the entry if it is in the
> opposite direction.
>
> This patch extends the check to also hint that if the action has the
> force flag set, then we will lookup the existing entry so that the
> force check at the end of skb_nfct_cached has the ability to delete
> the connection.
>
> Fixes: dd41d330b03 ("openvswitch: Add force commit.")
> CC: Pravin Shelar 
> CC: d...@openvswitch.org
> Signed-off-by: Joe Stringer 
> Signed-off-by: Greg Rose 
> ---
>  net/openvswitch/conntrack.c | 50 
> +++--
>  1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 08679eb..1260f2b 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -629,6 +629,33 @@ static int handle_fragments(struct net *net, struct 
> sw_flow_key *key,
> return ct;
>  }
>
> +struct nf_conn *ovs_ct_executed(struct net *net,
> +   const struct sw_flow_key *key,
> +   const struct ovs_conntrack_info *info,
> +   struct sk_buff *skb,
> +   bool *ct_executed)

Actually, some compilers will report this new warning:
net/openvswitch/conntrack.c:632:16: warning: symbol 'ovs_ct_executed'
was not declared. Should it be static?

Could you make this function static and repost?

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

Re: [ovs-dev] [PATCH net] openvswitch: Fix for force/commit action failures

[Joe Stringer]

On 14 July 2017 at 09:10, Greg Rose  wrote:
> When there is an established connection in direction A->B, it is
> possible to receive a packet on port B which then executes
> ct(commit,force) without first performing ct() - ie, a lookup.
> In this case, we would expect that this packet can delete the existing
> entry so that we can commit a connection with direction B->A. However,
> currently we only perform a check in skb_nfct_cached() for whether
> OVS_CS_F_TRACKED is set and OVS_CS_F_INVALID is not set, ie that a
> lookup previously occurred. In the above scenario, a lookup has not
> occurred but we should still be able to statelessly look up the
> existing entry and potentially delete the entry if it is in the
> opposite direction.
>
> This patch extends the check to also hint that if the action has the
> force flag set, then we will lookup the existing entry so that the
> force check at the end of skb_nfct_cached has the ability to delete
> the connection.
>
> Fixes: dd41d330b03 ("openvswitch: Add force commit.")
> CC: Pravin Shelar 
> CC: d...@openvswitch.org
> Signed-off-by: Joe Stringer 
> Signed-off-by: Greg Rose 
> ---

Thanks for the fix.

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

Re: [ovs-dev] [PATCH 3/3] dpctl: Add new 'ct-bkts' command.

[Ben Pfaff]

On Fri, Jul 14, 2017 at 10:11:04AM +, Fischetti, Antonio wrote:
> Thanks Ben for your feedback, my replies inline.
> 
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Tuesday, July 11, 2017 8:54 PM
> > To: Fischetti, Antonio 
> > Cc: d...@openvswitch.org
> > Subject: Re: [ovs-dev] [PATCH 3/3] dpctl: Add new 'ct-bkts' command.
> > 
> > On Fri, Jun 23, 2017 at 01:28:22PM +0100, antonio.fische...@intel.com
> > wrote:
> > > From: Antonio Fischetti 
> > >
> > > With the command:
> > >  ovs-appctl dpctl/ct-bkts
> > > shows the number of connections per bucket.
> > >
> > > By using a threshold:
> > >  ovs-appctl dpctl/ct-bkts gt=N
> > > for each bucket shows the number of connections when they
> > > are greater than N.
> > >
> > > Signed-off-by: Antonio Fischetti 
> > > Signed-off-by: Bhanuprakash Bodireddy 
> > > Co-authored-by: Bhanuprakash Bodireddy
> > 
> > 
> > Is this concept of buckets one that the kernel conntracker has too?  If
> > so, then I'm concerned about the definition of conn_per_buckets[] in
> > dpctl_ct_bkts(), since it has a hardcoded CONNTRACK_BUCKETS size and
> > nothing checks whether cte.bkt is in the right range.  If not, then
> > should this command be one that is limited to the userspace conntracker?
> 
> [AF] You're right, I didn't realize the concept of buckets 
> is in the kernel CT too, I did this implementation with the 
> userspace ConnTracker in mind.
> 
> Would it be ok to have a first implementation limited to the userspace CT 
> like:
> 
> dpctl_ct_bkts(...)
> { 
> if (!dpctl_p->is_appctl) { /* Not called by ovs-appctl command => exit. */
> dpctl_print(dpctl_p, "Command is available for UserSpace ConnTracker 
> only.\n");
> return 0;
> }
> 
> < implementation for Userspace CT here >
> ...
> 
> }
> 
> so that later on it could be extended to cover also the kernel CT case?

Is it much work to extend it to cover the kernel?  If not, it would be
better to cover both.  Otherwise, sure, this approach is an OK way to
start.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] system-traffic: 802.1ad: Add double VLAN match test case

[Joe Stringer]

On 15 June 2017 at 17:15, Eric Garver  wrote:
> On Thu, Jun 15, 2017 at 04:49:17PM -0700, Joe Stringer wrote:
>> On 8 June 2017 at 14:42, Joe Stringer  wrote:
>> > Yes, I'm the right person.
>> >
>> > At this point I'm looking for the tap device persistence issue to be
>> > addressed before running it on my tester box which runs tests on a
>> > variety of platforms.
>>
>> Hi Eric,
>>
>> I'm finding that things are still pretty unstable in the
>> system-traffic tests, particularly around kernels 4.8-4.9, but also
>> with regards to cleaning up properly after testsuite failures. Without
>> clean runs on master, it's a bit hard for me to run these tests as
>> well and observe that these tests are good on a range of kernels.
>
> Understood. I don't think there is any rush to get this particular patch
> in (or the IPv6 underlay tests for that matter). When you get back
> around to it let me know when/if they need rebased.
>
> Thanks.
> Eric.

No need - I just applied this to master. Thanks for your patience.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 0/4] system-traffic: Add basic IPv6 underlay tunnel tests

[Joe Stringer]

On 7 July 2017 at 11:32, Ben Pfaff  wrote:
> On Wed, Jun 07, 2017 at 07:54:23PM -0400, Eric Garver wrote:
>> Currently there is no system-traffic coverage for tunnels with IPv6 
>> underlays.
>> This series adds very basic ping tests for VXLANv6 and GENEVEv6.
>>
>> Note: GREv6 is not added by this series due to some incompatibilities between
>> OVS and Linux native ip6gretap. More info available in bugzilla [0].
>>
>> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1459605
>>
>> Eric Garver (4):
>>   system-common-macros: Add ip_addr_flags argument to ADD_VETH()
>>   system-common-macros: Add macros to create IPv6 tunnels
>>   system-traffic: datapath: Add vxlan6 test case.
>>   system-traffic: datapath: Add geneve6 test case.
>
> Joe, are you the right person to review these?

Slowly catching up on my backlog, sorry for the delay. Yes.

Eric, thanks for the series. For the first patch, I think that it'd be
nice to apply this for all IPv6 tests to speed them up. Given that we
assume control of the host for running these tests, I don't think
there's any situation where we actually want duplicate address
detection here. I looked to see if we could change the addresses in
the testsuite, for example to use link-local IPv6 addresses which I
think could avoid DAD, but if we did this then we would need to update
all of the pings and wgets and so on to specify a device; as it turns
out, even quite recent versions of wget do not parse the arguments
reliably to be able to do this. So, I think this would be more hassle
than it's worth. For now, we should stick with IPv6 ULAs (fc00:) for
the testsuite and just see if we can edge towards always disabling
DAD.

I intend to apply this series as-is shortly, with just the following
incremental to skip the geneve6 test when iproute can't configure it
properly:

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 2c3b2f9880c8..5b4c515cb0c5 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -382,6 +382,7 @@ AT_CLEANUP

AT_SETUP([datapath - ping over geneve6 tunnel])
OVS_CHECK_GENEVE()
+AT_SKIP_IF([! ip link add foo type geneve help 2>&1 | grep zerocsum
>/dev/null])

OVS_TRAFFIC_VSWITCHD_START()
ADD_BR([br-underlay])

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

Re: [ovs-dev] [PATCH 00/40] Fix static code analysis warnings.

[Shashank Ram]

Alin, thanks a lot for this important series! Could you please test out all 
these changes with Driver verifier and ensure that it does not introduce new 
regressions such as memory leaks, deadlocks etc.

Thanks,
Shashank


From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40:51 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 00/40] Fix static code analysis warnings.

The following patches are fixes found with the WDK 8.1 and 10
static code analysis.

Alin Serdean (40):
Found with WDK 10
  datapath-windows: Use only non executable memory
  datapath-windows: Use non-executable memory when allocating memory
  datapath-windows: Remove annotations in Switch.c
  datapath-windows: interfaceName overflow in IpHelper
  datapath-windows: Fix possible NULL dereference in IpFragment
  datapath-windows: Fix aligment in Stt
  datapath-windows: Add asserts to Stt
  datapath-windows: Suppress PAGED_CODE warnings
  datapath-windows: Fix possible NULL dereference in BufferMgmt
  datapath-windows: Fix possible NULL deference
  datapath-windows: Add an assert in recirculation
  datapath-windows: Add annotations for OvsAcquireCtrlLock
  datapath-windows: Add annotations for OvsReleaseCtrlLock
  datapath-windows: Remove function declarations from Tunnel.c
  datapath-windows: Add function annotations for OvsAcquireDatapathRead
  datapath-windows: Add function annotations for OvsAcquireDatapathWrite
  datapath-windows: Add function annotations for OvsReleaseDatapath
  datapath-windows: Add function annotations for OvsCancelIrp
  datapath-windows: Add function annotations for OvsTunnelFilterCancelIrp
  datapath-windows: Add function annotations for OvsCancelIrpDatapath
  datapath-windows: Add function annotations for OvsAcquireEventQueueLock
  datapath-windows: Add annotations for OvsReleaseEventQueueLock
  datapath-windows: Add annotations for OvsReleasePidHashLock
  datapath-windows: Add annotations for OvsAcquirePidHashLock
  datapath-windows: Fix spelling for OvsTunnelFilterSetIrpContext
  datapath-windows: Use annotations instead for macros
  datapath-windows: Add assert in OvsPartialCopyNBL
  datapath-windows: Fix possible NULL deference in OvsFullCopyNBL
  datapath-windows: Suppress warning in jhash
  datapath-windows: Add dummy parameter for NotifyRouteChange2
Found with WDK 8.1
  datapath-windows: prettify logging in iphelper
  datapath-windows: fix excessive stack usage in iphelper
  datapath-windows: Check return status when using APIs
  datapath-windows: Vport check RtlStringCbLengthW return value
  datapath-windows: Treat TCP_HDR_LEN static analysis warnings
  datapath-windows: Add annotation for OvsCtRelatedEntryCleaner
  datapath-windows: Add annotation for OvsIpFragmentEntryCleaner
  datapath-windows: Fix shared variables which use Interlocked functions
  datapath-windows: Fix static analysis warnings in OvsGetTcpPayloadLength
  datapath-windows: Fix static analysis warnings around ovsInstanceListLock

 datapath-windows/ovsext/Actions.c   |  5 +++
 datapath-windows/ovsext/BufferMgmt.c| 18 ++
 datapath-windows/ovsext/Conntrack-related.c |  7 ++--
 datapath-windows/ovsext/Conntrack.h | 17 -
 datapath-windows/ovsext/Datapath.c  |  8 +++--
 datapath-windows/ovsext/Datapath.h  |  8 +
 datapath-windows/ovsext/Debug.h | 18 --
 datapath-windows/ovsext/Event.c |  8 +
 datapath-windows/ovsext/Flow.c  |  2 +-
 datapath-windows/ovsext/Geneve.c|  5 ++-
 datapath-windows/ovsext/Gre.c   |  3 +-
 datapath-windows/ovsext/IpFragment.c|  4 +--
 datapath-windows/ovsext/IpHelper.c  | 53 +
 datapath-windows/ovsext/Jhash.c |  2 ++
 datapath-windows/ovsext/NetProto.h  |  2 +-
 datapath-windows/ovsext/Offload.c   |  9 +++--
 datapath-windows/ovsext/PacketParser.c  |  3 +-
 datapath-windows/ovsext/Stt.c   | 14 
 datapath-windows/ovsext/Switch.c|  8 ++---
 datapath-windows/ovsext/Switch.h| 10 ++
 datapath-windows/ovsext/Tunnel.c| 10 --
 datapath-windows/ovsext/TunnelFilter.c  |  5 ++-
 datapath-windows/ovsext/User.c  |  8 +
 datapath-windows/ovsext/Util.c  |  9 +++--
 datapath-windows/ovsext/Util.h  | 21 
 datapath-windows/ovsext/Vport.c |  9 +++--
 datapath-windows/ovsext/Vxlan.c |  7 ++--
 27 files changed, 161 insertions(+), 112 deletions(-)

--
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=6OuVHk-mnufSWzkKa74UkQ&m=4_aantYcWkgldEbd8X-s9FXQ8EGQRtaF2Eqi9ayiKbo&s=jJhd646Ak7DRhcVsVX9_Z5wcuPk0zJJ9jGsj36p6U1o&e=
__

Re: [ovs-dev] [PATCH 37/40] datapath-windows: Add annotation for OvsIpFragmentEntryCleaner

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 37/40] datapath-windows: Add annotation for 
OvsIpFragmentEntryCleaner

Make the function `OvsIpFragmentEntryCleaner` aware it is a kstart_routine.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/IpFragment.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/IpFragment.c 
b/datapath-windows/ovsext/IpFragment.c
index eb278ef..fd0d79a 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -31,7 +31,7 @@
 #define MAX_IPDATAGRAM_SIZE 65535

 /* Function declarations */
-static VOID OvsIpFragmentEntryCleaner(PVOID data);
+static KSTART_ROUTINE OvsIpFragmentEntryCleaner;
 static VOID OvsIpFragmentEntryDelete(POVS_IPFRAG_ENTRY entry, BOOLEAN 
checkExpiry);

 /* Global and static variables */
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 35/40] datapath-windows: Treat TCP_HDR_LEN static analysis warnings

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 35/40] datapath-windows: Treat TCP_HDR_LEN static 
analysis warnings

Using the shift operator in macros makes the static analyzer on WDK 8.1 
confused.

Switch to multiplication when trying to get the data offset of the TCP header.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/NetProto.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/NetProto.h 
b/datapath-windows/ovsext/NetProto.h
index 92d6611..9a17dee 100644
--- a/datapath-windows/ovsext/NetProto.h
+++ b/datapath-windows/ovsext/NetProto.h
@@ -68,7 +68,7 @@ typedef UINT64 IP4FragUnitLength;
 // length UINT for ipv6 header length.
 typedef UINT64 IP6UnitLength;

-#define TCP_HDR_LEN(tcph) IP4_UNITS_TO_BYTES((tcph)->doff)
+#define TCP_HDR_LEN(tcph) ((tcph)->doff * 4)
 #define TCP_DATA_LENGTH(iph, tcph)(ntohs(iph->tot_len) -\
IP4_HDR_LEN(iph) - TCP_HDR_LEN(tcph))

--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 40/40] datapath-windows: Fix static analysis warnings around ovsInstanceListLock

[Shashank Ram]

Comments inline.

Thanks,
Shashank


From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 40/40] datapath-windows: Fix static analysis warnings 
around ovsInstanceListLock

Check for return value when trying to initialize ovsInstanceListLock.

Also return the status back to caller of `OvsInitIpHelper`.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/IpHelper.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/datapath-windows/ovsext/IpHelper.c 
b/datapath-windows/ovsext/IpHelper.c
index 8ade997..791de13 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -1989,6 +1989,12 @@ OvsInitIpHelper(NDIS_HANDLE ndisFilterHandle)
 HANDLE threadHandle;
 UINT32 i;

+status = ExInitializeResourceLite(&ovsInstanceListLock);
+if (status != NDIS_STATUS_SUCCESS) {
+return status;
+}
+InitializeListHead(&ovsInstanceList);
+
 ovsFwdHashTable = (PLIST_ENTRY)OvsAllocateMemoryWithTag(
 sizeof(LIST_ENTRY) * OVS_FWD_HASH_TABLE_SIZE, OVS_IPHELPER_POOL_TAG);

@@ -2009,9 +2015,6 @@ OvsInitIpHelper(NDIS_HANDLE ndisFilterHandle)
 ipRouteNotificationHandle = NULL;
 unicastIPNotificationHandle = NULL;

-ExInitializeResourceLite(&ovsInstanceListLock);
-InitializeListHead(&ovsInstanceList);
-
 if (ovsFwdHashTable == NULL ||
 ovsRouteHashTable == NULL ||
 ovsNeighHashTable == NULL ||
@@ -2073,6 +2076,7 @@ init_cleanup:
 }
 ExDeleteResourceLite(&ovsInstanceListLock);
 NdisFreeSpinLock(&ovsIpHelperLock);
+return status;

>> Not needed if you return status below.

 }
 return STATUS_SUCCESS;
 }
--
2.10.2.windows.1
___

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

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

[YAMAMOTO Takashi]

\t is GNU sed extension.

Signed-off-by: YAMAMOTO Takashi 
---
 tests/packet-type-aware.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
index 1104078..2eee7f4 100644
--- a/tests/packet-type-aware.at
+++ b/tests/packet-type-aware.at
@@ -233,7 +233,7 @@ ovs-vsctl \
 
 ### Verify datapath configuration
 AT_CHECK([
-ovs-appctl dpif/show | grep -v hit | sed 's/\t//g' | sed 
's./[[0-9]]\{1,\}..'
+ovs-appctl dpif/show | grep -v hit | sed "s/$(printf \\t)//g" | sed 
's./[[0-9]]\{1,\}..'
 ], [0], [dnl
 br-in1:
 br-in1 65534: (dummy-internal)
-- 
2.5.4 (Apple Git-61)

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

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

[YAMAMOTO Takashi]

== is a GNU extension.

Signed-off-by: YAMAMOTO Takashi 
---
 tests/ovn.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/ovn.at b/tests/ovn.at
index efcbd91..06edcaa 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -7459,7 +7459,7 @@ test_packet() {
 local packet=$(echo $dst$src | sed 's/://g')${eth}
 hv=`vif_to_hv $inport`
 # If hypervisor 0 (localport) use the defhv parameter
-if test $hv == hv0; then
+if test $hv = hv0; then
 hv=$defhv
 fi
 vif=vif$inport
-- 
2.5.4 (Apple Git-61)

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

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

[YAMAMOTO Takashi]

SUS says:
When jobs reports the termination status of a job,
the shell removes its process ID from the list of those
"known in the current shell execution environment";

With NetBSD /bin/sh, the list involves zombie processes and
ends up with "can not fork" during test runs.
---
 tests/automake.mk  | 13 ++---
 tests/testsuite.unix.patch | 10 ++
 2 files changed, 16 insertions(+), 7 deletions(-)
 create mode 100644 tests/testsuite.unix.patch

diff --git a/tests/automake.mk b/tests/automake.mk
index 3118bbc..fef5db9 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -12,7 +12,8 @@ EXTRA_DIST += \
tests/atlocal.in \
$(srcdir)/package.m4 \
$(srcdir)/tests/testsuite \
-   $(srcdir)/tests/testsuite.patch
+   $(srcdir)/tests/testsuite.patch \
+   $(srcdir)/tests/testsuite.unix.patch
 
 COMMON_MACROS_AT = \
tests/ovsdb-macros.at \
@@ -125,7 +126,11 @@ SYSTEM_OFFLOADS_TESTSUITE_AT = \
 check_SCRIPTS += tests/atlocal
 
 TESTSUITE = $(srcdir)/tests/testsuite
+if WIN32
 TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
+else
+TESTSUITE_PATCH = $(srcdir)/tests/testsuite.unix.patch
+endif
 SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite
 SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite
 SYSTEM_OFFLOADS_TESTSUITE = $(srcdir)/tests/system-offloads-testsuite
@@ -249,16 +254,10 @@ clean-local:
 
 AUTOTEST = $(AUTOM4TE) --language=autotest
 
-if WIN32
 $(TESTSUITE): package.m4 $(TESTSUITE_AT) $(COMMON_MACROS_AT) $(TESTSUITE_PATCH)
$(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o testsuite.tmp $@.at
patch -p0 testsuite.tmp $(TESTSUITE_PATCH)
$(AM_V_at)mv testsuite.tmp $@
-else
-$(TESTSUITE): package.m4 $(TESTSUITE_AT) $(COMMON_MACROS_AT)
-   $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
-   $(AM_V_at)mv $@.tmp $@
-endif
 
 $(SYSTEM_KMOD_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT) 
$(SYSTEM_KMOD_TESTSUITE_AT) $(COMMON_MACROS_AT)
$(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
diff --git a/tests/testsuite.unix.patch b/tests/testsuite.unix.patch
new file mode 100644
index 000..f56daf0
--- /dev/null
+++ b/tests/testsuite.unix.patch
@@ -0,0 +1,10 @@
+--- testsuite  2017-07-14 13:41:25.0 +
 testsuite  2017-07-14 13:44:04.0 +
+@@ -5959,6 +5959,7 @@ $as_echo "$as_me: WARNING: unable to par
+   echo >&7
+ ) &
+ $at_job_control_off
++jobs > /dev/null || :  # drain the list of jobs
+ if $at_first; then
+   at_first=false
+   exec 6<"$at_job_fifo" 7>"$at_job_fifo"
-- 
2.5.4 (Apple Git-61)

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

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

[YAMAMOTO Takashi]

BRE alternative (\|) is an GNU sed extension. [1]
It isn't available in NetBSD sed.

[1] http://www.gnu.org/software/sed/manual/sed.html#Regular-Expressions
regexp1\|regexp2
Matches either regexp1 or regexp2. Use parentheses to use
complex alternative regular expressions. The matching process
tries each alternative in turn, from left to right, and the
first one that succeeds is used. It is a GNU extension.

Signed-off-by: YAMAMOTO Takashi 
---
 tests/pmd.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/pmd.at b/tests/pmd.at
index d041dce..f95a016 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -53,7 +53,7 @@ m4_define([CHECK_PMD_THREADS_CREATED], [
 ])
 
 m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
\)[[0-9]]*:/\1\2:/"])
-m4_define([SED_NUMA_CORE_QUEUE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
\)[[0-9]]*:/\1\2:/;s/\(queue-id: \)\(0 2 4 6\|1 3 5 
7\)/\1/"])
+m4_define([SED_NUMA_CORE_QUEUE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
\)[[0-9]]*:/\1\2:/;s/\(queue-id: \)0 2 4 
6/\1/;s/\(queue-id: \)1 3 5 7/\1/"])
 m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
 
 AT_SETUP([PMD - creating a thread/add-port])
-- 
2.5.4 (Apple Git-61)

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

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

[YAMAMOTO Takashi]

Because:
- It's no longer necessary
- It can cause problems for some utilities (e.g. ovs-pcap)
  Cf. http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=51152

Signed-off-by: YAMAMOTO Takashi 
---
 Documentation/intro/install/netbsd.rst | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/intro/install/netbsd.rst 
b/Documentation/intro/install/netbsd.rst
index c2c23e5..0b5b5c9 100644
--- a/Documentation/intro/install/netbsd.rst
+++ b/Documentation/intro/install/netbsd.rst
@@ -34,7 +34,6 @@ you need at least the following packages.
 - python27
 - py27-six
 - py27-xml
-- pkg_alternatives
 
 Some components have additional requirements. Refer to :doc:`general` for more
 information.
-- 
2.5.4 (Apple Git-61)

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

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

[YAMAMOTO Takashi]

Workaround pkg_alternative issue for NetBSD:
http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=51152

An alternative would be generating xenserver scripts from *.in,
but i'm not sure if it's appropriate for those scripts.

Signed-off-by: YAMAMOTO Takashi 
---
 tests/interface-reconfigure.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/interface-reconfigure.at b/tests/interface-reconfigure.at
index a4c0dca..f3914a4 100644
--- a/tests/interface-reconfigure.at
+++ b/tests/interface-reconfigure.at
@@ -681,7 +681,7 @@ EOF
 }
 
 ifr_run () {
-./interface-reconfigure --root-prefix="`pwd`" --no-syslog "$@"
+$PYTHON ./interface-reconfigure --root-prefix="`pwd`" --no-syslog "$@"
 }
 
 ifr_filter () {
-- 
2.5.4 (Apple Git-61)

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

Re: [ovs-dev] [PATCH 32/40] datapath-windows: fix excessive stack usage in iphelper

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 32/40] datapath-windows: fix excessive stack usage in 
iphelper

`OvsGetOrResolveIPNeigh` uses a stack over 1024 bytes.

Switch one parameter to be a pointer.

Found using WDK 8.1 static code analysis.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/IpHelper.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/datapath-windows/ovsext/IpHelper.c 
b/datapath-windows/ovsext/IpHelper.c
index cb97835..0af249e 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -485,7 +485,7 @@ OvsResolveIPNeighEntry(PMIB_IPNET_ROW2 ipNeigh)


 NTSTATUS
-OvsGetOrResolveIPNeigh(MIB_IF_ROW2 ipRow,
+OvsGetOrResolveIPNeigh(PMIB_IF_ROW2 ipRow,
UINT32 ipAddr,
PMIB_IPNET_ROW2 ipNeigh)
 {
@@ -494,8 +494,8 @@ OvsGetOrResolveIPNeigh(MIB_IF_ROW2 ipRow,
 ASSERT(ipNeigh);

 RtlZeroMemory(ipNeigh, sizeof (*ipNeigh));
-ipNeigh->InterfaceLuid.Value = ipRow.InterfaceLuid.Value;
-ipNeigh->InterfaceIndex = ipRow.InterfaceIndex;
+ipNeigh->InterfaceLuid.Value = ipRow->InterfaceLuid.Value;
+ipNeigh->InterfaceIndex = ipRow->InterfaceIndex;
 ipNeigh->Address.si_family = AF_INET;
 ipNeigh->Address.Ipv4.sin_addr.s_addr = ipAddr;

@@ -503,8 +503,8 @@ OvsGetOrResolveIPNeigh(MIB_IF_ROW2 ipRow,

 if (status != STATUS_SUCCESS) {
 RtlZeroMemory(ipNeigh, sizeof (*ipNeigh));
-ipNeigh->InterfaceLuid.Value = ipRow.InterfaceLuid.Value;
-ipNeigh->InterfaceIndex = ipRow.InterfaceIndex;
+ipNeigh->InterfaceLuid.Value = ipRow->InterfaceLuid.Value;
+ipNeigh->InterfaceIndex = ipRow->InterfaceIndex;
 ipNeigh->Address.si_family = AF_INET;
 ipNeigh->Address.Ipv4.sin_addr.s_addr = ipAddr;
 status = OvsResolveIPNeighEntry(ipNeigh);
@@ -1644,7 +1644,7 @@ OvsHandleFwdRequest(POVS_IP_HELPER_REQUEST request)
 if (ipAddr == 0) {
 ipAddr = request->fwdReq.tunnelKey.dst;
 }
-status = OvsGetOrResolveIPNeigh(instance->internalRow,
+status = OvsGetOrResolveIPNeigh(&instance->internalRow,
 ipAddr, &ipNeigh);
 if (status != STATUS_SUCCESS) {
 ExReleaseResourceLite(&instance->lock);
@@ -1936,11 +1936,10 @@ OvsStartIpHelper(PVOID data)
 MIB_IPNET_ROW2 ipNeigh;
 NTSTATUS status;
 POVS_IPHELPER_INSTANCE instance = 
(POVS_IPHELPER_INSTANCE)ipn->context;
-MIB_IF_ROW2 internalRow = instance->internalRow;
 NdisReleaseSpinLock(&ovsIpHelperLock);
 ExAcquireResourceExclusiveLite(&ovsInstanceListLock, TRUE);

-status = OvsGetOrResolveIPNeigh(internalRow,
+status = OvsGetOrResolveIPNeigh(&instance->internalRow,
 ipAddr, &ipNeigh);
 OvsUpdateIPNeighEntry(ipAddr, &ipNeigh, status);

--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 38/40] datapath-windows: Fix shared variables which use Interlocked functions

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 38/40] datapath-windows: Fix shared variables which 
use Interlocked functions

Instead of assigning a value directly to the variable use `InterlockedAdd`
with 0.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Switch.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
index 28c8ecf..1ac4fa7 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -143,7 +143,7 @@ OvsExtAttach(NDIS_HANDLE ndisFilterHandle,
 KeMemoryBarrier();

 cleanup:
-gOvsInAttach = FALSE;
+InterlockedExchange(&gOvsInAttach, 0);
 if (status != NDIS_STATUS_SUCCESS) {
 if (switchContext != NULL) {
 OvsDeleteSwitch(switchContext);
@@ -516,7 +516,7 @@ OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext)
 LONG icxRef = 0;

 do {
-ref = gOvsSwitchContextRefCount;
+ref = InterlockedAdd(&gOvsSwitchContextRefCount, 0);
 newRef = (0 == ref) ? 0 : ref - 1;
 icxRef = InterlockedCompareExchange(&gOvsSwitchContextRefCount,
 newRef,
@@ -538,7 +538,7 @@ OvsAcquireSwitchContext(VOID)
 BOOLEAN ret = FALSE;

 do {
-ref = gOvsSwitchContextRefCount;
+ref = InterlockedAdd(&gOvsSwitchContextRefCount, 0);
 newRef = (0 == ref) ? 0 : ref + 1;
 icxRef = InterlockedCompareExchange(&gOvsSwitchContextRefCount,
 newRef,
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [patch_v4 5/8] Userspace Datapath: Add TFTP support.

[Darrell Ball]

On 7/13/17, 3:19 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

On Wed, Jul 05, 2017 at 09:32:23PM -0700, Darrell Ball wrote:
> Both ipv4 and ipv6 are supported. Also, NAT support is included.
> 
> Signed-off-by: Darrell Ball 

Thanks for adding TFTP support!

The new is_tftp_ctl() function is very similar to is_ftp_ctl().  I
suggest factoring out the common code. 

Done

 Also, please write
ntohs(uh->udp_dst) == CT_IPPORT_TFTP
as
uh->udp_dst == htons(CT_IPPORT_TFTP)
to match our common practice.

Done


Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=9c4SIjTm0FSAj_H8NqdiJ_ev-2pC-f8yh5GImq31jQc&s=chDGxKSlcNUUZI1vTsll2lL9efA3RfYPUMqSY22mWgw&e=
 


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

Re: [ovs-dev] [patch_v4 4/8] Userspace Datapath: Add ALG infra and FTP.

[Darrell Ball]

Thanks a lot for the review Ben

On 7/13/17, 3:15 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

On Wed, Jul 05, 2017 at 09:32:22PM -0700, Darrell Ball wrote:
> ALG infra and FTP (both V4 and V6) support is added to the userspace
> datapath.  Also, NAT support is included.
> 
> Signed-off-by: Darrell Ball 

Thanks a lot for working on this.  It will be a valuable feature that
brings the userspace datapath closer to the kernel datapath features.

I have some comments.  I'm not too familiar with this area, so a lot of
my comments are ones that should help to make the code easier to
understand not just for me but for other newbies to ALG implementation.

The data structures introduced or modified in this patch are almost
comment-free.  The reader is left to guess important information like
the purpose of the structure, as well as per-member info like what lists
or hmaps a structure belongs to, what a "master" is, and so on.  Please
add some comments.

Done

The same seems warranted for the collection of macros that this
defines.  For example, what does FTP_MAX_PORT mean?  (If it's just the
maximum TCP port, what makes it FTP specific? etc.)

Done

In the name "alg_exp_node", I don't know what an exp is.

Done

I don't know what "delinate" means.

Name changed

What OS X problem does this allude to?
+/* CT_IPPORT_FTP is used in lieu of IPPORT_FTP as a workaround
+ * to handle OSX. */
+#define CT_IPPORT_FTP 21

I added comment that it was build related.

Usually we put the conversion on the constant side in comparisons like
the following, because compilers are better at optimizing it:
+return (ip_proto == IPPROTO_TCP &&
+(ntohs(th->tcp_src) == CT_IPPORT_FTP ||
+ ntohs(th->tcp_dst) == CT_IPPORT_FTP));

Got it

I guess that not everyone knows what "tuple space" is.  I had to think
about it for a while.  I think you basically mean the ports available
for NAT.  Maybe a more user friendly term could be used (at least in
user messages)?  Why is exhaustion "likely a user error"?  (I would have
guessed that it is more likely from some kind of DoS or port scan or
equivalent.)  How should a user respond?

Sure, DoS is valid too as the system may be unprotected. I expanded the
comments and added recommendations.


process_one() has a variable alg_nat_repl_addr that it zeros and then
appears to never use again.

I deprecated that variable use and I forgot to cleanup.

Also in process_one(), I think that this memcpy:
memcpy(&alg_exp_entry, alg_exp, sizeof alg_exp_entry);
can be written as just:
alg_exp_entry = *alg_exp;
although I don't know whether you have some expectation for padding etc.

structure copy is fine here; I thought I had caught all of these.


process_one() uses the expression "conn && is_ftp_ctl(pkt)" twice, and
the latter function is nontrivial.  Can it be evaluated just once?

Yes, thanks for pointing that out.

In conntrack_execute(), it seems odd to move the loop index declaration
out of the for statement.

This is day one conntrack code, but I can fix it here.

conn_to_ct_dpif_entry() does an xstrdup but I wasn't able to spot where
the string gets freed.

This is an existing API that I just used; the caller (in dpctl) frees the 
string.
I added a comment that the caller frees it.

I can't see how get_v4_byte_be() works properly on both big-endian and
little-endian systems.

get_v4_byte_be() shouldn't need the "& 0xff", since it returns a
uint8_t.

right, mask is not needed

In replace_substring(), it seems odd to use 8-bit quantities for sizes.
Also, it looks like 'delta' can be negative, in which case it should not
be plain char (which can be signed or unsigned given an ASCII character
set): if you really want 8-bit, use "signed char" or int8_t.

Thanks for catching this; this is a real potential bug depending on the 
compiler 
and settings. At one point, I checked and converted all signed values to either
int or int64_t; this looks like the only one left over.

The expression "remain_substring + delta" in replace_substring() kind of
threw me for a loop.  If you expand this based on variable definitions,
you get:

remain_substring + delta
 == (substr + substr_size) + (rep_str_size - substr_size)
 == substr + rep_str_size

In other words, the substr_size cancels out entirely, so that I think
the first four statements
+char delta = rep_str_size - substr_size;
+size_t move_size = total_size - substr_size;
+char *remain_substring = substr + substr_size;
+memmove(remain_substring + delta, remain_substring,
+move_size);
might as wel

Re: [ovs-dev] [PATCH 36/40] datapath-windows: Add annotation for OvsCtRelatedEntryCleaner

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 36/40] datapath-windows: Add annotation for 
OvsCtRelatedEntryCleaner

Make the function `OvsCtRelatedEntryCleaner` aware it is a kstart_routine.

Also, the function is not compliant with the coding standard.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Conntrack-related.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-related.c 
b/datapath-windows/ovsext/Conntrack-related.c
index 2d95bc2..16ed6f7 100644
--- a/datapath-windows/ovsext/Conntrack-related.c
+++ b/datapath-windows/ovsext/Conntrack-related.c
@@ -22,6 +22,7 @@ static UINT64 ctTotalRelatedEntries;
 static OVS_CT_THREAD_CTX ctRelThreadCtx;
 static PNDIS_RW_LOCK_EX ovsCtRelatedLockObj;
 extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
+KSTART_ROUTINE OvsCtRelatedEntryCleaner;

 static __inline UINT32
 OvsExtractCtRelatedKeyHash(OVS_CT_KEY *key)
@@ -170,12 +171,12 @@ OvsCtRelatedFlush()

 /*
  *
- * ovsCtRelatedEntryCleaner
+ * OvsCtRelatedEntryCleaner
  * Runs periodically and cleans up the related connections tracker
  *
  */
 VOID
-ovsCtRelatedEntryCleaner(PVOID data)
+OvsCtRelatedEntryCleaner(PVOID data)
 {
 POVS_CT_THREAD_CTX context = (POVS_CT_THREAD_CTX)data;
 PLIST_ENTRY link, next;
@@ -249,7 +250,7 @@ OvsInitCtRelated(POVS_SWITCH_CONTEXT context)
 /* Init CT Cleaner Thread */
 KeInitializeEvent(&ctRelThreadCtx.event, NotificationEvent, FALSE);
 status = PsCreateSystemThread(&threadHandle, SYNCHRONIZE, NULL, NULL,
-  NULL, ovsCtRelatedEntryCleaner,
+  NULL, OvsCtRelatedEntryCleaner,
   &ctRelThreadCtx);

 if (status != STATUS_SUCCESS) {
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 33/40] datapath-windows: Check return status when using APIs

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 33/40] datapath-windows: Check return status when 
using APIs

Check the return status of `ConvertInterfaceLuidToAlias` and 
`RtlStringCbLengthW`
and treat them accordingly.

Also remove unneeded initialization for `interfaceName`.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/IpHelper.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/datapath-windows/ovsext/IpHelper.c 
b/datapath-windows/ovsext/IpHelper.c
index 0af249e..8ade997 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -369,7 +369,7 @@ OvsGetRoute(SOCKADDR_INET *destinationAddress,
 SOCKADDR_INET crtSrcAddr = { 0 };
 MIB_IPFORWARD_ROW2 crtRoute = { 0 };
 POVS_IPHELPER_INSTANCE crtInstance = NULL;
-WCHAR interfaceName[IF_MAX_STRING_SIZE + 1] = { 0 };
+WCHAR interfaceName[IF_MAX_STRING_SIZE + 1];

 crtInstance = CONTAINING_RECORD(link, OVS_IPHELPER_INSTANCE, link);

@@ -394,11 +394,16 @@ OvsGetRoute(SOCKADDR_INET *destinationAddress,
 RtlCopyMemory(route, &crtRoute, sizeof(*route));
 *instance = crtInstance;

-
ConvertInterfaceLuidToAlias(&crtInstance->internalRow.InterfaceLuid,
-interfaceName, IF_MAX_STRING_SIZE + 1);
-RtlStringCbLengthW(interfaceName, IF_MAX_STRING_SIZE, &len);
+status =
+
ConvertInterfaceLuidToAlias(&crtInstance->internalRow.InterfaceLuid,
+interfaceName,
+IF_MAX_STRING_SIZE + 1);
+if (NT_SUCCESS(status)) {
+status = RtlStringCbLengthW(interfaceName, IF_MAX_STRING_SIZE,
+&len);
+}

-if (gOvsSwitchContext != NULL) {
+if (gOvsSwitchContext != NULL && NT_SUCCESS(status)) {
 NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock,
   &lockState, 0);
 *vport = OvsFindVportByHvNameW(gOvsSwitchContext,
@@ -608,11 +613,11 @@ OvsAddIpInterfaceNotification(PMIB_IPINTERFACE_ROW ipRow)

 InitializeListHead(&instance->link);
 ExInitializeResourceLite(&instance->lock);
-WCHAR interfaceName[IF_MAX_STRING_SIZE + 1] = { 0 };
+WCHAR interfaceName[IF_MAX_STRING_SIZE + 1];
 status = ConvertInterfaceLuidToAlias(&ipRow->InterfaceLuid,
  interfaceName,
  IF_MAX_STRING_SIZE + 1);
-if (gOvsSwitchContext == NULL) {
+if (gOvsSwitchContext == NULL || !NT_SUCCESS(status)) {
 goto error;
 }
 NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0);
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 34/40] datapath-windows: Vport check RtlStringCbLengthW return value

[Shashank Ram]

Minor nit inline.



From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 34/40] datapath-windows: Vport check 
RtlStringCbLengthW return value

The result of `RtlStringCbLengthW` is not currently checked and triggers
a warning using the WDK 8.1 static analysis.

This patch treats the result of `RtlStringCbLengthW`.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Vport.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 075f419..071bc31 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -1144,8 +1144,13 @@ GetNICAlias(PNDIS_SWITCH_NIC_PARAMETERS nicParam,
 if (status == STATUS_SUCCESS) {
 RtlStringCbPrintfW(portFriendlyName->String,
IF_MAX_STRING_SIZE, L"%s", interfaceName);
-RtlStringCbLengthW(portFriendlyName->String, IF_MAX_STRING_SIZE,
-   &len);
+status = RtlStringCbLengthW(portFriendlyName->String,
+IF_MAX_STRING_SIZE, &len);
+if (!NT_SUCCESS(status)) {
+OVS_LOG_ERROR("Fail to get the length of the string,"
+  "status: %x", status);
>> s/Fail/Failed

+return status;
+}
 portFriendlyName->Length = (USHORT)len;
 } else {
 OVS_LOG_ERROR("Fail to convert interface LUID to alias, status: 
%x",
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 31/40] datapath-windows: prettify logging in iphelper

[Shashank Ram]

Minor nits inline.

Thanks,
Shashank


From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 31/40] datapath-windows: prettify logging in  iphelper

Found by inspection.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/IpHelper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/IpHelper.c 
b/datapath-windows/ovsext/IpHelper.c
index 555351a..cb97835 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -933,7 +933,7 @@ OvsRegisterChangeNotification()
 status = NotifyRouteChange2(AF_INET, OvsChangeCallbackIpRoute, &dummy,
 TRUE, &ipRouteNotificationHandle);
 if (status != STATUS_SUCCESS) {
-OVS_LOG_ERROR("Fail to regiter ip route change, status: %x.",
+OVS_LOG_ERROR("Fail to register IP route change, status: %x.",

>> s/Fail/Failed

   status);
 goto register_cleanup;
 }
@@ -942,7 +942,8 @@ OvsRegisterChangeNotification()
   NULL, TRUE,
   &unicastIPNotificationHandle);
 if (status != STATUS_SUCCESS) {
-OVS_LOG_ERROR("Fail to regiter unicast ip change, status: %x.", 
status);
+OVS_LOG_ERROR("Fail to register UNICAST IP change, status: %x.",

>> s/Fail/Failed

+  status);
 }
 register_cleanup:
 if (status != STATUS_SUCCESS) {
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 30/40] datapath-windows: Add dummy parameter for NotifyRouteChange2

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 30/40] datapath-windows: Add dummy parameter for 
NotifyRouteChange2

Add a dummy parameter when using `NotifyRouteChange2` to keep static
static analysis happy.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/IpHelper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/IpHelper.c 
b/datapath-windows/ovsext/IpHelper.c
index e98dcd1..555351a 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -917,6 +917,7 @@ static NTSTATUS
 OvsRegisterChangeNotification()
 {
 NTSTATUS status;
+UINT dummy = 0;


 status = NotifyIpInterfaceChange(AF_INET, OvsChangeCallbackIpInterface,
@@ -928,7 +929,8 @@ OvsRegisterChangeNotification()
 return status;
 }

-status = NotifyRouteChange2(AF_INET, OvsChangeCallbackIpRoute, NULL,
+/* The CallerContext is dummy and should never be used */
+status = NotifyRouteChange2(AF_INET, OvsChangeCallbackIpRoute, &dummy,
 TRUE, &ipRouteNotificationHandle);
 if (status != STATUS_SUCCESS) {
 OVS_LOG_ERROR("Fail to regiter ip route change, status: %x.",
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 26/40] datapath-windows: Use annotations instead for macros

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 26/40] datapath-windows: Use annotations instead  
for macros

We can safely use function annotations to instead of defining out own macros.
Nuke implementation of `OVS_VERIFY_IRQL_LE` and OVS_VERIFY_IRQL (unused).

Add function annotations to the functions which were using OVS_VERIFY_IRQL_LE`.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Debug.h | 18 --
 datapath-windows/ovsext/Util.c  |  7 +++
 datapath-windows/ovsext/Util.h  |  6 ++
 3 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/datapath-windows/ovsext/Debug.h b/datapath-windows/ovsext/Debug.h
index 6de1812..c17f0e9 100644
--- a/datapath-windows/ovsext/Debug.h
+++ b/datapath-windows/ovsext/Debug.h
@@ -74,22 +74,4 @@ VOID OvsLog(UINT32 level, UINT32 flag, CHAR *funcName,
 #define OVS_LOG_WARN(_format, ...) \
OvsLog(OVS_DBG_WARN, OVS_DBG_MOD, __FUNCTION__, __LINE__, _format, 
__VA_ARGS__)

-#if DBG
-#define OVS_VERIFY_IRQL(_x)  \
-if (KeGetCurrentIrql() != (KIRQL)_x) { \
-OVS_LOG_WARN("expected IRQL %u, actual IRQL: %u", \
- _x, KeGetCurrentIrql()); \
-}
-
-#define OVS_VERIFY_IRQL_LE(_x)  \
-if (KeGetCurrentIrql() > (KIRQL)_x) { \
-OVS_LOG_WARN("expected IRQL <= %u, actual IRQL: %u", \
- _x, KeGetCurrentIrql()); \
-}
-
-#else
-#define OVS_VERIFY_IRQL(_x)
-#define OVS_VERIFY_IRQL_LE(_x)
-#endif
-
 #endif /* __DEBUG_H_ */
diff --git a/datapath-windows/ovsext/Util.c b/datapath-windows/ovsext/Util.c
index 3c9b052..abd38c2 100644
--- a/datapath-windows/ovsext/Util.c
+++ b/datapath-windows/ovsext/Util.c
@@ -25,10 +25,10 @@

 extern NDIS_HANDLE gOvsExtDriverHandle;

+_Use_decl_annotations_
 VOID*
 OvsAllocateMemoryWithTag(size_t size, ULONG tag)
 {
-OVS_VERIFY_IRQL_LE(DISPATCH_LEVEL);
 return NdisAllocateMemoryWithTagPriority(gOvsExtDriverHandle,
 (UINT32)size, tag, NormalPoolPriority);
 }
@@ -40,19 +40,18 @@ OvsFreeMemoryWithTag(VOID *ptr, ULONG tag)
 NdisFreeMemoryWithTagPriority(gOvsExtDriverHandle, ptr, tag);
 }

+_Use_decl_annotations_
 VOID *
 OvsAllocateMemory(size_t size)
 {
-OVS_VERIFY_IRQL_LE(DISPATCH_LEVEL);
 return NdisAllocateMemoryWithTagPriority(gOvsExtDriverHandle,
 (UINT32)size, OVS_MEMORY_TAG, NormalPoolPriority);
 }

+_Use_decl_annotations_
 VOID *
 OvsAllocateAlignedMemory(size_t size, UINT16 align)
 {
-OVS_VERIFY_IRQL_LE(DISPATCH_LEVEL);
-
 ASSERT((align == 8) || (align == 16));

 if ((align == 8) || (align == 16)) {
diff --git a/datapath-windows/ovsext/Util.h b/datapath-windows/ovsext/Util.h
index cccf9ff..bcb80b6 100644
--- a/datapath-windows/ovsext/Util.h
+++ b/datapath-windows/ovsext/Util.h
@@ -41,9 +41,15 @@
 #define OVS_GENEVE_POOL_TAG 'GNVO'
 #define OVS_IPFRAG_POOL_TAG 'FGVO'

+_IRQL_requires_max_(DISPATCH_LEVEL)
 VOID *OvsAllocateMemory(size_t size);
+
+_IRQL_requires_max_(DISPATCH_LEVEL)
 VOID *OvsAllocateMemoryWithTag(size_t size, ULONG tag);
+
+_IRQL_requires_max_(DISPATCH_LEVEL)
 VOID *OvsAllocateAlignedMemory(size_t size, UINT16 align);
+
 VOID *OvsAllocateMemoryPerCpu(size_t size, size_t count, ULONG tag);
 VOID OvsFreeMemory(VOID *ptr);
 VOID OvsFreeMemoryWithTag(VOID *ptr, ULONG tag);
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH v1 0/2] Add GTP vport based on upstream datapath

[Amar Padmanabhan]

Yeah, we are looking at tunnel termination in OVS, i.e. GGSN or PGW. I think 
what you mention Weiger is about an on-path device that also does some 
classification like some of the 5G proposals. I think Yi is also looking at it 
but that is not directly related to this patch set.
- Amar

On 7/14/17, 10:01 AM, "Joe Stringer"  wrote:

On 14 July 2017 at 04:53, Wieger IJntema  
wrote:
>> ovs-vsctl add-port br0 gtp-vport -- set interface gtp-vport \
>> ofport_request=2 type=gtp option:remote_ip=flow options:key=flow
>>
>> ovs-ofctl add-flow br0
>> "in_port=2,tun_src=192.168.60.141,tun_id=123, \
>> actions=set_field:02:00:00:00:00:00->eth_src, \
>> set_field:ff:ff:ff:ff:ff:ff->eth_dst,LOCAL"
>
>
> I just want to be sure. But this implicates that the incomming packet is
> already decapusulated by the kernel and it has attached metadata like the
> tunnel_id etc.
> a nicer solution would be that you can already match on tunnel_id before 
it
> is getting encapsulated. Then you can chose later if it needa 
decapsulation
> or just forward the packet.
> I'm not sure if it is a possibility?

I wonder if we're actually discussing two different use cases? I think
that Jiannan & co are interested in GGSN functionality, whereas if my
understanding serves me right what you're proposing sounds more like
SGSN functionality. This approach is specifically for the edge of the
GTP-tunnelled network so it's always wanting to perform encap/decap.
For this particular use case, the proposed approach has a couple of
nice attributes. Firstly, the tunneling follows the same model as all
of the existing tunneling code so it fits quite well without needing
to solve new problems for a new tunnel protocol type. Secondly, the
kernel can deal with filtering GTP-C traffic out of the stream to
forward to the appropriate GTP daemon, which means that OVS doesn't
need to be taught how to understand that traffic or forward it to
another program.


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

Re: [ovs-dev] [PATCH 19/40] datapath-windows: Add function annotations for OvsTunnelFilterCancelIrp

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 19/40] datapath-windows: Add function annotations for 
OvsTunnelFilterCancelIrp

The function should be aware that it is cancel routine.

This patch adds annotation for that.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/TunnelFilter.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/TunnelFilter.c 
b/datapath-windows/ovsext/TunnelFilter.c
index f5f82cb..6f4663e 100644
--- a/datapath-windows/ovsext/TunnelFilter.c
+++ b/datapath-windows/ovsext/TunnelFilter.c
@@ -172,8 +172,7 @@ static NTSTATUS 
OvsTunnelFilterThreadInit(POVS_TUNFLT_THREAD_CONTEXT threadCtx);
 static VOID OvsTunnelFilterThreadUninit(POVS_TUNFLT_THREAD_CONTEXT 
threadCtx);
 static VOID OvsTunnelFilterSetIrpContext(POVS_TUNFLT_REQUEST_LIST 
listRequests,
  POVS_TUNFLT_REQUEST request);
-static VOID OvsTunnelFilterCancelIrp(PDEVICE_OBJECT DeviceObject,
- PIRP Irp);
+DRIVER_CANCEL   OvsTunnelFilterCancelIrp;

 /*
  * Callout driver global variables
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 29/40] datapath-windows: Suppress warning in jhash

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 29/40] datapath-windows: Suppress warning in jhash

Suppress overflow warning to keep static code analysis happy.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Jhash.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/datapath-windows/ovsext/Jhash.c b/datapath-windows/ovsext/Jhash.c
index db08d0b..f42104d 100644
--- a/datapath-windows/ovsext/Jhash.c
+++ b/datapath-windows/ovsext/Jhash.c
@@ -121,6 +121,8 @@ OvsJhashBytes(const VOID *p_, SIZE_T n, UINT32 basis)
 memcpy(tmp, p, n);
 a += tmp[0];
 b += tmp[1];
+#pragma warning(suppress: 6385)
+/* Suppress buffer overflow, it is either zero or some random value */
 c += tmp[2];
 JhashFinal(&a, &b, &c);
 }
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 01/40] datapath-windows: Use only non executable memory

[Shashank Ram]

Comments inline.

Thanks,
Shashank


From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 01/40] datapath-windows: Use only non executable  
memory

Use only non-executable memory when using MmGetSystemAddressForMdlSafe.

Introduce a new function called OvsGetMdlWithLowPrio for readability.

Found using WDK 10 static code analysis.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/BufferMgmt.c   |  8 
 datapath-windows/ovsext/Datapath.c |  3 +--
 datapath-windows/ovsext/Flow.c |  2 +-
 datapath-windows/ovsext/Geneve.c   |  5 ++---
 datapath-windows/ovsext/Gre.c  |  3 +--
 datapath-windows/ovsext/Offload.c  |  9 -
 datapath-windows/ovsext/PacketParser.c |  3 +--
 datapath-windows/ovsext/Stt.c  |  8 +++-
 datapath-windows/ovsext/Util.h | 15 +++
 datapath-windows/ovsext/Vxlan.c|  7 +++
 10 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/datapath-windows/ovsext/BufferMgmt.c 
b/datapath-windows/ovsext/BufferMgmt.c
index 5048ada..6354781 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -1153,7 +1153,7 @@ FixFragmentHeader(PNET_BUFFER nb, UINT16 fragmentSize,

 mdl = NET_BUFFER_FIRST_MDL(nb);

-bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(mdl, LowPagePriority);
+bufferStart = (PUINT8)OvsGetMdlWithLowPrio(mdl);
 if (!bufferStart) {
 return NDIS_STATUS_RESOURCES;
 }
@@ -1211,7 +1211,7 @@ FixSegmentHeader(PNET_BUFFER nb, UINT16 segmentSize, 
UINT32 seqNumber,

 mdl = NET_BUFFER_FIRST_MDL(nb);

-bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(mdl, LowPagePriority);
+bufferStart = (PUINT8)OvsGetMdlWithLowPrio(mdl);
 if (!bufferStart) {
 return NDIS_STATUS_RESOURCES;
 }
@@ -1517,8 +1517,8 @@ OvsAllocateNBLFromBuffer(PVOID context,

 nb = NET_BUFFER_LIST_FIRST_NB(nbl);
 mdl = NET_BUFFER_CURRENT_MDL(nb);
-data = (PUINT8)MmGetSystemAddressForMdlSafe(mdl, LowPagePriority) +
-NET_BUFFER_CURRENT_MDL_OFFSET(nb);
+data = (PUINT8)OvsGetMdlWithLowPrio(mdl)
++ NET_BUFFER_CURRENT_MDL_OFFSET(nb);
 if (!data) {
 OvsCompleteNBL(switchContext, nbl, TRUE);
 return NULL;
diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 10412a1..21693af 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1588,8 +1588,7 @@ MapIrpOutputBuffer(PIRP irp,
 if (irp->MdlAddress == NULL) {
 return STATUS_INVALID_PARAMETER;
 }
-*buffer = MmGetSystemAddressForMdlSafe(irp->MdlAddress,
-   NormalPagePriority);
+*buffer = OvsGetMdlWithLowPrio(irp->MdlAddress);
 if (*buffer == NULL) {
 return STATUS_INSUFFICIENT_RESOURCES;
 }
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 852a22f..a7d6317 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -1933,7 +1933,7 @@ GetStartAddrNBL(const NET_BUFFER_LIST *_pNB)

 // Ethernet Header is a guaranteed safe access.
 curMdl = (NET_BUFFER_LIST_FIRST_NB(_pNB))->CurrentMdl;
-curBuffer =  MmGetSystemAddressForMdlSafe(curMdl, LowPagePriority);
+curBuffer = OvsGetMdlWithLowPrio(curMdl);
 if (!curBuffer) {
 return NULL;
 }
diff --git a/datapath-windows/ovsext/Geneve.c b/datapath-windows/ovsext/Geneve.c
index 43374e2..01b5416 100644
--- a/datapath-windows/ovsext/Geneve.c
+++ b/datapath-windows/ovsext/Geneve.c
@@ -157,8 +157,7 @@ NDIS_STATUS OvsEncapGeneve(POVS_VPORT_ENTRY vport,
 }

 curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
-   LowPagePriority);
+bufferStart = (PUINT8)OvsGetMdlWithLowPrio(curMdl);
 if (!bufferStart) {
 status = NDIS_STATUS_RESOURCES;
 goto ret_error;
@@ -286,7 +285,7 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT 
switchContext,
 curNbl = *newNbl;
 curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
 curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl, LowPagePriority)
+bufferStart = (PUINT8)OvsGetMdlWithLowPrio(curMdl)
   + NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
 if (!bufferStart) {
 status = NDIS_STATUS_RESOURCES;
diff --git a/datapath-windows/ovsext/Gre.c b/datapath-windows/ovsext/Gre.c
index f095742..461efe0 100644
--- a/datapath-windows/ovsext/Gre.c
+++ b/datapath-windows/ovsext/Gre.c
@@ -202,8 +202,7 @@ OvsDoEncapGre(POVS_VPORT_ENTRY vport,
 }

 curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curM

Re: [ovs-dev] [PATCH 28/40] datapath-windows: Fix possible NULL deference in OvsFullCopyNBL

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 28/40] datapath-windows: Fix possible NULL deference 
in OvsFullCopyNBL

Check if the first net buffer exists before trying to copy it.

Found using WDK 10 static code analysis.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/BufferMgmt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/datapath-windows/ovsext/BufferMgmt.c 
b/datapath-windows/ovsext/BufferMgmt.c
index c81ca2a..d913380 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -985,6 +985,9 @@ OvsFullCopyNBL(PVOID ovsContext,
 }

 nb = NET_BUFFER_LIST_FIRST_NB(nbl);
+if (nb == NULL) {
+return NULL;
+}

 if (NET_BUFFER_NEXT_NB(nb) == NULL) {
 return OvsCopySinglePacketNBL(context, nbl, nb, headRoom, copyNblInfo);
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 21/40] datapath-windows: Add function annotations for OvsAcquireEventQueueLock

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 21/40] datapath-windows: Add function annotations for 
OvsAcquireEventQueueLock

The function should be aware that it raises the dispatch level, saves the
dispatch level and acquires a lock.

This patch adds annotation for that.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Event.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/datapath-windows/ovsext/Event.c b/datapath-windows/ovsext/Event.c
index cabfebf..71fcd4b 100644
--- a/datapath-windows/ovsext/Event.c
+++ b/datapath-windows/ovsext/Event.c
@@ -51,6 +51,9 @@ OvsCleanupEventQueue()
 }
 }

+_IRQL_raises_(DISPATCH_LEVEL)
+_IRQL_saves_global_(OldIrql, eventQueueLockArr[eventId])
+_Acquires_lock_(eventQueueLockArr[eventId])
 static __inline VOID
 OvsAcquireEventQueueLock(int eventId)
 {
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 17/40] datapath-windows: Add function annotations for OvsReleaseDatapath

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 17/40] datapath-windows: Add function annotations for 
OvsReleaseDatapath

The function should be aware that it requires a certain dispatch level,
restores the dispatch level, requires lock held and releases a lock.

This patch adds annotation for that.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Switch.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/datapath-windows/ovsext/Switch.h b/datapath-windows/ovsext/Switch.h
index 5e856e2..18a9ec6 100644
--- a/datapath-windows/ovsext/Switch.h
+++ b/datapath-windows/ovsext/Switch.h
@@ -210,6 +210,10 @@ OvsAcquireDatapathWrite(OVS_DATAPATH *datapath,
dispatch ? NDIS_RWL_AT_DISPATCH_LEVEL : 0);
 }

+_IRQL_requires_(DISPATCH_LEVEL)
+_IRQL_restores_global_(OldIrql, lockState)
+_Requires_lock_held_(datapath->lock)
+_Releases_lock_(datapath->lock)
 static __inline VOID
 OvsReleaseDatapath(OVS_DATAPATH *datapath,
LOCK_STATE_EX *lockState)
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 25/40] datapath-windows: Fix spelling for OvsTunnelFilterSetIrpContext

[Shashank Ram]

Minor nit: Change commit message to "datapath-windows: Fix a spelling in the 
comment"

Thanks,
Shashank


From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 25/40] datapath-windows: Fix spelling for 
OvsTunnelFilterSetIrpContext

Found by inspection.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/TunnelFilter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/TunnelFilter.c 
b/datapath-windows/ovsext/TunnelFilter.c
index 6f4663e..9939415 100644
--- a/datapath-windows/ovsext/TunnelFilter.c
+++ b/datapath-windows/ovsext/TunnelFilter.c
@@ -1618,7 +1618,7 @@ OvsTunnelFilterSetIrpContext(POVS_TUNFLT_REQUEST_LIST 
listRequests,
 /*
  * --
  * This function is the Cancel routine to be called by the I/O Manager in the
- * case when the IRP is cancelled.
+ * case the IRP is canceled.
  * --
  */
 VOID
--
2.10.2.windows.1
___

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

Re: [ovs-dev] Very Urgent

[Aeberhardt Dania]

Outlook

Dear account User,
A few of your incoming mails were placed on pending status due to the recent 
upgrade on our database. In order to receive your messages,
click on the link below to login and wait for response from Webmail.
CLICK HERE
We apologies for any inconvenience and appreciate your understanding.
Thank You.

Copyright © 2017 Webmail .Inc . All rights reserved.






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

Re: [ovs-dev] [PATCH 27/40] datapath-windows: Add assert in OvsPartialCopyNBL

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 27/40] datapath-windows: Add assert in
OvsPartialCopyNBL

`srcNb` should never be NULL since it was copied over from another nbl.
Add an assertion just in case and to keep static analysis happy.

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

diff --git a/datapath-windows/ovsext/BufferMgmt.c 
b/datapath-windows/ovsext/BufferMgmt.c
index 53490fa..c81ca2a 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -802,6 +802,7 @@ OvsPartialCopyNBL(PVOID ovsContext,
  OVS_BUFFER_PRIVATE_FORWARD_CONTEXT;

 srcNb = NET_BUFFER_LIST_FIRST_NB(nbl);
+ASSERT(srcNb);
 OvsInitNBLContext(dstCtx, flags, NET_BUFFER_DATA_LENGTH(srcNb) - copySize,
   OVS_DPPORT_NUMBER_INVALID);

--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 24/40] datapath-windows: Add annotations for OvsAcquirePidHashLock

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 24/40] datapath-windows: Add annotations for 
OvsAcquirePidHashLock

Add annotations to the function ` OvsAcquirePidHashLock`.
We make it aware that it raises the dispatch level, where it saves the
dispatch level and it acquires a lock.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/User.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
index 3d4bebe..4693a8b 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -55,6 +55,9 @@ extern NL_POLICY nlFlowTunnelKeyPolicy[];
 extern UINT32 nlFlowTunnelKeyPolicyLen;
 DRIVER_CANCEL OvsCancelIrpDatapath;

+_IRQL_raises_(DISPATCH_LEVEL)
+_IRQL_saves_global_(OldIrql, gOvsSwitchContext->pidHashLock)
+_Acquires_lock_(gOvsSwitchContext->pidHashLock)
 static __inline VOID
 OvsAcquirePidHashLock()
 {
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 18/40] datapath-windows: Add function annotations for OvsCancelIrp

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 18/40] datapath-windows: Add function annotations for 
OvsCancelIrp

The function should be aware that it is cancel routine.

This patch adds annotation for that.

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

diff --git a/datapath-windows/ovsext/Event.c b/datapath-windows/ovsext/Event.c
index 2b54692..cabfebf 100644
--- a/datapath-windows/ovsext/Event.c
+++ b/datapath-windows/ovsext/Event.c
@@ -29,6 +29,7 @@
 LIST_ENTRY ovsEventQueueArr[OVS_MCAST_EVENT_TYPES_MAX];
 static NDIS_SPIN_LOCK eventQueueLockArr[OVS_MCAST_EVENT_TYPES_MAX];
 UINT32 ovsNumEventQueueArr[OVS_MCAST_EVENT_TYPES_MAX];
+DRIVER_CANCEL OvsCancelIrp;

 NTSTATUS
 OvsInitEventQueue()
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 16/40] datapath-windows: Add function annotations for OvsAcquireDatapathWrite

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 16/40] datapath-windows: Add function annotations for 
OvsAcquireDatapathWrite

The function should be aware that it raises the dispatch level, saves the
dispatch level and acquires a lock.

This patch adds annotation for that.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Switch.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/datapath-windows/ovsext/Switch.h b/datapath-windows/ovsext/Switch.h
index d76c462..5e856e2 100644
--- a/datapath-windows/ovsext/Switch.h
+++ b/datapath-windows/ovsext/Switch.h
@@ -197,6 +197,9 @@ OvsAcquireDatapathRead(OVS_DATAPATH *datapath,
   dispatch ? NDIS_RWL_AT_DISPATCH_LEVEL : 0);
 }

+_IRQL_raises_(DISPATCH_LEVEL)
+_IRQL_saves_global_(OldIrql, lockState)
+_Acquires_lock_(datapath->lock)
 static __inline VOID
 OvsAcquireDatapathWrite(OVS_DATAPATH *datapath,
 LOCK_STATE_EX *lockState,
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 23/40] datapath-windows: Add annotations for OvsReleasePidHashLock

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 23/40] datapath-windows: Add annotations for 
OvsReleasePidHashLock

Add function annotations for ` OvsReleasePidHashLock`.
We make it aware that it requires a certain dispatch level, that it
restores the dispatch level, that it requires a lock held and releases
a lock.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/User.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
index d2ef4aa..3d4bebe 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -61,6 +61,10 @@ OvsAcquirePidHashLock()
 NdisAcquireSpinLock(&(gOvsSwitchContext->pidHashLock));
 }

+_IRQL_requires_(DISPATCH_LEVEL)
+_IRQL_restores_global_(OldIrql, gOvsSwitchContext->pidHashLock)
+_Requires_lock_held_(gOvsSwitchContext->pidHashLock)
+_Releases_lock_(gOvsSwitchContext->pidHashLock)
 static __inline VOID
 OvsReleasePidHashLock()
 {
--
2.10.2.windows.1
___

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

Re: [ovs-dev] [PATCH 22/40] datapath-windows: Add annotations for OvsReleaseEventQueueLock

[Shashank Ram]

From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Thursday, July 13, 2017 9:40 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH 22/40] datapath-windows: Add annotations for 
OvsReleaseEventQueueLock

Add function annotations for ` OvsReleaseEventQueueLock`.
We make it aware that it requires a certain dispatch level, that it
restores the dispatch level, that it requires a lock held and releases
a lock.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Event.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/datapath-windows/ovsext/Event.c b/datapath-windows/ovsext/Event.c
index 71fcd4b..348f032 100644
--- a/datapath-windows/ovsext/Event.c
+++ b/datapath-windows/ovsext/Event.c
@@ -60,6 +60,10 @@ OvsAcquireEventQueueLock(int eventId)
 NdisAcquireSpinLock(&eventQueueLockArr[eventId]);
 }

+_IRQL_requires_(DISPATCH_LEVEL)
+_IRQL_restores_global_(OldIrql, eventQueueLockArr[eventId])
+_Requires_lock_held_(eventQueueLockArr[eventId])
+_Releases_lock_(eventQueueLockArr[eventId])
 static __inline VOID
 OvsReleaseEventQueueLock(int eventId)
 {
--
2.10.2.windows.1
___

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