[ovs-dev] [RFC PATCH kernel 02/10] openvswitch: remove custom alignment

[Flavio Leitner]

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 <jbenc at redhat.com>
> ---
>  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
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev