Reported-by: Justin Pettit
Signed-off-by: Andy Zhou
v1 -> v2
Address Jesse's review feedback:
o Fix the logic of exporting tun_id mask.
o Remove unnecessary check for swkey values.
o Kept a few optimizations of omitting export zero values
whenever i
hi,
> @@ -708,9 +711,7 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key,
> uint32_t key_len,
> return EINVAL;
> }
>
> -if (flow->in_port < OFPP_MAX
> -? flow->in_port >= MAX_PORTS
> -: flow->in_port != OFPP_LOCAL && flow->in_port != OFPP_NONE) {
> +
The upstream has the fix for dev_forward_skb() to reset pkt_type to
PACKET_HOST. the change set of 06a23fe31ca3992863721f21bdb0307af93da807
So the pkt_type of PACKET_OTHERHOST doesn't come in.
This patch is a workaround for older kernel, reset skb->pkt_type when
receiving pkt_type of PACKET_OTHERHO
Thomas Graf (3):
datapath: Rename skb_network_protocol() to __skb_network_protocol()
datapath: rhel: Account for RHEL specific backports
datapath: rhel: Move RHEL OVS hook registration to
netdev_rx_handler_register() backport
acinclude.m4| 3 +++
dat
Commit 4e022ec09e14 (Create specific types for ofp and odp port) broke
OpenFlow OFPP_PACKET_OUT requests that use in_port=OFPP_CONTROLLER. This
commit fixes the problem and adds a regression test.
CC: Alex Wang
Reported-by: YAMAMOTO Takashi
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c |
Moves the registration of the RHEL specific OVS hook to the compat
backport of netdev_rx_handler_register(). This moves the hook
unregistration from the RCU callback to the netdev_destroy()
callback directly.
This is purely cosmetic though, the RHEL hook is only used if the
IFF_OVS_DATAPATH flag i
The function skb_network_protocol() is already defined in
upstream but not an exported symbol.
Rename the OVS internal implementation to work around this.
Signed-off-by: Thomas Graf
---
datapath/linux/compat/gso.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/datapath
The following symbols have been backported to RHEL and the kernel
version is no longer an accurate indicator for their presence:
- skb_gso_segment
- netif_skb_features
- netif_needs_gso
Signed-off-by: Thomas Graf
---
acinclude.m4| 3 +++
datapath/linux/com
On Tue, Jul 09, 2013 at 05:18:34PM +0900, YAMAMOTO Takashi wrote:
> hi,
>
> > @@ -708,9 +711,7 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key,
> > uint32_t key_len,
> > return EINVAL;
> > }
> >
> > -if (flow->in_port < OFPP_MAX
> > -? flow->in_port >= MAX_PO
On Tue, Jul 9, 2013 at 9:00 AM, Thomas Graf wrote:
> The function skb_network_protocol() is already defined in
> upstream but not an exported symbol.
>
> Rename the OVS internal implementation to work around this.
>
Are you seeing problem with RHEL kernel backports?
OVS does not support upstream
On 07/09/2013 06:38 PM, Pravin Shelar wrote:
On Tue, Jul 9, 2013 at 9:00 AM, Thomas Graf wrote:
The function skb_network_protocol() is already defined in
upstream but not an exported symbol.
Rename the OVS internal implementation to work around this.
Are you seeing problem with RHEL kernel
The behavior seems a little surprising, since the "--if-exists" sounds like it
will be deleted if it exists, but it's actually just avoiding a fatal error and
the port will still exist. It would be nicer to give some sort of non-fatal
warning in that case. However, that's likely to cause break
With the latest version of sparse, the ATOMIC_VAR_INIT macro
generates the following warning. This patch suppresses it.
warning: Using plain integer as NULL pointer
Signed-off-by: Ethan Jackson
---
lib/ovs-atomic-pthreads.h |4
1 file changed, 4 insertions(+)
diff --git a/lib/ovs-ato
The latest version of sparse triggers the following warning in the
checksum tests. This patch suppresses it.
warning: too long initializer-string for array of char
Signed-off-by: Ethan Jackson
---
tests/test-csum.c |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a
Sorry Yamamoto and thanks Ben,
Please ignore my previous reply.
On Tue, Jul 9, 2013 at 9:24 AM, Ben Pfaff wrote:
> On Tue, Jul 09, 2013 at 05:18:34PM +0900, YAMAMOTO Takashi wrote:
> > hi,
> >
> > > @@ -708,9 +711,7 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr
> *key, uint32_t key_len,
On Tue, Jul 9, 2013 at 9:00 AM, Thomas Graf wrote:
> diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
> index fe7e359..d043eae 100644
> --- a/datapath/vport-netdev.c
> +++ b/datapath/vport-netdev.c
> @@ -46,9 +46,8 @@ MODULE_PARM_DESC(vlan_tso, "Enable TSO for VLAN packets");
> #end
On Tue, Jul 9, 2013 at 9:00 AM, Thomas Graf wrote:
> The function skb_network_protocol() is already defined in
> upstream but not an exported symbol.
>
> Rename the OVS internal implementation to work around this.
>
> Signed-off-by: Thomas Graf
Applied, thanks.
X-CudaMail-Whitelist-To: dev@open
On Tue, Jul 9, 2013 at 9:00 AM, Thomas Graf wrote:
> The following symbols have been backported to RHEL and the kernel
> version is no longer an accurate indicator for their presence:
>
> - skb_gso_segment
> - netif_skb_features
> - netif_needs_gso
>
> Signed-off-by: Thomas Graf
Also applied.
On 07/09/2013 08:06 PM, Jesse Gross wrote:
On Tue, Jul 9, 2013 at 9:00 AM, Thomas Graf wrote:
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index fe7e359..d043eae 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -46,9 +46,8 @@ MODULE_PARM_DESC(vlan_tso, "En
Moves the registration of the RHEL specific OVS hook to the compat
backport of netdev_rx_handler_register(). This moves the hook
unregistration from the RCU callback to the netdev_destroy()
callback directly.
This is purely cosmetic though, the RHEL hook is only used if the
IFF_OVS_DATAPATH flag i
On Tue, Jul 9, 2013 at 11:36 AM, Thomas Graf wrote:
> Moves the registration of the RHEL specific OVS hook to the compat
> backport of netdev_rx_handler_register(). This moves the hook
> unregistration from the RCU callback to the netdev_destroy()
> callback directly.
>
> This is purely cosmetic t
On Tue, Jul 09, 2013 at 10:16:35AM -0700, Ethan Jackson wrote:
> With the latest version of sparse, the ATOMIC_VAR_INIT macro
> generates the following warning. This patch suppresses it.
>
> warning: Using plain integer as NULL pointer
>
> Signed-off-by: Ethan Jackson
Does this actually occur
On Tue, Jul 09, 2013 at 10:16:36AM -0700, Ethan Jackson wrote:
> The latest version of sparse triggers the following warning in the
> checksum tests. This patch suppresses it.
>
> warning: too long initializer-string for array of char
>
> Signed-off-by: Ethan Jackson
Acked-by: Ben Pfaff
_
On Tue, Jul 9, 2013 at 12:25 AM, Andy Zhou wrote:
> diff --git a/datapath/flow.c b/datapath/flow.c
> index adb918f..a756a15 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
> @@ -1686,16 +1688,15 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key
> *swkey,
[...]
> - if (swkey->phy.in
On Tue, Jul 9, 2013 at 4:07 AM, Isaku Yamahata wrote:
> The upstream has the fix for dev_forward_skb() to reset pkt_type to
> PACKET_HOST. the change set of 06a23fe31ca3992863721f21bdb0307af93da807
> So the pkt_type of PACKET_OTHERHOST doesn't come in.
> This patch is a workaround for older kernel
Reported-by: Kris zhang
Signed-off-by: Jesse Gross
---
FAQ | 3 +++
1 file changed, 3 insertions(+)
diff --git a/FAQ b/FAQ
index 0210477..6b5d8da 100644
--- a/FAQ
+++ b/FAQ
@@ -146,6 +146,9 @@ A: The following table lists the Linux kernel versions
against which the
1.7.x 2.6.18 to
To indicate that a flow is not associated with any particular in port,
userspace may omit the IN_PORT attribute, which the kernel translates
internally to the special value DP_MAX_PORTS. After the megaflows
changes, this was no longer being done, resulting in it using port 0
(the internal port).
T
CC: Andy Zhou
Signed-off-by: Jesse Gross
---
datapath/datapath.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/datapath/datapath.h b/datapath/datapath.h
index d14a162..eda87fd 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -206,6 +206,6 @@ int ovs_execute_act
Thanks for the patch. It looks good.
On Tue, Jul 9, 2013 at 2:27 PM, Jesse Gross wrote:
> To indicate that a flow is not associated with any particular in port,
> userspace may omit the IN_PORT attribute, which the kernel translates
> internally to the special value DP_MAX_PORTS. After the mega
Looks good. Thanks.
On Tue, Jul 9, 2013 at 2:44 PM, Jesse Gross wrote:
> CC: Andy Zhou
> Signed-off-by: Jesse Gross
> ---
> datapath/datapath.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/datapath/datapath.h b/datapath/datapath.h
> index d14a162..eda87fd 100644
>
Thanks, I'll merge this shortly.
Ethan
On Tue, Jul 9, 2013 at 3:30 PM, Ben Pfaff wrote:
> On Tue, Jul 09, 2013 at 10:16:36AM -0700, Ethan Jackson wrote:
>> The latest version of sparse triggers the following warning in the
>> checksum tests. This patch suppresses it.
>>
>> warning: too long ini
This is mostly historical. Here's the whole story, as I remember it.
I guess that you remember at least some of this too.
Originally ovs-vswitchd had a text-based config file. To reconfigure,
you edited the file and then told ovs-vswitchd to reread it. When we
did the xenserver integration, we
Good point. I'll send out a cleaner patch in a bit.
Ethan
On Tue, Jul 9, 2013 at 3:29 PM, Ben Pfaff wrote:
> On Tue, Jul 09, 2013 at 10:16:35AM -0700, Ethan Jackson wrote:
>> With the latest version of sparse, the ATOMIC_VAR_INIT macro
>> generates the following warning. This patch suppresses
With the latest version of sparse, the ATOMIC_VAR_INIT macro
generates the following warning. This patch suppresses it.
warning: Using plain integer as NULL pointer
Signed-off-by: Ethan Jackson
---
include/sparse/pthread.h |3 +++
1 file changed, 3 insertions(+)
diff --git a/include/spars
Thanks, I applied both of these patches.
On Tue, Jul 9, 2013 at 3:16 PM, Andy Zhou wrote:
> Thanks for the patch. It looks good.
>
>
> On Tue, Jul 9, 2013 at 2:27 PM, Jesse Gross wrote:
>>
>> To indicate that a flow is not associated with any particular in port,
>> userspace may omit the IN_PORT
Thanks. An incremental patch is attached that should fix both issues
raised.
On Tue, Jul 9, 2013 at 1:59 PM, Jesse Gross wrote:
> On Tue, Jul 9, 2013 at 12:25 AM, Andy Zhou wrote:
> > diff --git a/datapath/flow.c b/datapath/flow.c
> > index adb918f..a756a15 100644
> > --- a/datapath/flow.c
> >
On Tue, Jul 09, 2013 at 03:29:20PM -0700, Ethan Jackson wrote:
> With the latest version of sparse, the ATOMIC_VAR_INIT macro
> generates the following warning. This patch suppresses it.
>
> warning: Using plain integer as NULL pointer
>
> Signed-off-by: Ethan Jackson
Acked-by: Ben Pfaff
Ple
Sorry, I meant to say TTL instead of ToS.
On Tue, Jul 9, 2013 at 4:43 PM, Jesse Gross wrote:
> I applied the combined result but after I did that I noticed that it
> introduces another bug: a ToS of 0 must be explicitly specified, there
> is no default value for this field. Therefore, when we ser
I applied the combined result but after I did that I noticed that it
introduces another bug: a ToS of 0 must be explicitly specified, there
is no default value for this field. Therefore, when we serialize it
back we should always include it.
Please be more careful when changing code like this, par
There is no default value for the tunnel TTL so it must always be
included in flow keys sent from userspace to kernel. The kernel
should also respect this convertion when sending flows to userspace
by always including the TTL in tunnel flows.
CC: Andy Zhou
Signed-off-by: Jesse Gross
---
datapat
There is no default value for the tunnel TTL, so it must be
specified when setting up a new flow. However, the flow rejection
log message indicates that the TTL must be non-zero, which is not
true.
CC: Andy Zhou
Signed-off-by: Jesse Gross
---
datapath/flow.c | 2 +-
1 file changed, 1 insertion(
Looks good. Did not know TTL=0 is allowed.
On Tue, Jul 9, 2013 at 4:51 PM, Jesse Gross wrote:
> There is no default value for the tunnel TTL so it must always be
> included in flow keys sent from userspace to kernel. The kernel
> should also respect this convertion when sending flows to userspa
Thanks, I applied both of these patches.
On Tue, Jul 9, 2013 at 5:11 PM, Andy Zhou wrote:
> Looks good. Did not know TTL=0 is allowed.
>
>
> On Tue, Jul 9, 2013 at 4:51 PM, Jesse Gross wrote:
>>
>> There is no default value for the tunnel TTL so it must always be
>> included in flow keys sent fr
On Tue, Jul 09, 2013 at 02:08:03PM -0700, Jesse Gross wrote:
> On Tue, Jul 9, 2013 at 4:07 AM, Isaku Yamahata wrote:
> > The upstream has the fix for dev_forward_skb() to reset pkt_type to
> > PACKET_HOST. the change set of 06a23fe31ca3992863721f21bdb0307af93da807
> > So the pkt_type of PACKET_OTH
On Mon, 2013-07-08 at 23:26 -0700, Jesse Gross wrote:
> On Mon, Jul 8, 2013 at 7:41 PM, Cong Wang wrote:
> > On Mon, 2013-07-08 at 09:28 -0700, Pravin Shelar wrote:
> >> On Mon, Jul 8, 2013 at 2:51 AM, Cong Wang wrote:
> >> > However, I noticed there is some problem with such design:
> >> >
> >>
On Tue, Jul 9, 2013 at 7:48 PM, Isaku Yamahata wrote:
> On Tue, Jul 09, 2013 at 02:08:03PM -0700, Jesse Gross wrote:
>> On Tue, Jul 9, 2013 at 4:07 AM, Isaku Yamahata
>> wrote:
>> > The upstream has the fix for dev_forward_skb() to reset pkt_type to
>> > PACKET_HOST. the change set of 06a23fe31c
On Tue, Jul 09, 2013 at 10:23:42PM -0700, Jesse Gross wrote:
> On Tue, Jul 9, 2013 at 7:48 PM, Isaku Yamahata wrote:
> > On Tue, Jul 09, 2013 at 02:08:03PM -0700, Jesse Gross wrote:
> >> On Tue, Jul 9, 2013 at 4:07 AM, Isaku Yamahata
> >> wrote:
> >> > The upstream has the fix for dev_forward_sk
47 matches
Mail list logo
