[ovs-dev] [PATCH] dpif-netdev: Unwildcard entire odp_port in dpif_netdev_mask_from_nlattrs().

[Jarno Rajahalme]

Looks good to me,

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>

> On Apr 4, 2014, at 8:45 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> One case in the dpif_netdev_mask_from_nlattrs() function accidentally
> wildcarded only a 16-bit subset of the mask's odp_port.  On little-endian
> machines this subset was the lower bits, which happened to work out OK,
> but on big-endian machines this subset was the upper bits, which doesn't
> work and causes a test failure.  (The problem was actually visible in the
> test expected results on little-endian machines, but we had not noticed.)
> 
> This commit unwildcards the whole field, fixing the problem, and updates
> the test expected results to match.
> 
> This fixes the failure of test 732 seen here:
> https://buildd.debian.org/status/fetch.php?pkg=openvswitch&arch=sparc&ver=2.1.0%2Bgit20140325-1&stamp=1396438624
> 
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> lib/dpif-netdev.c     | 10 ++++++++--
> tests/ofproto-dpif.at |  4 ++--
> 2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 5b9bfdb..15cf5c1 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1136,8 +1136,6 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len,
> 
>             return EINVAL;
>         }
> -        /* Force unwildcard the in_port. */
> -        mask->in_port.odp_port = u32_to_odp(UINT32_MAX);
>     } else {
>         enum mf_field_id id;
>         /* No mask key, unwildcard everything except fields whose
> @@ -1156,6 +1154,14 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len,
>         }
>     }
> 
> +    /* Force unwildcard the in_port.
> +     *
> +     * We need to do this even in the case where we unwildcard "everything"
> +     * above because "everything" only includes the 16-bit OpenFlow port number
> +     * mask->in_port.ofp_port, which only covers half of the 32-bit datapath
> +     * port number mask->in_port.odp_port. */
> +    mask->in_port.odp_port = u32_to_odp(UINT32_MAX);
> +
>     return 0;
> }
> 
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index e78788a..484b7a6 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -4119,8 +4119,8 @@ for i in 1 2 3 4; do
> done
> sleep 1
> AT_CHECK([ovs-appctl dpif/dump-flows br0 | STRIP_USED], [0], [dnl
> -skb_priority(0),skb_mark(0),in_port(1/0xffff),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0), packets:3, bytes:180, used:0.0s, actions:2
> -skb_priority(0),skb_mark(0),in_port(1/0xffff),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0), packets:3, bytes:180, used:0.0s, actions:drop
> +skb_priority(0),skb_mark(0),in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0), packets:3, bytes:180, used:0.0s, actions:2
> +skb_priority(0),skb_mark(0),in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0), packets:3, bytes:180, used:0.0s, actions:drop
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> -- 
> 1.8.5.3
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev