diff options
author | Daniel Borkmann <daniel@iogearbox.net> | 2018-03-30 16:50:16 -0400 |
---|---|---|
committer | Daniel Borkmann <daniel@iogearbox.net> | 2018-03-30 16:50:17 -0400 |
commit | 807ae7daf5fb9ba9ef688344ae7c0d8cbebd211c (patch) | |
tree | 630eb9ec8764b825abe20e0457df291dd0c1f9eb | |
parent | 1379ef828a18d8f81c526b25e4d5685caa2cfd65 (diff) | |
parent | 6ef6d84ceee2262a8c7a4247616c7eb71268b3f9 (diff) |
Merge branch 'bpf-sockmap-sg-api-fixes'
Prashant Bhole says:
====================
These patches fix sg api usage in sockmap. Previously sockmap didn't
use sg_init_table(), which caused hitting BUG_ON in sg api, when
CONFIG_DEBUG_SG is enabled
v1: added sg_init_table() calls wherever needed.
v2:
- Patch1 adds new helper function in sg api. sg_init_marker()
- Patch2 sg_init_marker() and sg_init_table() in appropriate places
Backgroud:
While reviewing v1, John Fastabend raised a valid point about
unnecessary memset in sg_init_table() because sockmap uses sg table
which embedded in a struct. As enclosing struct is zeroed out, there
is unnecessary memset in sg_init_table.
So Daniel Borkmann suggested to define another static inline function
in scatterlist.h which only initializes sg_magic. Also this function
will be called from sg_init_table. From this suggestion I defined a
function sg_init_marker() which sets sg_magic and calls sg_mark_end()
====================
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
-rw-r--r-- | include/linux/scatterlist.h | 18 | ||||
-rw-r--r-- | kernel/bpf/sockmap.c | 13 | ||||
-rw-r--r-- | lib/scatterlist.c | 9 |
3 files changed, 27 insertions, 13 deletions
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 22b2131bcdcd..aa5d4eb725f5 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h | |||
@@ -248,6 +248,24 @@ static inline void *sg_virt(struct scatterlist *sg) | |||
248 | return page_address(sg_page(sg)) + sg->offset; | 248 | return page_address(sg_page(sg)) + sg->offset; |
249 | } | 249 | } |
250 | 250 | ||
251 | /** | ||
252 | * sg_init_marker - Initialize markers in sg table | ||
253 | * @sgl: The SG table | ||
254 | * @nents: Number of entries in table | ||
255 | * | ||
256 | **/ | ||
257 | static inline void sg_init_marker(struct scatterlist *sgl, | ||
258 | unsigned int nents) | ||
259 | { | ||
260 | #ifdef CONFIG_DEBUG_SG | ||
261 | unsigned int i; | ||
262 | |||
263 | for (i = 0; i < nents; i++) | ||
264 | sgl[i].sg_magic = SG_MAGIC; | ||
265 | #endif | ||
266 | sg_mark_end(&sgl[nents - 1]); | ||
267 | } | ||
268 | |||
251 | int sg_nents(struct scatterlist *sg); | 269 | int sg_nents(struct scatterlist *sg); |
252 | int sg_nents_for_len(struct scatterlist *sg, u64 len); | 270 | int sg_nents_for_len(struct scatterlist *sg, u64 len); |
253 | struct scatterlist *sg_next(struct scatterlist *); | 271 | struct scatterlist *sg_next(struct scatterlist *); |
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c index 9192fdbcbccc..d2bda5aa25d7 100644 --- a/kernel/bpf/sockmap.c +++ b/kernel/bpf/sockmap.c | |||
@@ -341,7 +341,7 @@ retry: | |||
341 | md->sg_start++; | 341 | md->sg_start++; |
342 | if (md->sg_start == MAX_SKB_FRAGS) | 342 | if (md->sg_start == MAX_SKB_FRAGS) |
343 | md->sg_start = 0; | 343 | md->sg_start = 0; |
344 | memset(sg, 0, sizeof(*sg)); | 344 | sg_init_table(sg, 1); |
345 | 345 | ||
346 | if (md->sg_start == md->sg_end) | 346 | if (md->sg_start == md->sg_end) |
347 | break; | 347 | break; |
@@ -843,7 +843,7 @@ static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) | |||
843 | } | 843 | } |
844 | 844 | ||
845 | sg = md.sg_data; | 845 | sg = md.sg_data; |
846 | sg_init_table(sg, MAX_SKB_FRAGS); | 846 | sg_init_marker(sg, MAX_SKB_FRAGS); |
847 | rcu_read_unlock(); | 847 | rcu_read_unlock(); |
848 | 848 | ||
849 | lock_sock(sk); | 849 | lock_sock(sk); |
@@ -950,10 +950,14 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page, | |||
950 | 950 | ||
951 | lock_sock(sk); | 951 | lock_sock(sk); |
952 | 952 | ||
953 | if (psock->cork_bytes) | 953 | if (psock->cork_bytes) { |
954 | m = psock->cork; | 954 | m = psock->cork; |
955 | else | 955 | sg = &m->sg_data[m->sg_end]; |
956 | } else { | ||
956 | m = &md; | 957 | m = &md; |
958 | sg = m->sg_data; | ||
959 | sg_init_marker(sg, MAX_SKB_FRAGS); | ||
960 | } | ||
957 | 961 | ||
958 | /* Catch case where ring is full and sendpage is stalled. */ | 962 | /* Catch case where ring is full and sendpage is stalled. */ |
959 | if (unlikely(m->sg_end == m->sg_start && | 963 | if (unlikely(m->sg_end == m->sg_start && |
@@ -961,7 +965,6 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page, | |||
961 | goto out_err; | 965 | goto out_err; |
962 | 966 | ||
963 | psock->sg_size += size; | 967 | psock->sg_size += size; |
964 | sg = &m->sg_data[m->sg_end]; | ||
965 | sg_set_page(sg, page, size, offset); | 968 | sg_set_page(sg, page, size, offset); |
966 | get_page(page); | 969 | get_page(page); |
967 | m->sg_copy[m->sg_end] = true; | 970 | m->sg_copy[m->sg_end] = true; |
diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 53728d391d3a..06dad7a072fd 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c | |||
@@ -132,14 +132,7 @@ EXPORT_SYMBOL(sg_last); | |||
132 | void sg_init_table(struct scatterlist *sgl, unsigned int nents) | 132 | void sg_init_table(struct scatterlist *sgl, unsigned int nents) |
133 | { | 133 | { |
134 | memset(sgl, 0, sizeof(*sgl) * nents); | 134 | memset(sgl, 0, sizeof(*sgl) * nents); |
135 | #ifdef CONFIG_DEBUG_SG | 135 | sg_init_marker(sgl, nents); |
136 | { | ||
137 | unsigned int i; | ||
138 | for (i = 0; i < nents; i++) | ||
139 | sgl[i].sg_magic = SG_MAGIC; | ||
140 | } | ||
141 | #endif | ||
142 | sg_mark_end(&sgl[nents - 1]); | ||
143 | } | 136 | } |
144 | EXPORT_SYMBOL(sg_init_table); | 137 | EXPORT_SYMBOL(sg_init_table); |
145 | 138 | ||