[ovs-dev] datapath: Fix wrong push/pop ethernet validation

Message ID 20181102114514.7023-1-jcaamano@suse.com
State New
Headers show
Series
  • [ovs-dev] datapath: Fix wrong push/pop ethernet validation
Related show

Commit Message

Jaime Caamaño Ruiz Nov. 2, 2018, 11:45 a.m.
Upstream commit:
    commit 46ebe2834ba5b541f28ee72e556a3fed42c47570
    Author: Jaime Caamaño Ruiz <jcaamano@suse.com>
    Date:   Wed Oct 31 18:52:03 2018 +0100

    openvswitch: Fix push/pop ethernet validation

    When there are both pop and push ethernet header actions among the
    actions to be applied to a packet, an unexpected EINVAL (Invalid
    argument) error is obtained. This is due to mac_proto not being reset
    correctly when those actions are validated.

    Reported-at:
    https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
    Fixes: 91820da6ae85 ("openvswitch: add Ethernet push and pop actions")
    Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>

Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
Fixes: 6fcecb85ab ("datapath: add Ethernet push and pop actions")
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
---
 datapath/flow_netlink.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Gregory Rose Nov. 2, 2018, 3:31 p.m. | #1
On 11/2/2018 4:45 AM, Jaime Caamaño Ruiz wrote:
> Upstream commit:
>      commit 46ebe2834ba5b541f28ee72e556a3fed42c47570
>      Author: Jaime Caamaño Ruiz <jcaamano@suse.com>
>      Date:   Wed Oct 31 18:52:03 2018 +0100
>
>      openvswitch: Fix push/pop ethernet validation
>
>      When there are both pop and push ethernet header actions among the
>      actions to be applied to a packet, an unexpected EINVAL (Invalid
>      argument) error is obtained. This is due to mac_proto not being reset
>      correctly when those actions are validated.
>
>      Reported-at:
>      https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
>      Fixes: 91820da6ae85 ("openvswitch: add Ethernet push and pop actions")
>      Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
>
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
> Fixes: 6fcecb85ab ("datapath: add Ethernet push and pop actions")
> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
> ---
>   datapath/flow_netlink.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index c3f1baa05..ee0c18422 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -2998,7 +2998,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
>   			 * is already present */
>   			if (mac_proto != MAC_PROTO_NONE)
>   				return -EINVAL;
> -			mac_proto = MAC_PROTO_NONE;
> +			mac_proto = MAC_PROTO_ETHERNET;
>   			break;
>   
>   		case OVS_ACTION_ATTR_POP_ETH:
> @@ -3006,7 +3006,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
>   				return -EINVAL;
>   			if (vlan_tci & htons(VLAN_TAG_PRESENT))
>   				return -EINVAL;
> -			mac_proto = MAC_PROTO_ETHERNET;
> +			mac_proto = MAC_PROTO_NONE;
>   			break;
>   
>   		case OVS_ACTION_ATTR_PUSH_NSH:

Thanks for all your work on this Jaime!

Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Ben Pfaff Nov. 2, 2018, 8:46 p.m. | #2
On Fri, Nov 02, 2018 at 08:31:06AM -0700, Gregory Rose wrote:
> On 11/2/2018 4:45 AM, Jaime Caamaño Ruiz wrote:
> >Upstream commit:
> >     commit 46ebe2834ba5b541f28ee72e556a3fed42c47570
> >     Author: Jaime Caamaño Ruiz <jcaamano@suse.com>
> >     Date:   Wed Oct 31 18:52:03 2018 +0100
> >
> >     openvswitch: Fix push/pop ethernet validation
> >
> >     When there are both pop and push ethernet header actions among the
> >     actions to be applied to a packet, an unexpected EINVAL (Invalid
> >     argument) error is obtained. This is due to mac_proto not being reset
> >     correctly when those actions are validated.
> >
> >     Reported-at:
> >     https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
> >     Fixes: 91820da6ae85 ("openvswitch: add Ethernet push and pop actions")
> >     Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
> >
> >Reported-at:
> >https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
> >Fixes: 6fcecb85ab ("datapath: add Ethernet push and pop actions")
> >Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>

