On Thu, May 14, 2015 at 08:10:44PM +0200, Jiri Benc wrote:
> The custom alignment of struct ovs_key_ipv4_tunnel was originally introduced
> by commit 1139e241ec436 ("openvswitch: Compact sw_flow_key."). At that time,
> the size of the structure was not a multiply of 64bit. This is not the case
> anymore with addition of more fields. As this alignment is error prone (see
> the list of conditions in the commit that introduced the alignment) and is
> not needed anymore, remove it.
>
> The __packed attribute added to struct sw_flow_key.eth is useless now, too,
I think you mean sw_flow_key.phy here.
> for the same reason. In fact, there's a 2 byte hole after it with the
> current code.
That's correct.
Patch looks good to me, just the minor nit at the commit log.
fbl
> Also, the padding of the struct ovs_key_ipv4_tunnel became effectively zero
> and
> OVS_TUNNEL_KEY_SIZE together with the memset using it could be removed, too.
> But it is actually important to keep it, as in case new fields are added
> to the structure, the padding needs to be zeroed out. For safety, leave it,
> but document that it may be zero.
>
> Signed-off-by: Jiri Benc <[email protected]>
> ---
> net/openvswitch/flow.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
> index a076e445ccc2..2af6ffbf2f2e 100644
> --- a/net/openvswitch/flow.h
> +++ b/net/openvswitch/flow.h
> @@ -35,7 +35,7 @@
>
> struct sk_buff;
>
> -/* Used to memset ovs_key_ipv4_tunnel padding. */
> +/* Used to memset ovs_key_ipv4_tunnel padding (if there is any). */
> #define OVS_TUNNEL_KEY_SIZE \
> (offsetof(struct ovs_key_ipv4_tunnel, tp_dst) + \
> FIELD_SIZEOF(struct ovs_key_ipv4_tunnel, tp_dst))
> @@ -49,7 +49,7 @@ struct ovs_key_ipv4_tunnel {
> u8 ipv4_ttl;
> __be16 tp_src;
> __be16 tp_dst;
> -} __packed __aligned(4); /* Minimize padding. */
> +};
>
> struct ovs_tunnel_info {
> struct ovs_key_ipv4_tunnel tunnel;
> @@ -127,7 +127,7 @@ struct sw_flow_key {
> u32 priority; /* Packet QoS priority. */
> u32 skb_mark; /* SKB mark. */
> u16 in_port; /* Input switch port (or DP_MAX_PORTS).
> */
> - } __packed phy; /* Safe when right after 'tun_key'. */
> + } phy;
> u32 ovs_flow_hash; /* Datapath computed hash value. */
> u32 recirc_id; /* Recirculation ID. */
> struct {
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
