[PATCH RFC 00/14] shrink skb cb to 44 bytes

[PATCH RFC 00/14] shrink skb cb to 44 bytes

[Florian Westphal]

Following patches shrink all in-tree users of skb->cb[] so that kernel
still builds with skb->cb[] set to 44 bytes.

This would create a 4-byte hole, it would be easy to reorder skb
members so this hole is filled.

pahole dump for vmlinux with allyesconfig (+this patch series):

http://www.strlen.de/fw/pahole.txt.gz

remarks/known issues:
- adds __packet attribute to a few structures.
  Its needed to not have padding at end of the structure, else
  we get build assertion errors even if all members fit into cb[].
- checkpatch isn't happy yet.
- dccp changes are untested (its on my todo list)
- rxrpc change is untested (on todo list).
- wireless changes are untested, I don't own any of the affected hw.

The idea is to figure out what needs to be done to make build_skb() and
friends only touch the first 3 cachelines of the skb on all architectures.

We'd need to reduce skbuff by 40 bytes to achieve this for allyesconfig.

Other sk_buff reduction ideas being worked:

- move truesize to shinfo (which has 4 byte hole)
- turn ->data into offset on 64bit platforms (intrusive, > 1000 files
  affected)
- move pointers that are ususally not needed (nf_bridge, secpath) to the end,
  with flag field that tells us when pointer is valid (so we don't have
  to memset() them unconditionally at allocation time).
- seems we could already to this for the inner header fields since the're only
  valid once ->encapsulation / inner_protocol_type is set.

Comments and more ideas welcome.

[PATCH RFC 01/14] net: gro: shrink napi_gro_cb to fit into hypothetical 44-byte sized skb cb

[Florian Westphal]

>From 48 down to 42 bytes using reordering + bitfields.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/netdevice.h | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5897b4e..7427185 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1914,8 +1914,11 @@ struct napi_gro_cb {
 	/* This indicates where we are processing relative to skb->data. */
 	int data_offset;
 
-	/* This is non-zero if the packet cannot be merged with the new skb. */
-	u16	flush;
+	/* used in skb_gro_receive() slow path */
+	struct sk_buff *last;
+
+	/* Used in ipv6_gro_receive() and foo-over-udp */
+	u16	proto;
 
 	/* Save the IP ID here and check when we get to the transport layer */
 	u16	flush_id;
@@ -1926,11 +1929,11 @@ struct napi_gro_cb {
 	/* Start offset for remote checksum offload */
 	u16	gro_remcsum_start;
 
-	/* jiffies when first packet was created/queued */
-	unsigned long age;
+	/* used to support CHECKSUM_COMPLETE for tunneling protocols */
+	__wsum	csum;
 
-	/* Used in ipv6_gro_receive() and foo-over-udp */
-	u16	proto;
+	/* jiffies when first packet was created/queued */
+	unsigned int age;
 
 	/* This is non-zero if the packet may be of the same flow. */
 	u8	same_flow:1;
@@ -1950,16 +1953,11 @@ struct napi_gro_cb {
 #define NAPI_GRO_FREE_STOLEN_HEAD 2
 
 	/* Used in foo-over-udp, set in udp[46]_gro_receive */
-	u8	is_ipv6:1;
-
-	/* 7 bit hole */
+	u8	is_ipv6:1,
+		flush:1;
 
-	/* used to support CHECKSUM_COMPLETE for tunneling protocols */
-	__wsum	csum;
-
-	/* used in skb_gro_receive() slow path */
-	struct sk_buff *last;
-};
+	/* 6 bit hole */
+} __packed;
 
 #define NAPI_GRO_CB(skb) ((struct napi_gro_cb *)(skb)->cb)
 
-- 
2.0.5

[PATCH RFC 02/14] net: sched: reduce qdisc size to 24 byte

[Florian Westphal]

Its fine as-is, BUT infiniband cb includes qdisc_skb_cb
as first member so it can preserve its cb even if skb is
enqueued in a packet scheduler.

And I haven't found a way to change infiniband so that
it doesn't have to do that yet.
The infiniband cb is currently exactly 48 bytes, reducing
qdisc_cb also reduces ipoib_cb struct to 44 bytes as a result.

Reducing qdisc_cb means we can no longer use flow_keys struct
in sfq, but thats easy to work around, we don't need all its fields.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/sch_generic.h |  2 +-
 net/sched/sch_sfq.c       | 29 ++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index c605d30..baf7e96 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -252,7 +252,7 @@ struct qdisc_skb_cb {
 	unsigned int		pkt_len;
 	u16			slave_dev_queue_mapping;
 	u16			_pad;
-#define QDISC_CB_PRIV_LEN 20
+#define QDISC_CB_PRIV_LEN 16
 	unsigned char		data[QDISC_CB_PRIV_LEN];
 };
 
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index b877140..06485e8 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -158,10 +158,18 @@ static inline struct sfq_head *sfq_dep_head(struct sfq_sched_data *q, sfq_index
 
 /*
  * In order to be able to quickly rehash our queue when timer changes
- * q->perturbation, we store flow_keys in skb->cb[]
+ * q->perturbation, we store a few flow keys in skb->cb[]
+ *
+ * We're not using flow_key directly due to cb[] size limitation.
  */
+struct sfq_flow_keys {
+	__be32 src;
+	__be32 dst;
+	__be32 ports;
+};
+
 struct sfq_skb_cb {
-       struct flow_keys        keys;
+	struct sfq_flow_keys keys;
 };
 
 static inline struct sfq_skb_cb *sfq_skb_cb(const struct sk_buff *skb)
@@ -173,15 +181,26 @@ static inline struct sfq_skb_cb *sfq_skb_cb(const struct sk_buff *skb)
 static unsigned int sfq_hash(const struct sfq_sched_data *q,
 			     const struct sk_buff *skb)
 {
-	const struct flow_keys *keys = &sfq_skb_cb(skb)->keys;
+	const struct sfq_flow_keys *keys = &sfq_skb_cb(skb)->keys;
 	unsigned int hash;
 
 	hash = jhash_3words((__force u32)keys->dst,
-			    (__force u32)keys->src ^ keys->ip_proto,
+			    (__force u32)keys->src,
 			    (__force u32)keys->ports, q->perturbation);
 	return hash & (q->divisor - 1);
 }
 
+static void sfq_flow_dissect(const struct sk_buff *skb, struct sfq_flow_keys *flow)
+{
+	struct flow_keys flowk;
+
+	if (skb_flow_dissect(skb, &flowk)) {
+		flow->src = flowk.src ^ flowk.ip_proto,
+		flow->dst = flowk.dst;
+		flow->ports = flowk.ports;
+	}
+}
+
 static unsigned int sfq_classify(struct sk_buff *skb, struct Qdisc *sch,
 				 int *qerr)
 {
@@ -197,7 +216,7 @@ static unsigned int sfq_classify(struct sk_buff *skb, struct Qdisc *sch,
 
 	fl = rcu_dereference_bh(q->filter_list);
 	if (!fl) {
-		skb_flow_dissect(skb, &sfq_skb_cb(skb)->keys);
+		sfq_flow_dissect(skb, &sfq_skb_cb(skb)->keys);
 		return sfq_hash(q, skb) + 1;
 	}
 
-- 
2.0.5

[PATCH RFC 03/14] ipv6: use flag instead of u16 for hop in inet6_skb_parm

[Florian Westphal]

reduces size by 2 bytes.  hop was always either 0 or sizeof(struct ipv6hdr).

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/ipv6.h | 2 +-
 net/ipv6/af_inet6.c  | 4 ++--
 net/ipv6/datagram.c  | 8 ++++----
 net/ipv6/exthdrs.c   | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 4d5169f..61b9169 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -90,7 +90,6 @@ static inline struct ipv6hdr *ipipv6_hdr(const struct sk_buff *skb)
 struct inet6_skb_parm {
 	int			iif;
 	__be16			ra;
-	__u16			hop;
 	__u16			dst0;
 	__u16			srcrt;
 	__u16			dst1;
@@ -107,6 +106,7 @@ struct inet6_skb_parm {
 #define IP6SKB_REROUTED		4
 #define IP6SKB_ROUTERALERT	8
 #define IP6SKB_FRAGMENTED      16
+#define IP6SKB_HOPBYHOP        32
 };
 
 #define IP6CB(skb)	((struct inet6_skb_parm*)((skb)->cb))
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 6bafcc2..b96dd8d 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -678,8 +678,8 @@ bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb,
 	const struct ipv6_pinfo *np = inet6_sk(sk);
 
 	if (np->rxopt.all) {
-		if ((opt->hop && (np->rxopt.bits.hopopts ||
-				  np->rxopt.bits.ohopopts)) ||
+		if (((opt->flags & IP6SKB_HOPBYHOP) &&
+		     (np->rxopt.bits.hopopts || np->rxopt.bits.ohopopts)) ||
 		    (ip6_flowinfo((struct ipv6hdr *) skb_network_header(skb)) &&
 		     np->rxopt.bits.rxflow) ||
 		    (opt->srcrt && (np->rxopt.bits.srcrt ||
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index c215be7..8b6391e 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -531,8 +531,8 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 	}
 
 	/* HbH is allowed only once */
-	if (np->rxopt.bits.hopopts && opt->hop) {
-		u8 *ptr = nh + opt->hop;
+	if (np->rxopt.bits.hopopts && (opt->flags & IP6SKB_HOPBYHOP)) {
+		u8 *ptr = nh + sizeof(struct ipv6hdr);
 		put_cmsg(msg, SOL_IPV6, IPV6_HOPOPTS, (ptr[1]+1)<<3, ptr);
 	}
 
@@ -593,8 +593,8 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 		int hlim = ipv6_hdr(skb)->hop_limit;
 		put_cmsg(msg, SOL_IPV6, IPV6_2292HOPLIMIT, sizeof(hlim), &hlim);
 	}
-	if (np->rxopt.bits.ohopopts && opt->hop) {
-		u8 *ptr = nh + opt->hop;
+	if (np->rxopt.bits.ohopopts && (opt->flags & IP6SKB_HOPBYHOP)) {
+		u8 *ptr = nh + sizeof(struct ipv6hdr);
 		put_cmsg(msg, SOL_IPV6, IPV6_2292HOPOPTS, (ptr[1]+1)<<3, ptr);
 	}
 	if (np->rxopt.bits.odstopts && opt->dst0) {
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index a7bbbe4..ce203b0 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -632,7 +632,7 @@ int ipv6_parse_hopopts(struct sk_buff *skb)
 		return -1;
 	}
 
-	opt->hop = sizeof(struct ipv6hdr);
+	opt->flags |= IP6SKB_HOPBYHOP;
 	if (ip6_parse_tlv(tlvprochopopt_lst, skb)) {
 		skb->transport_header += (skb_transport_header(skb)[1] + 1) << 3;
 		opt = IP6CB(skb);
-- 
2.0.5

[PATCH RFC 04/14] drivers: wireless: rt2x00: move skb_dma to queue entry

[Florian Westphal]

... to shrink size needed for the skb frame desc.

Note that this is just the lowest hanging fruit I found, if you find
better/other means to shrink the space needed in skb->cb
(or to shrink it even further) -- great.

Cc: linux-wireless@vger.kernel.org
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 drivers/net/wireless/rt2x00/rt2400pci.c   |  5 ++---
 drivers/net/wireless/rt2x00/rt2500pci.c   |  5 ++---
 drivers/net/wireless/rt2x00/rt2800mmio.c  |  7 +++----
 drivers/net/wireless/rt2x00/rt2x00queue.c | 10 +++++-----
 drivers/net/wireless/rt2x00/rt2x00queue.h | 17 ++++++++---------
 drivers/net/wireless/rt2x00/rt61pci.c     |  5 ++---
 6 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index bdf5590..5c7927c 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -739,7 +739,6 @@ static bool rt2400pci_get_entry_state(struct queue_entry *entry)
 static void rt2400pci_clear_entry(struct queue_entry *entry)
 {
 	struct queue_entry_priv_mmio *entry_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 	u32 word;
 
 	if (entry->queue->qid == QID_RX) {
@@ -748,7 +747,7 @@ static void rt2400pci_clear_entry(struct queue_entry *entry)
 		rt2x00_desc_write(entry_priv->desc, 2, word);
 
 		rt2x00_desc_read(entry_priv->desc, 1, &word);
-		rt2x00_set_field32(&word, RXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
+		rt2x00_set_field32(&word, RXD_W1_BUFFER_ADDRESS, entry->skb_dma);
 		rt2x00_desc_write(entry_priv->desc, 1, word);
 
 		rt2x00_desc_read(entry_priv->desc, 0, &word);
@@ -1111,7 +1110,7 @@ static void rt2400pci_write_tx_desc(struct queue_entry *entry,
 	 * Start writing the descriptor words.
 	 */
 	rt2x00_desc_read(txd, 1, &word);
-	rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
+	rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, entry->skb_dma);
 	rt2x00_desc_write(txd, 1, word);
 
 	rt2x00_desc_read(txd, 2, &word);
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 79f4fe6..907d7c2 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -828,12 +828,11 @@ static bool rt2500pci_get_entry_state(struct queue_entry *entry)
 static void rt2500pci_clear_entry(struct queue_entry *entry)
 {
 	struct queue_entry_priv_mmio *entry_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 	u32 word;
 
 	if (entry->queue->qid == QID_RX) {
 		rt2x00_desc_read(entry_priv->desc, 1, &word);
-		rt2x00_set_field32(&word, RXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
+		rt2x00_set_field32(&word, RXD_W1_BUFFER_ADDRESS, entry->skb_dma);
 		rt2x00_desc_write(entry_priv->desc, 1, word);
 
 		rt2x00_desc_read(entry_priv->desc, 0, &word);
@@ -1264,7 +1263,7 @@ static void rt2500pci_write_tx_desc(struct queue_entry *entry,
 	 * Start writing the descriptor words.
 	 */
 	rt2x00_desc_read(txd, 1, &word);
-	rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
+	rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, entry->skb_dma);
 	rt2x00_desc_write(txd, 1, word);
 
 	rt2x00_desc_read(txd, 2, &word);
diff --git a/drivers/net/wireless/rt2x00/rt2800mmio.c b/drivers/net/wireless/rt2x00/rt2800mmio.c
index de4790b..45772a1 100644
--- a/drivers/net/wireless/rt2x00/rt2800mmio.c
+++ b/drivers/net/wireless/rt2x00/rt2800mmio.c
@@ -66,7 +66,7 @@ void rt2800mmio_write_tx_desc(struct queue_entry *entry,
 	 * Initialize TX descriptor
 	 */
 	word = 0;
-	rt2x00_set_field32(&word, TXD_W0_SD_PTR0, skbdesc->skb_dma);
+	rt2x00_set_field32(&word, TXD_W0_SD_PTR0, entry->skb_dma);
 	rt2x00_desc_write(txd, 0, word);
 
 	word = 0;
@@ -82,7 +82,7 @@ void rt2800mmio_write_tx_desc(struct queue_entry *entry,
 
 	word = 0;
 	rt2x00_set_field32(&word, TXD_W2_SD_PTR1,
-			   skbdesc->skb_dma + txwi_size);
+			   entry->skb_dma + txwi_size);
 	rt2x00_desc_write(txd, 2, word);
 
 	word = 0;
@@ -710,13 +710,12 @@ EXPORT_SYMBOL_GPL(rt2800mmio_get_entry_state);
 void rt2800mmio_clear_entry(struct queue_entry *entry)
 {
 	struct queue_entry_priv_mmio *entry_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
 	u32 word;
 
 	if (entry->queue->qid == QID_RX) {
 		rt2x00_desc_read(entry_priv->desc, 0, &word);
-		rt2x00_set_field32(&word, RXD_W0_SDP0, skbdesc->skb_dma);
+		rt2x00_set_field32(&word, RXD_W0_SDP0, entry->skb_dma);
 		rt2x00_desc_write(entry_priv->desc, 0, word);
 
 		rt2x00_desc_read(entry_priv->desc, 1, &word);
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index 68b620b..6c79266 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -95,7 +95,7 @@ struct sk_buff *rt2x00queue_alloc_rxskb(struct queue_entry *entry, gfp_t gfp)
 			return NULL;
 		}
 
-		skbdesc->skb_dma = skb_dma;
+		entry->skb_dma = skb_dma;
 		skbdesc->flags |= SKBDESC_DMA_MAPPED_RX;
 	}
 
@@ -107,10 +107,10 @@ int rt2x00queue_map_txskb(struct queue_entry *entry)
 	struct device *dev = entry->queue->rt2x00dev->dev;
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 
-	skbdesc->skb_dma =
+	entry->skb_dma =
 	    dma_map_single(dev, entry->skb->data, entry->skb->len, DMA_TO_DEVICE);
 
-	if (unlikely(dma_mapping_error(dev, skbdesc->skb_dma)))
+	if (unlikely(dma_mapping_error(dev, entry->skb_dma)))
 		return -ENOMEM;
 
 	skbdesc->flags |= SKBDESC_DMA_MAPPED_TX;
@@ -124,11 +124,11 @@ void rt2x00queue_unmap_skb(struct queue_entry *entry)
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 
 	if (skbdesc->flags & SKBDESC_DMA_MAPPED_RX) {
-		dma_unmap_single(dev, skbdesc->skb_dma, entry->skb->len,
+		dma_unmap_single(dev, entry->skb_dma, entry->skb->len,
 				 DMA_FROM_DEVICE);
 		skbdesc->flags &= ~SKBDESC_DMA_MAPPED_RX;
 	} else if (skbdesc->flags & SKBDESC_DMA_MAPPED_TX) {
-		dma_unmap_single(dev, skbdesc->skb_dma, entry->skb->len,
+		dma_unmap_single(dev, entry->skb_dma, entry->skb->len,
 				 DMA_TO_DEVICE);
 		skbdesc->flags &= ~SKBDESC_DMA_MAPPED_TX;
 	}
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.h b/drivers/net/wireless/rt2x00/rt2x00queue.h
index 2233b91..e6f6a2e 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.h
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.h
@@ -101,23 +101,19 @@ enum skb_frame_desc_flags {
  *	Note that this pointer could point to something outside
  *	of the scope of the skb->data pointer.
  * @iv: IV/EIV data used during encryption/decryption.
- * @skb_dma: (PCI-only) the DMA address associated with the sk buffer.
  * @entry: The entry to which this sk buffer belongs.
  */
 struct skb_frame_desc {
-	u8 flags;
-
-	u8 desc_len;
-	u8 tx_rate_idx;
-	u8 tx_rate_flags;
-
+	struct queue_entry *entry;
 	void *desc;
 
 	__le32 iv[2];
 
-	dma_addr_t skb_dma;
+	u8 flags;
 
-	struct queue_entry *entry;
+	u8 desc_len;
+	u8 tx_rate_idx;
+	u8 tx_rate_flags;
 };
 
 /**
@@ -372,6 +368,7 @@ enum queue_entry_flags {
  * @entry_idx: The entry index number.
  * @priv_data: Private data belonging to this queue entry. The pointer
  *	points to data specific to a particular driver and queue type.
+ * @skb_dma: (PCI-only) the DMA address associated with the sk buffer.
  * @status: Device specific status
  */
 struct queue_entry {
@@ -386,6 +383,8 @@ struct queue_entry {
 
 	u32 status;
 
+	dma_addr_t skb_dma;
+
 	void *priv_data;
 };
 
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 8194550..a0c8de1 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -1397,13 +1397,12 @@ static bool rt61pci_get_entry_state(struct queue_entry *entry)
 static void rt61pci_clear_entry(struct queue_entry *entry)
 {
 	struct queue_entry_priv_mmio *entry_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 	u32 word;
 
 	if (entry->queue->qid == QID_RX) {
 		rt2x00_desc_read(entry_priv->desc, 5, &word);
 		rt2x00_set_field32(&word, RXD_W5_BUFFER_PHYSICAL_ADDRESS,
-				   skbdesc->skb_dma);
+				   entry->skb_dma);
 		rt2x00_desc_write(entry_priv->desc, 5, word);
 
 		rt2x00_desc_read(entry_priv->desc, 0, &word);
@@ -1913,7 +1912,7 @@ static void rt61pci_write_tx_desc(struct queue_entry *entry,
 	if (entry->queue->qid != QID_BEACON) {
 		rt2x00_desc_read(txd, 6, &word);
 		rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
-				   skbdesc->skb_dma);
+				   entry->skb_dma);
 		rt2x00_desc_write(txd, 6, word);
 
 		rt2x00_desc_read(txd, 11, &word);
-- 
2.0.5

[PATCH RFC 05/14] drivers: wireless: ar5523: use container_of

[Florian Westphal]

Compile tested only due to lack of hw.

If we want to shrink skb->cb then we'd have to see about
reducing struct ieee80211_tx_info, which gets embedded inside
skb->cb[].

It provides a scratch space to be used by wireless drivers.
ar5523 uses the maximum space available today (40 bytes), but it seems
we don't need this -- data->skb pointer seems to always point back to the
skb whose cb buffer the data structure resides, iow, given a pointer to the
embedded control buffer we can infer the skb address.

Cc: linux-wireless@vger.kernel.org
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 drivers/net/wireless/ath/ar5523/ar5523.c | 8 ++++----
 drivers/net/wireless/ath/ar5523/ar5523.h | 1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ar5523/ar5523.c b/drivers/net/wireless/ath/ar5523/ar5523.c
index f920506..7410ac0 100644
--- a/drivers/net/wireless/ath/ar5523/ar5523.c
+++ b/drivers/net/wireless/ath/ar5523/ar5523.c
@@ -779,8 +779,6 @@ static void ar5523_tx(struct ieee80211_hw *hw,
 		ieee80211_stop_queues(hw);
 	}
 
-	data->skb = skb;
-
 	spin_lock_irqsave(&ar->tx_data_list_lock, flags);
 	list_add_tail(&data->list, &ar->tx_queue_pending);
 	spin_unlock_irqrestore(&ar->tx_data_list_lock, flags);
@@ -817,10 +815,12 @@ static void ar5523_tx_work_locked(struct ar5523 *ar)
 		if (!data)
 			break;
 
-		skb = data->skb;
+		txi = container_of((void *)data, struct ieee80211_tx_info, driver_data);
 		txqid = 0;
-		txi = IEEE80211_SKB_CB(skb);
+
+		skb = container_of((void *) txi, struct sk_buff, cb);
 		paylen = skb->len;
+
 		urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (!urb) {
 			ar5523_err(ar, "Failed to allocate TX urb\n");
diff --git a/drivers/net/wireless/ath/ar5523/ar5523.h b/drivers/net/wireless/ath/ar5523/ar5523.h
index 00c6fd3..9a322a6 100644
--- a/drivers/net/wireless/ath/ar5523/ar5523.h
+++ b/drivers/net/wireless/ath/ar5523/ar5523.h
@@ -74,7 +74,6 @@ struct ar5523_tx_cmd {
 struct ar5523_tx_data {
 	struct list_head	list;
 	struct ar5523		*ar;
-	struct sk_buff		*skb;
 	struct urb		*urb;
 };
 
-- 
2.0.5

[PATCH RFC 06/14] drivers: wireless: carl9170: shrink carl9170_tx_info

[Florian Westphal]

compile tested only.

its embededded inside rate_driver_data of the ieee80211_tx_info struct,
which in turn is stored in skb->cb[].

In order to shrink cb, we need to shrink ieee80211_tx_info
which means to downsize the users.

Alternatively, one might be able to remove kref but
its less intrusive/simpler to use a u32 for timeout handling.

Cc: linux-wireless@vger.kernel.org
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 drivers/net/wireless/ath/carl9170/carl9170.h |  4 ++--
 drivers/net/wireless/ath/carl9170/debug.c    |  2 +-
 drivers/net/wireless/ath/carl9170/tx.c       | 22 +++++++++++++++++-----
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/carl9170.h b/drivers/net/wireless/ath/carl9170/carl9170.h
index 237d0cd..a785300 100644
--- a/drivers/net/wireless/ath/carl9170/carl9170.h
+++ b/drivers/net/wireless/ath/carl9170/carl9170.h
@@ -491,9 +491,9 @@ struct carl9170_sta_info {
 };
 
 struct carl9170_tx_info {
-	unsigned long timeout;
-	struct ar9170 *ar;
+	u32 timeout32;
 	struct kref ref;
+	struct ar9170 *ar;
 };
 
 #define CHK_DEV_STATE(a, s)	(((struct ar9170 *)a)->state >= (s))
diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c
index 6808db4..d00ab9d 100644
--- a/drivers/net/wireless/ath/carl9170/debug.c
+++ b/drivers/net/wireless/ath/carl9170/debug.c
@@ -291,7 +291,7 @@ static void carl9170_debugfs_format_frame(struct ar9170 *ar,
 	    "pc:%.8x, to:%d ms\n", prefix, skb, txc->s.cookie,
 	    ieee80211_get_DA(hdr), get_seq_h(hdr),
 	    le16_to_cpu(txc->f.mac_control), le32_to_cpu(txc->f.phy_control),
-	    jiffies_to_msecs(jiffies - arinfo->timeout));
+	    jiffies_to_msecs(((u32)jiffies) - arinfo->timeout32));
 }
 
 
diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c
index ae86a600..85687aaf4 100644
--- a/drivers/net/wireless/ath/carl9170/tx.c
+++ b/drivers/net/wireless/ath/carl9170/tx.c
@@ -555,6 +555,18 @@ static void carl9170_tx_fill_rateinfo(struct ar9170 *ar, unsigned int rix,
 	}
 }
 
+static inline bool carl9170_time_is_before_jiffies(u32 timeout)
+{
+	u32 now = (u32) jiffies;
+	return (s32)(now - timeout) >= 0;
+}
+
+static inline bool carl9170_time_is_after_jiffies(u32 timeout)
+{
+	u32 now = (u32) jiffies;
+	return (s32)(now - timeout) < 0;
+}
+
 static void carl9170_check_queue_stop_timeout(struct ar9170 *ar)
 {
 	int i;
@@ -574,8 +586,8 @@ static void carl9170_check_queue_stop_timeout(struct ar9170 *ar)
 		txinfo = IEEE80211_SKB_CB(skb);
 		arinfo = (void *) txinfo->rate_driver_data;
 
-		if (time_is_before_jiffies(arinfo->timeout +
-		    msecs_to_jiffies(CARL9170_QUEUE_STUCK_TIMEOUT)) == true)
+		if (carl9170_time_is_before_jiffies(arinfo->timeout32 +
+		    (u32)msecs_to_jiffies(CARL9170_QUEUE_STUCK_TIMEOUT)) == true)
 			restart = true;
 
 next:
@@ -620,7 +632,7 @@ static void carl9170_tx_ampdu_timeout(struct ar9170 *ar)
 
 		txinfo = IEEE80211_SKB_CB(skb);
 		arinfo = (void *)txinfo->rate_driver_data;
-		if (time_is_after_jiffies(arinfo->timeout +
+		if (carl9170_time_is_after_jiffies(arinfo->timeout32 +
 		    msecs_to_jiffies(CARL9170_QUEUE_TIMEOUT)))
 			goto unlock;
 
@@ -1066,7 +1078,7 @@ static int carl9170_tx_prepare(struct ar9170 *ar,
 	txc->f.mac_control = mac_tmp;
 
 	arinfo = (void *)info->rate_driver_data;
-	arinfo->timeout = jiffies;
+	arinfo->timeout32 = (u32) jiffies;
 	arinfo->ar = ar;
 	kref_init(&arinfo->ref);
 	return 0;
@@ -1259,7 +1271,7 @@ static struct sk_buff *carl9170_tx_pick_skb(struct ar9170 *ar,
 	info = IEEE80211_SKB_CB(skb);
 	arinfo = (void *) info->rate_driver_data;
 
-	arinfo->timeout = jiffies;
+	arinfo->timeout32 = (u32) jiffies;
 	return skb;
 
 err_unlock:
-- 
2.0.5

[PATCH RFC 07/14] net: wireless: iwlwifi: shrink status private area

[Florian Westphal]

iwlwifi is only driver to use status_driver_data, and it only wants to
store u8 counter.

This is one patch needed to reduce size of ieee80211_tx_info,
which in turn is needed to allow reducing size of skb->cb[].

Cc: linux-wireless@vger.kernel.org
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 drivers/net/wireless/iwlwifi/mvm/rs.c | 2 +-
 drivers/net/wireless/iwlwifi/mvm/tx.c | 8 +++-----
 include/net/mac80211.h                | 2 +-
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/mvm/rs.c b/drivers/net/wireless/iwlwifi/mvm/rs.c
index 194bd1f..e29092b 100644
--- a/drivers/net/wireless/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/iwlwifi/mvm/rs.c
@@ -1054,7 +1054,7 @@ void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 	u32 ucode_rate;
 	struct rs_rate rate;
 	struct iwl_scale_tbl_info *curr_tbl, *other_tbl, *tmp_tbl;
-	u8 reduced_txp = (uintptr_t)info->status.status_driver_data[0];
+	u8 reduced_txp = (u8)info->status.status_driver_data[0];
 	struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
 	struct iwl_lq_sta *lq_sta = &mvmsta->lq_sta;
 
diff --git a/drivers/net/wireless/iwlwifi/mvm/tx.c b/drivers/net/wireless/iwlwifi/mvm/tx.c
index 07304e1..450635f 100644
--- a/drivers/net/wireless/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/iwlwifi/mvm/tx.c
@@ -683,9 +683,8 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
 		 */
 		info->status.tx_time =
 			le16_to_cpu(tx_resp->wireless_media_time);
-		BUILD_BUG_ON(ARRAY_SIZE(info->status.status_driver_data) < 1);
-		info->status.status_driver_data[0] =
-				(void *)(uintptr_t)tx_resp->reduced_tpc;
+		BUILD_BUG_ON(sizeof(info->status.status_driver_data) < 1);
+		info->status.status_driver_data[0] = tx_resp->reduced_tpc;
 
 		ieee80211_tx_status(mvm->hw, skb);
 	}
@@ -907,8 +906,7 @@ static void iwl_mvm_tx_info_from_ba_notif(struct ieee80211_tx_info *info,
 				    info);
 	/* TODO: not accounted if the whole A-MPDU failed */
 	info->status.tx_time = tid_data->tx_time;
-	info->status.status_driver_data[0] =
-		(void *)(uintptr_t)tid_data->reduced_tpc;
+	info->status.status_driver_data[0] = tid_data->reduced_tpc;
 }
 
 int iwl_mvm_rx_ba_notif(struct iwl_mvm *mvm, struct iwl_rx_cmd_buffer *rxb,
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d52914b..63c3708 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -755,7 +755,7 @@ struct ieee80211_tx_info {
 			u8 ampdu_len;
 			u8 antenna;
 			u16 tx_time;
-			void *status_driver_data[19 / sizeof(void *)];
+			u8 status_driver_data[1];
 		} status;
 		struct {
 			struct ieee80211_tx_rate driver_rates[
-- 
2.0.5

[PATCH RFC 08/14] net: wireless: mac80211: shrink ieee80211_tx_info

[Florian Westphal]

to make it fit into (future) 44-byte sized skb->cb[].

This works, since flags is only used to store values
from mac80211_tx_control_flags enum, and these are just 2 bits.
We can thus move this to the padding hole inside the union.

Also add BUILD_BUG_ON magic to make sure that the new flags
field doesn't share storage w. other members of the union.

Cc: linux-wireless@vger.kernel.org
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/mac80211.h | 11 ++++++++---
 net/mac80211/main.c    |  3 +++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 63c3708..36c2599 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -737,16 +737,21 @@ struct ieee80211_tx_info {
 					u8 use_cts_prot:1;
 					u8 short_preamble:1;
 					u8 skip_table:1;
-					/* 2 bytes free */
+					enum mac80211_tx_control_flags flags:2;
+					/* used for BUILD_BUG_ON validation that ->flags won't
+					 * overlap with jiffies below */
+					char flags_end[0];
 				};
 				/* only needed before rate control */
 				unsigned long jiffies;
+
+				/* used for BUILD_BUG_ON validation that ->flags won't
+				 * overlap with other members of this union. */
+				char union_end[0];
 			};
 			/* NB: vif can be NULL for injected frames */
 			struct ieee80211_vif *vif;
 			struct ieee80211_key_conf *hw_key;
-			u32 flags;
-			/* 4 bytes free */
 		} control;
 		struct {
 			struct ieee80211_tx_rate rates[IEEE80211_TX_MAX_RATES];
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 5e09d35..9e8c807 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1218,6 +1218,9 @@ static int __init ieee80211_init(void)
 	BUILD_BUG_ON(offsetof(struct ieee80211_tx_info, driver_data) +
 		     IEEE80211_TX_INFO_DRIVER_DATA_SIZE > sizeof(skb->cb));
 
+	BUILD_BUG_ON(offsetof(struct ieee80211_tx_info, control.flags_end) <
+		     offsetof(struct ieee80211_tx_info, control.union_end));
+
 	ret = rc80211_minstrel_init();
 	if (ret)
 		return ret;
-- 
2.0.5

[PATCH RFC 09/14] net: wireless: mac80211: shrink private driver area

[Florian Westphal]

This makes the structure fit 44-byte sized skb->cb[].

Cc: linux-wireless@vger.kernel.org
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 drivers/net/wireless/ath/ath9k/ath9k.h | 2 +-
 include/net/mac80211.h                 | 8 +++-----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 0f8e946..2ed77cf 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -184,12 +184,12 @@ struct ath_frame_info {
 	struct ath_buf *bf;
 	u16 framelen;
 	s8 txq;
-	enum ath9k_key_type keytype;
 	u8 keyix;
 	u8 rtscts_rate;
 	u8 retries : 7;
 	u8 baw_tracked : 1;
 	u8 tx_power;
+	enum ath9k_key_type keytype:2;
 };
 
 struct ath_rxbuf {
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 36c2599..372c25a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -627,11 +627,11 @@ enum mac80211_rate_control_flags {
 };
 
 
-/* there are 40 bytes if you don't need the rateset to be kept */
-#define IEEE80211_TX_INFO_DRIVER_DATA_SIZE 40
+/* there are 32 bytes if you don't need the rateset to be kept */
+#define IEEE80211_TX_INFO_DRIVER_DATA_SIZE 32
 
 /* if you do need the rateset, then you have less space */
-#define IEEE80211_TX_INFO_RATE_DRIVER_DATA_SIZE 24
+#define IEEE80211_TX_INFO_RATE_DRIVER_DATA_SIZE 16
 
 /* maximum number of rate stages */
 #define IEEE80211_TX_MAX_RATES	4
@@ -765,8 +765,6 @@ struct ieee80211_tx_info {
 		struct {
 			struct ieee80211_tx_rate driver_rates[
 				IEEE80211_TX_MAX_RATES];
-			u8 pad[4];
-
 			void *rate_driver_data[
 				IEEE80211_TX_INFO_RATE_DRIVER_DATA_SIZE / sizeof(void *)];
 		};
-- 
2.0.5

[PATCH RFC 10/14] dccp: keep failed options on stack

[Florian Westphal]

dccp_skb_cb uses all available skb->cb size.

To eventually reduce skb->cb we thus need to shrink the existing users.
Keeping this state (only needed in rare case of invalid option) on the
stack would reduce dccp_skb_cb by 4 bytes.  This change doesn't alter
dccp_skb_cb size due to padding; we'll remove the padding in next patch.

Cc: dccp@vger.kernel.org
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/dccp.h |  9 ++++++++-
 net/dccp/dccp.h      | 26 ++++++++++++++------------
 net/dccp/input.c     | 17 ++++++++++-------
 net/dccp/ipv4.c      | 29 +++++++++++++++++++----------
 net/dccp/ipv6.c      | 29 +++++++++++++++++++----------
 net/dccp/minisocks.c |  9 +++++----
 net/dccp/options.c   | 10 ++++++----
 net/dccp/output.c    | 29 ++++++++++++++++-------------
 8 files changed, 97 insertions(+), 61 deletions(-)

diff --git a/include/linux/dccp.h b/include/linux/dccp.h
index 439ff69..8806848 100644
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -166,6 +166,13 @@ struct dccp_request_sock {
 	__u32			 dreq_timestamp_time;
 };
 
+/* dccp_reset_state - Temporary information for reset generation
+ * @dccpd_reset_data: Data1..3 fields (depend on @dccpd_reset_code)
+ */
+struct dccp_reset_state {
+	__u8 dccpd_reset_data[3];
+};
+
 static inline struct dccp_request_sock *dccp_rsk(const struct request_sock *req)
 {
 	return (struct dccp_request_sock *)req;
@@ -174,7 +181,7 @@ static inline struct dccp_request_sock *dccp_rsk(const struct request_sock *req)
 extern struct inet_timewait_death_row dccp_death_row;
 
 extern int dccp_parse_options(struct sock *sk, struct dccp_request_sock *dreq,
-			      struct sk_buff *skb);
+			      struct sk_buff *skb, struct dccp_reset_state *rst);
 
 struct dccp_options_received {
 	u64	dccpor_ndp:48;
diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
index e4c144f..520c566 100644
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -281,14 +281,16 @@ struct sock *dccp_v4_request_recv_sock(struct sock *sk, struct sk_buff *skb,
 				       struct dst_entry *dst);
 struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
 			    struct request_sock *req,
-			    struct request_sock **prev);
+			    struct request_sock **prev,
+			    struct dccp_reset_state *rst);
 
 int dccp_child_process(struct sock *parent, struct sock *child,
-		       struct sk_buff *skb);
+		       struct sk_buff *skb, struct dccp_reset_state *rst);
 int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
-			   struct dccp_hdr *dh, unsigned int len);
+			   struct dccp_hdr *dh, unsigned int len, struct dccp_reset_state *rst);
 int dccp_rcv_established(struct sock *sk, struct sk_buff *skb,
-			 const struct dccp_hdr *dh, const unsigned int len);
+			 const struct dccp_hdr *dh, const unsigned int len,
+			 struct dccp_reset_state *rst);
 
 int dccp_init_sock(struct sock *sk, const __u8 ctl_sock_initialized);
 void dccp_destroy_sock(struct sock *sk);
@@ -321,7 +323,8 @@ unsigned int dccp_poll(struct file *file, struct socket *sock,
 		       poll_table *wait);
 int dccp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len);
 
-struct sk_buff *dccp_ctl_make_reset(struct sock *sk, struct sk_buff *skb);
+struct sk_buff *dccp_ctl_make_reset(struct sock *sk, struct sk_buff *skb,
+				    const struct dccp_reset_state *rst);
 int dccp_send_reset(struct sock *sk, enum dccp_reset_codes code);
 void dccp_send_close(struct sock *sk, const int active);
 int dccp_invalid_packet(struct sk_buff *skb);
@@ -339,13 +342,12 @@ static inline int dccp_bad_service_code(const struct sock *sk,
 
 /**
  * dccp_skb_cb  -  DCCP per-packet control information
+ * @dccpd_seq: sequence number
+ * @dccpd_ack_seq: acknowledgment number subheader field value
  * @dccpd_type: one of %dccp_pkt_type (or unknown)
  * @dccpd_ccval: CCVal field (5.1), see e.g. RFC 4342, 8.1
  * @dccpd_reset_code: one of %dccp_reset_codes
- * @dccpd_reset_data: Data1..3 fields (depend on @dccpd_reset_code)
  * @dccpd_opt_len: total length of all options (5.8) in the packet
- * @dccpd_seq: sequence number
- * @dccpd_ack_seq: acknowledgment number subheader field value
  *
  * This is used for transmission as well as for reception.
  */
@@ -356,13 +358,13 @@ struct dccp_skb_cb {
 		struct inet6_skb_parm	h6;
 #endif
 	} header;
+	__u64 dccpd_seq;
+	__u64 dccpd_ack_seq;
+
 	__u8  dccpd_type:4;
 	__u8  dccpd_ccval:4;
-	__u8  dccpd_reset_code,
-	      dccpd_reset_data[3];
+	__u8  dccpd_reset_code;
 	__u16 dccpd_opt_len;
-	__u64 dccpd_seq;
-	__u64 dccpd_ack_seq;
 };
 
 #define DCCP_SKB_CB(__skb) ((struct dccp_skb_cb *)&((__skb)->cb[0]))
diff --git a/net/dccp/input.c b/net/dccp/input.c
index 3bd14e8..8de7a37 100644
--- a/net/dccp/input.c
+++ b/net/dccp/input.c
@@ -366,12 +366,13 @@ discard:
 }
 
 int dccp_rcv_established(struct sock *sk, struct sk_buff *skb,
-			 const struct dccp_hdr *dh, const unsigned int len)
+			 const struct dccp_hdr *dh, const unsigned int len,
+			 struct dccp_reset_state *rst)
 {
 	if (dccp_check_seqno(sk, skb))
 		goto discard;
 
-	if (dccp_parse_options(sk, NULL, skb))
+	if (dccp_parse_options(sk, NULL, skb, rst))
 		return 1;
 
 	dccp_handle_ackvec_processing(sk, skb);
@@ -388,7 +389,8 @@ EXPORT_SYMBOL_GPL(dccp_rcv_established);
 static int dccp_rcv_request_sent_state_process(struct sock *sk,
 					       struct sk_buff *skb,
 					       const struct dccp_hdr *dh,
-					       const unsigned int len)
+					       const unsigned int len,
+					       struct dccp_reset_state *rst)
 {
 	/*
 	 *  Step 4: Prepare sequence numbers in REQUEST
@@ -421,7 +423,7 @@ static int dccp_rcv_request_sent_state_process(struct sock *sk,
 		 * dccp_v4_do_rcv() sends a Reset. The Reset code depends on
 		 * the option type and is set in dccp_parse_options().
 		 */
-		if (dccp_parse_options(sk, NULL, skb))
+		if (dccp_parse_options(sk, NULL, skb, rst))
 			return 1;
 
 		/* Obtain usec RTT sample from SYN exchange (used by TFRC). */
@@ -572,7 +574,7 @@ static int dccp_rcv_respond_partopen_state_process(struct sock *sk,
 }
 
 int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
-			   struct dccp_hdr *dh, unsigned int len)
+			   struct dccp_hdr *dh, unsigned int len, struct dccp_reset_state *rst)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
 	struct dccp_skb_cb *dcb = DCCP_SKB_CB(skb);
@@ -641,7 +643,7 @@ int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
 	}
 
 	/*  Step 8: Process options */
-	if (dccp_parse_options(sk, NULL, skb))
+	if (dccp_parse_options(sk, NULL, skb, rst))
 		return 1;
 
 	/*
@@ -667,7 +669,7 @@ int dccp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
 
 	switch (sk->sk_state) {
 	case DCCP_REQUESTING:
-		queued = dccp_rcv_request_sent_state_process(sk, skb, dh, len);
+		queued = dccp_rcv_request_sent_state_process(sk, skb, dh, len, rst);
 		if (queued >= 0)
 			return queued;
 
@@ -716,6 +718,7 @@ EXPORT_SYMBOL_GPL(dccp_rcv_state_process);
  */
 u32 dccp_sample_rtt(struct sock *sk, long delta)
 {
+
 	/* dccpor_elapsed_time is either zeroed out or set and > 0 */
 	delta -= dccp_sk(sk)->dccps_options_received.dccpor_elapsed_time * 10;
 
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index e45b968..e908e27 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -444,7 +444,8 @@ put_and_exit:
 }
 EXPORT_SYMBOL_GPL(dccp_v4_request_recv_sock);
 
-static struct sock *dccp_v4_hnd_req(struct sock *sk, struct sk_buff *skb)
+static struct sock *dccp_v4_hnd_req(struct sock *sk, struct sk_buff *skb,
+				    struct dccp_reset_state *rst)
 {
 	const struct dccp_hdr *dh = dccp_hdr(skb);
 	const struct iphdr *iph = ip_hdr(skb);
@@ -455,7 +456,7 @@ static struct sock *dccp_v4_hnd_req(struct sock *sk, struct sk_buff *skb)
 						       dh->dccph_sport,
 						       iph->saddr, iph->daddr);
 	if (req != NULL)
-		return dccp_check_req(sk, skb, req, prev);
+		return dccp_check_req(sk, skb, req, prev, rst);
 
 	nsk = inet_lookup_established(sock_net(sk), &dccp_hashinfo,
 				      iph->saddr, dh->dccph_sport,
@@ -527,7 +528,8 @@ out:
 	return err;
 }
 
-static void dccp_v4_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb)
+static void __dccp_v4_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb,
+				     const struct dccp_reset_state *rst)
 {
 	int err;
 	const struct iphdr *rxiph;
@@ -547,7 +549,7 @@ static void dccp_v4_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb)
 	if (dst == NULL)
 		return;
 
-	skb = dccp_ctl_make_reset(ctl_sk, rxskb);
+	skb = dccp_ctl_make_reset(ctl_sk, rxskb, rst);
 	if (skb == NULL)
 		goto out;
 
@@ -569,6 +571,11 @@ out:
 	 dst_release(dst);
 }
 
+static void dccp_v4_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb)
+{
+	__dccp_v4_ctl_send_reset(sk, rxskb, NULL);
+}
+
 static void dccp_v4_reqsk_destructor(struct request_sock *req)
 {
 	dccp_feat_list_purge(&dccp_rsk(req)->dreq_featneg);
@@ -632,7 +639,7 @@ int dccp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
 		goto drop_and_free;
 
 	dreq = dccp_rsk(req);
-	if (dccp_parse_options(sk, dreq, skb))
+	if (dccp_parse_options(sk, dreq, skb, NULL))
 		goto drop_and_free;
 
 	if (security_inet_conn_request(sk, skb, req))
@@ -672,9 +679,10 @@ EXPORT_SYMBOL_GPL(dccp_v4_conn_request);
 int dccp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	struct dccp_hdr *dh = dccp_hdr(skb);
+	struct dccp_reset_state reset_state = {};
 
 	if (sk->sk_state == DCCP_OPEN) { /* Fast path */
-		if (dccp_rcv_established(sk, skb, dh, skb->len))
+		if (dccp_rcv_established(sk, skb, dh, skb->len, &reset_state))
 			goto reset;
 		return 0;
 	}
@@ -703,24 +711,24 @@ int dccp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 	 *	 dccp_rcv_state_process
 	 */
 	if (sk->sk_state == DCCP_LISTEN) {
-		struct sock *nsk = dccp_v4_hnd_req(sk, skb);
+		struct sock *nsk = dccp_v4_hnd_req(sk, skb, &reset_state);
 
 		if (nsk == NULL)
 			goto discard;
 
 		if (nsk != sk) {
-			if (dccp_child_process(sk, nsk, skb))
+			if (dccp_child_process(sk, nsk, skb, &reset_state))
 				goto reset;
 			return 0;
 		}
 	}
 
-	if (dccp_rcv_state_process(sk, skb, dh, skb->len))
+	if (dccp_rcv_state_process(sk, skb, dh, skb->len, &reset_state))
 		goto reset;
 	return 0;
 
 reset:
-	dccp_v4_ctl_send_reset(sk, skb);
+	__dccp_v4_ctl_send_reset(sk, skb, &reset_state);
 discard:
 	kfree_skb(skb);
 	return 0;
@@ -898,6 +906,7 @@ no_dccp_socket:
 	if (dh->dccph_type != DCCP_PKT_RESET) {
 		DCCP_SKB_CB(skb)->dccpd_reset_code =
 					DCCP_RESET_CODE_NO_CONNECTION;
+
 		dccp_v4_ctl_send_reset(sk, skb);
 	}
 
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 6bcaa33..980f103 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -270,7 +270,8 @@ static void dccp_v6_reqsk_destructor(struct request_sock *req)
 	kfree_skb(inet_rsk(req)->pktopts);
 }
 
-static void dccp_v6_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb)
+static void __dccp_v6_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb,
+				     const struct dccp_reset_state *rst)
 {
 	const struct ipv6hdr *rxip6h;
 	struct sk_buff *skb;
@@ -285,7 +286,7 @@ static void dccp_v6_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb)
 	if (!ipv6_unicast_destination(rxskb))
 		return;
 
-	skb = dccp_ctl_make_reset(ctl_sk, rxskb);
+	skb = dccp_ctl_make_reset(ctl_sk, rxskb, rst);
 	if (skb == NULL)
 		return;
 
@@ -316,6 +317,11 @@ static void dccp_v6_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb)
 	kfree_skb(skb);
 }
 
+static void dccp_v6_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb)
+{
+	__dccp_v6_ctl_send_reset(sk, rxskb, NULL);
+}
+
 static struct request_sock_ops dccp6_request_sock_ops = {
 	.family		= AF_INET6,
 	.obj_size	= sizeof(struct dccp6_request_sock),
@@ -326,7 +332,8 @@ static struct request_sock_ops dccp6_request_sock_ops = {
 	.syn_ack_timeout = dccp_syn_ack_timeout,
 };
 
-static struct sock *dccp_v6_hnd_req(struct sock *sk,struct sk_buff *skb)
+static struct sock *dccp_v6_hnd_req(struct sock *sk,struct sk_buff *skb,
+				    struct dccp_reset_state *rst)
 {
 	const struct dccp_hdr *dh = dccp_hdr(skb);
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
@@ -339,7 +346,7 @@ static struct sock *dccp_v6_hnd_req(struct sock *sk,struct sk_buff *skb)
 							&iph->daddr,
 							inet6_iif(skb));
 	if (req != NULL)
-		return dccp_check_req(sk, skb, req, prev);
+		return dccp_check_req(sk, skb, req, prev, rst);
 
 	nsk = __inet6_lookup_established(sock_net(sk), &dccp_hashinfo,
 					 &iph->saddr, dh->dccph_sport,
@@ -394,7 +401,7 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
 		goto drop_and_free;
 
 	dreq = dccp_rsk(req);
-	if (dccp_parse_options(sk, dreq, skb))
+	if (dccp_parse_options(sk, dreq, skb, NULL))
 		goto drop_and_free;
 
 	if (security_inet_conn_request(sk, skb, req))
@@ -616,6 +623,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sk_buff *opt_skb = NULL;
+	struct dccp_reset_state reset_state = {};
 
 	/* Imagine: socket is IPv6. IPv4 packet arrives,
 	   goes to IPv4 receive handler and backlogged.
@@ -657,7 +665,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 		opt_skb = skb_clone(skb, GFP_ATOMIC);
 
 	if (sk->sk_state == DCCP_OPEN) { /* Fast path */
-		if (dccp_rcv_established(sk, skb, dccp_hdr(skb), skb->len))
+		if (dccp_rcv_established(sk, skb, dccp_hdr(skb), skb->len, &reset_state))
 			goto reset;
 		if (opt_skb) {
 			/* XXX This is where we would goto ipv6_pktoptions. */
@@ -691,7 +699,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	 *	 dccp_rcv_state_process
 	 */
 	if (sk->sk_state == DCCP_LISTEN) {
-		struct sock *nsk = dccp_v6_hnd_req(sk, skb);
+		struct sock *nsk = dccp_v6_hnd_req(sk, skb, &reset_state);
 
 		if (nsk == NULL)
 			goto discard;
@@ -701,7 +709,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 		 * the new socket..
 		 */
 		if (nsk != sk) {
-			if (dccp_child_process(sk, nsk, skb))
+			if (dccp_child_process(sk, nsk, skb, &reset_state))
 				goto reset;
 			if (opt_skb != NULL)
 				__kfree_skb(opt_skb);
@@ -709,7 +717,8 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 		}
 	}
 
-	if (dccp_rcv_state_process(sk, skb, dccp_hdr(skb), skb->len))
+	if (dccp_rcv_state_process(sk, skb, dccp_hdr(skb), skb->len,
+				   &reset_state))
 		goto reset;
 	if (opt_skb) {
 		/* XXX This is where we would goto ipv6_pktoptions. */
@@ -718,7 +727,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	return 0;
 
 reset:
-	dccp_v6_ctl_send_reset(sk, skb);
+	__dccp_v6_ctl_send_reset(sk, skb, &reset_state);
 discard:
 	if (opt_skb != NULL)
 		__kfree_skb(opt_skb);
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index b50dc43..7dc5fe3 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -153,7 +153,8 @@ EXPORT_SYMBOL_GPL(dccp_create_openreq_child);
  */
 struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
 			    struct request_sock *req,
-			    struct request_sock **prev)
+			    struct request_sock **prev,
+			    struct dccp_reset_state *rst)
 {
 	struct sock *child = NULL;
 	struct dccp_request_sock *dreq = dccp_rsk(req);
@@ -193,7 +194,7 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
 		goto drop;
 	}
 
-	if (dccp_parse_options(sk, dreq, skb))
+	if (dccp_parse_options(sk, dreq, skb, rst))
 		 goto drop;
 
 	child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL);
@@ -224,14 +225,14 @@ EXPORT_SYMBOL_GPL(dccp_check_req);
  *  the new socket.
  */
 int dccp_child_process(struct sock *parent, struct sock *child,
-		       struct sk_buff *skb)
+		       struct sk_buff *skb, struct dccp_reset_state *rst)
 {
 	int ret = 0;
 	const int state = child->sk_state;
 
 	if (!sock_owned_by_user(child)) {
 		ret = dccp_rcv_state_process(child, skb, dccp_hdr(skb),
-					     skb->len);
+					     skb->len, rst);
 
 		/* Wakeup parent, send SIGIO */
 		if (state == DCCP_RESPOND && child->sk_state != state)
diff --git a/net/dccp/options.c b/net/dccp/options.c
index 9bce318..7848fbf 100644
--- a/net/dccp/options.c
+++ b/net/dccp/options.c
@@ -49,7 +49,7 @@ u64 dccp_decode_value_var(const u8 *bf, const u8 len)
  * @dreq: request socket to use during connection setup, or NULL
  */
 int dccp_parse_options(struct sock *sk, struct dccp_request_sock *dreq,
-		       struct sk_buff *skb)
+		       struct sk_buff *skb, struct dccp_reset_state *rst)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
 	const struct dccp_hdr *dh = dccp_hdr(skb);
@@ -258,9 +258,11 @@ out_invalid_option:
 out_featneg_failed:
 	DCCP_WARN("DCCP(%p): Option %d (len=%d) error=%u\n", sk, opt, len, rc);
 	DCCP_SKB_CB(skb)->dccpd_reset_code = rc;
-	DCCP_SKB_CB(skb)->dccpd_reset_data[0] = opt;
-	DCCP_SKB_CB(skb)->dccpd_reset_data[1] = len > 0 ? value[0] : 0;
-	DCCP_SKB_CB(skb)->dccpd_reset_data[2] = len > 1 ? value[1] : 0;
+	if (rst) {
+		rst->dccpd_reset_data[0] = opt;
+		rst->dccpd_reset_data[1] = len > 0 ? value[0] : 0;
+		rst->dccpd_reset_data[2] = len > 1 ? value[1] : 0;
+	}
 	return -1;
 }
 
diff --git a/net/dccp/output.c b/net/dccp/output.c
index 0248e8a..f00cdf3 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -42,7 +42,8 @@ static struct sk_buff *dccp_skb_entail(struct sock *sk, struct sk_buff *skb)
  * IP so it can do the same plus pass the packet off to the
  * device.
  */
-static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
+static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb,
+			     enum dccp_reset_codes reset_code)
 {
 	if (likely(skb != NULL)) {
 		struct inet_sock *inet = inet_sk(sk);
@@ -126,8 +127,7 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
 			dp->dccps_awl = dp->dccps_iss;
 			break;
 		case DCCP_PKT_RESET:
-			dccp_hdr_reset(skb)->dccph_reset_code =
-							dcb->dccpd_reset_code;
+			dccp_hdr_reset(skb)->dccph_reset_code = reset_code;
 			break;
 		}
 
@@ -277,7 +277,7 @@ static void dccp_xmit_packet(struct sock *sk)
 		DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_DATA;
 	}
 
-	err = dccp_transmit_skb(sk, skb);
+	err = dccp_transmit_skb(sk, skb, 0);
 	if (err)
 		dccp_pr_debug("transmit_skb() returned err=%d\n", err);
 	/*
@@ -387,7 +387,7 @@ int dccp_retransmit_skb(struct sock *sk)
 	/* this count is used to distinguish original and retransmitted skb */
 	inet_csk(sk)->icsk_retransmits++;
 
-	return dccp_transmit_skb(sk, skb_clone(sk->sk_send_head, GFP_ATOMIC));
+	return dccp_transmit_skb(sk, skb_clone(sk->sk_send_head, GFP_ATOMIC), 0);
 }
 
 struct sk_buff *dccp_make_response(struct sock *sk, struct dst_entry *dst,
@@ -448,7 +448,8 @@ response_failed:
 EXPORT_SYMBOL_GPL(dccp_make_response);
 
 /* answer offending packet in @rcv_skb with Reset from control socket @ctl */
-struct sk_buff *dccp_ctl_make_reset(struct sock *sk, struct sk_buff *rcv_skb)
+struct sk_buff *dccp_ctl_make_reset(struct sock *sk, struct sk_buff *rcv_skb,
+				    const struct dccp_reset_state *rst)
 {
 	struct dccp_hdr *rxdh = dccp_hdr(rcv_skb), *dh;
 	struct dccp_skb_cb *dcb = DCCP_SKB_CB(rcv_skb);
@@ -481,7 +482,10 @@ struct sk_buff *dccp_ctl_make_reset(struct sock *sk, struct sk_buff *rcv_skb)
 		break;
 	case DCCP_RESET_CODE_OPTION_ERROR:	/* fall through */
 	case DCCP_RESET_CODE_MANDATORY_ERROR:
-		memcpy(dhr->dccph_reset_data, dcb->dccpd_reset_data, 3);
+		BUILD_BUG_ON(sizeof(dhr->dccph_reset_data) != sizeof(rst->dccpd_reset_data));
+		if (rst)
+			memcpy(dhr->dccph_reset_data, rst->dccpd_reset_data,
+					sizeof(dhr->dccph_reset_data));
 		break;
 	}
 	/*
@@ -519,9 +523,8 @@ int dccp_send_reset(struct sock *sk, enum dccp_reset_codes code)
 	/* Reserve space for headers and prepare control bits. */
 	skb_reserve(skb, sk->sk_prot->max_header);
 	DCCP_SKB_CB(skb)->dccpd_type	   = DCCP_PKT_RESET;
-	DCCP_SKB_CB(skb)->dccpd_reset_code = code;
 
-	return dccp_transmit_skb(sk, skb);
+	return dccp_transmit_skb(sk, skb, code);
 }
 
 /*
@@ -555,7 +558,7 @@ int dccp_connect(struct sock *sk)
 
 	DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_REQUEST;
 
-	dccp_transmit_skb(sk, dccp_skb_entail(sk, skb));
+	dccp_transmit_skb(sk, dccp_skb_entail(sk, skb), 0);
 	DCCP_INC_STATS(DCCP_MIB_ACTIVEOPENS);
 
 	/* Timer for repeating the REQUEST until an answer. */
@@ -586,7 +589,7 @@ void dccp_send_ack(struct sock *sk)
 		/* Reserve space for headers */
 		skb_reserve(skb, sk->sk_prot->max_header);
 		DCCP_SKB_CB(skb)->dccpd_type = DCCP_PKT_ACK;
-		dccp_transmit_skb(sk, skb);
+		dccp_transmit_skb(sk, skb, 0);
 	}
 }
 
@@ -652,7 +655,7 @@ void dccp_send_sync(struct sock *sk, const u64 ackno,
 	 */
 	dccp_sk(sk)->dccps_sync_scheduled = 0;
 
-	dccp_transmit_skb(sk, skb);
+	dccp_transmit_skb(sk, skb, 0);
 }
 
 EXPORT_SYMBOL_GPL(dccp_send_sync);
@@ -694,5 +697,5 @@ void dccp_send_close(struct sock *sk, const int active)
 		inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
 					  DCCP_TIMEOUT_INIT, DCCP_RTO_MAX);
 	}
-	dccp_transmit_skb(sk, skb);
+	dccp_transmit_skb(sk, skb, 0);
 }
-- 
2.0.5

[PATCH RFC 11/14] dccp: reduce size of dccp_skb_cb to 40 bytes

[Florian Westphal]

... to eventually reduce skb->cb size.

Cc: dccp@vger.kernel.org
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/dccp/dccp.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
index 520c566..e8a981b 100644
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -343,11 +343,11 @@ static inline int dccp_bad_service_code(const struct sock *sk,
 /**
  * dccp_skb_cb  -  DCCP per-packet control information
  * @dccpd_seq: sequence number
+ * @dccpd_opt_len: total length of all options (5.8) in the packet
  * @dccpd_ack_seq: acknowledgment number subheader field value
  * @dccpd_type: one of %dccp_pkt_type (or unknown)
  * @dccpd_ccval: CCVal field (5.1), see e.g. RFC 4342, 8.1
  * @dccpd_reset_code: one of %dccp_reset_codes
- * @dccpd_opt_len: total length of all options (5.8) in the packet
  *
  * This is used for transmission as well as for reception.
  */
@@ -358,13 +358,13 @@ struct dccp_skb_cb {
 		struct inet6_skb_parm	h6;
 #endif
 	} header;
-	__u64 dccpd_seq;
-	__u64 dccpd_ack_seq;
 
-	__u8  dccpd_type:4;
-	__u8  dccpd_ccval:4;
-	__u8  dccpd_reset_code;
-	__u16 dccpd_opt_len;
+	__u64 dccpd_seq:48,
+	      dccpd_opt_len:16;
+	__u64 dccpd_ack_seq:50,
+	      dccpd_type:4,
+	      dccpd_ccval:4,
+	      dccpd_reset_code:4;
 };
 
 #define DCCP_SKB_CB(__skb) ((struct dccp_skb_cb *)&((__skb)->cb[0]))
-- 
2.0.5

[PATCH RFC 12/14] rxrpc: use 32bit jiffies on 64bit platforms, too

[Florian Westphal]

Its enough for resend decision and reduces rxrpc private cb structure
by 8 bytes.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/rxrpc/ar-ack.c      | 72 +++++++++++++++++++++++++++++++------------------
 net/rxrpc/ar-internal.h |  4 +--
 net/rxrpc/ar-output.c   |  6 ++---
 3 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index e0547f5..f0602bc 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -63,6 +63,9 @@ unsigned rxrpc_rx_mtu = 5692;
  */
 unsigned rxrpc_rx_jumbo_max = 4;
 
+/* we use 32bit jiffies even on 64bit machines to save space in skb->cb */
+#define rxrpc_resend_time_stamp ((u32) jiffies)
+
 static const char *rxrpc_acks(u8 reason)
 {
 	static const char *const str[] = {
@@ -185,7 +188,7 @@ void rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason,
  * set the resend timer
  */
 static void rxrpc_set_resend(struct rxrpc_call *call, u8 resend,
-			     unsigned long resend_at)
+			     s32 resend_delta)
 {
 	read_lock_bh(&call->state_lock);
 	if (call->state >= RXRPC_CALL_COMPLETE)
@@ -199,7 +202,7 @@ static void rxrpc_set_resend(struct rxrpc_call *call, u8 resend,
 	if (resend & 2) {
 		_debug("MODIFY RESEND TIMER");
 		set_bit(RXRPC_CALL_RUN_RTIMER, &call->flags);
-		mod_timer(&call->resend_timer, resend_at);
+		mod_timer(&call->resend_timer, jiffies + resend_delta);
 	} else {
 		_debug("KILL RESEND TIMER");
 		del_timer_sync(&call->resend_timer);
@@ -209,6 +212,11 @@ static void rxrpc_set_resend(struct rxrpc_call *call, u8 resend,
 	read_unlock_bh(&call->state_lock);
 }
 
+static bool rxrpc_resend_needed(const struct rxrpc_skb_priv *sp)
+{
+	return ((s32)((rxrpc_resend_time_stamp) - (sp->resend_at)) >= 0);
+}
+
 /*
  * resend packets
  */
@@ -217,7 +225,8 @@ static void rxrpc_resend(struct rxrpc_call *call)
 	struct rxrpc_skb_priv *sp;
 	struct rxrpc_header *hdr;
 	struct sk_buff *txb;
-	unsigned long *p_txb, resend_at;
+	unsigned long *p_txb;
+	s32 resend_at_delta;
 	bool stop;
 	int loop;
 	u8 resend;
@@ -229,12 +238,14 @@ static void rxrpc_resend(struct rxrpc_call *call)
 
 	stop = false;
 	resend = 0;
-	resend_at = 0;
+	resend_at_delta = 0;
 
 	for (loop = call->acks_tail;
 	     loop != call->acks_head || stop;
 	     loop = (loop + 1) &  (call->acks_winsz - 1)
 	     ) {
+		s32 resend_delta;
+
 		p_txb = call->acks_window + loop;
 		smp_read_barrier_depends();
 		if (*p_txb & 1)
@@ -243,6 +254,8 @@ static void rxrpc_resend(struct rxrpc_call *call)
 		txb = (struct sk_buff *) *p_txb;
 		sp = rxrpc_skb(txb);
 
+		resend_delta = sp->resend_at - rxrpc_resend_time_stamp;
+
 		if (sp->need_resend) {
 			sp->need_resend = false;
 
@@ -257,26 +270,27 @@ static void rxrpc_resend(struct rxrpc_call *call)
 			       ntohl(sp->hdr.serial), ntohl(sp->hdr.seq));
 			if (rxrpc_send_packet(call->conn->trans, txb) < 0) {
 				stop = true;
-				sp->resend_at = jiffies + 3;
+				resend_delta = 3;
 			} else {
-				sp->resend_at =
-					jiffies + rxrpc_resend_timeout;
+				resend_delta = rxrpc_resend_timeout;
+				sp->resend_at = rxrpc_resend_time_stamp +
+						  resend_delta;
 			}
 		}
 
-		if (time_after_eq(jiffies + 1, sp->resend_at)) {
+		if (rxrpc_resend_needed(sp)) {
 			sp->need_resend = true;
 			resend |= 1;
 		} else if (resend & 2) {
-			if (time_before(sp->resend_at, resend_at))
-				resend_at = sp->resend_at;
+			if (resend_delta < resend_at_delta)
+				resend_at_delta = resend_delta;
 		} else {
-			resend_at = sp->resend_at;
+			resend_at_delta = resend_delta;
 			resend |= 2;
 		}
 	}
 
-	rxrpc_set_resend(call, resend, resend_at);
+	rxrpc_set_resend(call, resend, resend_at_delta);
 	_leave("");
 }
 
@@ -287,7 +301,8 @@ static void rxrpc_resend_timer(struct rxrpc_call *call)
 {
 	struct rxrpc_skb_priv *sp;
 	struct sk_buff *txb;
-	unsigned long *p_txb, resend_at;
+	unsigned long *p_txb;
+	s32 resend_at_delta;
 	int loop;
 	u8 resend;
 
@@ -298,7 +313,7 @@ static void rxrpc_resend_timer(struct rxrpc_call *call)
 		return;
 
 	resend = 0;
-	resend_at = 0;
+	resend_at_delta = 0;
 
 	for (loop = call->acks_unacked;
 	     loop != call->acks_head;
@@ -313,19 +328,21 @@ static void rxrpc_resend_timer(struct rxrpc_call *call)
 
 		if (sp->need_resend) {
 			;
-		} else if (time_after_eq(jiffies + 1, sp->resend_at)) {
+		} else if (rxrpc_resend_needed(sp)) {
 			sp->need_resend = true;
 			resend |= 1;
 		} else if (resend & 2) {
-			if (time_before(sp->resend_at, resend_at))
-				resend_at = sp->resend_at;
+			s32 resend_delta = sp->resend_at - rxrpc_resend_time_stamp;
+
+			if (resend_delta < resend_at_delta)
+				resend_at_delta = resend_delta;
 		} else {
-			resend_at = sp->resend_at;
+			resend_at_delta = sp->resend_at - rxrpc_resend_time_stamp;
 			resend |= 2;
 		}
 	}
 
-	rxrpc_set_resend(call, resend, resend_at);
+	rxrpc_set_resend(call, resend, resend_at_delta);
 	_leave("");
 }
 
@@ -340,9 +357,10 @@ static int rxrpc_process_soft_ACKs(struct rxrpc_call *call,
 {
 	struct rxrpc_skb_priv *sp;
 	struct sk_buff *txb;
-	unsigned long *p_txb, resend_at;
+	unsigned long *p_txb;
 	int loop;
 	u8 sacks[RXRPC_MAXACKS], resend;
+	s32 resend_at_delta;
 
 	_enter("{%d,%d},{%d},",
 	       call->acks_hard,
@@ -353,7 +371,7 @@ static int rxrpc_process_soft_ACKs(struct rxrpc_call *call,
 		goto protocol_error;
 
 	resend = 0;
-	resend_at = 0;
+	resend_at_delta = 0;
 	for (loop = 0; loop < ack->nAcks; loop++) {
 		p_txb = call->acks_window;
 		p_txb += (call->acks_tail + loop) & (call->acks_winsz - 1);
@@ -398,19 +416,21 @@ static int rxrpc_process_soft_ACKs(struct rxrpc_call *call,
 			resend |= 1;
 		} else if (sp->need_resend) {
 			;
-		} else if (time_after_eq(jiffies + 1, sp->resend_at)) {
+		} else if (rxrpc_resend_needed(sp)) {
 			sp->need_resend = true;
 			resend |= 1;
 		} else if (resend & 2) {
-			if (time_before(sp->resend_at, resend_at))
-				resend_at = sp->resend_at;
+			s32 resend_delta = sp->resend_at - rxrpc_resend_time_stamp;
+
+			if (resend_delta < resend_at_delta)
+				resend_at_delta = resend_delta;
 		} else {
-			resend_at = sp->resend_at;
+			resend_at_delta = sp->resend_at - rxrpc_resend_time_stamp;
 			resend |= 2;
 		}
 	}
 
-	rxrpc_set_resend(call, resend, resend_at);
+	rxrpc_set_resend(call, resend, resend_at_delta);
 	_leave(" = 0");
 	return 0;
 
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index ba9fd36..81d1949 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -81,7 +81,7 @@ struct rxrpc_sock {
  */
 struct rxrpc_skb_priv {
 	struct rxrpc_call	*call;		/* call with which associated */
-	unsigned long		resend_at;	/* time in jiffies at which to resend */
+	u32			resend_at;	/* time at which to resend (32 bit jiffies) */
 	union {
 		unsigned int	offset;		/* offset into buffer of next read */
 		int		remain;		/* amount of space remaining for next write */
@@ -90,7 +90,7 @@ struct rxrpc_skb_priv {
 	};
 
 	struct rxrpc_header	hdr;		/* RxRPC packet header from this packet */
-};
+} __packed;
 
 #define rxrpc_skb(__skb) ((struct rxrpc_skb_priv *) &(__skb)->cb)
 
diff --git a/net/rxrpc/ar-output.c b/net/rxrpc/ar-output.c
index 8331c95..f04e89a 100644
--- a/net/rxrpc/ar-output.c
+++ b/net/rxrpc/ar-output.c
@@ -486,11 +486,11 @@ static void rxrpc_queue_packet(struct rxrpc_call *call, struct sk_buff *skb,
 	_proto("Tx DATA %%%u { #%u }",
 	       ntohl(sp->hdr.serial), ntohl(sp->hdr.seq));
 
-	sp->need_resend = false;
-	sp->resend_at = jiffies + rxrpc_resend_timeout;
+	sp->resend_at = (u32) jiffies;
+	sp->resend_at += rxrpc_resend_timeout;
 	if (!test_and_set_bit(RXRPC_CALL_RUN_RTIMER, &call->flags)) {
 		_debug("run timer");
-		call->resend_timer.expires = sp->resend_at;
+		call->resend_timer.expires = jiffies + rxrpc_resend_timeout;
 		add_timer(&call->resend_timer);
 	}
 
-- 
2.0.5

[PATCH RFC 13/14] net: tcp: don't assert sock_skb_cb_check_size

[Florian Westphal]

Its required fro af_packet and all protocol that make use of

SOCK_SKB_CB(skb)->dropcount

via sock_queue_rcv_skb.

But tcp doesn't seem to do that.  This avoids build failure
with a 44byte sized tcp cb.

Cc: Eyal Birger <eyal.birger@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/tcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 4b57ea8..9d72a0f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3005,11 +3005,12 @@ static void __init tcp_init_mem(void)
 
 void __init tcp_init(void)
 {
+	struct sk_buff *skb = NULL;
 	unsigned long limit;
 	int max_rshare, max_wshare, cnt;
 	unsigned int i;
 
-	sock_skb_cb_check_size(sizeof(struct tcp_skb_cb));
+	BUILD_BUG_ON(sizeof(struct tcp_skb_cb) > sizeof(skb->cb));
 
 	percpu_counter_init(&tcp_sockets_allocated, 0, GFP_KERNEL);
 	percpu_counter_init(&tcp_orphan_count, 0, GFP_KERNEL);
-- 
2.0.5

[PATCH RFC 14/14] net: introduce and use KERNEL_MAX_HEADER_PARSE_ADDRLEN

[Florian Westphal]

af_packet sockets call dev->header_ops->parse to stash the hwaddr in
skb->cb.

This currently works without problems since no hw address exceeds 20
bytes.  When we'd reduce skb->cb in the future we might silently
corrupt fields after skb->cb.  So add compile-time assertions to all
implementations of header_ops->parse.

The largest address is infiniband (20); the largest address that can
be returned by a header_ops->parse call however is 16 (Firewire).

add KERNEL_MAX_HEADER_PARSE_ADDRLEN as a way to indicate the largest
possible value returned by dev_parse_header().

Original idea by David Miller.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 drivers/firewire/net.c                    | 1 +
 drivers/net/wireless/airo.c               | 2 ++
 drivers/net/wireless/hostap/hostap_main.c | 2 ++
 include/linux/netdevice.h                 | 7 +++++++
 net/ethernet/eth.c                        | 2 ++
 net/mac802154/iface.c                     | 1 +
 net/packet/af_packet.c                    | 3 ++-
 net/phonet/af_phonet.c                    | 2 ++
 8 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 2c68da1..ce7b45c 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -276,6 +276,7 @@ static void fwnet_header_cache_update(struct hh_cache *hh,
 static int fwnet_header_parse(const struct sk_buff *skb, unsigned char *haddr)
 {
 	memcpy(haddr, skb->dev->dev_addr, FWNET_ALEN);
+	dev_validate_header_parse_addrlen(FWNET_ALEN);
 
 	return FWNET_ALEN;
 }
diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index e71a2ce..ba6497e 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -2438,6 +2438,8 @@ EXPORT_SYMBOL(stop_airo_card);
 static int wll_header_parse(const struct sk_buff *skb, unsigned char *haddr)
 {
 	memcpy(haddr, skb_mac_header(skb) + 10, ETH_ALEN);
+	dev_validate_header_parse_addrlen(ETH_ALEN);
+
 	return ETH_ALEN;
 }
 
diff --git a/drivers/net/wireless/hostap/hostap_main.c b/drivers/net/wireless/hostap/hostap_main.c
index 52919ad..fe760e9 100644
--- a/drivers/net/wireless/hostap/hostap_main.c
+++ b/drivers/net/wireless/hostap/hostap_main.c
@@ -582,6 +582,8 @@ static int hostap_80211_header_parse(const struct sk_buff *skb,
 				     unsigned char *haddr)
 {
 	memcpy(haddr, skb_mac_header(skb) + 10, ETH_ALEN); /* addr2 */
+	dev_validate_header_parse_addrlen(ETH_ALEN);
+
 	return ETH_ALEN;
 }
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7427185..bd1365a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -268,6 +268,13 @@ struct header_ops {
 				const unsigned char *haddr);
 };
 
+/* largest possible return value of header_ops->parse methods */
+#define KERNEL_MAX_HEADER_PARSE_ADDRLEN	16
+static inline void dev_validate_header_parse_addrlen(unsigned int alen)
+{
+	BUILD_BUG_ON(alen > KERNEL_MAX_HEADER_PARSE_ADDRLEN);
+}
+
 /* These flag bits are private to the generic network queueing
  * layer, they may not be explicitly referenced by any other
  * code.
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 238f38d..6d27123 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -240,6 +240,8 @@ int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr)
 {
 	const struct ethhdr *eth = eth_hdr(skb);
 	memcpy(haddr, eth->h_source, ETH_ALEN);
+	dev_validate_header_parse_addrlen(ETH_ALEN);
+
 	return ETH_ALEN;
 }
 EXPORT_SYMBOL(eth_header_parse);
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 6fb6bdf..97eb50c 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -433,6 +433,7 @@ mac802154_header_parse(const struct sk_buff *skb, unsigned char *haddr)
 		return 0;
 	}
 
+	dev_validate_header_parse_addrlen(sizeof(*addr));
 	*addr = hdr.source;
 	return sizeof(*addr);
 }
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 9db8369..1a4dcbb 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1816,7 +1816,8 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
 		skb = nskb;
 	}
 
-	sock_skb_cb_check_size(sizeof(*PACKET_SKB_CB(skb)) + MAX_ADDR_LEN - 8);
+	sock_skb_cb_check_size(sizeof(*PACKET_SKB_CB(skb)) +
+			       KERNEL_MAX_HEADER_PARSE_ADDRLEN);
 
 	sll = &PACKET_SKB_CB(skb)->sa.ll;
 	sll->sll_hatype = dev->type;
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 32ab87d..617ac42 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -146,6 +146,8 @@ static int pn_header_parse(const struct sk_buff *skb, unsigned char *haddr)
 {
 	const u8 *media = skb_mac_header(skb);
 	*haddr = *media;
+	dev_validate_header_parse_addrlen(sizeof(*haddr));
+
 	return 1;
 }
 
-- 
2.0.5

Re: [PATCH RFC 09/14] net: wireless: mac80211: shrink private driver area

[Johannes Berg]

On Mon, 2015-03-02 at 18:40 +0100, Florian Westphal wrote:
> This makes the structure fit 44-byte sized skb->cb[].
> 
> Cc: linux-wireless@vger.kernel.org
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  drivers/net/wireless/ath/ath9k/ath9k.h | 2 +-
>  include/net/mac80211.h                 | 8 +++-----
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 0f8e946..2ed77cf 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h

This is clearly not a mac80211 patch.

johannes

Re: [PATCH RFC 08/14] net: wireless: mac80211: shrink ieee80211_tx_info

[Johannes Berg]

On Mon, 2015-03-02 at 18:40 +0100, Florian Westphal wrote:
> to make it fit into (future) 44-byte sized skb->cb[].
> 
> This works, since flags is only used to store values
> from mac80211_tx_control_flags enum, and these are just 2 bits.
> We can thus move this to the padding hole inside the union.
> 
> Also add BUILD_BUG_ON magic to make sure that the new flags
> field doesn't share storage w. other members of the union.

This is really ugly - what's the point of this?

Mind you - we are actually acutely out of space and would rather have
*more*, not less.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 08/14] net: wireless: mac80211: shrink ieee80211_tx_info

[Florian Westphal]

Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2015-03-02 at 18:40 +0100, Florian Westphal wrote:
> > to make it fit into (future) 44-byte sized skb->cb[].
> > 
> > This works, since flags is only used to store values
> > from mac80211_tx_control_flags enum, and these are just 2 bits.
> > We can thus move this to the padding hole inside the union.
> > 
> > Also add BUILD_BUG_ON magic to make sure that the new flags
> > field doesn't share storage w. other members of the union.
> 
> This is really ugly - what's the point of this?

Eventually reducing skb size to make it fit into 3 cachelines again even
on 64bit architectures.  For that 40 bytes need to go.

> Mind you - we are actually acutely out of space and would rather have
> *more*, not less.

:-(

I'm not familiar with mac80211, aside from that it seemed to me
that 40 byte cb would be doable, given enough work.

Where are to main problems, exactly?

I known that pushing something into ->cb is a lot easier than e.g. keeping
extra state on stack, but, IMO cb should really only be used when you
need to associate data strictly with an skb so that this data is still
availabe even when skb gets queued somewehere.

Is there a document somewhere that lists all of the per-skb data that
mac80211 needs to store (or wants to store)?

Thanks,
Florian

Re: [PATCH RFC 08/14] net: wireless: mac80211: shrink ieee80211_tx_info

[Johannes Berg]

On Mon, 2015-03-02 at 20:03 +0100, Florian Westphal wrote:

> Eventually reducing skb size to make it fit into 3 cachelines again even
> on 64bit architectures.  For that 40 bytes need to go.

That seems like a worthy goal I guess (I guess you should've copied a
patch 0/N to us).

> > Mind you - we are actually acutely out of space and would rather have
> > *more*, not less.
> 
> :-(
> 
> I'm not familiar with mac80211, aside from that it seemed to me
> that 40 byte cb would be doable, given enough work.

Right now it is, mostly, yes.

> Where are to main problems, exactly?

Well, that depends. Right now clearly we're not really using all of it
as you saw (even if this patch moving bits here and there is really
ugly) but there are multiple things:
 1) Of course mac80211 isn't static, it keeps getting developed! Right
now for
    example we need to fix single TCP flow throughput over wifi, and for
that we
    need a timestamp. That won't even fit into skb->cb any more right
now; we'll
    probably be able to get away with (ab)using skb->tstamp or doing
reshuffling
    similar to yours to get some space, but that's just lucky this time.
 2) We're actually out of flags that are kept from TX generation to TX
    destruction and it's almost certain that we'll need to add more
things.

Also, we already do a lot of bit twiddling here, doing it even more
makes the code even harder to follow. It's bad enough as is if you ask
me.

> I known that pushing something into ->cb is a lot easier than e.g. keeping
> extra state on stack, but, IMO cb should really only be used when you
> need to associate data strictly with an skb so that this data is still
> availabe even when skb gets queued somewehere.

Right, and we do that. We've in the past moved out data from here to
elsewhere, but it's extremely tedious and error-prone, and I'm not sure
we have much that we can possibly move now, since we do need to hang on
to SKBs in many cases like client powersaving etc.

> Is there a document somewhere that lists all of the per-skb data that
> mac80211 needs to store (or wants to store)?

Not really, sorry.

> +                                       enum mac80211_tx_control_flags flags:2;
> +                                       /* used for BUILD_BUG_ON validation that ->flags won't
> +                                        * overlap with jiffies below */
> +                                       char flags_end[0];

That "jiffies" should probably say "other members (currently jiffies)"
or something ... I got a bit confused here.

johannes

Re: [PATCH RFC 08/14] net: wireless: mac80211: shrink ieee80211_tx_info

[Florian Westphal]

Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> On Mon, 2015-03-02 at 20:03 +0100, Florian Westphal wrote:
> 
> > Eventually reducing skb size to make it fit into 3 cachelines again even
> > on 64bit architectures.  For that 40 bytes need to go.
> 
> That seems like a worthy goal I guess (I guess you should've copied a
> patch 0/N to us).

Indeed, sorry.
archived cover letter: http://marc.info/?l=linux-netdev&m=142531804325076&w=2

> > I'm not familiar with mac80211, aside from that it seemed to me
> > that 40 byte cb would be doable, given enough work.
> 
> Right now it is, mostly, yes.
> 
> > Where are to main problems, exactly?
> 
> Well, that depends. Right now clearly we're not really using all of it
> as you saw (even if this patch moving bits here and there is really
> ugly) but there are multiple things:
>  1) Of course mac80211 isn't static, it keeps getting developed! Right
> now for
>     example we need to fix single TCP flow throughput over wifi, and for
> that we
>     need a timestamp. That won't even fit into skb->cb any more right
> now; we'll
>     probably be able to get away with (ab)using skb->tstamp or doing
> reshuffling
>     similar to yours to get some space, but that's just lucky this time.
>  2) We're actually out of flags that are kept from TX generation to TX
>     destruction and it's almost certain that we'll need to add more
> things.
> 
> Also, we already do a lot of bit twiddling here, doing it even more
> makes the code even harder to follow. It's bad enough as is if you ask
> me.

True.

> > I known that pushing something into ->cb is a lot easier than e.g. keeping
> > extra state on stack, but, IMO cb should really only be used when you
> > need to associate data strictly with an skb so that this data is still
> > availabe even when skb gets queued somewehere.
> 
> Right, and we do that. We've in the past moved out data from here to
> elsewhere, but it's extremely tedious and error-prone, and I'm not sure
> we have much that we can possibly move now, since we do need to hang on
> to SKBs in many cases like client powersaving etc.

I see.  I cannot comment any further at the moment, I will have to dig
into mac80211 more.

Thanks for your comments!
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 00/14] shrink skb cb to 44 bytes

[Eric Dumazet]

On Mon, 2015-03-02 at 18:40 +0100, Florian Westphal wrote:
> Following patches shrink all in-tree users of skb->cb[] so that kernel
> still builds with skb->cb[] set to 44 bytes.
> 
> This would create a 4-byte hole, it would be easy to reorder skb
> members so this hole is filled.
> 
> pahole dump for vmlinux with allyesconfig (+this patch series):
> 
> http://www.strlen.de/fw/pahole.txt.gz
> 
> remarks/known issues:
> - adds __packet attribute to a few structures.
>   Its needed to not have padding at end of the structure, else
>   we get build assertion errors even if all members fit into cb[].
> - checkpatch isn't happy yet.
> - dccp changes are untested (its on my todo list)
> - rxrpc change is untested (on todo list).
> - wireless changes are untested, I don't own any of the affected hw.
> 
> The idea is to figure out what needs to be done to make build_skb() and
> friends only touch the first 3 cachelines of the skb on all architectures.
> 
> We'd need to reduce skbuff by 40 bytes to achieve this for allyesconfig.
> 
> Other sk_buff reduction ideas being worked:
> 
> - move truesize to shinfo (which has 4 byte hole)
> - turn ->data into offset on 64bit platforms (intrusive, > 1000 files
>   affected)
> - move pointers that are ususally not needed (nf_bridge, secpath) to the end,
>   with flag field that tells us when pointer is valid (so we don't have
>   to memset() them unconditionally at allocation time).
> - seems we could already to this for the inner header fields since the're only
>   valid once ->encapsulation / inner_protocol_type is set.
> 
> Comments and more ideas welcome.

Size of skb->cb[] is not the major factor. Trying to gain 4 or 8 bytes
is not going to improve performance a lot.

The real problem is that we clear it in skb_alloc()/build_skb(), instead
of each layer doing so at demand, and only the part that matters for
this layer.

Basically, skb->cb[] could be 80 or 160 bytes instead of 48, and we
should not care, as long as no layer does a stupid/lazy 

memset(skb->cb, 0, sizeof(skb->cb))

Presumably skb_clone() could be extended to receive the length of
skb->cb[] that current layer cares about.

Re: [PATCH RFC 00/14] shrink skb cb to 44 bytes

[Florian Westphal]

Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > remarks/known issues:
> > - adds __packet attribute to a few structures.
> >   Its needed to not have padding at end of the structure, else
> >   we get build assertion errors even if all members fit into cb[].
> > - checkpatch isn't happy yet.
> > - dccp changes are untested (its on my todo list)
> > - rxrpc change is untested (on todo list).
> > - wireless changes are untested, I don't own any of the affected hw.
> > 
> > The idea is to figure out what needs to be done to make build_skb() and
> > friends only touch the first 3 cachelines of the skb on all architectures.
> > 
> > We'd need to reduce skbuff by 40 bytes to achieve this for allyesconfig.
> > 
> > Other sk_buff reduction ideas being worked:
> > 
> > - move truesize to shinfo (which has 4 byte hole)
> > - turn ->data into offset on 64bit platforms (intrusive, > 1000 files
> >   affected)
> > - move pointers that are ususally not needed (nf_bridge, secpath) to the end,
> >   with flag field that tells us when pointer is valid (so we don't have
> >   to memset() them unconditionally at allocation time).
> > - seems we could already to this for the inner header fields since the're only
> >   valid once ->encapsulation / inner_protocol_type is set.
> > 
> > Comments and more ideas welcome.
> 
> Size of skb->cb[] is not the major factor. Trying to gain 4 or 8 bytes
> is not going to improve performance a lot.

Thats right, goal is to have build_skb etc. only touch 3 cachelines,
and have those layers that need some particular feature initialise it on
demand.

We could move skb->cb to end of skb, so that its not covered by the
initial allocation memset anymore.

Obviously that won't buy us anything at this point since gro needs
to zero it out (GRO skb cb is 48 bytes).

> The real problem is that we clear it in skb_alloc()/build_skb(), instead
> of each layer doing so at demand, and only the part that matters for
> this layer.

Thats right.  Do you think its worth to already move cb[] near the end
of skb and alter build_skb to not clear it anymore?

Which of the ideas, in your opinion, is worth pursuing first (if any)?

I'd love to get rid of nf_bridge* pointer and use percpu storage
area for it but I did not yet find a simple way to deal with when
skb that uses this percpu state leaves bridge code and is then dropped
or queued later -- we need to make sure that another skb isn't accidentally
believed to have valid nf_bridge context.

kfree_skb doesn't seem to be a nice place to add bridge crap to, and
neither is netif_rx...
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 00/14] shrink skb cb to 44 bytes

[Eric Dumazet]

On Mon, 2015-03-02 at 21:42 +0100, Florian Westphal wrote:

> Thats right.  Do you think its worth to already move cb[] near the end
> of skb and alter build_skb to not clear it anymore?
> 
> Which of the ideas, in your opinion, is worth pursuing first (if any)?

moving cb[] near the end will void my patches to use one cache line per
skb in TCP receive queue ( or write queue)

971f10eca186ca tcp: better TCP_SKB_CB layout to reduce cache line misses


I have worked a bit (3 months ago) about doing the skb->cb[] selective
clearing, but a lot of alloc_skb() users _assume_ it is already
cleared. 

That seemed a lot of work to me, because of the many alloc_skb()
variants we have. But definitely worth trying to complete.

Re: [PATCH RFC 00/14] shrink skb cb to 44 bytes

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Mar 2015 11:49:23 -0800

> Size of skb->cb[] is not the major factor. Trying to gain 4 or 8 bytes
> is not going to improve performance a lot.
> 
> The real problem is that we clear it in skb_alloc()/build_skb(), instead
> of each layer doing so at demand, and only the part that matters for
> this layer.
> 
> Basically, skb->cb[] could be 80 or 160 bytes instead of 48, and we
> should not care, as long as no layer does a stupid/lazy 
> 
> memset(skb->cb, 0, sizeof(skb->cb))
> 
> Presumably skb_clone() could be extended to receive the length of
> skb->cb[] that current layer cares about.

Regardless, I think Florian's work has value.

Re: [PATCH RFC 00/14] shrink skb cb to 44 bytes

[Eric Dumazet]

On Mon, 2015-03-02 at 17:17 -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 02 Mar 2015 11:49:23 -0800
> 
> > Size of skb->cb[] is not the major factor. Trying to gain 4 or 8 bytes
> > is not going to improve performance a lot.
> > 
> > The real problem is that we clear it in skb_alloc()/build_skb(), instead
> > of each layer doing so at demand, and only the part that matters for
> > this layer.
> > 
> > Basically, skb->cb[] could be 80 or 160 bytes instead of 48, and we
> > should not care, as long as no layer does a stupid/lazy 
> > 
> > memset(skb->cb, 0, sizeof(skb->cb))
> > 
> > Presumably skb_clone() could be extended to receive the length of
> > skb->cb[] that current layer cares about.
> 
> Regardless, I think Florian's work has value.

Of course. I hope my answer was not implying the contrary !

48 -> 44 cb change, with the 8 bytes alignment and various __packed
tricks that might confuse compilers on some arches, will be hard to
quantify in term of performances on all arches.

About the GRO layout change, reason why 'struct sk_buff *last;' is at
the end of struct napi_gro_cb is that this field is not used in fast
path.

Note : We could try to use one bit in skb to advertise zero shinfo(skb).

Many skbs have a zeroed shinfo() (but shinfo->dataref == 1) , and
dereferencing skb_shinfo adds a cache line miss. 

-> We could avoid memset(shinfo, 0, offsetof(struct skb_shared_info,
dataref)) & atomic_set(&shinfo->dataref, 1); 

 in alloc_skb() and friends completely.

Unfortunately this kind of change would be quite invasive...

Re: [PATCH RFC 00/14] shrink skb cb to 44 bytes

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Mar 2015 20:02:05 -0800

> About the GRO layout change, reason why 'struct sk_buff *last;' is at
> the end of struct napi_gro_cb is that this field is not used in fast
> path.

Understood.

While reviewing this I noticed that the jiffies timestamp in GRO cb
could really be u32 if we want instead of full "unsigned long".

> Note : We could try to use one bit in skb to advertise zero shinfo(skb).
> 
> Many skbs have a zeroed shinfo() (but shinfo->dataref == 1) , and
> dereferencing skb_shinfo adds a cache line miss. 
> 
> -> We could avoid memset(shinfo, 0, offsetof(struct skb_shared_info,
> dataref)) & atomic_set(&shinfo->dataref, 1); 
> 
>  in alloc_skb() and friends completely.
> 
> Unfortunately this kind of change would be quite invasive...

Right, all these kinds of things touch everything.

Re: [PATCH RFC 05/14] drivers: wireless: ar5523: use container_of

[Pontus Fuchs]

On 2015-03-02 18:40, Florian Westphal wrote:
> Compile tested only due to lack of hw.
>
> If we want to shrink skb->cb then we'd have to see about
> reducing struct ieee80211_tx_info, which gets embedded inside
> skb->cb[].
>
> It provides a scratch space to be used by wireless drivers.
> ar5523 uses the maximum space available today (40 bytes), but it seems
> we don't need this -- data->skb pointer seems to always point back to the
> skb whose cb buffer the data structure resides, iow, given a pointer to the
> embedded control buffer we can infer the skb address.
>
> Cc: linux-wireless@vger.kernel.org
> Signed-off-by: Florian Westphal <fw@strlen.de>

Tested-by: Pontus Fuchs <pontus.fuchs@gmail.com>

Re: [PATCH RFC 00/14] shrink skb cb to 44 bytes

[Florian Westphal]

David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 02 Mar 2015 20:02:05 -0800
> 
> > About the GRO layout change, reason why 'struct sk_buff *last;' is at
> > the end of struct napi_gro_cb is that this field is not used in fast
> > path.
> 
> Understood.

Moved it back to end, thanks!

> While reviewing this I noticed that the jiffies timestamp in GRO cb
> could really be u32 if we want instead of full "unsigned long".

Made it an u16 at the moment -- then __packed is no longer needed
and it fits in 40 bytes.

> > Note : We could try to use one bit in skb to advertise zero shinfo(skb).
> > 
> > Many skbs have a zeroed shinfo() (but shinfo->dataref == 1) , and
> > dereferencing skb_shinfo adds a cache line miss. 
> > 
> > -> We could avoid memset(shinfo, 0, offsetof(struct skb_shared_info,
> > dataref)) & atomic_set(&shinfo->dataref, 1); 
> > 
> >  in alloc_skb() and friends completely.
> > 
> > Unfortunately this kind of change would be quite invasive...
>
> Right, all these kinds of things touch everything.

Indeed, but thanks for the hint Eric -- I'll investigate.

Re: [PATCH RFC 14/14] net: introduce and use KERNEL_MAX_HEADER_PARSE_ADDRLEN

[Willem de Bruijn]

> index 6fb6bdf..97eb50c 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -433,6 +433,7 @@ mac802154_header_parse(const struct sk_buff *skb, unsigned char *haddr)
>                 return 0;
>         }
>
> +       dev_validate_header_parse_addrlen(sizeof(*addr));
>         *addr = hdr.source;
>         return sizeof(*addr);
>  }
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 9db8369..1a4dcbb 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1816,7 +1816,8 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
>                 skb = nskb;
>         }
>
> -       sock_skb_cb_check_size(sizeof(*PACKET_SKB_CB(skb)) + MAX_ADDR_LEN - 8);
> +       sock_skb_cb_check_size(sizeof(*PACKET_SKB_CB(skb)) +
> +                              KERNEL_MAX_HEADER_PARSE_ADDRLEN);

KERNEL_MAX_HEADER_PARSE_ADDRLEN - 8, as the first 8
bytes are still written into the space allocated in packet_skb_cb?

Re: [PATCH RFC 14/14] net: introduce and use KERNEL_MAX_HEADER_PARSE_ADDRLEN

[Florian Westphal]

Willem de Bruijn <willemb@google.com> wrote:
> > index 6fb6bdf..97eb50c 100644
> > --- a/net/mac802154/iface.c
> > +++ b/net/mac802154/iface.c
> > @@ -433,6 +433,7 @@ mac802154_header_parse(const struct sk_buff *skb, unsigned char *haddr)
> >                 return 0;
> >         }
> >
> > +       dev_validate_header_parse_addrlen(sizeof(*addr));
> >         *addr = hdr.source;
> >         return sizeof(*addr);
> >  }
> > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > index 9db8369..1a4dcbb 100644
> > --- a/net/packet/af_packet.c
> > +++ b/net/packet/af_packet.c
> > @@ -1816,7 +1816,8 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
> >                 skb = nskb;
> >         }
> >
> > -       sock_skb_cb_check_size(sizeof(*PACKET_SKB_CB(skb)) + MAX_ADDR_LEN - 8);
> > +       sock_skb_cb_check_size(sizeof(*PACKET_SKB_CB(skb)) +
> > +                              KERNEL_MAX_HEADER_PARSE_ADDRLEN);
> 
> KERNEL_MAX_HEADER_PARSE_ADDRLEN - 8, as the first 8
> bytes are still written into the space allocated in packet_skb_cb?

Good catch, thanks!

I'll make this a FIELD_SIZEOF construct using sll_addr to make
it a bit more obvious where that 8 is coming from.