* [PATCH net-next] net: Move skb decrypted field, avoid explicity copy
@ 2018-07-17 7:18 Stefano Brivio
2018-07-17 8:18 ` kbuild test robot
2018-07-17 9:25 ` kbuild test robot
0 siblings, 2 replies; 3+ messages in thread
From: Stefano Brivio @ 2018-07-17 7:18 UTC (permalink / raw)
To: David S. Miller; +Cc: Boris Pismenny, Stephen Rothwell, netdev
Commit 784abe24c903 ("net: Add decrypted field to skb")
introduced a 'decrypted' field that is explicitly copied on skb
copy and clone.
Move it between headers_start[0] and headers_end[0], so that we
don't need to copy it explicitly as it's copied by the memcpy()
in __copy_skb_header().
While at it, drop the assignment in __skb_clone(), it was
already redundant.
This doesn't change the size of sk_buff or cacheline boundaries.
The 15-bits hole before tc_index becomes a 14-bits hole, and
will be again a 15-bits hole when this change is merged with
commit 8b7008620b84 ("net: Don't copy pfmemalloc flag in
__copy_skb_header()").
Fixes: 784abe24c903 ("net: Add decrypted field to skb")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
include/linux/skbuff.h | 9 ++++-----
net/core/skbuff.c | 9 +++------
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3ceb8dcc54da..14bc9ebe30f2 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -630,7 +630,6 @@ typedef unsigned char *sk_buff_data_t;
* @hash: the packet hash
* @queue_mapping: Queue mapping for multiqueue devices
* @xmit_more: More SKBs are pending for this queue
- * @decrypted: Decrypted SKB
* @ndisc_nodetype: router type (from link layer)
* @ooo_okay: allow the mapping of a socket to a queue to be changed
* @l4_hash: indicate hash is a canonical 4-tuple hash over transport
@@ -641,6 +640,7 @@ typedef unsigned char *sk_buff_data_t;
* @no_fcs: Request NIC to treat last 4 bytes as Ethernet FCS
* @csum_not_inet: use CRC32c to resolve CHECKSUM_PARTIAL
* @dst_pending_confirm: need to confirm neighbour
+ * @decrypted: Decrypted SKB
* @napi_id: id of the NAPI struct this skb came from
* @secmark: security marking
* @mark: Generic packet mark
@@ -737,11 +737,7 @@ struct sk_buff {
peeked:1,
head_frag:1,
xmit_more:1,
-#ifdef CONFIG_TLS_DEVICE
- decrypted:1;
-#else
__unused:1;
-#endif
/* fields enclosed in headers_start/headers_end are copied
* using a single memcpy() in __copy_skb_header()
@@ -797,6 +793,9 @@ struct sk_buff {
__u8 tc_redirected:1;
__u8 tc_from_ingress:1;
#endif
+#ifdef CONFIG_TLS_DEVICE
+ __u8 decrypted:1;
+#endif
#ifdef CONFIG_NET_SCHED
__u16 tc_index; /* traffic control index */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cfd6c6f35f9c..b8a563b2d2df 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -805,9 +805,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
* It is not yet because we do not want to have a 16 bit hole
*/
new->queue_mapping = old->queue_mapping;
-#ifdef CONFIG_TLS_DEVICE
- new->decrypted = old->decrypted;
-#endif
memcpy(&new->headers_start, &old->headers_start,
offsetof(struct sk_buff, headers_end) -
@@ -836,6 +833,9 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
#ifdef CONFIG_XPS
CHECK_SKB_FIELD(sender_cpu);
#endif
+#ifdef CONFIG_TLS_DEVICE
+ CHECK_SKB_FIELD(decrypted);
+#endif
#ifdef CONFIG_NET_SCHED
CHECK_SKB_FIELD(tc_index);
#endif
@@ -868,9 +868,6 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
C(head_frag);
C(data);
C(truesize);
-#ifdef CONFIG_TLS_DEVICE
- C(decrypted);
-#endif
refcount_set(&n->users, 1);
atomic_inc(&(skb_shinfo(skb)->dataref));
--
2.15.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: Move skb decrypted field, avoid explicity copy
2018-07-17 7:18 [PATCH net-next] net: Move skb decrypted field, avoid explicity copy Stefano Brivio
@ 2018-07-17 8:18 ` kbuild test robot
2018-07-17 9:25 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2018-07-17 8:18 UTC (permalink / raw)
To: Stefano Brivio
Cc: kbuild-all, David S. Miller, Boris Pismenny, Stephen Rothwell,
netdev
[-- Attachment #1: Type: text/plain, Size: 5173 bytes --]
Hi Stefano,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Stefano-Brivio/net-Move-skb-decrypted-field-avoid-explicity-copy/20180717-152125
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=ia64
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:10,
from include/linux/list.h:9,
from include/linux/module.h:9,
from net//core/skbuff.c:41:
net//core/skbuff.c: In function '__copy_skb_header':
>> net//core/skbuff.c:787:31: error: attempt to take address of bit-field structure member 'decrypted'
BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
^~~~~~~
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^~~~~~~~~~~~~~~~
net//core/skbuff.c:787:2: note: in expansion of macro 'BUILD_BUG_ON'
BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
^~~~~~~~~~~~
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
net//core/skbuff.c:787:15: note: in expansion of macro 'offsetof'
BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
^~~~~~~~
net//core/skbuff.c:837:2: note: in expansion of macro 'CHECK_SKB_FIELD'
CHECK_SKB_FIELD(decrypted);
^~~~~~~~~~~~~~~
net//core/skbuff.c:789:31: error: attempt to take address of bit-field structure member 'decrypted'
BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
^~~~~~~
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^~~~~~~~~~~~~~~~
net//core/skbuff.c:789:2: note: in expansion of macro 'BUILD_BUG_ON'
BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
^~~~~~~~~~~~
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
net//core/skbuff.c:789:15: note: in expansion of macro 'offsetof'
BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
^~~~~~~~
net//core/skbuff.c:837:2: note: in expansion of macro 'CHECK_SKB_FIELD'
CHECK_SKB_FIELD(decrypted);
^~~~~~~~~~~~~~~
vim +/decrypted +787 net//core/skbuff.c
795bb1c0 Jesper Dangaard Brouer 2016-02-08 784
b1937227 Eric Dumazet 2014-09-28 785 /* Make sure a field is enclosed inside headers_start/headers_end section */
b1937227 Eric Dumazet 2014-09-28 786 #define CHECK_SKB_FIELD(field) \
b1937227 Eric Dumazet 2014-09-28 @787 BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
b1937227 Eric Dumazet 2014-09-28 788 offsetof(struct sk_buff, headers_start)); \
b1937227 Eric Dumazet 2014-09-28 789 BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
b1937227 Eric Dumazet 2014-09-28 790 offsetof(struct sk_buff, headers_end)); \
b1937227 Eric Dumazet 2014-09-28 791
:::::: The code at line 787 was first introduced by commit
:::::: b1937227316417aa7568d01e6fa1f272e98fb890 net: reorganize sk_buff for faster __copy_skb_header()
:::::: TO: Eric Dumazet <edumazet@google.com>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50921 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: Move skb decrypted field, avoid explicity copy
2018-07-17 7:18 [PATCH net-next] net: Move skb decrypted field, avoid explicity copy Stefano Brivio
2018-07-17 8:18 ` kbuild test robot
@ 2018-07-17 9:25 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2018-07-17 9:25 UTC (permalink / raw)
To: Stefano Brivio
Cc: kbuild-all, David S. Miller, Boris Pismenny, Stephen Rothwell,
netdev
[-- Attachment #1: Type: text/plain, Size: 6201 bytes --]
Hi Stefano,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Stefano-Brivio/net-Move-skb-decrypted-field-avoid-explicity-copy/20180717-152125
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/module.h:9,
from net//core/skbuff.c:41:
net//core/skbuff.c: In function '__copy_skb_header':
net//core/skbuff.c:787:31: error: attempt to take address of bit-field structure member 'decrypted'
BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^~~~~~~~~~~~~~~~
>> net//core/skbuff.c:787:2: note: in expansion of macro 'BUILD_BUG_ON'
BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
^~~~~~~~~~~~
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
>> net//core/skbuff.c:787:15: note: in expansion of macro 'offsetof'
BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
^~~~~~~~
>> net//core/skbuff.c:837:2: note: in expansion of macro 'CHECK_SKB_FIELD'
CHECK_SKB_FIELD(decrypted);
^~~~~~~~~~~~~~~
net//core/skbuff.c:789:31: error: attempt to take address of bit-field structure member 'decrypted'
BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^~~~~~~~~~~~~~~~
net//core/skbuff.c:789:2: note: in expansion of macro 'BUILD_BUG_ON'
BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
^~~~~~~~~~~~
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
net//core/skbuff.c:789:15: note: in expansion of macro 'offsetof'
BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
^~~~~~~~
>> net//core/skbuff.c:837:2: note: in expansion of macro 'CHECK_SKB_FIELD'
CHECK_SKB_FIELD(decrypted);
^~~~~~~~~~~~~~~
vim +/CHECK_SKB_FIELD +837 net//core/skbuff.c
784
785 /* Make sure a field is enclosed inside headers_start/headers_end section */
786 #define CHECK_SKB_FIELD(field) \
> 787 BUILD_BUG_ON(offsetof(struct sk_buff, field) < \
788 offsetof(struct sk_buff, headers_start)); \
789 BUILD_BUG_ON(offsetof(struct sk_buff, field) > \
790 offsetof(struct sk_buff, headers_end)); \
791
792 static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
793 {
794 new->tstamp = old->tstamp;
795 /* We do not copy old->sk */
796 new->dev = old->dev;
797 memcpy(new->cb, old->cb, sizeof(old->cb));
798 skb_dst_copy(new, old);
799 #ifdef CONFIG_XFRM
800 new->sp = secpath_get(old->sp);
801 #endif
802 __nf_copy(new, old, false);
803
804 /* Note : this field could be in headers_start/headers_end section
805 * It is not yet because we do not want to have a 16 bit hole
806 */
807 new->queue_mapping = old->queue_mapping;
808
809 memcpy(&new->headers_start, &old->headers_start,
810 offsetof(struct sk_buff, headers_end) -
811 offsetof(struct sk_buff, headers_start));
812 CHECK_SKB_FIELD(protocol);
813 CHECK_SKB_FIELD(csum);
814 CHECK_SKB_FIELD(hash);
815 CHECK_SKB_FIELD(priority);
816 CHECK_SKB_FIELD(skb_iif);
817 CHECK_SKB_FIELD(vlan_proto);
818 CHECK_SKB_FIELD(vlan_tci);
819 CHECK_SKB_FIELD(transport_header);
820 CHECK_SKB_FIELD(network_header);
821 CHECK_SKB_FIELD(mac_header);
822 CHECK_SKB_FIELD(inner_protocol);
823 CHECK_SKB_FIELD(inner_transport_header);
824 CHECK_SKB_FIELD(inner_network_header);
825 CHECK_SKB_FIELD(inner_mac_header);
826 CHECK_SKB_FIELD(mark);
827 #ifdef CONFIG_NETWORK_SECMARK
828 CHECK_SKB_FIELD(secmark);
829 #endif
830 #ifdef CONFIG_NET_RX_BUSY_POLL
831 CHECK_SKB_FIELD(napi_id);
832 #endif
833 #ifdef CONFIG_XPS
834 CHECK_SKB_FIELD(sender_cpu);
835 #endif
836 #ifdef CONFIG_TLS_DEVICE
> 837 CHECK_SKB_FIELD(decrypted);
838 #endif
839 #ifdef CONFIG_NET_SCHED
840 CHECK_SKB_FIELD(tc_index);
841 #endif
842
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64662 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-07-17 10:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-17 7:18 [PATCH net-next] net: Move skb decrypted field, avoid explicity copy Stefano Brivio
2018-07-17 8:18 ` kbuild test robot
2018-07-17 9:25 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).