Re: [ovs-dev] seq: Correct example in comment.

[Ilya Maximets]

On 23.01.2019 1:43, Ben Pfaff wrote:
> On Tue, Jan 22, 2019 at 12:52:56PM +0300, Ilya Maximets wrote:
>> On 22.01.2019 4:14, Ben Pfaff wrote:
>>> It was deceptive for the example to imply that a seq can be declared
>>> directly, because the API only allows for creating a new one on the heap.
>>>
>>> Signed-off-by: Ben Pfaff 
>>
>> Thanks.
>> Maybe you can additionally fix the C99-style comment here to not use
>> a bad style inside the example.
>>
>> Anyway,
>> Acked-by: Ilya Maximets 
> 
> Thanks, I applied this to master.
> 
> It's tricky to avoid C99 comments in code samples inside regular C
> comments, because C comments don't nest.  Do you have a suggestion on
> how to do it?

Oops. Yes, you're right, it's a bit tricky.

So, I'm not sure if this needed, but I see 2 possible solutions:

  1. Add some spaces like / * Comment * /.
 But this increases the line length over 79 chars here.

  2. Make it look not like a comment at all. i.e. Make it look like a
 a simple text. For example:

 if (ovs_list_is_singleton(&queue)) {  <-- The 'if' test here is 
optional.

 This doesn't give a bad comment example and obviously should be removed
 while copying to the real code.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2] rhel: bug fix upgrade path in kmod fedora spec file

[Flavio Leitner]

On Tue, Jan 22, 2019 at 03:02:30PM -0800, Greg Rose wrote:
> From: Martin Xu 
> 
> This patch removes the "Conflicts" tag and adds "Obsoletes" tag.
> 
> With the conflicts tag, when a user attempts to install or upgrade with
> the same version as already installed, the conflict kicks in. Otherwise,
> such is allowed with --replacepkgs.
> 
> Obsoletes is needed for the upgrade path from kmod-openvswitch to
> openvswitch-kmod.
> 
> Fixes: 22c33c3039 (rhel: support kmod build against mulitple kernel
> versions, fedora)
> 
> VMware-BZ: #2249788
> 
> Signed-off-by: Martin Xu 
> Signed-off-by: Greg Rose 
> CC: Flavio Leitner 
> CC: Yi-Hung Wei 
> CC: Yifeng Sun 
> CC: Zak Whittington 
> CC: Ben Pfaff 
> ---

LGTM
Acked-by: Flavio Leitner 


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

Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS

[Anju Thomas]

Hi Ben,

Thanks for reply. I just have one doubt . Reply inline 

Regards
Anju

-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Wednesday, January 23, 2019 5:57 AM
To: Anju Thomas 
Cc: d...@openvswitch.org; Keshav Gupta 
Subject: Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS

On Mon, Jan 21, 2019 at 07:01:33AM +, Anju Thomas wrote:
> On Thu, Jan 17, 2019 at 04:49:20AM +, Anju Thomas wrote:
> > Currently OVS maintains explicit packet drop/error counters only on port
> > level. Packets that are dropped as part of normal OpenFlow processing 
> > are
> > counted in flow stats of “drop” flows or as table misses in table stats.
> > These can only be interpreted by controllers that know the semantics of
> > the configured OpenFlow pipeline. Without that knowledge, it is 
> > impossible
> > for an OVS user to obtain e.g. the total number of packets dropped due 
> > to
> > OpenFlow rules.
> 
> Thanks for the patch!  I agree with your motivations--it is useful to 
> understand why packets are dropped.  I have some comments to add to Ilya's.
> 
> It looks like the drop actions that this formats in
> format_odp_drop_action() can't necessarily be parsed by 
> odp_actions_from_string().  Usually we expect this.  (Probably the syntax 
> should be adjusted to make parsing more straightforward.)   
> odp_actions_from string is used to convert string to action for parsing .But 
> we are using the drop action reason only fo display. We do not want to 
> provide an option for its parsing. Is my understanding correct.

Why not?  Ordinarily, anything that can be output can be input.  It is useful 
for testing to be able to input datapath actions.

Agreed. Ideally yes. But in this case the error are set due to some error 
during translation like *bridge not found 
*too many resubmits 
*recursion too deep 
*too many resubmits 
*sack too deeo
*no recirculation context
*too many mpls labels
* invalid tunnel metadata
* unsupported packet type
* ecn mismatch at tunnel decapsulation
* forwarding disabled
* pipeline drop

Except pipeline drop I don’t understand how it will make sense to inject others 
as input . Can you explain if I am missing something ?


> It looks like xlate_error maps one-to-one to drop reasons (except why is 
> XLATE_FORWARDING_DISABLED mapped to OVS_DROP_REASON_MAX?), so do we really 
> want different enumerations?  Mapping back and forth is a bit of a slog, and 
> there's already a way to translate xlate_errors to strings.
> 
>  .We just kept it separate for better code readability and also that 
> XLATE_** error were more in the ofproto layer and we wanted something in the 
> datapath . If you believe that that is not justified compared to the slogging 
> then I will modify this .  What do you say ?

I'd leave them the same for now, until there's a need for them to be different.


 Sure. I will keep them same error viz XLATE_** codes.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 0/3] revert port duplicate checking optimization

[Flavio Leitner]

Hi,

Just a reminder about this revert.

On Thu, Dec 13, 2018 at 12:30:47PM -0200, Flavio Leitner wrote:
> 
> This should be applied to branch-2.10 as well.

And now branch-2.11

Thanks,
fbl

> 
> (BTW, I had CC few folks in the patchset, but I am only seeing Guru, so
>  I am adding them to this email just in case)
> 
> Thanks,
> fbl
> 
> On Thu, Dec 13, 2018 at 12:24:45PM -0200, Flavio Leitner wrote:
> > The optimization introduced a regression in OSP environments using
> > internal ports in other netns. Their networking configuration is lost
> > when the service is restarted because the ports are recreated now.
> > 
> > Before the patch it checked using netlink if the port with a specific
> > "name" was already there. The check is a lookup in all ports attached
> > to the DP regardless of the port's netns.
> > 
> > After the patch it relies on the kernel to identify that situation.
> > Unfortunately the only protection there is register_netdevice() which
> > fails only if the port with that name exists in the current netns.
> > 
> > If the port is in another netns, it will get a new dp_port and because
> > of that userspace will delete the old port. At this point the original
> > port is gone from the other netns and there a fresh port in the current
> > netns.
> > 
> > This patchset reverts the original commit and the two other follow ups.
> > 
> > Flavio Leitner (3):
> >   Revert "dpif-netlink: Don't destroy and recreate port if it exists"
> >   Revert "ofproto-dpif: Check for EBUSY as well"
> >   Revert "ofproto-dpif: Let the dpif report when a port is a duplicate."
> > 
> >  lib/dpif-netlink.c | 4 ++--
> >  lib/dpif.c | 9 ++---
> >  ofproto/ofproto-dpif.c | 7 ---
> >  3 files changed, 8 insertions(+), 12 deletions(-)
> > 
> > -- 
> > 2.17.2
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[ovs-dev] [PATCH] Fix test 'testing ovn -- IP packet buffering' on Windows

[Alin Gabriel Serdean]

The test fails on Windows because of:
<--cut-->
ovn-nbctl: sw0: invalid network address: 2001;1\64
ovn-nbctl: sw1: invalid network address: 2002;1\64
<--cut-->

This is due to the fact msys converts '::1' into ';1'.

Use IPv6 long form instead of its short variant.

Signed-off-by: Alin Gabriel Serdean 
---
 tests/ovn.at | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/ovn.at b/tests/ovn.at
index 5f387fc07..f54f24c74 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -11858,12 +11858,12 @@ ovn-nbctl create Logical_Router name=lr0 
options:chassis=hv1
 ovn-nbctl ls-add sw0
 ovn-nbctl ls-add sw1
 
-ovn-nbctl lrp-add lr0 sw0 00:00:01:01:02:03 192.168.1.1/24 2001::1/64
+ovn-nbctl lrp-add lr0 sw0 00:00:01:01:02:03 192.168.1.1/24 
2001:0:0:0:0:0:0:1/64
 ovn-nbctl lsp-add sw0 rp-sw0 -- set Logical_Switch_Port rp-sw0 \
 type=router options:router-port=sw0 \
 -- lsp-set-addresses rp-sw0 router
 
-ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 2002::1/64
+ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 2002:0:0:0:0:0:0:1/64
 ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \
 type=router options:router-port=sw1 \
 -- lsp-set-addresses rp-sw1 router
-- 
2.16.1.windows.1

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

Re: [ovs-dev] [PATCH] Fix test 'testing ovn -- IP packet buffering' on Windows

[Numan Siddique]

On Wed, Jan 23, 2019 at 4:14 PM Alin Gabriel Serdean 
wrote:

> The test fails on Windows because of:
> <--cut-->
> ovn-nbctl: sw0: invalid network address: 2001;1\64
> ovn-nbctl: sw1: invalid network address: 2002;1\64
> <--cut-->
>
> This is due to the fact msys converts '::1' into ';1'.
>
> Use IPv6 long form instead of its short variant.
>
> Signed-off-by: Alin Gabriel Serdean 
>

Acked-by: Numan Siddique 

We generally tend to use the short form and we don't even realize it
breaks windows.  Probably checkpatch can check it and flag an error.
I will give it a try to add this check in checkpatch utility.

Thanks
Numan




> ---
>  tests/ovn.at | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 5f387fc07..f54f24c74 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -11858,12 +11858,12 @@ ovn-nbctl create Logical_Router name=lr0
> options:chassis=hv1
>  ovn-nbctl ls-add sw0
>  ovn-nbctl ls-add sw1
>
> -ovn-nbctl lrp-add lr0 sw0 00:00:01:01:02:03 192.168.1.1/24 2001::1/64
> +ovn-nbctl lrp-add lr0 sw0 00:00:01:01:02:03 192.168.1.1/24
> 2001:0:0:0:0:0:0:1/64
>  ovn-nbctl lsp-add sw0 rp-sw0 -- set Logical_Switch_Port rp-sw0 \
>  type=router options:router-port=sw0 \
>  -- lsp-set-addresses rp-sw0 router
>
> -ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 2002::1/64
> +ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24
> 2002:0:0:0:0:0:0:1/64
>  ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \
>  type=router options:router-port=sw1 \
>  -- lsp-set-addresses rp-sw1 router
> --
> 2.16.1.windows.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 0/3] gcc-plugins: Introduce stackinit plugin

[Kees Cook]

This adds a new plugin "stackinit" that attempts to perform unconditional
initialization of all stack variables[1]. It has wider effects than
GCC_PLUGIN_STRUCTLEAK_BYREF_ALL=y since BYREF_ALL does not consider
non-structures. A notable weakness is that padding bytes in many cases
remain uninitialized since GCC treats these bytes as "undefined". I'm
hoping we can improve the compiler (or the plugin) to cover that too.
(It's worth noting that BYREF_ALL actually does handle the padding --
I think this is due to the different method of detecting if initialization
is needed.)

Included is a tree-wide change to move switch variables up and out of
their switch and into the top-level variable declarations.

Included is a set of test cases for evaluating stack initialization,
which checks for padding, different types, etc.

Feedback welcome! :)

-Kees

[1] 
https://lkml.kernel.org/r/CA+55aFykZL+cSBJjBBts7ebEFfyGPdMzTmLSxKnT_29=j94...@mail.gmail.com

Kees Cook (3):
  treewide: Lift switch variables out of switches
  gcc-plugins: Introduce stackinit plugin
  lib: Introduce test_stackinit module

 arch/x86/xen/enlighten_pv.c   |   7 +-
 drivers/char/pcmcia/cm4000_cs.c   |   2 +-
 drivers/char/ppdev.c  |  20 +-
 drivers/gpu/drm/drm_edid.c|   4 +-
 drivers/gpu/drm/i915/intel_display.c  |   2 +-
 drivers/gpu/drm/i915/intel_pm.c   |   4 +-
 drivers/net/ethernet/intel/e1000/e1000_main.c |   3 +-
 drivers/tty/n_tty.c   |   3 +-
 drivers/usb/gadget/udc/net2280.c  |   5 +-
 fs/fcntl.c|   3 +-
 lib/Kconfig.debug |   9 +
 lib/Makefile  |   1 +
 lib/test_stackinit.c  | 327 ++
 mm/shmem.c|   5 +-
 net/core/skbuff.c |   4 +-
 net/ipv6/ip6_gre.c|   4 +-
 net/ipv6/ip6_tunnel.c |   4 +-
 net/openvswitch/flow_netlink.c|   7 +-
 scripts/Makefile.gcc-plugins  |   6 +
 scripts/gcc-plugins/Kconfig   |   9 +
 scripts/gcc-plugins/gcc-common.h  |  11 +-
 scripts/gcc-plugins/stackinit_plugin.c|  79 +
 security/tomoyo/common.c  |   3 +-
 security/tomoyo/condition.c   |   7 +-
 security/tomoyo/util.c|   4 +-
 25 files changed, 484 insertions(+), 49 deletions(-)
 create mode 100644 lib/test_stackinit.c
 create mode 100644 scripts/gcc-plugins/stackinit_plugin.c

-- 
2.17.1

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

[ovs-dev] [PATCH 3/3] lib: Introduce test_stackinit module

[Kees Cook]

Adds test for stack initialization coverage. We have several build options
that control the level of stack variable initialization. This test lets us
visualize which options cover which cases, and provide tests for options
that are currently not available (padding initialization).

All options pass the explicit initialization cases and the partial
initializers (even with padding):

test_stackinit: u8_zero ok
test_stackinit: u16_zero ok
test_stackinit: u32_zero ok
test_stackinit: u64_zero ok
test_stackinit: char_array_zero ok
test_stackinit: small_hole_zero ok
test_stackinit: big_hole_zero ok
test_stackinit: packed_zero ok
test_stackinit: small_hole_dynamic_partial ok
test_stackinit: big_hole_dynamic_partial ok
test_stackinit: packed_static_partial ok
test_stackinit: small_hole_static_partial ok
test_stackinit: big_hole_static_partial ok

The results of the other tests (which contain no explicit initialization),
change based on the build's configured compiler instrumentation.

No options:

test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_runtime_partial FAIL (uninit bytes: 23)
test_stackinit: big_hole_runtime_partial FAIL (uninit bytes: 127)
test_stackinit: small_hole_runtime_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_runtime_all FAIL (uninit bytes: 61)
test_stackinit: u8 FAIL (uninit bytes: 1)
test_stackinit: u16 FAIL (uninit bytes: 2)
test_stackinit: u32 FAIL (uninit bytes: 4)
test_stackinit: u64 FAIL (uninit bytes: 8)
test_stackinit: char_array FAIL (uninit bytes: 16)
test_stackinit: small_hole FAIL (uninit bytes: 24)
test_stackinit: big_hole FAIL (uninit bytes: 128)
test_stackinit: user FAIL (uninit bytes: 32)
test_stackinit: failures: 16

CONFIG_GCC_PLUGIN_STRUCTLEAK=y
This only tries to initialize structs with __user markings:

test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_runtime_partial FAIL (uninit bytes: 23)
test_stackinit: big_hole_runtime_partial FAIL (uninit bytes: 127)
test_stackinit: small_hole_runtime_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_runtime_all FAIL (uninit bytes: 61)
test_stackinit: u8 FAIL (uninit bytes: 1)
test_stackinit: u16 FAIL (uninit bytes: 2)
test_stackinit: u32 FAIL (uninit bytes: 4)
test_stackinit: u64 FAIL (uninit bytes: 8)
test_stackinit: char_array FAIL (uninit bytes: 16)
test_stackinit: small_hole FAIL (uninit bytes: 24)
test_stackinit: big_hole FAIL (uninit bytes: 128)
test_stackinit: user ok
test_stackinit: failures: 15

CONFIG_GCC_PLUGIN_STRUCTLEAK=y
CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL=y
This initializes all structures passed by reference (scalars and strings
remain uninitialized, but padding is wiped):

test_stackinit: small_hole_static_all ok
test_stackinit: big_hole_static_all ok
test_stackinit: small_hole_dynamic_all ok
test_stackinit: big_hole_dynamic_all ok
test_stackinit: small_hole_runtime_partial ok
test_stackinit: big_hole_runtime_partial ok
test_stackinit: small_hole_runtime_all ok
test_stackinit: big_hole_runtime_all ok
test_stackinit: u8 FAIL (uninit bytes: 1)
test_stackinit: u16 FAIL (uninit bytes: 2)
test_stackinit: u32 FAIL (uninit bytes: 4)
test_stackinit: u64 FAIL (uninit bytes: 8)
test_stackinit: char_array FAIL (uninit bytes: 16)
test_stackinit: small_hole ok
test_stackinit: big_hole ok
test_stackinit: user ok
test_stackinit: failures: 5

CONFIG_GCC_PLUGIN_STACKINIT=y
This initializes all variables, but has no special padding handling:

test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_runtime_partial ok
test_stackinit: big_hole_runtime_partial ok
test_stackinit: small_hole_runtime_all ok
test_stackinit: big_hole_runtime_all ok
test_stackinit: u8 ok
test_stackinit: u16 ok
test_stackinit: u32 ok
test_stackinit: u64 ok
test_stackinit: char_array ok
test_stackinit: small_hole ok
test_stackinit: big_hole ok
test_stackinit: user ok
test_stackinit: failures: 4

Signed-off-by: Kees Cook 
---
 lib/Kconfig.debug|   9 ++
 lib/Makefile |   1 +
 lib/test_stackinit.c | 327 +++
 3 files changed, 337 insertions(+)
 create mode 100644 lib/test_stackinit.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d4df5b24d75e..09788af9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2001,6 +2001,15 @@ config TEST_OBJAGG
 
  If unsure, say N.
 
+config TEST_STACKINIT
+   tristate "Test level of stack variabl

[ovs-dev] [PATCH 2/3] gcc-plugins: Introduce stackinit plugin

[Kees Cook]

This attempts to duplicate the proposed gcc option -finit-local-vars[1]
in an effort to implement the "always initialize local variables" kernel
development goal[2].

Enabling CONFIG_GCC_PLUGIN_STACKINIT should stop all "uninitialized
stack variable" flaws as long as they don't depend on being zero. :)

[1] https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00615.html
[2] 
https://lkml.kernel.org/r/CA+55aFykZL+cSBJjBBts7ebEFfyGPdMzTmLSxKnT_29=j94...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 scripts/Makefile.gcc-plugins   |  6 ++
 scripts/gcc-plugins/Kconfig|  9 +++
 scripts/gcc-plugins/gcc-common.h   | 11 +++-
 scripts/gcc-plugins/stackinit_plugin.c | 79 ++
 4 files changed, 102 insertions(+), 3 deletions(-)
 create mode 100644 scripts/gcc-plugins/stackinit_plugin.c

diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 35042d96cf5d..2483121d781c 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -12,6 +12,12 @@ export DISABLE_LATENT_ENTROPY_PLUGIN
 
 gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV) += sancov_plugin.so
 
+gcc-plugin-$(CONFIG_GCC_PLUGIN_STACKINIT)  += stackinit_plugin.so
+ifdef CONFIG_GCC_PLUGIN_STACKINIT
+DISABLE_STACKINIT_PLUGIN += -fplugin-arg-stackinit_plugin-disable
+endif
+export DISABLE_STACKINIT_PLUGIN
+
 gcc-plugin-$(CONFIG_GCC_PLUGIN_STRUCTLEAK) += structleak_plugin.so
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE)  \
+= -fplugin-arg-structleak_plugin-verbose
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index d45f7f36b859..b117fe83f1d3 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -66,6 +66,15 @@ config GCC_PLUGIN_LATENT_ENTROPY
   * https://grsecurity.net/
   * https://pax.grsecurity.net/
 
+config GCC_PLUGIN_STACKINIT
+   bool "Initialize all stack variables to zero by default"
+   depends on GCC_PLUGINS
+   depends on !GCC_PLUGIN_STRUCTLEAK
+   help
+ This plugin zero-initializes all stack variables. This is more
+ comprehensive than GCC_PLUGIN_STRUCTLEAK, and attempts to
+ duplicate the proposed -finit-local-vars gcc build flag.
+
 config GCC_PLUGIN_STRUCTLEAK
bool "Force initialization of variables containing userspace addresses"
# Currently STRUCTLEAK inserts initialization out of live scope of
diff --git a/scripts/gcc-plugins/gcc-common.h b/scripts/gcc-plugins/gcc-common.h
index 552d5efd7cb7..f690b4deeabd 100644
--- a/scripts/gcc-plugins/gcc-common.h
+++ b/scripts/gcc-plugins/gcc-common.h
@@ -76,6 +76,14 @@
 #include "c-common.h"
 #endif
 
+#if BUILDING_GCC_VERSION > 4005
+#include "c-tree.h"
+#else
+/* should come from c-tree.h if only it were installed for gcc 4.5... */
+#define C_TYPE_FIELDS_READONLY(TYPE) TREE_LANG_FLAG_1(TYPE)
+extern bool global_bindings_p (void);
+#endif
+
 #if BUILDING_GCC_VERSION <= 4008
 #include "tree-flow.h"
 #else
@@ -158,9 +166,6 @@ void dump_gimple_stmt(pretty_printer *, gimple, int, int);
 #define TYPE_NAME_POINTER(node) IDENTIFIER_POINTER(TYPE_NAME(node))
 #define TYPE_NAME_LENGTH(node) IDENTIFIER_LENGTH(TYPE_NAME(node))
 
-/* should come from c-tree.h if only it were installed for gcc 4.5... */
-#define C_TYPE_FIELDS_READONLY(TYPE) TREE_LANG_FLAG_1(TYPE)
-
 static inline tree build_const_char_string(int len, const char *str)
 {
tree cstr, elem, index, type;
diff --git a/scripts/gcc-plugins/stackinit_plugin.c 
b/scripts/gcc-plugins/stackinit_plugin.c
new file mode 100644
index ..41055cd7098e
--- /dev/null
+++ b/scripts/gcc-plugins/stackinit_plugin.c
@@ -0,0 +1,79 @@
+/* SPDX-License: GPLv2 */
+/*
+ * This will zero-initialize local stack variables. (Though structure
+ * padding may remain uninitialized in certain cases.)
+ *
+ * Implements Florian Weimer's "-finit-local-vars" gcc patch as a plugin:
+ * https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00615.html
+ *
+ * Plugin skeleton code thanks to PaX Team.
+ *
+ * Options:
+ * -fplugin-arg-stackinit_plugin-disable
+ */
+
+#include "gcc-common.h"
+
+__visible int plugin_is_GPL_compatible;
+
+static struct plugin_info stackinit_plugin_info = {
+   .version= "20190122",
+   .help   = "disable\tdo not activate plugin\n",
+};
+
+static void finish_decl(void *event_data, void *data)
+{
+   tree decl = (tree)event_data;
+   tree type;
+
+   if (TREE_CODE (decl) != VAR_DECL)
+   return;
+
+   if (DECL_EXTERNAL (decl))
+   return;
+
+   if (DECL_INITIAL (decl) != NULL_TREE)
+   return;
+
+   if (global_bindings_p ())
+   return;
+
+   type = TREE_TYPE (decl);
+   if (AGGREGATE_TYPE_P (type))
+   DECL_INITIAL (decl) = build_constructor (type, NULL);
+   else
+   DECL_INITIAL (decl) = fold_convert (type, integer_zero_node);
+}
+
+__visible int plugin_init(struct plugin_name_args *plu

[ovs-dev] [PATCH 1/3] treewide: Lift switch variables out of switches

[Kees Cook]

Variables declared in a switch statement before any case statements
cannot be initialized, so move all instances out of the switches.
After this, future always-initialized stack variables will work
and not throw warnings like this:

fs/fcntl.c: In function ‘send_sigio_to_task’:
fs/fcntl.c:738:13: warning: statement will never be executed 
[-Wswitch-unreachable]
   siginfo_t si;
 ^~

Signed-off-by: Kees Cook 
---
 arch/x86/xen/enlighten_pv.c   |  7 ---
 drivers/char/pcmcia/cm4000_cs.c   |  2 +-
 drivers/char/ppdev.c  | 20 ---
 drivers/gpu/drm/drm_edid.c|  4 ++--
 drivers/gpu/drm/i915/intel_display.c  |  2 +-
 drivers/gpu/drm/i915/intel_pm.c   |  4 ++--
 drivers/net/ethernet/intel/e1000/e1000_main.c |  3 ++-
 drivers/tty/n_tty.c   |  3 +--
 drivers/usb/gadget/udc/net2280.c  |  5 ++---
 fs/fcntl.c|  3 ++-
 mm/shmem.c|  5 +++--
 net/core/skbuff.c |  4 ++--
 net/ipv6/ip6_gre.c|  4 ++--
 net/ipv6/ip6_tunnel.c |  4 ++--
 net/openvswitch/flow_netlink.c|  7 +++
 security/tomoyo/common.c  |  3 ++-
 security/tomoyo/condition.c   |  7 ---
 security/tomoyo/util.c|  4 ++--
 18 files changed, 45 insertions(+), 46 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index c54a493e139a..a79d4b548a08 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -907,14 +907,15 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
 static int xen_write_msr_safe(unsigned int msr, unsigned low, unsigned high)
 {
int ret;
+#ifdef CONFIG_X86_64
+   unsigned which;
+   u64 base;
+#endif
 
ret = 0;
 
switch (msr) {
 #ifdef CONFIG_X86_64
-   unsigned which;
-   u64 base;
-
case MSR_FS_BASE:   which = SEGBASE_FS; goto set;
case MSR_KERNEL_GS_BASE:which = SEGBASE_GS_USER; goto set;
case MSR_GS_BASE:   which = SEGBASE_GS_KERNEL; goto set;
diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
index 7a4eb86aedac..7211dc0e6f4f 100644
--- a/drivers/char/pcmcia/cm4000_cs.c
+++ b/drivers/char/pcmcia/cm4000_cs.c
@@ -663,6 +663,7 @@ static void monitor_card(struct timer_list *t)
 {
struct cm4000_dev *dev = from_timer(dev, t, timer);
unsigned int iobase = dev->p_dev->resource[0]->start;
+   unsigned char flags0;
unsigned short s;
struct ptsreq ptsreq;
int i, atrc;
@@ -731,7 +732,6 @@ static void monitor_card(struct timer_list *t)
}
 
switch (dev->mstate) {
-   unsigned char flags0;
case M_CARDOFF:
DEBUGP(4, dev, "M_CARDOFF\n");
flags0 = inb(REG_FLAGS0(iobase));
diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index 1ae77b41050a..d77c97e4f996 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -359,14 +359,19 @@ static int pp_do_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
struct pp_struct *pp = file->private_data;
struct parport *port;
void __user *argp = (void __user *)arg;
+   struct ieee1284_info *info;
+   unsigned char reg;
+   unsigned char mask;
+   int mode;
+   s32 time32[2];
+   s64 time64[2];
+   struct timespec64 ts;
+   int ret;
 
/* First handle the cases that don't take arguments. */
switch (cmd) {
case PPCLAIM:
{
-   struct ieee1284_info *info;
-   int ret;
-
if (pp->flags & PP_CLAIMED) {
dev_dbg(&pp->pdev->dev, "you've already got it!\n");
return -EINVAL;
@@ -517,15 +522,6 @@ static int pp_do_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
 
port = pp->pdev->port;
switch (cmd) {
-   struct ieee1284_info *info;
-   unsigned char reg;
-   unsigned char mask;
-   int mode;
-   s32 time32[2];
-   s64 time64[2];
-   struct timespec64 ts;
-   int ret;
-
case PPRSTATUS:
reg = parport_read_status(port);
if (copy_to_user(argp, ®, sizeof(reg)))
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b506e3622b08..8f93956c1628 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3942,12 +3942,12 @@ static void drm_edid_to_eld(struct drm_connector 
*connector, struct edid *edid)
}
 
for_each_cea_db(cea, i, start, end) {
+   int sad_count;
+
db = &cea[i];

Re: [ovs-dev] [PATCH] Fix test 'testing ovn -- IP packet buffering' on Windows

[Alin Gabriel Serdean]

> On 23 Jan 2019, at 12:58, Numan Siddique  wrote:
> 
> 
> 
> On Wed, Jan 23, 2019 at 4:14 PM Alin Gabriel Serdean  > wrote:
> The test fails on Windows because of:
> <--cut-->
> ovn-nbctl: sw0: invalid network address: 2001;1\64
> ovn-nbctl: sw1: invalid network address: 2002;1\64
> <--cut-->
> 
> This is due to the fact msys converts '::1' into ';1'.
> 
> Use IPv6 long form instead of its short variant.
> 
> Signed-off-by: Alin Gabriel Serdean  >
> 
> Acked-by: Numan Siddique mailto:nusid...@redhat.com>>
> 
No worries :)
I’m trying to bring up Cirrus CI for Windows which should give some
more visibility.
> We generally tend to use the short form and we don't even realize it
> breaks windows.  Probably checkpatch can check it and flag an error.
> I will give it a try to add this check in checkpatch utility.
That would be a nice addition. Thanks for giving it a try!
> 
> Thanks
> Numan
> 
> 


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

Re: [ovs-dev] [PATCH 1/3] treewide: Lift switch variables out of switches

[Greg KH]

On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
> 
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed 
> [-Wswitch-unreachable]
>siginfo_t si;
>  ^~

That's a pain, so this means we can't have any new variables in { }
scope except for at the top of a function?

That's going to be a hard thing to keep from happening over time, as
this is valid C :(

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

Re: [ovs-dev] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jann Horn via dev]

On Wed, Jan 23, 2019 at 1:04 PM Greg KH  wrote:
> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> > Variables declared in a switch statement before any case statements
> > cannot be initialized, so move all instances out of the switches.
> > After this, future always-initialized stack variables will work
> > and not throw warnings like this:
> >
> > fs/fcntl.c: In function ‘send_sigio_to_task’:
> > fs/fcntl.c:738:13: warning: statement will never be executed 
> > [-Wswitch-unreachable]
> >siginfo_t si;
> >  ^~
>
> That's a pain, so this means we can't have any new variables in { }
> scope except for at the top of a function?

AFAICS this only applies to switch statements (because they jump to a
case and don't execute stuff at the start of the block), not blocks
after if/while/... .

> That's going to be a hard thing to keep from happening over time, as
> this is valid C :(
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] treewide: Lift switch variables out of switches

[Ard Biesheuvel]

On Wed, 23 Jan 2019 at 13:09, Jann Horn  wrote:
>
> On Wed, Jan 23, 2019 at 1:04 PM Greg KH  wrote:
> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> > > Variables declared in a switch statement before any case statements
> > > cannot be initialized, so move all instances out of the switches.
> > > After this, future always-initialized stack variables will work
> > > and not throw warnings like this:
> > >
> > > fs/fcntl.c: In function ‘send_sigio_to_task’:
> > > fs/fcntl.c:738:13: warning: statement will never be executed 
> > > [-Wswitch-unreachable]
> > >siginfo_t si;
> > >  ^~
> >
> > That's a pain, so this means we can't have any new variables in { }
> > scope except for at the top of a function?
>
> AFAICS this only applies to switch statements (because they jump to a
> case and don't execute stuff at the start of the block), not blocks
> after if/while/... .
>

I guess that means it may apply to other cases where you do a 'goto'
into the middle of a for() loop, for instance (at the first
iteration), which is also a valid pattern.

Is there any way to tag these assignments so the diagnostic disregards them?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpdk: Limit DPDK memory usage.

[Aaron Conole]

Ilya Maximets  writes:

> Aaron, what do you think about supporting tags like 'Based-on' in ovsrobot ?
> For example, patchew in QEMU parses 'Based-on: ' tags.
> Maybe we can use patchwork ids or something similar.

I think there's a way to do it without a tag in the commit message.  I
don't like having a tag for this sort of information in the commit
message itself since it isn't useful to preserve in the commit history
(and can serve to clutter things up).

Currently, we use [PATCH branch-XXX] to imply that the series needs to
be based on a specific branch.  The ovsrobot automatically creates
branches based on patch series.  Maybe it makes sense to enhance that so
that some subject metadata marker (series-[0-9]+) would automatically
check out the series and then apply the patches.  This also lets a
reviewer know at a glance that the patches are destined to be applied
after a specific series which is already posted.  One downside is that
it's more subject line data.  I can't think of others at the moment, but
I'm sure there are some.  Since the robot doesn't currently clean up
branches, it's entirely possible to even base a series on a severely old
branch (so that should get fixed at some point).

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

Re: [ovs-dev] [PATCH V2 0/2] Do not rewrite fields with the same values as

[Eli Britstein]

On 1/23/2019 2:14 AM, Ben Pfaff wrote:
> On Fri, Jan 11, 2019 at 08:28:57AM +0200, Eli Britstein wrote:
>> Hi
>>
>> This patch set avoids unnecessary rewrite actions to fields with the
>> same values as matched on.
>>
>> Patch 1 is a pre-step by defining ovs key structs using macros
>>
>> Patch 2 avoids the unnecessary rewrites and adapts the tests accordingly
> I like the goal of the patch, but it makes openvswitch.h unreadable and
> completely different from Linux upstream.  One of the goals of that
> "sed" script is to avoid having it be different from upstream.  I think

unreadable - well, it's a point of view, but i don't think that's alone 
is a justified argument not to change

completely different - that's correct. why don't we just push the same 
change to the Linux upstream to keep it the same?

> that we should come up with a way to do this that avoids the problem.
> Offhand, I can think of a couple of ways:
>
>  - Write a script that parses openvswitch.h in a simple way and
>extracts the needed information into a file that can be
>#included.
I think that would be by far more complicated and unreadable solution
>
>  - Just list the relevant field names in odp-util.c in the
>relevant functions.  From field names, it's easy to get
>offsets (with offsetof) and sizes (MEMBER_SIZEOF).  It might
>be problematic to keep these in sync if the structures
>changed, but they will not because the Linux ABI is stable.

though it is stable, if it happens that someone needs to change it, with 
this solution we will have to change in more than one place the same 
change. I don't think this is acceptable.

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

Re: [ovs-dev] [PATCH 1/3] treewide: Lift switch variables out of switches

[William Kucharski]

> On Jan 23, 2019, at 5:09 AM, Jann Horn  wrote:
> 
> AFAICS this only applies to switch statements (because they jump to a
> case and don't execute stuff at the start of the block), not blocks
> after if/while/... .

It bothers me that we are going out of our way to deprecate valid C constructs
in favor of placing the declarations elsewhere.

As current compiler warnings would catch any reference before initialization
usage anyway, it seems like we are letting a compiler warning rather than the
language standard dictate syntax.

Certainly if we want to make it a best practice coding style issue we can, and
then an appropriate note explaining why should be added to
Documentation/process/coding-style.rst.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev,v6] Improved Packet Drop Statistics in OVS

[Anju Thomas]

Hi Ilya ,
Some queries w.r.t your comments .

Regards
Anju

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Friday, January 18, 2019 5:33 PM
To: Anju Thomas ; d...@openvswitch.org
Cc: Keshav Gupta ; Stokes, Ian ; 
Ben Pfaff 
Subject: Re: [ovs-dev,v6] Improved Packet Drop Statistics in OVS

Hi.
Thanks for working on this.
Some general comments:

1. This patch consists of two separate features:
   - Reporting the drop reason in flow dumps.
   - Counters for different drop types.
   I still think that there should be patch-set of two separate patches.
   This will simplify the review and allow to apply them separately/speed
   up accepting.

2. There are some issues with the patch formatting. The commit message is
   indented like in the output of 'git show' command.  So, it's double
   shifted after applying in git. Please remove the additional indentation.
   You probably should start using 'git format-patch'.

   Second issue:

   Applying: Improved Packet Drop Statistics in OVS
   .git/rebase-apply/patch:415: new blank line at EOF.
   +
   warning: 1 line adds whitespace errors.

   One more thing is that for better readability it's a common practice to
   limit the width of lines in commit-message to 72 characters. For example,
   you may strip not important fields of the dump-flows output. Like this:

   recirc_id(0),in_port(5),<...>, actions:drop:recursion too deep


Other comments inline.

 I use 
a)git format-patch -1 -s 
b)  ./utilities/checkpatch.py -1 

Are these the correct parameters that I should be using ?

Best regards, Ilya Maximets. 

On 17.01.2019 7:49, Anju Thomas wrote:
> Currently OVS maintains explicit packet drop/error counters only on port
> level. Packets that are dropped as part of normal OpenFlow processing are
> counted in flow stats of “drop” flows or as table misses in table stats.
> These can only be interpreted by controllers that know the semantics of
> the configured OpenFlow pipeline. Without that knowledge, it is impossible
> for an OVS user to obtain e.g. the total number of packets dropped due to
> OpenFlow rules.
> 
> Furthermore, there are numerous other reasons for which packets can be
> dropped by OVS slow path that are not related to the OpenFlow pipeline.
> The generated datapath flow entries include a drop action to avoid further
> expensive upcalls to the slow path, but subsequent packets dropped by the
> datapath are not accounted anywhere.
> 
> Finally, the datapath itself drops packets in certain error situations.
> Also, these drops are today not accounted for.
> 
> This makes it difficult for OVS users to monitor packet drop in an OVS
> instance and to alert a management system in case of a unexpected increase
> of such drops. Also OVS trouble-shooters face difficulties in analysing
> packet drops.
> 
> With this patch we implement following changes to address the issues
> mentioned above.
> 
> 1. Account and categorize all the packet drops in OVS.
> 2. Account & classify “drop” action packet drops according to the drop
>reason.
> 3. Identify and account all the silent packet drop scenarios.
> 4. Display these drops in ovs-appctl coverage/show
> 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows
>to print drop reason along with drop action
> 
> A detailed presentation on this was presented at OvS conference 2017 and
> link for the corresponding presentation is available at:
> 
> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
> 
> Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows 
> displaying drop reason along with drop action.
> 
>  The idea is to use the coverage infrastructure to maintain the drops
> 
> $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev
> flow-dump from pmd on cpu core: 0
> 
> recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
>  packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep
> 
> $ ovs-appctl dpif/dump-flows br-int
> 
> recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
>  packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep
> recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), 
> packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep
> 
>In subsequent commits, we are planning to see if we can use this 
> infrastructure to create a
>wrapper to clear and display counters as well.
> 
> Co-authored-by: Rohith Basavaraja 
> Co-authored-by: Keshav Gupta 
> Signed-off-by: Anju Thomas 
> Signed-off-by: Rohith Basavaraja 
> Signed-off-by: Keshav Gupta 
> Signed-off-by: Anju Thomas 
> Signed-off-by: Rohith Basavaraja 
> Signed-off-by: Keshav Gupta 
> ---
>  datapath/linux/compat/include/linux/openvswitch.h |  51 ++
>  lib/coverage.c 

Re: [ovs-dev] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Greg KH  wrote:
> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>> Variables declared in a switch statement before any case statements
>> cannot be initialized, so move all instances out of the switches.
>> After this, future always-initialized stack variables will work
>> and not throw warnings like this:
>> 
>> fs/fcntl.c: In function ‘send_sigio_to_task’:
>> fs/fcntl.c:738:13: warning: statement will never be executed 
>> [-Wswitch-unreachable]
>>siginfo_t si;
>>  ^~
>
> That's a pain, so this means we can't have any new variables in { }
> scope except for at the top of a function?
>
> That's going to be a hard thing to keep from happening over time, as
> this is valid C :(

Not all valid C is meant to be used! ;)

Anyway, I think you're mistaking the limitation to arbitrary blocks
while it's only about the switch block IIUC.

Can't have:

switch (i) {
int j;
case 0:
/* ... */
}

because it can't be turned into:

switch (i) {
int j = 0; /* not valid C */
case 0:
/* ... */
}

but can have e.g.:

switch (i) {
case 0:
{
int j = 0;
/* ... */
}
}

I think Kees' approach of moving such variable declarations to the
enclosing block scope is better than adding another nesting block.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Jani Nikula  wrote:
> On Wed, 23 Jan 2019, Greg KH  wrote:
>> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>>> Variables declared in a switch statement before any case statements
>>> cannot be initialized, so move all instances out of the switches.
>>> After this, future always-initialized stack variables will work
>>> and not throw warnings like this:
>>> 
>>> fs/fcntl.c: In function ‘send_sigio_to_task’:
>>> fs/fcntl.c:738:13: warning: statement will never be executed 
>>> [-Wswitch-unreachable]
>>>siginfo_t si;
>>>  ^~
>>
>> That's a pain, so this means we can't have any new variables in { }
>> scope except for at the top of a function?
>>
>> That's going to be a hard thing to keep from happening over time, as
>> this is valid C :(
>
> Not all valid C is meant to be used! ;)
>
> Anyway, I think you're mistaking the limitation to arbitrary blocks
> while it's only about the switch block IIUC.
>
> Can't have:
>
>   switch (i) {
>   int j;
>   case 0:
>   /* ... */
>   }
>
> because it can't be turned into:
>
>   switch (i) {
>   int j = 0; /* not valid C */
>   case 0:
>   /* ... */
>   }
>
> but can have e.g.:
>
>   switch (i) {
>   case 0:
>   {
>   int j = 0;
>   /* ... */
>   }
>   }
>
> I think Kees' approach of moving such variable declarations to the
> enclosing block scope is better than adding another nesting block.

PS. The patch is

Reviewed-by: Jani Nikula 

and the drivers/gpu/drm/i915/* parts are

Acked-by: Jani Nikula 

for merging via whichever tree is appropriate. (There'll be minor
conflicts with in-flight work in our -next tree, but no biggie.)


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev,v6] Improved Packet Drop Statistics in OVS

[Ilya Maximets]

On 23.01.2019 16:53, Anju Thomas wrote:
> Hi Ilya ,
> Some queries w.r.t your comments .
> 
> Regards
> Anju
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
> Sent: Friday, January 18, 2019 5:33 PM
> To: Anju Thomas ; d...@openvswitch.org
> Cc: Keshav Gupta ; Stokes, Ian ; 
> Ben Pfaff 
> Subject: Re: [ovs-dev,v6] Improved Packet Drop Statistics in OVS
> 
> Hi.
> Thanks for working on this.
> Some general comments:
> 
> 1. This patch consists of two separate features:
>- Reporting the drop reason in flow dumps.
>- Counters for different drop types.
>I still think that there should be patch-set of two separate patches.
>This will simplify the review and allow to apply them separately/speed
>up accepting.
> 
> 2. There are some issues with the patch formatting. The commit message is
>indented like in the output of 'git show' command.  So, it's double
>shifted after applying in git. Please remove the additional indentation.
>You probably should start using 'git format-patch'.
> 
>Second issue:
> 
>Applying: Improved Packet Drop Statistics in OVS
>.git/rebase-apply/patch:415: new blank line at EOF.
>+
>warning: 1 line adds whitespace errors.
> 
>One more thing is that for better readability it's a common practice to
>limit the width of lines in commit-message to 72 characters. For example,
>you may strip not important fields of the dump-flows output. Like this:
> 
>recirc_id(0),in_port(5),<...>, actions:drop:recursion too deep
> 
> 
> Other comments inline.
> 
>  I use 
> a)git format-patch -1 -s 
> b)  ./utilities/checkpatch.py -1 
> 
> Are these the correct parameters that I should be using ?

Commands looks fine. If the patch looks same in your git repo, I'd suggest you
to re-format the patch by hands.

Also, I just spotted that you have a lot of duplicated sign-offs in the patch.

> 
> Best regards, Ilya Maximets. 
> 
> On 17.01.2019 7:49, Anju Thomas wrote:
>> Currently OVS maintains explicit packet drop/error counters only on port
>> level. Packets that are dropped as part of normal OpenFlow processing are
>> counted in flow stats of “drop” flows or as table misses in table stats.
>> These can only be interpreted by controllers that know the semantics of
>> the configured OpenFlow pipeline. Without that knowledge, it is 
>> impossible
>> for an OVS user to obtain e.g. the total number of packets dropped due to
>> OpenFlow rules.
>>
>> Furthermore, there are numerous other reasons for which packets can be
>> dropped by OVS slow path that are not related to the OpenFlow pipeline.
>> The generated datapath flow entries include a drop action to avoid 
>> further
>> expensive upcalls to the slow path, but subsequent packets dropped by the
>> datapath are not accounted anywhere.
>>
>> Finally, the datapath itself drops packets in certain error situations.
>> Also, these drops are today not accounted for.
>>
>> This makes it difficult for OVS users to monitor packet drop in an OVS
>> instance and to alert a management system in case of a unexpected 
>> increase
>> of such drops. Also OVS trouble-shooters face difficulties in analysing
>> packet drops.
>>
>> With this patch we implement following changes to address the issues
>> mentioned above.
>>
>> 1. Account and categorize all the packet drops in OVS.
>> 2. Account & classify “drop” action packet drops according to the drop
>>reason.
>> 3. Identify and account all the silent packet drop scenarios.
>> 4. Display these drops in ovs-appctl coverage/show
>> 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows
>>to print drop reason along with drop action
>>
>> A detailed presentation on this was presented at OvS conference 2017 and
>> link for the corresponding presentation is available at:
>> 
>> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
>>
>> Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows 
>> displaying drop reason along with drop action.
>>
>>  The idea is to use the coverage infrastructure to maintain the drops
>>
>> $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev
>> flow-dump from pmd on cpu core: 0
>> 
>> recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
>>  packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep
>>
>> $ ovs-appctl dpif/dump-flows br-int
>> 
>> recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
>>  packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep
>> recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), 
>> packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep
>>
>>In subsequent commits, we are planning to see if we can use this 
>> infrastructure to create a
>>wrapper

Re: [ovs-dev] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Edwin Zimmerman]

On Wed, 23 Jan 2019, Jani Nikula  wrote:
> On Wed, 23 Jan 2019, Greg KH  wrote:
> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> >> Variables declared in a switch statement before any case statements
> >> cannot be initialized, so move all instances out of the switches.
> >> After this, future always-initialized stack variables will work
> >> and not throw warnings like this:
> >>
> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
> >> fs/fcntl.c:738:13: warning: statement will never be executed 
> >> [-Wswitch-unreachable]
> >>siginfo_t si;
> >>  ^~
> >
> > That's a pain, so this means we can't have any new variables in { }
> > scope except for at the top of a function?
> >
> > That's going to be a hard thing to keep from happening over time, as
> > this is valid C :(
> 
> Not all valid C is meant to be used! ;)

Very true.  The other thing to keep in mind is the burden of enforcing a 
prohibition on a valid C construct like this.  
It seems to me that patch reviewers and maintainers have enough to do without 
forcing them to watch for variable
declarations in switch statements.  Automating this prohibition, should it be 
accepted, seems like a good idea to me.

-Edwin Zimmerman

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

Re: [ovs-dev] [PATCH] ovn: Add pkg-config support via libovn.pc.

[Aaron Conole]

Ben Pfaff  writes:

> From: 竹內宏輝 Hiroki Takeuchi 
>
> Submitted-at: https://github.com/openvswitch/ovs/pull/270
> Signed-off-by: Hiroki Takeuchi 
> ---
> I'm sending this patch as a squashed version of what was submitted at
> Github.  I know that pkg-config is somewhat controversial, so I'd like to
> hear what the denizens of ovs-dev have to say about including a libovn.pc.

I like pkg-config, as a general rule.  I wasn't aware that it was
controversial, but I'm definitely not plugged into the latest trends in
library development.

> Thanks,
>
> Ben.
>
>  configure.ac |  1 +
>  ovn/lib/automake.mk  |  3 +++
>  ovn/lib/libovn.pc.in | 11 +++
>  3 files changed, 15 insertions(+)
>  create mode 100644 ovn/lib/libovn.pc.in
>
> diff --git a/configure.ac b/configure.ac
> index 505e3d041e93..473454265532 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -201,6 +201,7 @@ AC_CONFIG_FILES(lib/libopenvswitch.pc)
>  AC_CONFIG_FILES(lib/libsflow.pc)
>  AC_CONFIG_FILES(ofproto/libofproto.pc)
>  AC_CONFIG_FILES(ovsdb/libovsdb.pc)
> +AC_CONFIG_FILES(ovn/lib/libovn.pc)
>  AC_CONFIG_FILES(include/openvswitch/version.h)
>  
>  dnl This makes sure that include/openflow gets created in the build 
> directory.
> diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk
> index 6178fc2d5aa4..cb216f626293 100644
> --- a/ovn/lib/automake.mk
> +++ b/ovn/lib/automake.mk
> @@ -24,6 +24,9 @@ nodist_ovn_lib_libovn_la_SOURCES = \
>   ovn/lib/ovn-sb-idl.c \
>   ovn/lib/ovn-sb-idl.h
>  
> +pkgconfig_DATA += \
> + ovn/lib/libovn.pc

Is it intended that the OVN library will be used by anything other than
OVN?
If so, it's probably also good to enhance the ABI version
information to work similarly to the way the openvswitch library is
versioned.
If not, maybe it's not a good idea to have a pkg-config
entry for the library (since it would encourage non-ovn utilities to
link to it without any kind of ABI contract).

I don't have any idea whether it would be useful to provide access to
the OVN library to other projects.

> +
>  # ovn-sb IDL
>  OVSIDL_BUILT += \
>   ovn/lib/ovn-sb-idl.c \
> diff --git a/ovn/lib/libovn.pc.in b/ovn/lib/libovn.pc.in
> new file mode 100644
> index ..6d9b22be6568
> --- /dev/null
> +++ b/ovn/lib/libovn.pc.in
> @@ -0,0 +1,11 @@
> +prefix=@prefix@
> +exec_prefix=@exec_prefix@
> +libdir=@libdir@
> +includedir=@includedir@
> +
> +Name: libovn
> +Description: Open Virtual Network for Open vSwitch

I think the Description here should include the word 'library' - but
it's just a nit.

> +Version: @VERSION@
> +Libs: -L${libdir} -lovn
> +Libs.private: @LIBS@
> +Cflags: -I${includedir}/ovn

I believe this file should also have a Requires: stanza for
libopenvswitch (since any application linking to the ovn library will
need to link to the openvswitch library as well).

At the moment, it could be very restrictive:

  Requires: libopenvswitch = @VERSION@

But it will probably need to be relaxed in the future (to some minimum
version).  I admit I don't know enough about the ovn library to make an
informed suggestion here.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Fix test 'testing ovn -- IP packet buffering' on Windows

[Ben Pfaff]

On Wed, Jan 23, 2019 at 12:41:10PM +0200, Alin Gabriel Serdean wrote:
> The test fails on Windows because of:
> <--cut-->
> ovn-nbctl: sw0: invalid network address: 2001;1\64
> ovn-nbctl: sw1: invalid network address: 2002;1\64
> <--cut-->
> 
> This is due to the fact msys converts '::1' into ';1'.
> 
> Use IPv6 long form instead of its short variant.
> 
> Signed-off-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Edwin Zimmerman  wrote:
> On Wed, 23 Jan 2019, Jani Nikula  wrote:
>> On Wed, 23 Jan 2019, Greg KH  wrote:
>> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>> >> Variables declared in a switch statement before any case statements
>> >> cannot be initialized, so move all instances out of the switches.
>> >> After this, future always-initialized stack variables will work
>> >> and not throw warnings like this:
>> >>
>> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
>> >> fs/fcntl.c:738:13: warning: statement will never be executed 
>> >> [-Wswitch-unreachable]
>> >>siginfo_t si;
>> >>  ^~
>> >
>> > That's a pain, so this means we can't have any new variables in { }
>> > scope except for at the top of a function?
>> >
>> > That's going to be a hard thing to keep from happening over time, as
>> > this is valid C :(
>> 
>> Not all valid C is meant to be used! ;)
>
> Very true.  The other thing to keep in mind is the burden of enforcing
> a prohibition on a valid C construct like this.  It seems to me that
> patch reviewers and maintainers have enough to do without forcing them
> to watch for variable declarations in switch statements.  Automating
> this prohibition, should it be accepted, seems like a good idea to me.

Considering that the treewide diffstat to fix this is:

 18 files changed, 45 insertions(+), 46 deletions(-)

and using the gcc plugin in question will trigger the switch-unreachable
warning, I think we're good. There'll probably be the occasional
declarations that pass through, and will get fixed afterwards.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [Intel-wired-lan] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jeff Kirsher]

On Wed, 2019-01-23 at 03:03 -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
> 
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed [-
> Wswitch-unreachable]
>siginfo_t si;
>  ^~
> 
> Signed-off-by: Kees Cook 

Acked-by: Jeff Kirsher 

For the e1000 changes.

> ---
>  arch/x86/xen/enlighten_pv.c   |  7 ---
>  drivers/char/pcmcia/cm4000_cs.c   |  2 +-
>  drivers/char/ppdev.c  | 20 -
> --
>  drivers/gpu/drm/drm_edid.c|  4 ++--
>  drivers/gpu/drm/i915/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c   |  4 ++--
>  drivers/net/ethernet/intel/e1000/e1000_main.c |  3 ++-
>  drivers/tty/n_tty.c   |  3 +--
>  drivers/usb/gadget/udc/net2280.c  |  5 ++---
>  fs/fcntl.c|  3 ++-
>  mm/shmem.c|  5 +++--
>  net/core/skbuff.c |  4 ++--
>  net/ipv6/ip6_gre.c|  4 ++--
>  net/ipv6/ip6_tunnel.c |  4 ++--
>  net/openvswitch/flow_netlink.c|  7 +++
>  security/tomoyo/common.c  |  3 ++-
>  security/tomoyo/condition.c   |  7 ---
>  security/tomoyo/util.c|  4 ++--
>  18 files changed, 45 insertions(+), 46 deletions(-)

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

[ovs-dev] [PATCH v3] Enhancing ICMPv6 support

[Vishal Deep Ajmera]

This patch adds support for match and set ICMPv6
"reserved" and "nd options type" fields.

v1->v2: Fixed compiler and sparse warnings.
v2->v3: Updated NEWS, simplified miniflow_extract for ICMPv6,
updated usage for tcp_flags and igmpgroup_ipv4.

Vishal Deep Ajmera (1):
  Support for match & set ICMPv6 reserved and options type fields

 NEWS  |   2 +
 build-aux/extract-ofp-fields  |   1 +
 datapath/linux/compat/include/linux/openvswitch.h |   8 ++
 include/openvswitch/flow.h|   6 +-
 include/openvswitch/match.h   |   3 +
 include/openvswitch/meta-flow.h   |  28 +
 lib/flow.c|  58 +--
 lib/match.c   |  21 
 lib/meta-flow.c   |  38 +++
 lib/meta-flow.xml |  12 +++
 lib/nx-match.c|   8 ++
 lib/odp-execute.c |  32 ++
 lib/odp-util.c| 120 +-
 lib/packets.c |  33 ++
 lib/packets.h |   3 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 tests/ofproto.at  |   4 +-
 17 files changed, 364 insertions(+), 14 deletions(-)

-- 
1.9.1

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

[ovs-dev] [PATCH v3] Support for match & set ICMPv6 reserved and options type fields

[Vishal Deep Ajmera]

Currently OVS supports all ARP protocol fields as OXM match fields to
implement the relevant ARP procedures for IPv4. This includes support
for matching copying and setting ARP fields. In IPv6 ARP has been
replaced by ICMPv6 neighbor discovery (ND) procedures, neighbor
advertisement and neighbor solicitation.

The support for ICMPv6 fields in OVS is not complete for the use cases
equivalent to ARP in IPv4. OVS lacks support for matching, copying and
setting the “ND option type” and “ND reserved” fields. Without these user
cannot implement all ICMPv6 ND procedures for IPv6 support.

This commit adds additional OXM fields to OVS for ICMPv6 “ND option type“
and ICMPv6 “ND reserved” using the OXM extension mechanism. This allows
support for parsing these fields from an ICMPv6 packet header and extending
the OpenFlow protocol with specifications for these new OXM fields for
matching, copying and setting.

Signed-off-by: Vishal Deep Ajmera 
Co-authored-by: Ashvin Lakshmikantha 
Signed-off-by: Ashvin Lakshmikantha 
---
 NEWS  |   2 +
 build-aux/extract-ofp-fields  |   1 +
 datapath/linux/compat/include/linux/openvswitch.h |   8 ++
 include/openvswitch/flow.h|   6 +-
 include/openvswitch/match.h   |   3 +
 include/openvswitch/meta-flow.h   |  28 +
 lib/flow.c|  58 +--
 lib/match.c   |  21 
 lib/meta-flow.c   |  38 +++
 lib/meta-flow.xml |  12 +++
 lib/nx-match.c|   8 ++
 lib/odp-execute.c |  32 ++
 lib/odp-util.c| 120 +-
 lib/packets.c |  33 ++
 lib/packets.h |   3 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 tests/ofproto.at  |   4 +-
 17 files changed, 364 insertions(+), 14 deletions(-)

diff --git a/NEWS b/NEWS
index 358c9b9..6ffac90 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,8 @@ Post-v2.10.0
  * Add option for simple round-robin based Rxq to PMD assignment.
It can be set with pmd-rxq-assign.
  * Add support for DPDK 18.11
+ * ICMPv6 ND enhancements: support for match and set ND options type
+   and reserved fields.
- Add 'symmetric_l3' hash function.
- OVS now honors 'updelay' and 'downdelay' for bonds with LACP configured.
- ovs-vswitchd:
diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 3592594..e159a1d 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -71,6 +71,7 @@ OXM_CLASSES = {"NXM_OF_":(0,  0x, 
'extension'),
"OXM_OF_":(0,  0x8000, 'standard'),
"OXM_OF_PKT_REG": (0,  0x8001, 'standard'),
"ONFOXM_ET_": (0x4f4e4600, 0x, 'standard'),
+   "ERICOXM_OF_":(0,  0x1000, 'extension'),
 
# This is the experimenter OXM class for Nicira, which is the
# one that OVS would be using instead of NXM_OF_ and NXM_NX_
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 9b087f1..d5aa09d 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -375,6 +375,7 @@ enum ovs_key_attr {
 #ifndef __KERNEL__
/* Only used within userspace data path. */
OVS_KEY_ATTR_PACKET_TYPE,  /* be32 packet type */
+   OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
 #endif
 
__OVS_KEY_ATTR_MAX
@@ -489,6 +490,13 @@ struct ovs_key_nd {
__u8nd_tll[ETH_ALEN];
 };
 
+#ifndef __KERNEL__
+struct ovs_key_nd_extensions {
+__be32  nd_reserved;
+__u8nd_options_type;
+};
+#endif
+
 #define OVS_CT_LABELS_LEN_32   4
 #define OVS_CT_LABELS_LEN  (OVS_CT_LABELS_LEN_32 * sizeof(__u32))
 struct ovs_key_ct_labels {
diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index 5d2cf09..57b6c92 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -144,7 +144,8 @@ struct flow {
 struct in6_addr nd_target;  /* IPv6 neighbor discovery (ND) target. */
 struct eth_addr arp_sha;/* ARP/ND source hardware address. */
 struct eth_addr arp_tha;/* ARP/ND target hardware address. */
-ovs_be16 tcp_flags; /* TCP flags. With L3 to avoid matching L4. */
+ovs_be16 tcp_flags; /* TCP flags/ICMPv6 ND options type.
+ * With L3 to avoid matching L4. */
 ovs_be16 pad2;  /* Pad to 64 bits. */
 struct ovs_key_nsh nsh; /* Network Service Header keys */
 
@@ -153,7 +154,8 @@ struct flow 

Re: [ovs-dev] [PATCH v2] Support for match & set ICMPv6 reserved and options type fields

[Vishal Deep Ajmera]

> -Original Message-
> From: Ben Pfaff 
> Sent: Saturday, January 19, 2019 12:04 AM
> To: Vishal Deep Ajmera 
> Cc: d...@openvswitch.org; Ashvin Lakshmikantha
> 
> Subject: Re: [ovs-dev] [PATCH v2] Support for match & set ICMPv6 reserved and
> options type fields
> 
> On Thu, Jan 17, 2019 at 11:14:26AM +, Vishal Deep Ajmera wrote:
> > Currently OVS supports all ARP protocol fields as OXM match fields to
> > implement the relevant ARP procedures for IPv4. This includes support
> > for matching copying and setting ARP fields. In IPv6 ARP has been
> > replaced by ICMPv6 neighbor discovery (ND) procedures, neighbor
> > advertisement and neighbor solicitation.
> >
> > The support for ICMPv6 fields in OVS is not complete for the use cases
> > equivalent to ARP in IPv4. OVS lacks support for matching, copying and
> > setting the “ND option type” and “ND reserved” fields. Without these
> > user cannot implement all ICMPv6 ND procedures for IPv6 support.
> >
> > This commit adds additional OXM fields to OVS for ICMPv6 “ND option
> > type“ and ICMPv6 “ND reserved” using the OXM extension mechanism. This
> > allows support for parsing these fields from an ICMPv6 packet header
> > and extending the OpenFlow protocol with specifications for these new
> > OXM fields for matching, copying and setting.
> >
> > Signed-off-by: Vishal Deep Ajmera 
> > Co-authored-by: Ashvin Lakshmikantha
> > 
> > Signed-off-by: Ashvin Lakshmikantha
> > 
> 
> Thanks for working to make OVS better!
> 
> It looks like miniflow_extract() calls data_pull() for the RSO flags field 
> without
> first checking to see whether the message is long enough.
> This is dangerous.
>
Thanks Ben for reviewing this patch.

I have refactored this code addressing your comments in V3.
 
> This cast should not be needed:
> 
> rso_flags = (uint32_t *) data_pull(&data,
>&size, sizeof(uint32_t));
> 
> The code for populating opt_type[0] and opt_type[1] into the miniflow is
> confusing.  It looks like only one of these can be nonzero, and if either one 
> is
> present then we put it in the same spot (in the tcp_flags)?  If that's so, 
> then why
> bother distinguishing them during parsing?  (And how should flow_compose_l4()
> know where to put them?
> Currently it doesn't bother with them at all.)
> 
Fixed in V3.

> The comments on struct flow should describe the new uses of tcp_flags and
> igmp_group_ip4.
> 
Fixed in V3.

> Please add an item to describe the new feature in NEWS.
>
Fixed in V3.
 
> Please add parsing tests to odp.at.
> 
I will add parsing test in subsequent patch V4.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Support for match & set ICMPv6 reserved and options type fields

[0-day Robot]

Bleep bloop.  Greetings Vishal Deep Ajmera, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 Support for match & set ICMPv6 reserved and options type 
fields
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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

[ovs-dev] Sueldos, Salarios y Prestaciones

[Ultimos Lugares]

Curso Fiscal- Webinar Interactivo - Miércoles 06 de Febrero



ACTUALIZACIÓN FISCAL 2019
Sueldos, salarios y prestaciones.

Estar al día en los recientes cambios fiscales derivados de la miscelánea 
fiscal y las reformas fiscales para el ejercicio 2019, para hacer los ajustes 
necesarios e indispensables en el tema del timbrado y manejo contable de las 
nóminas.

Ejes Temáticos:

• Guía de llenado y sus principales cambios publicados por el SAT.

• Conceptos y catálogo para nómina.

• Principales cambios para el ejercicio 2019.

Para mayor información, responder sobre este correo con la palabra Sueldos + 
los siguientes datos:

NOMBRE:

TELÉFONO:

EMPRESA: 

Llamanos al (045) 55 1554 6630
www.Innovalearn.mx


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

[ovs-dev] Excel - Webinar

[Campos, filtros, etiquetas y valores.]

Cursos escenciales - Webinar Interactivo – Viernes 22 de Febrero

Bases de datos y tablas dinámicas en Excel

Nuestro curso está diseñado para enseñarte a importar o construir bases de 
datos correctamente estructuradas para la creación
de tablas dinámicas y todas las herramientas que la herramienta nos ofrece para 
esta función.

Identificaremos y aplicaremos las funciones y utilidades del software para 
tareas propias del área financiera más comunes
en el ámbito empresarial. 

Ejes Temáticos:

• Estructura y composición de una base de datos.
• Asistente para creación de tablas dinámicas.
• Campos, filtros, etiquetas y valores.
• Diseño de tabla dinámica.
• Herramientas de tabla.

Para mayor información, responder sobre este correo con la palabra EXCEL + los 
siguientes datos:

NOMBRE:

TELÉFONO:

EMPRESA: 

Llamanos al (045) 55 1554 6630
www.Innovalearn.mx 


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

Re: [ovs-dev] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Kees Cook]

On Thu, Jan 24, 2019 at 4:44 AM Jani Nikula  wrote:
>
> On Wed, 23 Jan 2019, Edwin Zimmerman  wrote:
> > On Wed, 23 Jan 2019, Jani Nikula  wrote:
> >> On Wed, 23 Jan 2019, Greg KH  wrote:
> >> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> >> >> Variables declared in a switch statement before any case statements
> >> >> cannot be initialized, so move all instances out of the switches.
> >> >> After this, future always-initialized stack variables will work
> >> >> and not throw warnings like this:
> >> >>
> >> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
> >> >> fs/fcntl.c:738:13: warning: statement will never be executed 
> >> >> [-Wswitch-unreachable]
> >> >>siginfo_t si;
> >> >>  ^~
> >> >
> >> > That's a pain, so this means we can't have any new variables in { }
> >> > scope except for at the top of a function?

Just in case this wasn't clear: no, it's just the switch statement
before the first "case". I cannot imagine how bad it would be if we
couldn't have block-scoped variables! Heh. :)

> >> >
> >> > That's going to be a hard thing to keep from happening over time, as
> >> > this is valid C :(
> >>
> >> Not all valid C is meant to be used! ;)
> >
> > Very true.  The other thing to keep in mind is the burden of enforcing
> > a prohibition on a valid C construct like this.  It seems to me that
> > patch reviewers and maintainers have enough to do without forcing them
> > to watch for variable declarations in switch statements.  Automating
> > this prohibition, should it be accepted, seems like a good idea to me.
>
> Considering that the treewide diffstat to fix this is:
>
>  18 files changed, 45 insertions(+), 46 deletions(-)
>
> and using the gcc plugin in question will trigger the switch-unreachable
> warning, I think we're good. There'll probably be the occasional
> declarations that pass through, and will get fixed afterwards.

Yeah, that was my thinking as well: it's a rare use, and we get a
warning when it comes up.

Thanks!

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

Re: [ovs-dev] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Matthew Wilcox]

On Wed, Jan 23, 2019 at 04:17:30PM +0200, Jani Nikula wrote:
> Can't have:
> 
>   switch (i) {
>   int j;
>   case 0:
>   /* ... */
>   }
> 
> because it can't be turned into:
> 
>   switch (i) {
>   int j = 0; /* not valid C */
>   case 0:
>   /* ... */
>   }
> 
> but can have e.g.:
> 
>   switch (i) {
>   case 0:
>   {
>   int j = 0;
>   /* ... */
>   }
>   }
> 
> I think Kees' approach of moving such variable declarations to the
> enclosing block scope is better than adding another nesting block.

Another nesting level would be bad, but I think this is OK:

switch (i) {
case 0: {
int j = 0;
/* ... */
}
case 1: {
void *p = q;
/* ... */
}
}

I can imagine Kees' patch might have a bad effect on stack consumption,
unless GCC can be relied on to be smart enough to notice the
non-overlapping liveness of the vriables and use the same stack slots
for both.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [Branch 2.10 Patch] compat: Fixup ipv6 fragmentation on 4.9.135+ kernels

[Greg Rose]

Upstream commit 648700f76b03 ("inet: frags: use rhashtables...") changed
how ipv6 fragmentation is implemented.  This patch was backported to
the upstream stable 4.9.x kernel starting at 4.9.135.

This patch creates the compatibility layer changes required to both
compile and also operate correctly with ipv6 fragmentation on these
kernels. Check if the inet_frags 'rnd' field is present to key on
whether the upstream patch is present.  Also update Travis to the
latest 4.9 kernel release so that this patch is compile tested.

Passes Travis:
https://travis-ci.org/gvrose8192/ovs-experimental/builds/478033409

Cc: William Tu 
Cc: Yi-Hung Wei 
Cc: Yifeng Sun 
Acked-by: Yi-Hung Wei 
Signed-off-by: Greg Rose 
Signed-off-by: Ben Pfaff 
---
 .travis.yml|  2 +-
 acinclude.m4   |  3 ++
 datapath/linux/compat/nf_conntrack_reasm.c | 54 --
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 998b33d..3370052 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -37,7 +37,7 @@ env:
   - KERNEL=3.16.54 DPDK=1 OPTS="--enable-shared"
   - KERNEL=4.15.18
   - KERNEL=4.14.63
-  - KERNEL=4.9.120
+  - KERNEL=4.9.149
   - KERNEL=4.4.148
   - KERNEL=3.16.57
   - TESTSUITE=1 LIBS=-ljemalloc
diff --git a/acinclude.m4 b/acinclude.m4
index be918fc..65c0105 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -895,6 +895,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   [OVS_DEFINE([HAVE_VOID_NDO_GET_STATS64])])
   OVS_GREP_IFELSE([$KSRC/include/linux/timer.h], [init_timer_deferrable],
   [OVS_DEFINE([HAVE_INIT_TIMER_DEFERRABLE])])
+  OVS_FIND_FIELD_IFELSE([$KSRC/include/net/inet_frag.h], [inet_frags],
+[rnd],
+[OVS_DEFINE([HAVE_INET_FRAGS_RND])])
 
   if cmp -s datapath/linux/kcompat.h.new \
 datapath/linux/kcompat.h >/dev/null 2>&1; then
diff --git a/datapath/linux/compat/nf_conntrack_reasm.c 
b/datapath/linux/compat/nf_conntrack_reasm.c
index ce13112..9d77d98 100644
--- a/datapath/linux/compat/nf_conntrack_reasm.c
+++ b/datapath/linux/compat/nf_conntrack_reasm.c
@@ -99,6 +99,7 @@ static inline u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
return 1 << (ipv6_get_dsfield(ipv6h) & INET_ECN_MASK);
 }
 
+#ifdef HAVE_INET_FRAGS_RND
 static unsigned int nf_hash_frag(__be32 id, const struct in6_addr *saddr,
 const struct in6_addr *daddr)
 {
@@ -125,6 +126,7 @@ static unsigned int nf_hashfn(struct inet_frag_queue *q)
return nf_hash_frag(nq->id, &nq->saddr, &nq->daddr);
 }
 
+#endif /* HAVE_INET_FRAGS_RND */
 static void nf_ct_frag6_expire(unsigned long data)
 {
struct frag_queue *fq;
@@ -133,9 +135,14 @@ static void nf_ct_frag6_expire(unsigned long data)
fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
net = get_net_from_netns_frags6(fq->q.net);
 
+#ifdef HAVE_INET_FRAGS_RND
ip6_expire_frag_queue(net, fq, &nf_frags);
+#else
+   ip6_expire_frag_queue(net, fq);
+#endif
 }
 
+#ifdef HAVE_INET_FRAGS_RND
 /* Creation primitives. */
 static inline struct frag_queue *fq_find(struct net *net, __be32 id,
 u32 user, struct in6_addr *src,
@@ -168,7 +175,27 @@ static inline struct frag_queue *fq_find(struct net *net, 
__be32 id,
}
return container_of(q, struct frag_queue, q);
 }
+#else
+static struct frag_queue *fq_find(struct net *net, __be32 id, u32 user,
+ const struct ipv6hdr *hdr, int iif)
+{
+   struct frag_v6_compare_key key = {
+   .id = id,
+   .saddr = hdr->saddr,
+   .daddr = hdr->daddr,
+   .user = user,
+   .iif = iif,
+   };
+   struct inet_frag_queue *q;
+
+   q = inet_frag_find(&net->nf_frag.frags, &key);
+   if (!q)
+   return NULL;
+
+   return container_of(q, struct frag_queue, q);
+}
 
+#endif  /* HAVE_INET_FRAGS_RND */
 
 static int nf_ct_frag6_queue(struct frag_queue *fq, struct sk_buff *skb,
 const struct frag_hdr *fhdr, int nhoff)
@@ -317,7 +344,11 @@ found:
return 0;
 
 discard_fq:
+#ifdef HAVE_INET_FRAGS_RND
inet_frag_kill(&fq->q, &nf_frags);
+#else
+   inet_frag_kill(&fq->q);
+#endif
 err:
return -1;
 }
@@ -339,7 +370,11 @@ nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff 
*prev,  struct net_devic
intpayload_len;
u8 ecn;
 
+#ifdef HAVE_INET_FRAGS_RND
inet_frag_kill(&fq->q, &nf_frags);
+#else
+   inet_frag_kill(&fq->q);
+#endif
 
WARN_ON(head == NULL);
WARN_ON(NFCT_FRAG6_CB(head)->offset != 0);
@@ -561,8 +596,13 @@ int rpl_nf_ct_frag6_gather(struct net *net, struct sk_buff 
*skb, u32 user)
 #endif
 
skb_orphan(skb);
+#ifdef HAVE_INET_FRAGS_RND
fq = fq_find(net, fhdr->identification, user, &hdr->saddr, &hdr->daddr,
  

[ovs-dev] [Branch 2.9 Patch] compat: Fixup ipv6 fragmentation on 4.9.135+ kernels

[Greg Rose]

Upstream commit 648700f76b03 ("inet: frags: use rhashtables...") changed
how ipv6 fragmentation is implemented.  This patch was backported to
the upstream stable 4.9.x kernel starting at 4.9.135.

This patch creates the compatibility layer changes required to both
compile and also operate correctly with ipv6 fragmentation on these
kernels. Check if the inet_frags 'rnd' field is present to key on
whether the upstream patch is present.  Also update Travis to the
latest 4.9 kernel release so that this patch is compile tested.

Passes Travis:
https://travis-ci.org/gvrose8192/ovs-experimental/builds/478033409

Cc: William Tu 
Cc: Yi-Hung Wei 
Cc: Yifeng Sun 
Acked-by: Yi-Hung Wei 
Signed-off-by: Greg Rose 
Signed-off-by: Ben Pfaff 
---
 .travis.yml|  2 +-
 acinclude.m4   |  3 ++
 datapath/linux/compat/nf_conntrack_reasm.c | 54 --
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index ef907ca..ab3817d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -34,7 +34,7 @@ env:
   - KERNEL=3.16.54 DPDK=1 OPTS="--enable-shared"
   - KERNEL=4.15.3
   - KERNEL=4.14.19
-  - KERNEL=4.9.81
+  - KERNEL=4.9.149
   - KERNEL=4.4.115
   - KERNEL=4.1.49
   - KERNEL=3.10.108
diff --git a/acinclude.m4 b/acinclude.m4
index bf790fe..5b7f36c 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -812,6 +812,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/inet_frag.h],
   frag_percpu_counter_batch[],
   [OVS_DEFINE([HAVE_FRAG_PERCPU_COUNTER_BATCH])])
+  OVS_FIND_FIELD_IFELSE([$KSRC/include/net/inet_frag.h], [inet_frags],
+[rnd],
+[OVS_DEFINE([HAVE_INET_FRAGS_RND])])
 
   if cmp -s datapath/linux/kcompat.h.new \
 datapath/linux/kcompat.h >/dev/null 2>&1; then
diff --git a/datapath/linux/compat/nf_conntrack_reasm.c 
b/datapath/linux/compat/nf_conntrack_reasm.c
index ce13112..9d77d98 100644
--- a/datapath/linux/compat/nf_conntrack_reasm.c
+++ b/datapath/linux/compat/nf_conntrack_reasm.c
@@ -99,6 +99,7 @@ static inline u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
return 1 << (ipv6_get_dsfield(ipv6h) & INET_ECN_MASK);
 }
 
+#ifdef HAVE_INET_FRAGS_RND
 static unsigned int nf_hash_frag(__be32 id, const struct in6_addr *saddr,
 const struct in6_addr *daddr)
 {
@@ -125,6 +126,7 @@ static unsigned int nf_hashfn(struct inet_frag_queue *q)
return nf_hash_frag(nq->id, &nq->saddr, &nq->daddr);
 }
 
+#endif /* HAVE_INET_FRAGS_RND */
 static void nf_ct_frag6_expire(unsigned long data)
 {
struct frag_queue *fq;
@@ -133,9 +135,14 @@ static void nf_ct_frag6_expire(unsigned long data)
fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
net = get_net_from_netns_frags6(fq->q.net);
 
+#ifdef HAVE_INET_FRAGS_RND
ip6_expire_frag_queue(net, fq, &nf_frags);
+#else
+   ip6_expire_frag_queue(net, fq);
+#endif
 }
 
+#ifdef HAVE_INET_FRAGS_RND
 /* Creation primitives. */
 static inline struct frag_queue *fq_find(struct net *net, __be32 id,
 u32 user, struct in6_addr *src,
@@ -168,7 +175,27 @@ static inline struct frag_queue *fq_find(struct net *net, 
__be32 id,
}
return container_of(q, struct frag_queue, q);
 }
+#else
+static struct frag_queue *fq_find(struct net *net, __be32 id, u32 user,
+ const struct ipv6hdr *hdr, int iif)
+{
+   struct frag_v6_compare_key key = {
+   .id = id,
+   .saddr = hdr->saddr,
+   .daddr = hdr->daddr,
+   .user = user,
+   .iif = iif,
+   };
+   struct inet_frag_queue *q;
+
+   q = inet_frag_find(&net->nf_frag.frags, &key);
+   if (!q)
+   return NULL;
+
+   return container_of(q, struct frag_queue, q);
+}
 
+#endif  /* HAVE_INET_FRAGS_RND */
 
 static int nf_ct_frag6_queue(struct frag_queue *fq, struct sk_buff *skb,
 const struct frag_hdr *fhdr, int nhoff)
@@ -317,7 +344,11 @@ found:
return 0;
 
 discard_fq:
+#ifdef HAVE_INET_FRAGS_RND
inet_frag_kill(&fq->q, &nf_frags);
+#else
+   inet_frag_kill(&fq->q);
+#endif
 err:
return -1;
 }
@@ -339,7 +370,11 @@ nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff 
*prev,  struct net_devic
intpayload_len;
u8 ecn;
 
+#ifdef HAVE_INET_FRAGS_RND
inet_frag_kill(&fq->q, &nf_frags);
+#else
+   inet_frag_kill(&fq->q);
+#endif
 
WARN_ON(head == NULL);
WARN_ON(NFCT_FRAG6_CB(head)->offset != 0);
@@ -561,8 +596,13 @@ int rpl_nf_ct_frag6_gather(struct net *net, struct sk_buff 
*skb, u32 user)
 #endif
 
skb_orphan(skb);
+#ifdef HAVE_INET_FRAGS_RND
fq = fq_find(net, fhdr->identification, user, &hdr->saddr, &hdr->daddr,
 ip6_frag_ecn(hdr));
+#else
+ 

Re: [ovs-dev] [PATCH] datapath: return -EEXIST if inet6_add_protocol fails

[Yifeng Sun]

Thanks, looks good to me.

Reviewed-by: Yifeng Sun 

On Tue, Jan 22, 2019 at 3:45 PM Greg Rose  wrote:

> Our code to determine whether receive functionality will work with
> ip6 gre depends on the return of -EEXIST but inet6_add_protocol()
> returns a -1 on failure to grab the pointer via a cmpxchg op.  Just
> set the error return to -EEXIST to help out the vport init function.
>
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-January/048090.html
> Reported-by: Ken Ajiro 
> Signed-off-by: Greg Rose 
> ---
>  datapath/linux/compat/ip6_gre.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/datapath/linux/compat/ip6_gre.c
> b/datapath/linux/compat/ip6_gre.c
> index 1827852..2ffdda5 100644
> --- a/datapath/linux/compat/ip6_gre.c
> +++ b/datapath/linux/compat/ip6_gre.c
> @@ -2802,6 +2802,13 @@ int rpl_ip6gre_init(void)
> if (err < 0) {
> pr_info("%s: can't add protocol\n", __func__);
> unregister_pernet_device(&ip6gre_net_ops);
> +   /*
> +* inet6_add_protocol will return a -1 if it fails
> +* to grab the pointer but the vport initialization
> +* expects a return value of -EEXIST.  Set err to
> +* -EEXIST here to ensure proper handling.
> +*/
> +   err = -EEXIST;
> goto ip6_gre_loaded;
> }
>
> --
> 1.8.3.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn: Add pkg-config support via libovn.pc.

[Ben Pfaff]

On Wed, Jan 23, 2019 at 10:17:40AM -0500, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > From: 竹內宏輝 Hiroki Takeuchi 
> >
> > Submitted-at: https://github.com/openvswitch/ovs/pull/270
> > Signed-off-by: Hiroki Takeuchi 
> > ---
> > I'm sending this patch as a squashed version of what was submitted at
> > Github.  I know that pkg-config is somewhat controversial, so I'd like to
> > hear what the denizens of ovs-dev have to say about including a libovn.pc.
> 
> I like pkg-config, as a general rule.  I wasn't aware that it was
> controversial, but I'm definitely not plugged into the latest trends in
> library development.

I think I misremembered this: it is libtool .la files that are
deprecated, not pkg-config .pc files.  Reference:
https://www.netfort.gr.jp/~dancer/column/libpkg-guide/libpkg-guide.html#pkgconfig

Never mind, then.

I'll ask Hiroki to comment on the rest here.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath: return -EEXIST if inet6_add_protocol fails

[Ben Pfaff]

Thanks, applied to master and branch-2.11.

On Wed, Jan 23, 2019 at 11:28:25AM -0800, Yifeng Sun wrote:
> Thanks, looks good to me.
> 
> Reviewed-by: Yifeng Sun 
> 
> On Tue, Jan 22, 2019 at 3:45 PM Greg Rose  wrote:
> 
> > Our code to determine whether receive functionality will work with
> > ip6 gre depends on the return of -EEXIST but inet6_add_protocol()
> > returns a -1 on failure to grab the pointer via a cmpxchg op.  Just
> > set the error return to -EEXIST to help out the vport init function.
> >
> > Reported-at:
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-January/048090.html
> > Reported-by: Ken Ajiro 
> > Signed-off-by: Greg Rose 
> > ---
> >  datapath/linux/compat/ip6_gre.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/datapath/linux/compat/ip6_gre.c
> > b/datapath/linux/compat/ip6_gre.c
> > index 1827852..2ffdda5 100644
> > --- a/datapath/linux/compat/ip6_gre.c
> > +++ b/datapath/linux/compat/ip6_gre.c
> > @@ -2802,6 +2802,13 @@ int rpl_ip6gre_init(void)
> > if (err < 0) {
> > pr_info("%s: can't add protocol\n", __func__);
> > unregister_pernet_device(&ip6gre_net_ops);
> > +   /*
> > +* inet6_add_protocol will return a -1 if it fails
> > +* to grab the pointer but the vport initialization
> > +* expects a return value of -EEXIST.  Set err to
> > +* -EEXIST here to ensure proper handling.
> > +*/
> > +   err = -EEXIST;
> > goto ip6_gre_loaded;
> > }
> >
> > --
> > 1.8.3.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [Branch 2.10 Patch] compat: Fixup ipv6 fragmentation on 4.9.135+ kernels

[Ben Pfaff]

On Wed, Jan 23, 2019 at 11:23:57AM -0800, Greg Rose wrote:
> Upstream commit 648700f76b03 ("inet: frags: use rhashtables...") changed
> how ipv6 fragmentation is implemented.  This patch was backported to
> the upstream stable 4.9.x kernel starting at 4.9.135.
> 
> This patch creates the compatibility layer changes required to both
> compile and also operate correctly with ipv6 fragmentation on these
> kernels. Check if the inet_frags 'rnd' field is present to key on
> whether the upstream patch is present.  Also update Travis to the
> latest 4.9 kernel release so that this patch is compile tested.
> 
> Passes Travis:
> https://travis-ci.org/gvrose8192/ovs-experimental/builds/478033409
> 
> Cc: William Tu 
> Cc: Yi-Hung Wei 
> Cc: Yifeng Sun 
> Acked-by: Yi-Hung Wei 
> Signed-off-by: Greg Rose 
> Signed-off-by: Ben Pfaff 

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

Re: [ovs-dev] [Branch 2.9 Patch] compat: Fixup ipv6 fragmentation on 4.9.135+ kernels

[Ben Pfaff]

On Wed, Jan 23, 2019 at 11:26:35AM -0800, Greg Rose wrote:
> Upstream commit 648700f76b03 ("inet: frags: use rhashtables...") changed
> how ipv6 fragmentation is implemented.  This patch was backported to
> the upstream stable 4.9.x kernel starting at 4.9.135.
> 
> This patch creates the compatibility layer changes required to both
> compile and also operate correctly with ipv6 fragmentation on these
> kernels. Check if the inet_frags 'rnd' field is present to key on
> whether the upstream patch is present.  Also update Travis to the
> latest 4.9 kernel release so that this patch is compile tested.
> 
> Passes Travis:
> https://travis-ci.org/gvrose8192/ovs-experimental/builds/478033409
> 
> Cc: William Tu 
> Cc: Yi-Hung Wei 
> Cc: Yifeng Sun 
> Acked-by: Yi-Hung Wei 
> Signed-off-by: Greg Rose 
> Signed-off-by: Ben Pfaff 

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

Re: [ovs-dev] [PATCH V2 0/2] Do not rewrite fields with the same values as

[Ben Pfaff]

On Wed, Jan 23, 2019 at 01:17:21PM +, Eli Britstein wrote:
> 
> On 1/23/2019 2:14 AM, Ben Pfaff wrote:
> > On Fri, Jan 11, 2019 at 08:28:57AM +0200, Eli Britstein wrote:
> >> Hi
> >>
> >> This patch set avoids unnecessary rewrite actions to fields with the
> >> same values as matched on.
> >>
> >> Patch 1 is a pre-step by defining ovs key structs using macros
> >>
> >> Patch 2 avoids the unnecessary rewrites and adapts the tests accordingly
> > I like the goal of the patch, but it makes openvswitch.h unreadable and
> > completely different from Linux upstream.  One of the goals of that
> > "sed" script is to avoid having it be different from upstream.  I think
> 
> unreadable - well, it's a point of view, but i don't think that's alone 
> is a justified argument not to change
> 
> completely different - that's correct. why don't we just push the same 
> change to the Linux upstream to keep it the same?

OK, let me know when you get it upstream and I'll look again.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2] rhel: bug fix upgrade path in kmod fedora spec file

[Ben Pfaff]

On Wed, Jan 23, 2019 at 07:45:56AM -0200, Flavio Leitner wrote:
> On Tue, Jan 22, 2019 at 03:02:30PM -0800, Greg Rose wrote:
> > From: Martin Xu 
> > 
> > This patch removes the "Conflicts" tag and adds "Obsoletes" tag.
> > 
> > With the conflicts tag, when a user attempts to install or upgrade with
> > the same version as already installed, the conflict kicks in. Otherwise,
> > such is allowed with --replacepkgs.
> > 
> > Obsoletes is needed for the upgrade path from kmod-openvswitch to
> > openvswitch-kmod.
> > 
> > Fixes: 22c33c3039 (rhel: support kmod build against mulitple kernel
> > versions, fedora)
> > 
> > VMware-BZ: #2249788
> > 
> > Signed-off-by: Martin Xu 
> > Signed-off-by: Greg Rose 
> > CC: Flavio Leitner 
> > CC: Yi-Hung Wei 
> > CC: Yifeng Sun 
> > CC: Zak Whittington 
> > CC: Ben Pfaff 
> > ---
> 
> LGTM
> Acked-by: Flavio Leitner 

Thanks Greg, Martin, Flavio.  I applied this to master and branch-2.11.
Let me know if it should be backported further.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] compat: Fixup ipv6 fragmentation on 4.9.135+ kernels

[0-day Robot]

Bleep bloop.  Greetings Greg Rose, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 compat: Fixup ipv6 fragmentation on 4.9.135+ kernels
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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

Re: [ovs-dev] compat: Fixup ipv6 fragmentation on 4.9.135+ kernels

[0-day Robot]

Bleep bloop.  Greetings Greg Rose, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 compat: Fixup ipv6 fragmentation on 4.9.135+ kernels
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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

Re: [ovs-dev] [PATCH 0/3] revert port duplicate checking optimization

[Ben Pfaff]

I'm not so happy about reverting without having the followup ready.  How
close are we to having the followup?  Basically we've got two problems
here.  Without the revert, we have one of them; with the revert, we have
the other one.  I'd rather not trade one for the other, that's not
ideal.  It would be much better to fix both in one shot.

On Wed, Jan 23, 2019 at 07:55:36AM -0200, Flavio Leitner wrote:
> 
> Hi,
> 
> Just a reminder about this revert.
> 
> On Thu, Dec 13, 2018 at 12:30:47PM -0200, Flavio Leitner wrote:
> > 
> > This should be applied to branch-2.10 as well.
> 
> And now branch-2.11
> 
> Thanks,
> fbl
> 
> > 
> > (BTW, I had CC few folks in the patchset, but I am only seeing Guru, so
> >  I am adding them to this email just in case)
> > 
> > Thanks,
> > fbl
> > 
> > On Thu, Dec 13, 2018 at 12:24:45PM -0200, Flavio Leitner wrote:
> > > The optimization introduced a regression in OSP environments using
> > > internal ports in other netns. Their networking configuration is lost
> > > when the service is restarted because the ports are recreated now.
> > > 
> > > Before the patch it checked using netlink if the port with a specific
> > > "name" was already there. The check is a lookup in all ports attached
> > > to the DP regardless of the port's netns.
> > > 
> > > After the patch it relies on the kernel to identify that situation.
> > > Unfortunately the only protection there is register_netdevice() which
> > > fails only if the port with that name exists in the current netns.
> > > 
> > > If the port is in another netns, it will get a new dp_port and because
> > > of that userspace will delete the old port. At this point the original
> > > port is gone from the other netns and there a fresh port in the current
> > > netns.
> > > 
> > > This patchset reverts the original commit and the two other follow ups.
> > > 
> > > Flavio Leitner (3):
> > >   Revert "dpif-netlink: Don't destroy and recreate port if it exists"
> > >   Revert "ofproto-dpif: Check for EBUSY as well"
> > >   Revert "ofproto-dpif: Let the dpif report when a port is a duplicate."
> > > 
> > >  lib/dpif-netlink.c | 4 ++--
> > >  lib/dpif.c | 9 ++---
> > >  ofproto/ofproto-dpif.c | 7 ---
> > >  3 files changed, 8 insertions(+), 12 deletions(-)
> > > 
> > > -- 
> > > 2.17.2
> > 
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS

[Ben Pfaff]

On Wed, Jan 23, 2019 at 09:52:02AM +, Anju Thomas wrote:
> On Mon, Jan 21, 2019 at 07:01:33AM +, Anju Thomas wrote:
> > On Thu, Jan 17, 2019 at 04:49:20AM +, Anju Thomas wrote:
> > > Currently OVS maintains explicit packet drop/error counters only on port
> > > level. Packets that are dropped as part of normal OpenFlow processing 
> > > are
> > > counted in flow stats of “drop” flows or as table misses in table 
> > > stats.
> > > These can only be interpreted by controllers that know the semantics 
> > > of
> > > the configured OpenFlow pipeline. Without that knowledge, it is 
> > > impossible
> > > for an OVS user to obtain e.g. the total number of packets dropped 
> > > due to
> > > OpenFlow rules.
> > 
> > Thanks for the patch!  I agree with your motivations--it is useful to 
> > understand why packets are dropped.  I have some comments to add to Ilya's.
> > 
> > It looks like the drop actions that this formats in
> > format_odp_drop_action() can't necessarily be parsed by 
> > odp_actions_from_string().  Usually we expect this.  (Probably the syntax 
> > should be adjusted to make parsing more straightforward.)   
> > odp_actions_from string is used to convert string to action for parsing 
> > .But we are using the drop action reason only fo display. We do not want to 
> > provide an option for its parsing. Is my understanding correct.
> 
> Why not?  Ordinarily, anything that can be output can be input.  It is useful 
> for testing to be able to input datapath actions.
> 
> Agreed. Ideally yes. But in this case the error are set due to some error 
> during translation like *bridge not found 
> *too many resubmits 
> *recursion too deep 
> *too many resubmits 
> *sack too deeo
> *no recirculation context
> *too many mpls labels
> * invalid tunnel metadata
> * unsupported packet type
> * ecn mismatch at tunnel decapsulation
> * forwarding disabled
> * pipeline drop
> 
> Except pipeline drop I don’t understand how it will make sense to inject 
> others as input . Can you explain if I am missing something ?

I don't have an immediate use case for injecting these.

At the very least, I want the output format to be one that is parseable,
whether we decide to actually parse it now or not.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] seq: Correct example in comment.

[Ben Pfaff]

On Wed, Jan 23, 2019 at 11:34:22AM +0300, Ilya Maximets wrote:
> On 23.01.2019 1:43, Ben Pfaff wrote:
> > On Tue, Jan 22, 2019 at 12:52:56PM +0300, Ilya Maximets wrote:
> >> On 22.01.2019 4:14, Ben Pfaff wrote:
> >>> It was deceptive for the example to imply that a seq can be declared
> >>> directly, because the API only allows for creating a new one on the heap.
> >>>
> >>> Signed-off-by: Ben Pfaff 
> >>
> >> Thanks.
> >> Maybe you can additionally fix the C99-style comment here to not use
> >> a bad style inside the example.
> >>
> >> Anyway,
> >> Acked-by: Ilya Maximets 
> > 
> > Thanks, I applied this to master.
> > 
> > It's tricky to avoid C99 comments in code samples inside regular C
> > comments, because C comments don't nest.  Do you have a suggestion on
> > how to do it?
> 
> Oops. Yes, you're right, it's a bit tricky.
> 
> So, I'm not sure if this needed, but I see 2 possible solutions:
> 
>   1. Add some spaces like / * Comment * /.
>  But this increases the line length over 79 chars here.
> 
>   2. Make it look not like a comment at all. i.e. Make it look like a
>  a simple text. For example:
> 
>  if (ovs_list_is_singleton(&queue)) {  <-- The 'if' test here is 
> optional.
> 
>  This doesn't give a bad comment example and obviously should be removed
>  while copying to the real code.
> 
> Best regards, Ilya Maximets.

Good ideas.  I like #2 better than #1.  I sent a fix.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Kees Cook]

On Thu, Jan 24, 2019 at 8:18 AM Matthew Wilcox  wrote:
>
> On Wed, Jan 23, 2019 at 04:17:30PM +0200, Jani Nikula wrote:
> > Can't have:
> >
> >   switch (i) {
> >   int j;
> >   case 0:
> >   /* ... */
> >   }
> >
> > because it can't be turned into:
> >
> >   switch (i) {
> >   int j = 0; /* not valid C */
> >   case 0:
> >   /* ... */
> >   }
> >
> > but can have e.g.:
> >
> >   switch (i) {
> >   case 0:
> >   {
> >   int j = 0;
> >   /* ... */
> >   }
> >   }
> >
> > I think Kees' approach of moving such variable declarations to the
> > enclosing block scope is better than adding another nesting block.
>
> Another nesting level would be bad, but I think this is OK:
>
> switch (i) {
> case 0: {
> int j = 0;
> /* ... */
> }
> case 1: {
> void *p = q;
> /* ... */
> }
> }
>
> I can imagine Kees' patch might have a bad effect on stack consumption,
> unless GCC can be relied on to be smart enough to notice the
> non-overlapping liveness of the vriables and use the same stack slots
> for both.

GCC is reasonable at this. The main issue, though, was most of these
places were using the variables in multiple case statements, so they
couldn't be limited to a single block (or they'd need to be manually
repeated in each block, which is even more ugly, IMO).

Whatever the consensus, I'm happy to tweak the patch.

Thanks!

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

[ovs-dev] TNT Express Consignment Notification

[TNT Import Clearance]

Dear Customer,
 A shipment has been arranged for you via TNT.
 The shipment is scheduled for delivery tomorrow 25th January, 2019 and has TNT 
consignment number: 142596001.
 
SHIPMENT DETAILS OVERVIEW: (View Enclosed)
  
Shipment reference : Invoice # 10623-24
Description : document
 Download Attachment for invoice and full shipment details.
  
 
 
---
This message and any attachment are confidential and may be privileged or 
otherwise protected from disclosure. 
If you are not the intended recipient, please telephone or email the sender and 
delete this message and any attachment from your system.  
If you are not the intended recipient you must not copy this message or 
attachment or disclose the contents to any other person.
 Please consider the environmental impact before printing this document and its 
attachment(s). 
Print black and white and double-sided where possible. 
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Urgent Payment Notification

[Mrs. Mary John via dev]

To the Attention of .

Sir,

I wish to bring to your notice that PARIS Club in alliance with World Bank held 
series of meetings with African Heads of States at Addis Abbaba late December 
2018 in order to offset the debts which African countries owed to Europeans, 
Americans, Asians and Australia citizens and the accord reached was that single 
paper should be issued to each certified contractor or beneficiary of any fund 
whose fund remains unpaid to enhance immediate and unconditional remittance of 
such funds to the beneficiary hence this contact to your-good-self as it was 
ascertained that your fund is among the unpaid funds basically for no concrete 
reasons which the African countries were blamed for inappropriate handling of 
the payment procedures which contradicted UN Charter 1141 Section 5b which 
empowers any contractor or beneficiary the right to claim his/her fund without 
prejudice.

Consequently, Your fund which is still on hold was among those to be paid out 
of the $2.5B loan which the PARIS Club gave to African Countries purposely to 
offset these debts and improve African economies. It might interest you to know 
also that the paper which authorizes due release of your fund is to be issued 
by the US Justice Department which was designated by the UN Charter 1142 
Section 3a authentically for actualization of this accord. Therefore, for you 
to be paid, you need to contact the US Justice Department on an official Email 
to be given to you upon response for swift processing of the said fund in your 
name and due remittance of such also by the designated bank which name will be 
made known to you upon response also.

As you may know, this was in complete adherence to the demand of the World 
leaders that African countries pay their citizens any unpaid debt held in 
African banks or elsewhere therefore it behooves you to comply and be paid as 
we wish you all the best as you move towards ending your predicaments of not 
being paid for long by the African countries.

Regards,

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

[ovs-dev] suspicious skiplist code

[Ben Pfaff]

I'm really suspicious about the code in lib/skiplist.c, because I
noticed that it writes, but never reads, the 'height' member of struct
skiplist_node.  I suspect that, therefore, in some circumstances the
code can read past the end of the 'forward' array in skiplist_node.

I'd appreciate it if someone out there has time to verify that I'm right
or I'm wrong.

Thanks,

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