Applied to master, thanks Jaime and Greg!
Gregory Rose Nov. 2, 2018, 8:57 p.m. | #3
On 11/2/2018 1:46 PM, Ben Pfaff wrote:
> On Fri, Nov 02, 2018 at 08:31:06AM -0700, Gregory Rose wrote:
>> On 11/2/2018 4:45 AM, Jaime Caamaño Ruiz wrote:
>>> Upstream commit:
>>>      commit 46ebe2834ba5b541f28ee72e556a3fed42c47570
>>>      Author: Jaime Caamaño Ruiz <jcaamano@suse.com>
>>>      Date:   Wed Oct 31 18:52:03 2018 +0100
>>>
>>>      openvswitch: Fix push/pop ethernet validation
>>>
>>>      When there are both pop and push ethernet header actions among the
>>>      actions to be applied to a packet, an unexpected EINVAL (Invalid
>>>      argument) error is obtained. This is due to mac_proto not being reset
>>>      correctly when those actions are validated.
>>>
>>>      Reported-at:
>>>      https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
>>>      Fixes: 91820da6ae85 ("openvswitch: add Ethernet push and pop actions")
>>>      Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
>>>
>>> Reported-at:
>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
>>> Fixes: 6fcecb85ab ("datapath: add Ethernet push and pop actions")
>>> Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
> Applied to master, thanks Jaime and Greg!

Thanks Ben!  It looks like it needs backporting to 2.10 and 2.9 as well.

- Greg
Ben Pfaff Nov. 2, 2018, 11:16 p.m. | #4
On Fri, Nov 02, 2018 at 01:57:08PM -0700, Gregory Rose wrote:
> On 11/2/2018 1:46 PM, Ben Pfaff wrote:
> >On Fri, Nov 02, 2018 at 08:31:06AM -0700, Gregory Rose wrote:
> >>On 11/2/2018 4:45 AM, Jaime Caamaño Ruiz wrote:
> >>>Upstream commit:
> >>>     commit 46ebe2834ba5b541f28ee72e556a3fed42c47570
> >>>     Author: Jaime Caamaño Ruiz <jcaamano@suse.com>
> >>>     Date:   Wed Oct 31 18:52:03 2018 +0100
> >>>
> >>>     openvswitch: Fix push/pop ethernet validation
> >>>
> >>>     When there are both pop and push ethernet header actions among the
> >>>     actions to be applied to a packet, an unexpected EINVAL (Invalid
> >>>     argument) error is obtained. This is due to mac_proto not being reset
> >>>     correctly when those actions are validated.
> >>>
> >>>     Reported-at:
> >>>     https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
> >>>     Fixes: 91820da6ae85 ("openvswitch: add Ethernet push and pop actions")
> >>>     Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
> >>>
> >>>Reported-at:
> >>>https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html
> >>>Fixes: 6fcecb85ab ("datapath: add Ethernet push and pop actions")
> >>>Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
> >Applied to master, thanks Jaime and Greg!
> 
> Thanks Ben!  It looks like it needs backporting to 2.10 and 2.9 as well.

Done.

Patch

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index c3f1baa05..ee0c18422 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -2998,7 +2998,7 @@  static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
 			 * is already present */
 			if (mac_proto != MAC_PROTO_NONE)
 				return -EINVAL;
-			mac_proto = MAC_PROTO_NONE;
+			mac_proto = MAC_PROTO_ETHERNET;
 			break;
 
 		case OVS_ACTION_ATTR_POP_ETH:
@@ -3006,7 +3006,7 @@  static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
 				return -EINVAL;
 			if (vlan_tci & htons(VLAN_TAG_PRESENT))
 				return -EINVAL;
-			mac_proto = MAC_PROTO_ETHERNET;
+			mac_proto = MAC_PROTO_NONE;
 			break;
 
 		case OVS_ACTION_ATTR_PUSH_NSH: