Netdev List
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
To: Pravin Shelar <pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>,
	"David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jiri Benc <jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Joe Stringer
	<joestringer-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>,
	Justin Pettit <jpettit-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH] openvswitch: reduce padding in struct sw_flow_key
Date: Fri, 18 Mar 2016 14:34:28 +0100	[thread overview]
Message-ID: <1458308084-268272-1-git-send-email-arnd@arndb.de> (raw)

It's been a while since the last time sw_flow_key was made smaller in
1139e241ec43 ("openvswitch: Compact sw_flow_key."), and it has seen five
patches adding new members since then.

With the current linux-next kernel and gcc-6.0 on ARM, this has tipped
us slightly over the stack frame warning limit of 1024 bytes:

net/openvswitch/datapath.c: In function 'ovs_flow_cmd_set':
net/openvswitch/datapath.c:1202:1: error: the frame size of 1032 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

This slightly rearranges the members in struct sw_flow_key to minimize
the amount of padding we have between them, bringing us again slightly
below the warning limit (checking all files against 128 bytes limit):

datapath.c: In function 'get_flow_actions.constprop':
datapath.c:1083:1: warning: the frame size of 464 bytes is larger than 128 bytes
datapath.c: In function 'ovs_flow_cmd_new':
datapath.c:1061:1: warning: the frame size of 984 bytes is larger than 128 bytes
datapath.c: In function 'ovs_flow_cmd_del':
datapath.c:1336:1: warning: the frame size of 528 bytes is larger than 128 bytes
datapath.c: In function 'ovs_flow_cmd_get':
datapath.c:1261:1: warning: the frame size of 504 bytes is larger than 128 bytes
datapath.c: In function 'ovs_flow_cmd_set':
datapath.c:1202:1: warning: the frame size of 1016 bytes is larger than 128 bytes
datapath.c: In function 'queue_gso_packets':
datapath.c:379:1: warning: the frame size of 472 bytes is larger than 128 bytes
flow_table.c: In function 'masked_flow_lookup':
flow_table.c:489:1: warning: the frame size of 488 bytes is larger than 128 bytes
flow_netlink.c: In function 'validate_and_copy_set_tun':
flow_netlink.c:1994:1: warning: the frame size of 512 bytes is larger than 128 bytes

This means it's still too large really, we just don't warn about it any more,
and will get the warning again once another member is added. My patch is a
band-aid at best, but more work is needed here. One problem is that
ovs_flow_cmd_new and ovs_flow_cmd_set have two copies of struct sw_flow_key on
the stack, one of them nested within sw_flow_mask. If we could reduce that to
a single instance, the stack usage would be cut in half here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 00a93babd06a ("openvswitch: add tunnel protocol to sw_flow_key")
Fixes: c2ac66735870 ("openvswitch: Allow matching on conntrack label")
Fixes: 182e3042e15d ("openvswitch: Allow matching on conntrack mark")
Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
Fixes: 971427f353f3 ("openvswitch: Add recirc and hash action.")
---
 net/openvswitch/flow.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 1d055c559eaf..41d15c50a43f 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -63,7 +63,11 @@ struct sw_flow_key {
 		u32	skb_mark;	/* SKB mark. */
 		u16	in_port;	/* Input switch port (or DP_MAX_PORTS). */
 	} __packed phy; /* Safe when right after 'tun_key'. */
-	u8 tun_proto;			/* Protocol of encapsulating tunnel. */
+	struct {
+		__be16 src;		/* TCP/UDP/SCTP source port. */
+		__be16 dst;		/* TCP/UDP/SCTP destination port. */
+		__be16 flags;		/* TCP flags. */
+	} tp;
 	u32 ovs_flow_hash;		/* Datapath computed hash value.  */
 	u32 recirc_id;			/* Recirculation ID.  */
 	struct {
@@ -83,11 +87,6 @@ struct sw_flow_key {
 			u8     frag;	/* One of OVS_FRAG_TYPE_*. */
 		} ip;
 	};
-	struct {
-		__be16 src;		/* TCP/UDP/SCTP source port. */
-		__be16 dst;		/* TCP/UDP/SCTP destination port. */
-		__be16 flags;		/* TCP flags. */
-	} tp;
 	union {
 		struct {
 			struct {
@@ -114,11 +113,12 @@ struct sw_flow_key {
 	};
 	struct {
 		/* Connection tracking fields. */
-		u16 zone;
 		u32 mark;
+		u16 zone;
 		u8 state;
 		struct ovs_key_ct_labels labels;
 	} ct;
+	u8 tun_proto;			/* Protocol of encapsulating tunnel. */
 
 } __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as longs. */
 
-- 
2.7.0

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

             reply	other threads:[~2016-03-18 13:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-18 13:34 Arnd Bergmann [this message]
     [not found] ` <1458308084-268272-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
2016-03-18 22:22   ` [PATCH] openvswitch: reduce padding in struct sw_flow_key Jesse Gross

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1458308084-268272-1-git-send-email-arnd@arndb.de \
    --to=arnd-r2ngtmty4d4@public.gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
    --cc=dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org \
    --cc=jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=joestringer-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org \
    --cc=jpettit-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox