* [PATCH v2 0/2] net/sctp: Avoid allocating high order memory with kmalloc()
From: Konstantin Khorenko @ 2018-08-03 16:21 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: oleg.babin, netdev, linux-sctp, David S . Miller, Vlad Yasevich,
Neil Horman, Xin Long, Andrey Ryabinin, Konstantin Khorenko
In-Reply-To: <20180724173647.GA8881@localhost.localdomain>
Each SCTP association can have up to 65535 input and output streams.
For each stream type an array of sctp_stream_in or sctp_stream_out
structures is allocated using kmalloc_array() function. This function
allocates physically contiguous memory regions, so this can lead
to allocation of memory regions of very high order, i.e.:
sizeof(struct sctp_stream_out) == 24,
((65535 * 24) / 4096) == 383 memory pages (4096 byte per page),
which means 9th memory order.
This can lead to a memory allocation failures on the systems
under a memory stress.
We actually do not need these arrays of memory to be physically
contiguous. Possible simple solution would be to use kvmalloc()
instread of kmalloc() as kvmalloc() can allocate physically scattered
pages if contiguous pages are not available. But the problem
is that the allocation can happed in a softirq context with
GFP_ATOMIC flag set, and kvmalloc() cannot be used in this scenario.
So the other possible solution is to use flexible arrays instead of
contiguios arrays of memory so that the memory would be allocated
on a per-page basis.
This patchset replaces kvmalloc() with flex_array usage.
It consists of two parts:
* First patch is preparatory - it mechanically wraps all direct
access to assoc->stream.out[] and assoc->stream.in[] arrays
with SCTP_SO() and SCTP_SI() wrappers so that later a direct
array access could be easily changed to an access to a
flex_array (or any other possible alternative).
* Second patch replaces kmalloc_array() with flex_array usage.
Oleg Babin (2):
net/sctp: Make wrappers for accessing in/out streams
net/sctp: Replace in/out stream arrays with flex_array
include/net/sctp/structs.h | 31 ++++----
net/sctp/chunk.c | 6 +-
net/sctp/outqueue.c | 11 +--
net/sctp/socket.c | 4 +-
net/sctp/stream.c | 165 +++++++++++++++++++++++++++++--------------
net/sctp/stream_interleave.c | 20 +++---
net/sctp/stream_sched.c | 13 ++--
net/sctp/stream_sched_prio.c | 22 +++---
net/sctp/stream_sched_rr.c | 8 +--
9 files changed, 175 insertions(+), 105 deletions(-)
v2 changes:
sctp_stream_in() users are updated to provide stream as an argument,
sctp_stream_{in,out}_ptr() are now just sctp_stream_{in,out}().
Performance results:
====================
* Kernel: v4.18-rc6 - stock and with 2 patches from Oleg (earlier in this thread)
* Node: CPU (8 cores): Intel(R) Xeon(R) CPU E31230 @ 3.20GHz
RAM: 32 Gb
* netperf: taken from https://github.com/HewlettPackard/netperf.git,
compiled from sources with sctp support
* netperf server and client are run on the same node
* ip link set lo mtu 1500
The script used to run tests:
# cat run_tests.sh
#!/bin/bash
for test in SCTP_STREAM SCTP_STREAM_MANY SCTP_RR SCTP_RR_MANY; do
echo "TEST: $test";
for i in `seq 1 3`; do
echo "Iteration: $i";
set -x
netperf -t $test -H localhost -p 22222 -S 200000,200000 -s 200000,200000 \
-l 60 -- -m 1452;
set +x
done
done
================================================
Results (a bit reformatted to be more readable):
Recv Send Send
Socket Socket Message Elapsed
Size Size Size Time Throughput
bytes bytes bytes secs. 10^6bits/sec
v4.18-rc7 v4.18-rc7 + fixes
TEST: SCTP_STREAM
212992 212992 1452 60.21 1125.52 1247.04
212992 212992 1452 60.20 1376.38 1149.95
212992 212992 1452 60.20 1131.40 1163.85
TEST: SCTP_STREAM_MANY
212992 212992 1452 60.00 1111.00 1310.05
212992 212992 1452 60.00 1188.55 1130.50
212992 212992 1452 60.00 1108.06 1162.50
===========
Local /Remote
Socket Size Request Resp. Elapsed Trans.
Send Recv Size Size Time Rate
bytes Bytes bytes bytes secs. per sec
v4.18-rc7 v4.18-rc7 + fixes
TEST: SCTP_RR
212992 212992 1 1 60.00 45486.98 46089.43
212992 212992 1 1 60.00 45584.18 45994.21
212992 212992 1 1 60.00 45703.86 45720.84
TEST: SCTP_RR_MANY
212992 212992 1 1 60.00 40.75 40.77
212992 212992 1 1 60.00 40.58 40.08
212992 212992 1 1 60.00 39.98 39.97
--
2.15.1
^ permalink raw reply
* [PATCH v2 1/2] net/sctp: Make wrappers for accessing in/out streams
From: Konstantin Khorenko @ 2018-08-03 16:21 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: oleg.babin, netdev, linux-sctp, David S . Miller, Vlad Yasevich,
Neil Horman, Xin Long, Andrey Ryabinin, Konstantin Khorenko
In-Reply-To: <20180803162102.19540-1-khorenko@virtuozzo.com>
This patch introduces wrappers for accessing in/out streams indirectly.
This will enable to replace physically contiguous memory arrays
of streams with flexible arrays (or maybe any other appropriate
mechanism) which do memory allocation on a per-page basis.
Signed-off-by: Oleg Babin <obabin@virtuozzo.com>
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
v2 changes:
sctp_stream_in() users are updated to provide stream as an argument,
sctp_stream_{in,out}_ptr() are now just sctp_stream_{in,out}().
---
include/net/sctp/structs.h | 30 +++++++-----
net/sctp/chunk.c | 6 ++-
net/sctp/outqueue.c | 11 +++--
net/sctp/socket.c | 4 +-
net/sctp/stream.c | 107 +++++++++++++++++++++++++------------------
net/sctp/stream_interleave.c | 20 ++++----
net/sctp/stream_sched.c | 13 +++---
net/sctp/stream_sched_prio.c | 22 ++++-----
net/sctp/stream_sched_rr.c | 8 ++--
9 files changed, 124 insertions(+), 97 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index dbe1b911a24d..dc48c8e2b293 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -394,37 +394,35 @@ void sctp_stream_update(struct sctp_stream *stream, struct sctp_stream *new);
/* What is the current SSN number for this stream? */
#define sctp_ssn_peek(stream, type, sid) \
- ((stream)->type[sid].ssn)
+ (sctp_stream_##type((stream), (sid))->ssn)
/* Return the next SSN number for this stream. */
#define sctp_ssn_next(stream, type, sid) \
- ((stream)->type[sid].ssn++)
+ (sctp_stream_##type((stream), (sid))->ssn++)
/* Skip over this ssn and all below. */
#define sctp_ssn_skip(stream, type, sid, ssn) \
- ((stream)->type[sid].ssn = ssn + 1)
+ (sctp_stream_##type((stream), (sid))->ssn = ssn + 1)
/* What is the current MID number for this stream? */
#define sctp_mid_peek(stream, type, sid) \
- ((stream)->type[sid].mid)
+ (sctp_stream_##type((stream), (sid))->mid)
/* Return the next MID number for this stream. */
#define sctp_mid_next(stream, type, sid) \
- ((stream)->type[sid].mid++)
+ (sctp_stream_##type((stream), (sid))->mid++)
/* Skip over this mid and all below. */
#define sctp_mid_skip(stream, type, sid, mid) \
- ((stream)->type[sid].mid = mid + 1)
-
-#define sctp_stream_in(asoc, sid) (&(asoc)->stream.in[sid])
+ (sctp_stream_##type((stream), (sid))->mid = mid + 1)
/* What is the current MID_uo number for this stream? */
#define sctp_mid_uo_peek(stream, type, sid) \
- ((stream)->type[sid].mid_uo)
+ (sctp_stream_##type((stream), (sid))->mid_uo)
/* Return the next MID_uo number for this stream. */
#define sctp_mid_uo_next(stream, type, sid) \
- ((stream)->type[sid].mid_uo++)
+ (sctp_stream_##type((stream), (sid))->mid_uo++)
/*
* Pointers to address related SCTP functions.
@@ -1433,8 +1431,8 @@ struct sctp_stream_in {
};
struct sctp_stream {
- struct sctp_stream_out *out;
- struct sctp_stream_in *in;
+ struct flex_array *out;
+ struct flex_array *in;
__u16 outcnt;
__u16 incnt;
/* Current stream being sent, if any */
@@ -1456,6 +1454,14 @@ struct sctp_stream {
struct sctp_stream_interleave *si;
};
+struct sctp_stream_out *sctp_stream_out(const struct sctp_stream *stream,
+ __u16 sid);
+struct sctp_stream_in *sctp_stream_in(const struct sctp_stream *stream,
+ __u16 sid);
+
+#define SCTP_SO(s, i) sctp_stream_out((s), (i))
+#define SCTP_SI(s, i) sctp_stream_in((s), (i))
+
#define SCTP_STREAM_CLOSED 0x00
#define SCTP_STREAM_OPEN 0x01
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index bfb9f812e2ef..ce8087846f05 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -325,7 +325,8 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
if (SCTP_PR_TTL_ENABLED(chunk->sinfo.sinfo_flags) &&
time_after(jiffies, chunk->msg->expires_at)) {
struct sctp_stream_out *streamout =
- &chunk->asoc->stream.out[chunk->sinfo.sinfo_stream];
+ SCTP_SO(&chunk->asoc->stream,
+ chunk->sinfo.sinfo_stream);
if (chunk->sent_count) {
chunk->asoc->abandoned_sent[SCTP_PR_INDEX(TTL)]++;
@@ -339,7 +340,8 @@ int sctp_chunk_abandoned(struct sctp_chunk *chunk)
} else if (SCTP_PR_RTX_ENABLED(chunk->sinfo.sinfo_flags) &&
chunk->sent_count > chunk->sinfo.sinfo_timetolive) {
struct sctp_stream_out *streamout =
- &chunk->asoc->stream.out[chunk->sinfo.sinfo_stream];
+ SCTP_SO(&chunk->asoc->stream,
+ chunk->sinfo.sinfo_stream);
chunk->asoc->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
streamout->ext->abandoned_sent[SCTP_PR_INDEX(RTX)]++;
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index d68aa33485a9..d74d00b29942 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -80,7 +80,7 @@ static inline void sctp_outq_head_data(struct sctp_outq *q,
q->out_qlen += ch->skb->len;
stream = sctp_chunk_stream_no(ch);
- oute = q->asoc->stream.out[stream].ext;
+ oute = SCTP_SO(&q->asoc->stream, stream)->ext;
list_add(&ch->stream_list, &oute->outq);
}
@@ -101,7 +101,7 @@ static inline void sctp_outq_tail_data(struct sctp_outq *q,
q->out_qlen += ch->skb->len;
stream = sctp_chunk_stream_no(ch);
- oute = q->asoc->stream.out[stream].ext;
+ oute = SCTP_SO(&q->asoc->stream, stream)->ext;
list_add_tail(&ch->stream_list, &oute->outq);
}
@@ -372,7 +372,7 @@ static int sctp_prsctp_prune_sent(struct sctp_association *asoc,
sctp_insert_list(&asoc->outqueue.abandoned,
&chk->transmitted_list);
- streamout = &asoc->stream.out[chk->sinfo.sinfo_stream];
+ streamout = SCTP_SO(&asoc->stream, chk->sinfo.sinfo_stream);
asoc->sent_cnt_removable--;
asoc->abandoned_sent[SCTP_PR_INDEX(PRIO)]++;
streamout->ext->abandoned_sent[SCTP_PR_INDEX(PRIO)]++;
@@ -416,7 +416,7 @@ static int sctp_prsctp_prune_unsent(struct sctp_association *asoc,
asoc->abandoned_unsent[SCTP_PR_INDEX(PRIO)]++;
if (chk->sinfo.sinfo_stream < asoc->stream.outcnt) {
struct sctp_stream_out *streamout =
- &asoc->stream.out[chk->sinfo.sinfo_stream];
+ SCTP_SO(&asoc->stream, chk->sinfo.sinfo_stream);
streamout->ext->abandoned_unsent[SCTP_PR_INDEX(PRIO)]++;
}
@@ -1082,6 +1082,7 @@ static void sctp_outq_flush_data(struct sctp_flush_ctx *ctx,
/* Finally, transmit new packets. */
while ((chunk = sctp_outq_dequeue_data(ctx->q)) != NULL) {
__u32 sid = ntohs(chunk->subh.data_hdr->stream);
+ __u8 stream_state = SCTP_SO(&ctx->asoc->stream, sid)->state;
/* Has this chunk expired? */
if (sctp_chunk_abandoned(chunk)) {
@@ -1091,7 +1092,7 @@ static void sctp_outq_flush_data(struct sctp_flush_ctx *ctx,
continue;
}
- if (ctx->asoc->stream.out[sid].state == SCTP_STREAM_CLOSED) {
+ if (stream_state == SCTP_STREAM_CLOSED) {
sctp_outq_head_data(ctx->q, chunk);
break;
}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index ce620e878538..4582ab25bc4e 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1905,7 +1905,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
goto err;
}
- if (unlikely(!asoc->stream.out[sinfo->sinfo_stream].ext)) {
+ if (unlikely(!SCTP_SO(&asoc->stream, sinfo->sinfo_stream)->ext)) {
err = sctp_stream_init_ext(&asoc->stream, sinfo->sinfo_stream);
if (err)
goto err;
@@ -6958,7 +6958,7 @@ static int sctp_getsockopt_pr_streamstatus(struct sock *sk, int len,
if (!asoc || params.sprstat_sid >= asoc->stream.outcnt)
goto out;
- streamoute = asoc->stream.out[params.sprstat_sid].ext;
+ streamoute = SCTP_SO(&asoc->stream, params.sprstat_sid)->ext;
if (!streamoute) {
/* Not allocated yet, means all stats are 0 */
params.sprstat_abandoned_unsent = 0;
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index f1f1d1b232ba..56fadeec7cba 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -37,6 +37,18 @@
#include <net/sctp/sm.h>
#include <net/sctp/stream_sched.h>
+struct sctp_stream_out *sctp_stream_out(const struct sctp_stream *stream,
+ __u16 sid)
+{
+ return ((struct sctp_stream_out *)(stream->out)) + sid;
+}
+
+struct sctp_stream_in *sctp_stream_in(const struct sctp_stream *stream,
+ __u16 sid)
+{
+ return ((struct sctp_stream_in *)(stream->in)) + sid;
+}
+
/* Migrates chunks from stream queues to new stream queues if needed,
* but not across associations. Also, removes those chunks to streams
* higher than the new max.
@@ -78,34 +90,35 @@ static void sctp_stream_outq_migrate(struct sctp_stream *stream,
* sctp_stream_update will swap ->out pointers.
*/
for (i = 0; i < outcnt; i++) {
- kfree(new->out[i].ext);
- new->out[i].ext = stream->out[i].ext;
- stream->out[i].ext = NULL;
+ kfree(SCTP_SO(new, i)->ext);
+ SCTP_SO(new, i)->ext = SCTP_SO(stream, i)->ext;
+ SCTP_SO(stream, i)->ext = NULL;
}
}
for (i = outcnt; i < stream->outcnt; i++)
- kfree(stream->out[i].ext);
+ kfree(SCTP_SO(stream, i)->ext);
}
static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
gfp_t gfp)
{
- struct sctp_stream_out *out;
+ struct flex_array *out;
+ size_t elem_size = sizeof(struct sctp_stream_out);
- out = kmalloc_array(outcnt, sizeof(*out), gfp);
+ out = kmalloc_array(outcnt, elem_size, gfp);
if (!out)
return -ENOMEM;
if (stream->out) {
memcpy(out, stream->out, min(outcnt, stream->outcnt) *
- sizeof(*out));
+ elem_size);
kfree(stream->out);
}
if (outcnt > stream->outcnt)
- memset(out + stream->outcnt, 0,
- (outcnt - stream->outcnt) * sizeof(*out));
+ memset(((struct sctp_stream_out *)out) + stream->outcnt, 0,
+ (outcnt - stream->outcnt) * elem_size);
stream->out = out;
@@ -115,22 +128,23 @@ static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt,
gfp_t gfp)
{
- struct sctp_stream_in *in;
+ struct flex_array *in;
+ size_t elem_size = sizeof(struct sctp_stream_in);
- in = kmalloc_array(incnt, sizeof(*stream->in), gfp);
+ in = kmalloc_array(incnt, elem_size, gfp);
if (!in)
return -ENOMEM;
if (stream->in) {
memcpy(in, stream->in, min(incnt, stream->incnt) *
- sizeof(*in));
+ elem_size);
kfree(stream->in);
}
if (incnt > stream->incnt)
- memset(in + stream->incnt, 0,
- (incnt - stream->incnt) * sizeof(*in));
+ memset(((struct sctp_stream_in *)in) + stream->incnt, 0,
+ (incnt - stream->incnt) * elem_size);
stream->in = in;
@@ -162,7 +176,7 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
stream->outcnt = outcnt;
for (i = 0; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_OPEN;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_OPEN;
sched->init(stream);
@@ -193,7 +207,7 @@ int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid)
soute = kzalloc(sizeof(*soute), GFP_KERNEL);
if (!soute)
return -ENOMEM;
- stream->out[sid].ext = soute;
+ SCTP_SO(stream, sid)->ext = soute;
return sctp_sched_init_sid(stream, sid, GFP_KERNEL);
}
@@ -205,7 +219,7 @@ void sctp_stream_free(struct sctp_stream *stream)
sched->free(stream);
for (i = 0; i < stream->outcnt; i++)
- kfree(stream->out[i].ext);
+ kfree(SCTP_SO(stream, i)->ext);
kfree(stream->out);
kfree(stream->in);
}
@@ -215,12 +229,12 @@ void sctp_stream_clear(struct sctp_stream *stream)
int i;
for (i = 0; i < stream->outcnt; i++) {
- stream->out[i].mid = 0;
- stream->out[i].mid_uo = 0;
+ SCTP_SO(stream, i)->mid = 0;
+ SCTP_SO(stream, i)->mid_uo = 0;
}
for (i = 0; i < stream->incnt; i++)
- stream->in[i].mid = 0;
+ SCTP_SI(stream, i)->mid = 0;
}
void sctp_stream_update(struct sctp_stream *stream, struct sctp_stream *new)
@@ -273,8 +287,8 @@ static bool sctp_stream_outq_is_empty(struct sctp_stream *stream,
for (i = 0; i < str_nums; i++) {
__u16 sid = ntohs(str_list[i]);
- if (stream->out[sid].ext &&
- !list_empty(&stream->out[sid].ext->outq))
+ if (SCTP_SO(stream, sid)->ext &&
+ !list_empty(&SCTP_SO(stream, sid)->ext->outq))
return false;
}
@@ -361,11 +375,11 @@ int sctp_send_reset_streams(struct sctp_association *asoc,
if (out) {
if (str_nums)
for (i = 0; i < str_nums; i++)
- stream->out[str_list[i]].state =
+ SCTP_SO(stream, str_list[i])->state =
SCTP_STREAM_CLOSED;
else
for (i = 0; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_CLOSED;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_CLOSED;
}
asoc->strreset_chunk = chunk;
@@ -380,11 +394,11 @@ int sctp_send_reset_streams(struct sctp_association *asoc,
if (str_nums)
for (i = 0; i < str_nums; i++)
- stream->out[str_list[i]].state =
+ SCTP_SO(stream, str_list[i])->state =
SCTP_STREAM_OPEN;
else
for (i = 0; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_OPEN;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_OPEN;
goto out;
}
@@ -418,7 +432,7 @@ int sctp_send_reset_assoc(struct sctp_association *asoc)
/* Block further xmit of data until this request is completed */
for (i = 0; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_CLOSED;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_CLOSED;
asoc->strreset_chunk = chunk;
sctp_chunk_hold(asoc->strreset_chunk);
@@ -429,7 +443,7 @@ int sctp_send_reset_assoc(struct sctp_association *asoc)
asoc->strreset_chunk = NULL;
for (i = 0; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_OPEN;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_OPEN;
return retval;
}
@@ -609,10 +623,10 @@ struct sctp_chunk *sctp_process_strreset_outreq(
}
for (i = 0; i < nums; i++)
- stream->in[ntohs(str_p[i])].mid = 0;
+ SCTP_SI(stream, ntohs(str_p[i]))->mid = 0;
} else {
for (i = 0; i < stream->incnt; i++)
- stream->in[i].mid = 0;
+ SCTP_SI(stream, i)->mid = 0;
}
result = SCTP_STRRESET_PERFORMED;
@@ -683,11 +697,11 @@ struct sctp_chunk *sctp_process_strreset_inreq(
if (nums)
for (i = 0; i < nums; i++)
- stream->out[ntohs(str_p[i])].state =
+ SCTP_SO(stream, ntohs(str_p[i]))->state =
SCTP_STREAM_CLOSED;
else
for (i = 0; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_CLOSED;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_CLOSED;
asoc->strreset_chunk = chunk;
asoc->strreset_outstanding = 1;
@@ -786,11 +800,11 @@ struct sctp_chunk *sctp_process_strreset_tsnreq(
* incoming and outgoing streams.
*/
for (i = 0; i < stream->outcnt; i++) {
- stream->out[i].mid = 0;
- stream->out[i].mid_uo = 0;
+ SCTP_SO(stream, i)->mid = 0;
+ SCTP_SO(stream, i)->mid_uo = 0;
}
for (i = 0; i < stream->incnt; i++)
- stream->in[i].mid = 0;
+ SCTP_SI(stream, i)->mid = 0;
result = SCTP_STRRESET_PERFORMED;
@@ -979,15 +993,18 @@ struct sctp_chunk *sctp_process_strreset_resp(
sizeof(__u16);
if (result == SCTP_STRRESET_PERFORMED) {
+ struct sctp_stream_out *sout;
if (nums) {
for (i = 0; i < nums; i++) {
- stream->out[ntohs(str_p[i])].mid = 0;
- stream->out[ntohs(str_p[i])].mid_uo = 0;
+ sout = SCTP_SO(stream, ntohs(str_p[i]));
+ sout->mid = 0;
+ sout->mid_uo = 0;
}
} else {
for (i = 0; i < stream->outcnt; i++) {
- stream->out[i].mid = 0;
- stream->out[i].mid_uo = 0;
+ sout = SCTP_SO(stream, i);
+ sout->mid = 0;
+ sout->mid_uo = 0;
}
}
@@ -995,7 +1012,7 @@ struct sctp_chunk *sctp_process_strreset_resp(
}
for (i = 0; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_OPEN;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_OPEN;
*evp = sctp_ulpevent_make_stream_reset_event(asoc, flags,
nums, str_p, GFP_ATOMIC);
@@ -1050,15 +1067,15 @@ struct sctp_chunk *sctp_process_strreset_resp(
asoc->adv_peer_ack_point = asoc->ctsn_ack_point;
for (i = 0; i < stream->outcnt; i++) {
- stream->out[i].mid = 0;
- stream->out[i].mid_uo = 0;
+ SCTP_SO(stream, i)->mid = 0;
+ SCTP_SO(stream, i)->mid_uo = 0;
}
for (i = 0; i < stream->incnt; i++)
- stream->in[i].mid = 0;
+ SCTP_SI(stream, i)->mid = 0;
}
for (i = 0; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_OPEN;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_OPEN;
*evp = sctp_ulpevent_make_assoc_reset_event(asoc, flags,
stsn, rtsn, GFP_ATOMIC);
@@ -1072,7 +1089,7 @@ struct sctp_chunk *sctp_process_strreset_resp(
if (result == SCTP_STRRESET_PERFORMED)
for (i = number; i < stream->outcnt; i++)
- stream->out[i].state = SCTP_STREAM_OPEN;
+ SCTP_SO(stream, i)->state = SCTP_STREAM_OPEN;
else
stream->outcnt = number;
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index d3764c181299..0a78cdf86463 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -197,7 +197,7 @@ static struct sctp_ulpevent *sctp_intl_retrieve_partial(
__u32 next_fsn = 0;
int is_last = 0;
- sin = sctp_stream_in(ulpq->asoc, event->stream);
+ sin = sctp_stream_in(&ulpq->asoc->stream, event->stream);
skb_queue_walk(&ulpq->reasm, pos) {
struct sctp_ulpevent *cevent = sctp_skb2event(pos);
@@ -278,7 +278,7 @@ static struct sctp_ulpevent *sctp_intl_retrieve_reassembled(
__u32 pd_len = 0;
__u32 mid = 0;
- sin = sctp_stream_in(ulpq->asoc, event->stream);
+ sin = sctp_stream_in(&ulpq->asoc->stream, event->stream);
skb_queue_walk(&ulpq->reasm, pos) {
struct sctp_ulpevent *cevent = sctp_skb2event(pos);
@@ -368,7 +368,7 @@ static struct sctp_ulpevent *sctp_intl_reasm(struct sctp_ulpq *ulpq,
sctp_intl_store_reasm(ulpq, event);
- sin = sctp_stream_in(ulpq->asoc, event->stream);
+ sin = sctp_stream_in(&ulpq->asoc->stream, event->stream);
if (sin->pd_mode && event->mid == sin->mid &&
event->fsn == sin->fsn)
retval = sctp_intl_retrieve_partial(ulpq, event);
@@ -575,7 +575,7 @@ static struct sctp_ulpevent *sctp_intl_retrieve_partial_uo(
__u32 next_fsn = 0;
int is_last = 0;
- sin = sctp_stream_in(ulpq->asoc, event->stream);
+ sin = sctp_stream_in(&ulpq->asoc->stream, event->stream);
skb_queue_walk(&ulpq->reasm_uo, pos) {
struct sctp_ulpevent *cevent = sctp_skb2event(pos);
@@ -659,7 +659,7 @@ static struct sctp_ulpevent *sctp_intl_retrieve_reassembled_uo(
__u32 pd_len = 0;
__u32 mid = 0;
- sin = sctp_stream_in(ulpq->asoc, event->stream);
+ sin = sctp_stream_in(&ulpq->asoc->stream, event->stream);
skb_queue_walk(&ulpq->reasm_uo, pos) {
struct sctp_ulpevent *cevent = sctp_skb2event(pos);
@@ -750,7 +750,7 @@ static struct sctp_ulpevent *sctp_intl_reasm_uo(struct sctp_ulpq *ulpq,
sctp_intl_store_reasm_uo(ulpq, event);
- sin = sctp_stream_in(ulpq->asoc, event->stream);
+ sin = sctp_stream_in(&ulpq->asoc->stream, event->stream);
if (sin->pd_mode_uo && event->mid == sin->mid_uo &&
event->fsn == sin->fsn_uo)
retval = sctp_intl_retrieve_partial_uo(ulpq, event);
@@ -774,7 +774,7 @@ static struct sctp_ulpevent *sctp_intl_retrieve_first_uo(struct sctp_ulpq *ulpq)
skb_queue_walk(&ulpq->reasm_uo, pos) {
struct sctp_ulpevent *cevent = sctp_skb2event(pos);
- csin = sctp_stream_in(ulpq->asoc, cevent->stream);
+ csin = sctp_stream_in(&ulpq->asoc->stream, cevent->stream);
if (csin->pd_mode_uo)
continue;
@@ -875,7 +875,7 @@ static struct sctp_ulpevent *sctp_intl_retrieve_first(struct sctp_ulpq *ulpq)
skb_queue_walk(&ulpq->reasm, pos) {
struct sctp_ulpevent *cevent = sctp_skb2event(pos);
- csin = sctp_stream_in(ulpq->asoc, cevent->stream);
+ csin = sctp_stream_in(&ulpq->asoc->stream, cevent->stream);
if (csin->pd_mode)
continue;
@@ -1053,7 +1053,7 @@ static void sctp_intl_abort_pd(struct sctp_ulpq *ulpq, gfp_t gfp)
__u16 sid;
for (sid = 0; sid < stream->incnt; sid++) {
- struct sctp_stream_in *sin = &stream->in[sid];
+ struct sctp_stream_in *sin = SCTP_SI(stream, sid);
__u32 mid;
if (sin->pd_mode_uo) {
@@ -1247,7 +1247,7 @@ static void sctp_handle_fwdtsn(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk)
static void sctp_intl_skip(struct sctp_ulpq *ulpq, __u16 sid, __u32 mid,
__u8 flags)
{
- struct sctp_stream_in *sin = sctp_stream_in(ulpq->asoc, sid);
+ struct sctp_stream_in *sin = sctp_stream_in(&ulpq->asoc->stream, sid);
struct sctp_stream *stream = &ulpq->asoc->stream;
if (flags & SCTP_FTSN_U_BIT) {
diff --git a/net/sctp/stream_sched.c b/net/sctp/stream_sched.c
index f5fcd425232a..a6c04a94b08f 100644
--- a/net/sctp/stream_sched.c
+++ b/net/sctp/stream_sched.c
@@ -161,7 +161,7 @@ int sctp_sched_set_sched(struct sctp_association *asoc,
/* Give the next scheduler a clean slate. */
for (i = 0; i < asoc->stream.outcnt; i++) {
- void *p = asoc->stream.out[i].ext;
+ void *p = SCTP_SO(&asoc->stream, i)->ext;
if (!p)
continue;
@@ -175,7 +175,7 @@ int sctp_sched_set_sched(struct sctp_association *asoc,
asoc->outqueue.sched = n;
n->init(&asoc->stream);
for (i = 0; i < asoc->stream.outcnt; i++) {
- if (!asoc->stream.out[i].ext)
+ if (!SCTP_SO(&asoc->stream, i)->ext)
continue;
ret = n->init_sid(&asoc->stream, i, GFP_KERNEL);
@@ -217,7 +217,7 @@ int sctp_sched_set_value(struct sctp_association *asoc, __u16 sid,
if (sid >= asoc->stream.outcnt)
return -EINVAL;
- if (!asoc->stream.out[sid].ext) {
+ if (!SCTP_SO(&asoc->stream, sid)->ext) {
int ret;
ret = sctp_stream_init_ext(&asoc->stream, sid);
@@ -234,7 +234,7 @@ int sctp_sched_get_value(struct sctp_association *asoc, __u16 sid,
if (sid >= asoc->stream.outcnt)
return -EINVAL;
- if (!asoc->stream.out[sid].ext)
+ if (!SCTP_SO(&asoc->stream, sid)->ext)
return 0;
return asoc->outqueue.sched->get(&asoc->stream, sid, value);
@@ -252,7 +252,7 @@ void sctp_sched_dequeue_done(struct sctp_outq *q, struct sctp_chunk *ch)
* priority stream comes in.
*/
sid = sctp_chunk_stream_no(ch);
- sout = &q->asoc->stream.out[sid];
+ sout = SCTP_SO(&q->asoc->stream, sid);
q->asoc->stream.out_curr = sout;
return;
}
@@ -272,8 +272,9 @@ void sctp_sched_dequeue_common(struct sctp_outq *q, struct sctp_chunk *ch)
int sctp_sched_init_sid(struct sctp_stream *stream, __u16 sid, gfp_t gfp)
{
struct sctp_sched_ops *sched = sctp_sched_ops_from_stream(stream);
+ struct sctp_stream_out_ext *ext = SCTP_SO(stream, sid)->ext;
- INIT_LIST_HEAD(&stream->out[sid].ext->outq);
+ INIT_LIST_HEAD(&ext->outq);
return sched->init_sid(stream, sid, gfp);
}
diff --git a/net/sctp/stream_sched_prio.c b/net/sctp/stream_sched_prio.c
index 7997d35dd0fd..2245083a98f2 100644
--- a/net/sctp/stream_sched_prio.c
+++ b/net/sctp/stream_sched_prio.c
@@ -75,10 +75,10 @@ static struct sctp_stream_priorities *sctp_sched_prio_get_head(
/* No luck. So we search on all streams now. */
for (i = 0; i < stream->outcnt; i++) {
- if (!stream->out[i].ext)
+ if (!SCTP_SO(stream, i)->ext)
continue;
- p = stream->out[i].ext->prio_head;
+ p = SCTP_SO(stream, i)->ext->prio_head;
if (!p)
/* Means all other streams won't be initialized
* as well.
@@ -165,7 +165,7 @@ static void sctp_sched_prio_sched(struct sctp_stream *stream,
static int sctp_sched_prio_set(struct sctp_stream *stream, __u16 sid,
__u16 prio, gfp_t gfp)
{
- struct sctp_stream_out *sout = &stream->out[sid];
+ struct sctp_stream_out *sout = SCTP_SO(stream, sid);
struct sctp_stream_out_ext *soute = sout->ext;
struct sctp_stream_priorities *prio_head, *old;
bool reschedule = false;
@@ -186,7 +186,7 @@ static int sctp_sched_prio_set(struct sctp_stream *stream, __u16 sid,
return 0;
for (i = 0; i < stream->outcnt; i++) {
- soute = stream->out[i].ext;
+ soute = SCTP_SO(stream, i)->ext;
if (soute && soute->prio_head == old)
/* It's still in use, nothing else to do here. */
return 0;
@@ -201,7 +201,7 @@ static int sctp_sched_prio_set(struct sctp_stream *stream, __u16 sid,
static int sctp_sched_prio_get(struct sctp_stream *stream, __u16 sid,
__u16 *value)
{
- *value = stream->out[sid].ext->prio_head->prio;
+ *value = SCTP_SO(stream, sid)->ext->prio_head->prio;
return 0;
}
@@ -215,7 +215,7 @@ static int sctp_sched_prio_init(struct sctp_stream *stream)
static int sctp_sched_prio_init_sid(struct sctp_stream *stream, __u16 sid,
gfp_t gfp)
{
- INIT_LIST_HEAD(&stream->out[sid].ext->prio_list);
+ INIT_LIST_HEAD(&SCTP_SO(stream, sid)->ext->prio_list);
return sctp_sched_prio_set(stream, sid, 0, gfp);
}
@@ -233,9 +233,9 @@ static void sctp_sched_prio_free(struct sctp_stream *stream)
*/
sctp_sched_prio_unsched_all(stream);
for (i = 0; i < stream->outcnt; i++) {
- if (!stream->out[i].ext)
+ if (!SCTP_SO(stream, i)->ext)
continue;
- prio = stream->out[i].ext->prio_head;
+ prio = SCTP_SO(stream, i)->ext->prio_head;
if (prio && list_empty(&prio->prio_sched))
list_add(&prio->prio_sched, &list);
}
@@ -255,7 +255,7 @@ static void sctp_sched_prio_enqueue(struct sctp_outq *q,
ch = list_first_entry(&msg->chunks, struct sctp_chunk, frag_list);
sid = sctp_chunk_stream_no(ch);
stream = &q->asoc->stream;
- sctp_sched_prio_sched(stream, stream->out[sid].ext);
+ sctp_sched_prio_sched(stream, SCTP_SO(stream, sid)->ext);
}
static struct sctp_chunk *sctp_sched_prio_dequeue(struct sctp_outq *q)
@@ -297,7 +297,7 @@ static void sctp_sched_prio_dequeue_done(struct sctp_outq *q,
* this priority.
*/
sid = sctp_chunk_stream_no(ch);
- soute = q->asoc->stream.out[sid].ext;
+ soute = SCTP_SO(&q->asoc->stream, sid)->ext;
prio = soute->prio_head;
sctp_sched_prio_next_stream(prio);
@@ -317,7 +317,7 @@ static void sctp_sched_prio_sched_all(struct sctp_stream *stream)
__u16 sid;
sid = sctp_chunk_stream_no(ch);
- sout = &stream->out[sid];
+ sout = SCTP_SO(stream, sid);
if (sout->ext)
sctp_sched_prio_sched(stream, sout->ext);
}
diff --git a/net/sctp/stream_sched_rr.c b/net/sctp/stream_sched_rr.c
index 1155692448f1..52ba743fa7a7 100644
--- a/net/sctp/stream_sched_rr.c
+++ b/net/sctp/stream_sched_rr.c
@@ -100,7 +100,7 @@ static int sctp_sched_rr_init(struct sctp_stream *stream)
static int sctp_sched_rr_init_sid(struct sctp_stream *stream, __u16 sid,
gfp_t gfp)
{
- INIT_LIST_HEAD(&stream->out[sid].ext->rr_list);
+ INIT_LIST_HEAD(&SCTP_SO(stream, sid)->ext->rr_list);
return 0;
}
@@ -120,7 +120,7 @@ static void sctp_sched_rr_enqueue(struct sctp_outq *q,
ch = list_first_entry(&msg->chunks, struct sctp_chunk, frag_list);
sid = sctp_chunk_stream_no(ch);
stream = &q->asoc->stream;
- sctp_sched_rr_sched(stream, stream->out[sid].ext);
+ sctp_sched_rr_sched(stream, SCTP_SO(stream, sid)->ext);
}
static struct sctp_chunk *sctp_sched_rr_dequeue(struct sctp_outq *q)
@@ -154,7 +154,7 @@ static void sctp_sched_rr_dequeue_done(struct sctp_outq *q,
/* Last chunk on that msg, move to the next stream */
sid = sctp_chunk_stream_no(ch);
- soute = q->asoc->stream.out[sid].ext;
+ soute = SCTP_SO(&q->asoc->stream, sid)->ext;
sctp_sched_rr_next_stream(&q->asoc->stream);
@@ -173,7 +173,7 @@ static void sctp_sched_rr_sched_all(struct sctp_stream *stream)
__u16 sid;
sid = sctp_chunk_stream_no(ch);
- soute = stream->out[sid].ext;
+ soute = SCTP_SO(stream, sid)->ext;
if (soute)
sctp_sched_rr_sched(stream, soute);
}
--
2.15.1
^ permalink raw reply related
* Re: [PATCH][v3] netfilter: use kvmalloc_array to allocate memory for hashtable
From: Pablo Neira Ayuso @ 2018-08-03 16:37 UTC (permalink / raw)
To: Li RongQing; +Cc: netdev, kadlec, fw, netfilter-devel, coreteam, edumazet
In-Reply-To: <1532505133-9398-1-git-send-email-lirongqing@baidu.com>
On Wed, Jul 25, 2018 at 03:52:13PM +0800, Li RongQing wrote:
> nf_ct_alloc_hashtable is used to allocate memory for conntrack,
> NAT bysrc and expectation hashtable. Assuming 64k bucket size,
> which means 7th order page allocation, __get_free_pages, called
> by nf_ct_alloc_hashtable, will trigger the direct memory reclaim
> and stall for a long time, when system has lots of memory stress
>
> so replace combination of __get_free_pages and vzalloc with
> kvmalloc_array, which provides a overflow check and a fallback
> if no high order memory is available, and do not retry to reclaim
> memory, reduce stall
Applied, thanks.
^ permalink raw reply
* RE: [PATCH v2 1/2] net/sctp: Make wrappers for accessing in/out streams
From: David Laight @ 2018-08-03 16:41 UTC (permalink / raw)
To: 'Konstantin Khorenko', Marcelo Ricardo Leitner
Cc: oleg.babin@gmail.com, netdev@vger.kernel.org,
linux-sctp@vger.kernel.org, David S . Miller, Vlad Yasevich,
Neil Horman, Xin Long, Andrey Ryabinin
In-Reply-To: <20180803162102.19540-2-khorenko@virtuozzo.com>
From: Konstantin Khorenko
> Sent: 03 August 2018 17:21
...
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -37,6 +37,18 @@
> #include <net/sctp/sm.h>
> #include <net/sctp/stream_sched.h>
>
> +struct sctp_stream_out *sctp_stream_out(const struct sctp_stream *stream,
> + __u16 sid)
> +{
> + return ((struct sctp_stream_out *)(stream->out)) + sid;
> +}
> +
> +struct sctp_stream_in *sctp_stream_in(const struct sctp_stream *stream,
> + __u16 sid)
> +{
> + return ((struct sctp_stream_in *)(stream->in)) + sid;
> +}
> +
Those look like they ought to be static inlines in the header file.
Otherwise you'll be making SCTP performance worse that it is already.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* RE: [PATCH v2 0/2] net/sctp: Avoid allocating high order memory with kmalloc()
From: David Laight @ 2018-08-03 16:43 UTC (permalink / raw)
To: 'Konstantin Khorenko', Marcelo Ricardo Leitner
Cc: oleg.babin@gmail.com, netdev@vger.kernel.org,
linux-sctp@vger.kernel.org, David S . Miller, Vlad Yasevich,
Neil Horman, Xin Long, Andrey Ryabinin
In-Reply-To: <20180803162102.19540-1-khorenko@virtuozzo.com>
From: Konstantin Khorenko
> Sent: 03 August 2018 17:21
>
> Each SCTP association can have up to 65535 input and output streams.
> For each stream type an array of sctp_stream_in or sctp_stream_out
> structures is allocated using kmalloc_array() function. This function
> allocates physically contiguous memory regions, so this can lead
> to allocation of memory regions of very high order, i.e.:
...
Given how useless SCTP streams are, does anything actually use
more than about 4?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [pull request][net-next 00/10] Mellanox, mlx5 and devlink updates 2018-07-31
From: Ido Schimmel @ 2018-08-03 16:41 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Eran Ben Elisha, David Miller, saeedm, netdev, jiri,
alexander.duyck, helgaas
In-Reply-To: <20180802155315.48aaab19@cakuba.netronome.com>
On Thu, Aug 02, 2018 at 03:53:15PM -0700, Jakub Kicinski wrote:
> No one is requesting full RED offload here.. if someone sets the
> parameters you can't support you simply won't offload them. And ignore
> the parameters which only make sense in software terms. Look at the
> docs for mlxsw:
>
> https://github.com/Mellanox/mlxsw/wiki/Queues-Management#offloading-red
>
> It says "not offloaded" in a number of places.
>
...
> It's generally preferable to implement a subset of exiting well defined
> API than create vendor knobs, hence hardly a misuse.
Sorry for derailing the discussion, but you mentioned some points that
have been bothering me for a while.
I think we didn't do a very good job with buffer management and this is
exactly why you see some parameters marked as "not offloaded". Take the
"limit" (queue size) for example. It's configured via devlink-sb, by
setting a quota on the number of bytes that can be queued for the port
and TC (queue) that RED manages. See:
https://github.com/Mellanox/mlxsw/wiki/Quality-of-Service#pool-binding
It would have been much better and user friendly to not ignore this
parameter and have users configure the limit using existing interfaces
(tc), instead of creating a discrepancy between the software and
hardware data paths by configuring the hardware directly via devlink-sb.
I believe devlink-sb is mainly the result of Linux's short comings in
this area and our lack of perspective back then. While the qdisc layer
(Linux's shared buffers) works for end hosts, it requires enhancements
(mainly on ingress) for switches (physical/virtual) that forward
packets.
For example, switches (I'm familiar with Mellanox ASICs, but I assume
the concept is similar in other ASICs) have ingress buffers where
packets are stored while going through the pipeline. Once out of the
pipeline you know from which port and queue the packet should egress. In
case you have both lossless and lossy traffic in your network you
probably want to classify it into different ingress buffers and mark the
buffers where the lossless traffic is stored as such, so that PFC frames
would be emitted above a certain threshold.
This is currently configured using dcbnl, but it lacks a software model
which means that packets that are forwarded by the kernel don't get the
same treatment (e.g., skb priority isn't set). It also means that when
you want to limit the number of packets that are queued *from* a certain
port and ingress buffer you resort to tools such as devlink-sb that end
up colliding with existing tools (tc).
I was thinking (not too much...) about modelling the above using ingress
qdiscs. They don't do any queueing, but more of accounting. Once the
egress qdisc dequeues the packet, you give credit back to the ingress
qdisc from which the packet came from. I believe that modelling these
buffers using the qdisc layer is the right abstraction.
Would appreciate hearing your thoughts on the above.
^ permalink raw reply
* Re: [patch net-next] net: sched: fix flush on non-existing chain
From: David Miller @ 2018-08-03 16:45 UTC (permalink / raw)
To: jiri; +Cc: netdev, jhs, xiyou.wangcong, jakub.kicinski, idosch, mlxsw
In-Reply-To: <20180803090847.1199-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 3 Aug 2018 11:08:47 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> User was able to perform filter flush on chain 0 even if it didn't have
> any filters in it. With the patch that avoided implicit chain 0
> creation, this changed. So in case user wants filter flush on chain
> which does not exist, just return success. There's no reason for non-0
> chains to behave differently than chain 0, so do the same for them.
>
> Reported-by: Ido Schimmel <idosch@mellanox.com>
> Fixes: f71e0ca4db18 ("net: sched: Avoid implicit chain 0 creation")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Applied.
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Stefano Brivio @ 2018-08-03 16:52 UTC (permalink / raw)
To: Ben Pfaff
Cc: Matteo Croce, Pravin B Shelar, jpettit, gvrose8192, netdev, dev,
Jiri Benc, Aaron Conole
In-Reply-To: <20180731220657.GC29662@ovn.org>
Hi Ben,
On Tue, 31 Jul 2018 15:06:57 -0700
Ben Pfaff <blp@ovn.org> wrote:
> This is an awkward problem to try to solve with sockets because of the
> nature of sockets, which are strictly first-in first-out. What you
> really want is something closer to the algorithm that we use in
> ovs-vswitchd to send packets to an OpenFlow controller. When the
> channel becomes congested, then for each packet to be sent to the
> controller, OVS appends it to a queue associated with its input port.
> (This could be done on a more granular basis than just port.) If the
> maximum amount of queued packets is reached, then OVS discards a packet
> from the longest queue. When space becomes available in the channel,
> OVS round-robins through the queues to send a packet. This achieves
> pretty good fairness but it can't be done with sockets because you can't
> drop a packet that is already queued to one.
Thanks for your feedback. What you describe is, though, functionally
equivalent to what this patch does, minus the per-port queueing limit.
However, instead of having one explicit queue for each port, and
then fetching packets in a round-robin fashion from all the queues, we
implemented this with a single queue and choose insertion points while
queueing in such a way that the result is equivalent. This way, we
avoid the massive overhead associated with having one queue per each
port (we can have up to 2^16 ports), and cycling over them.
Let's say we have two ports, A and B, and three upcalls are sent for
each port. If we implement one queue for each port as you described, we
end up with this:
.---------------- - - -
| A1 | A2 | A3 |
'---------------- - - -
.---------------- - - -
| B1 | B2 | B3 |
'---------------- - - -
and then send upcalls in this order: A1, B1, A2, B2, A3, B3.
What we are doing here with a single queue is inserting the upcalls
directly in this order:
.------------------------------- - - -
| A1 | B1 | A2 | B2 | A3 | B3 |
'------------------------------- - - -
and dequeueing from the head.
About the per-port queueing limit: we currently have a global one
(UPCALL_QUEUE_MAX_LEN), while the per-port limit is simply given by
implementation constraints in our case:
if (dp->upcalls.count[pos->port_no] == U8_MAX - 1) {
err = -ENOSPC;
goto out_clear;
}
but we can easily swap that U8_MAX - 1 with another macro or a
configurable value, if there's any value in doing that.
> My current thought is that any fairness scheme we implement directly in
> the kernel is going to need to evolve over time. Maybe we could do
> something flexible with BPF and maps, instead of hard-coding it.
Honestly, I fail to see what else we might want to do here, other than
adding a simple mechanism for fairness, to solve the specific issue at
hand. Flexibility would probably come at a higher cost. We could easily
make limits configurable if needed. Do you have anything else in mind?
--
Stefano
^ permalink raw reply
* Re: [PATCH v3 net-next 5/9] net: stmmac: Add MDIO related functions for XGMAC2
From: Andrew Lunn @ 2018-08-03 16:56 UTC (permalink / raw)
To: Jose Abreu
Cc: netdev, David S. Miller, Joao Pinto, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <53ac7d723d9d16a0b048433e85c2d7a8fafeef17.1533311285.git.joabreu@synopsys.com>
On Fri, Aug 03, 2018 at 04:50:23PM +0100, Jose Abreu wrote:
> Add the MDIO related funcionalities for the new IP block XGMAC2.
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 0/3] l2tp: sanitise MTU handling on sessions
From: David Miller @ 2018-08-03 17:04 UTC (permalink / raw)
To: g.nault; +Cc: netdev, jchapman
In-Reply-To: <cover.1533289827.git.g.nault@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Fri, 3 Aug 2018 12:38:32 +0200
> Most of the code handling sessions' MTU has no effect. The ->mtu field
> in struct l2tp_session might be used at session creation time, but
> neither PPP nor Ethernet pseudo-wires take updates into account.
>
> L2TP sessions don't have a concept of MTU, which is the reason why
> ->mtu is mostly ignored. MTU should remain a network device thing.
> Therefore this patch set does not try to propagate/update ->mtu to/from
> the device. That would complicate the code unnecessarily. Instead this
> field and the associated ioctl commands and netlink attributes are
> removed.
>
> Patch #1 defines l2tp_tunnel_dst_mtu() in order to simplify the
> following patches. Then patches #2 and #3 remove MTU handling from PPP
> and Ethernet pseudo-wires respectively.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] rxrpc: Push iov_iter up from rxrpc_kernel_recv_data() to caller
From: David Miller @ 2018-08-03 19:46 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <153331241619.5289.329306820096612046.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Fri, 03 Aug 2018 17:06:56 +0100
> Push iov_iter up from rxrpc_kernel_recv_data() to its caller to allow
> non-contiguous iovs to be passed down, thereby permitting file reading to
> be simplified in the AFS filesystem in a future patch.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied, thanks David.
^ permalink raw reply
* [PATCH iproute2] ip link: don't stop batch processing
From: Matteo Croce @ 2018-08-03 17:49 UTC (permalink / raw)
To: netdev, stephen
When 'ip link show dev DEVICE' is processed in a batch mode, ip exits
and stop processing further commands.
This because ipaddr_list_flush_or_save() calls exit() to avoid printing
the link information twice.
Replace the exit with a classic goto out instruction.
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
ip/ipaddress.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 6c306ab7..b7b78f6e 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1920,7 +1920,7 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
exit(1);
}
delete_json_obj();
- exit(0);
+ goto out;
}
if (filter.family != AF_PACKET) {
--
2.17.1
^ permalink raw reply related
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: Florian Fainelli @ 2018-08-03 17:57 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linville, andrew, vivien.didelot
In-Reply-To: <20180801.093219.1548098195820479887.davem@davemloft.net>
On 08/01/2018 09:32 AM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Mon, 30 Jul 2018 15:26:24 -0700
>
>> On 07/17/2018 08:36 AM, Florian Fainelli wrote:
>>> Allow re-purposing the wol->sopass storage area to specify a bitmask of filters
>>> (programmed previously via ethtool::rxnfc) to be used as wake-up patterns.
>>
>> John, David, can you provide some feedback if the approach is
>> acceptable? I will address Andrew's comment about the user friendliness
>> and allow providing a comma separate list of filter identifiers.
>>
>> One usability issue with this approach is that one cannot specify
>> wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time,
>> since it uses the same location in the ioctl() structure that is being
>> passed. Do you see this as a problem?
>
> Once again we are stuck in this weird situation, a sort of limbo.
>
> On the one hand, I don't want to block your work on the ethtool
> netlink stuff being done.
>
> However it is clear that by using netlink attributes, it would
> be so much cleaner.
>
> I honestly don't know what to say at this time. I wish I had
> a clear piece of advice and a way for everyone to move forward,
> and usually I do, but this time I really don't :-/
>
That's fine, let me submit the first few patches that are per-requisite
but don't actually introduce the WAKE_FILTER support. Once Michal's
ethtool/netlink work gets merged I can quickly extend that in a way that
supports wake-on-LAN using configured filters.
Does the current approach of specifying a bitmask of filters looks
reasonable to you though?
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] ppp: mppe: Remove VLA usage
From: David Miller @ 2018-08-03 19:55 UTC (permalink / raw)
To: keescook; +Cc: netdev, paulus, linux-ppp, linux-kernel
In-Reply-To: <20180803163745.GA45715@beast>
From: Kees Cook <keescook@chromium.org>
Date: Fri, 3 Aug 2018 09:37:45 -0700
> In the quest to remove all stack VLA usage from the kernel[1], this
> removes the discouraged use of AHASH_REQUEST_ON_STACK (and associated
> VLA) by switching to shash directly and keeping the associated descriptor
> allocated with the regular state on the heap.
>
> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Paul suggested this go via netdev directly:
> https://lkml.kernel.org/r/20180803031315.GA30807@fergus
Applied, thanks Kees.
^ permalink raw reply
* Re: [PATCH iproute2] ip link: don't stop batch processing
From: Dave Taht @ 2018-08-03 18:08 UTC (permalink / raw)
To: mcroce; +Cc: Linux Kernel Network Developers, Stephen Hemminger
In-Reply-To: <20180803174933.24600-1-mcroce@redhat.com>
On Fri, Aug 3, 2018 at 10:50 AM Matteo Croce <mcroce@redhat.com> wrote:
>
> When 'ip link show dev DEVICE' is processed in a batch mode, ip exits
> and stop processing further commands.
> This because ipaddr_list_flush_or_save() calls exit() to avoid printing
> the link information twice.
> Replace the exit with a classic goto out instruction.
>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
one thing I noticed in iproute2-next last week is that
( echo qdisc show dev eno1; sleep 5; echo qdisc show dev eno1; ) | tc -b -
batches the whole thing up to emerge on exit, only.
It didn't used to do that, the output of every command came out as it
completed. I used to use that to timestamp and save the overhead of
invoking the tc utility on openwrt while monitoring qdisc stats in
https://github.com/tohojo/flent/blob/master/misc/tc_iterate.c
alternatively adding timed/timestamped output to tc like -c count or
-I interval would be useful.
> ---
> ip/ipaddress.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 6c306ab7..b7b78f6e 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -1920,7 +1920,7 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
> exit(1);
> }
> delete_json_obj();
> - exit(0);
> + goto out;
> }
>
> if (filter.family != AF_PACKET) {
> --
> 2.17.1
>
--
Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619
^ permalink raw reply
* [PATCH net-next 0/4] net: dsa and systemport WoL changes
From: Florian Fainelli @ 2018-08-03 18:08 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, andrew, vivien.didelot, davem
Hi David,
This patch series extracts what was previously submitted as part of the
"WAKE_FILTER" Wake-on-LAN patch series into patches that do not.
Changes in this series:
- properly align the dsa_is_cpu_port() check in first patch
Florian Fainelli (4):
net: dsa: bcm_sf2: Allow targeting CPU ports for CFP rules
net: dsa: bcm_sf2: Disable learning while in WoL
net: systemport: Do not re-configure upon WoL interrupt
net: systemport: Create helper to set MPD
drivers/net/dsa/bcm_sf2.c | 12 +++++++++++-
drivers/net/dsa/bcm_sf2_cfp.c | 3 ++-
drivers/net/dsa/bcm_sf2_regs.h | 2 ++
drivers/net/ethernet/broadcom/bcmsysport.c | 24 ++++++++++++++----------
4 files changed, 29 insertions(+), 12 deletions(-)
--
2.14.1
^ permalink raw reply
* [PATCH net-next 1/4] net: dsa: bcm_sf2: Allow targeting CPU ports for CFP rules
From: Florian Fainelli @ 2018-08-03 18:08 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, andrew, vivien.didelot, davem
In-Reply-To: <20180803180844.1010-1-f.fainelli@gmail.com>
ds->enabled_port_mask only contains a bitmask of user-facing enabled
ports, we also need to allow programming CFP rules that target CPU ports
(e.g: ports 5 and 8).
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2_cfp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index b89acaee12d4..1e37b65aab93 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -755,7 +755,8 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
port_num = fs->ring_cookie / SF2_NUM_EGRESS_QUEUES;
if (fs->ring_cookie == RX_CLS_FLOW_DISC ||
- !dsa_is_user_port(ds, port_num) ||
+ !(dsa_is_user_port(ds, port_num) ||
+ dsa_is_cpu_port(ds, port_num)) ||
port_num >= priv->hw_params.num_ports)
return -EINVAL;
/*
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 2/4] net: dsa: bcm_sf2: Disable learning while in WoL
From: Florian Fainelli @ 2018-08-03 18:08 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, andrew, vivien.didelot, davem
In-Reply-To: <20180803180844.1010-1-f.fainelli@gmail.com>
When we are in Wake-on-LAN, we operate with the host sofware not running
a network stack, so we want to the switch to flood packets in order to
cause a system wake-up when matching specific filters (unicast or
multicast). This was not necessary before since we supported Magic
Packet which are targeting a broadcast MAC address which the switch
already floods.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 12 +++++++++++-
drivers/net/dsa/bcm_sf2_regs.h | 2 ++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index ac96ff40d37e..e0066adcd2f3 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -166,6 +166,11 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
reg &= ~P_TXQ_PSM_VDD(port);
core_writel(priv, reg, CORE_MEM_PSM_VDD_CTRL);
+ /* Enable learning */
+ reg = core_readl(priv, CORE_DIS_LEARN);
+ reg &= ~BIT(port);
+ core_writel(priv, reg, CORE_DIS_LEARN);
+
/* Enable Broadcom tags for that port if requested */
if (priv->brcm_tag_mask & BIT(port))
b53_brcm_hdr_setup(ds, port);
@@ -222,8 +227,13 @@ static void bcm_sf2_port_disable(struct dsa_switch *ds, int port,
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
u32 reg;
- if (priv->wol_ports_mask & (1 << port))
+ /* Disable learning while in WoL mode */
+ if (priv->wol_ports_mask & (1 << port)) {
+ reg = core_readl(priv, CORE_DIS_LEARN);
+ reg |= BIT(port);
+ core_writel(priv, reg, CORE_DIS_LEARN);
return;
+ }
if (port == priv->moca_port)
bcm_sf2_port_intr_disable(priv, port);
diff --git a/drivers/net/dsa/bcm_sf2_regs.h b/drivers/net/dsa/bcm_sf2_regs.h
index 3ccd5a865dcb..0a1e530d52b7 100644
--- a/drivers/net/dsa/bcm_sf2_regs.h
+++ b/drivers/net/dsa/bcm_sf2_regs.h
@@ -168,6 +168,8 @@ enum bcm_sf2_reg_offs {
#define CORE_SWITCH_CTRL 0x00088
#define MII_DUMB_FWDG_EN (1 << 6)
+#define CORE_DIS_LEARN 0x000f0
+
#define CORE_SFT_LRN_CTRL 0x000f8
#define SW_LEARN_CNTL(x) (1 << (x))
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 3/4] net: systemport: Do not re-configure upon WoL interrupt
From: Florian Fainelli @ 2018-08-03 18:08 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, andrew, vivien.didelot, davem
In-Reply-To: <20180803180844.1010-1-f.fainelli@gmail.com>
We already properly resume from Wake-on-LAN whether such a condition
occured or not, no need to process the WoL interrupt for functional
changes since that could race with other settings.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 631617d95769..7faad9e1a6f9 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1102,10 +1102,8 @@ static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
if (priv->irq0_stat & INTRL2_0_TX_RING_FULL)
bcm_sysport_tx_reclaim_all(priv);
- if (priv->irq0_stat & INTRL2_0_MPD) {
+ if (priv->irq0_stat & INTRL2_0_MPD)
netdev_info(priv->netdev, "Wake-on-LAN interrupt!\n");
- bcm_sysport_resume_from_wol(priv);
- }
if (!priv->is_lite)
goto out;
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 4/4] net: systemport: Create helper to set MPD
From: Florian Fainelli @ 2018-08-03 18:08 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, andrew, vivien.didelot, davem
In-Reply-To: <20180803180844.1010-1-f.fainelli@gmail.com>
Create a helper function to turn on/off MPD, this will be used to avoid
duplicating code as we are going to add additional types of wake-up
types.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 7faad9e1a6f9..284581c9680e 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1041,17 +1041,25 @@ static int bcm_sysport_poll(struct napi_struct *napi, int budget)
return work_done;
}
-static void bcm_sysport_resume_from_wol(struct bcm_sysport_priv *priv)
+static void mpd_enable_set(struct bcm_sysport_priv *priv, bool enable)
{
u32 reg;
+ reg = umac_readl(priv, UMAC_MPD_CTRL);
+ if (enable)
+ reg |= MPD_EN;
+ else
+ reg &= ~MPD_EN;
+ umac_writel(priv, reg, UMAC_MPD_CTRL);
+}
+
+static void bcm_sysport_resume_from_wol(struct bcm_sysport_priv *priv)
+{
/* Stop monitoring MPD interrupt */
intrl2_0_mask_set(priv, INTRL2_0_MPD);
/* Clear the MagicPacket detection logic */
- reg = umac_readl(priv, UMAC_MPD_CTRL);
- reg &= ~MPD_EN;
- umac_writel(priv, reg, UMAC_MPD_CTRL);
+ mpd_enable_set(priv, false);
netif_dbg(priv, wol, priv->netdev, "resumed from WOL\n");
}
@@ -2447,9 +2455,7 @@ static int bcm_sysport_suspend_to_wol(struct bcm_sysport_priv *priv)
/* Do not leave the UniMAC RBUF matching only MPD packets */
if (!timeout) {
- reg = umac_readl(priv, UMAC_MPD_CTRL);
- reg &= ~MPD_EN;
- umac_writel(priv, reg, UMAC_MPD_CTRL);
+ mpd_enable_set(priv, false);
netif_err(priv, wol, ndev, "failed to enter WOL mode\n");
return -ETIMEDOUT;
}
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next 1/4] net: dsa: bcm_sf2: Allow targeting CPU ports for CFP rules
From: Andrew Lunn @ 2018-08-03 18:22 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, vivien.didelot, davem
In-Reply-To: <20180803180844.1010-2-f.fainelli@gmail.com>
On Fri, Aug 03, 2018 at 11:08:41AM -0700, Florian Fainelli wrote:
> ds->enabled_port_mask only contains a bitmask of user-facing enabled
> ports, we also need to allow programming CFP rules that target CPU ports
> (e.g: ports 5 and 8).
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/4] net: dsa: bcm_sf2: Disable learning while in WoL
From: Andrew Lunn @ 2018-08-03 18:24 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, vivien.didelot, davem
In-Reply-To: <20180803180844.1010-3-f.fainelli@gmail.com>
On Fri, Aug 03, 2018 at 11:08:42AM -0700, Florian Fainelli wrote:
> When we are in Wake-on-LAN, we operate with the host sofware not running
> a network stack, so we want to the switch to flood packets in order to
> cause a system wake-up when matching specific filters (unicast or
> multicast). This was not necessary before since we supported Magic
> Packet which are targeting a broadcast MAC address which the switch
> already floods.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 4/4] net: systemport: Create helper to set MPD
From: Andrew Lunn @ 2018-08-03 18:25 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, vivien.didelot, davem
In-Reply-To: <20180803180844.1010-5-f.fainelli@gmail.com>
On Fri, Aug 03, 2018 at 11:08:44AM -0700, Florian Fainelli wrote:
> Create a helper function to turn on/off MPD, this will be used to avoid
> duplicating code as we are going to add additional types of wake-up
> types.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* KCM - recvmsg() mangles packets?
From: Dominique Martinet @ 2018-08-03 18:28 UTC (permalink / raw)
To: Tom Herbert, netdev
[-- Attachment #1: Type: text/plain, Size: 1787 bytes --]
I've been playing with KCM on a 4.18.0-rc7 kernel and I'm running in a
problem where the iovec filled by recvmsg() is mangled up: it is filled
by the length of one packet, but contains (truncated) data from another
packet, rendering KCM unuseable.
(I haven't tried old kernels to see for how long this is broken/try to
bisect; I might if there's no progress but this might be simpler than I
think)
I've attached a reproducer, a simple program that forks, creates a tcp
server/client, attach the server socket to a kcm socket, and in an
infinite loop sends varying-length messages from the client to the
server.
The loop stops when the server gets a message which length is not the
length indicated in the packet header, rather fast (I can make it run
for a while if I slow down emission, or if I run a verbose tcpdump for
example)
In the quiet version on a VM on my laptop, I get this output:
[root@f2 ~]# gcc -g -l bcc -o kcm kcm.c
[root@f2 ~]# ./kcm
client is starting
server is starting
server is receiving data
Got 14, expected 27 on 1th message: 22222222222222; flags: 80
The client sends message deterministacally, first one is 14 bytes filled
with 1, second one is 27 bytes filled with 2, third one is 9 bytes
filled with 3 etc (final digit is actually a \0 instead)
As we can see, the server received 14 '2', and the header size matches
the second message header, so something went wrong™.
Flags 0x80 is MSG_EOR meaning recvmsg copied the full message.
This happens even if I reduce the VMs CPU to 1, so I was thinking some
irq messes with the sock between skb_peek and the actual copy of the
data (as this deos work if I send slowly!), but even disabling
irq/preempt doesn't seem to help so I'm not sure what to try next.
Any idea?
Thanks,
--
Dominique Martinet
[-- Attachment #2: kcm.c --]
[-- Type: text/x-csrc, Size: 4860 bytes --]
/*
* A sample program of KCM.
* Originally https://gist.github.com/peo3/fd0e266a3852d3422c08854aba96bff5
*
* $ gcc -lbcc kcm-sample.c
* $ ./a.out 10000
*/
#include <err.h>
#include <errno.h>
#include <netdb.h>
#include <poll.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <netinet/in.h>
#include <linux/kcm.h>
#include <bcc/bpf_common.h>
#include <bcc/libbpf.h>
struct my_proto {
struct _hdr {
uint32_t len;
} hdr;
char data[32];
} __attribute__((packed));
// use htons to use LE header size, since load_half does a first convertion
// from network byte order
const char *bpf_prog_string = " \
ssize_t bpf_prog1(struct __sk_buff *skb) \
{ \
return bpf_htons(load_half(skb, 0)) + 4; \
}";
int servsock_init(int port)
{
int s, error;
struct sockaddr_in addr;
s = socket(AF_INET, SOCK_STREAM, 0);
addr.sin_family = AF_INET;
addr.sin_port = htons(port);
addr.sin_addr.s_addr = INADDR_ANY;
error = bind(s, (struct sockaddr *)&addr, sizeof(addr));
if (error == -1)
err(EXIT_FAILURE, "bind");
error = listen(s, 10);
if (error == -1)
err(EXIT_FAILURE, "listen");
return s;
}
int bpf_init(void)
{
int fd, map_fd;
void *mod;
int key;
long long value = 0;
mod = bpf_module_create_c_from_string(bpf_prog_string, 0, NULL, 0);
fd = bpf_prog_load(
BPF_PROG_TYPE_SOCKET_FILTER,
"bpf_prog1",
bpf_function_start(mod, "bpf_prog1"),
bpf_function_size(mod, "bpf_prog1"),
bpf_module_license(mod),
bpf_module_kern_version(mod),
0, NULL, 0);
if (fd == -1)
exit(1);
return fd;
}
void client(int port)
{
int s, error;
struct sockaddr_in addr;
struct hostent *host;
struct my_proto my_msg;
int len;
printf("client is starting\n");
s = socket(AF_INET, SOCK_STREAM, 0);
if (s == -1)
err(EXIT_FAILURE, "socket");
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
addr.sin_port = htons(port);
host = gethostbyname("localhost");
if (host == NULL)
err(EXIT_FAILURE, "gethostbyname");
memcpy(&addr.sin_addr, host->h_addr, host->h_length);
error = connect(s, (struct sockaddr *)&addr, sizeof(addr));
if (error == -1)
err(EXIT_FAILURE, "connect");
len = sprintf(my_msg.data, "1234567890123456789012345678901");
my_msg.data[len] = '\0';
my_msg.hdr.len = len + 1;
int i = 1;
while(1) {
my_msg.hdr.len = (i++ * 1312739ULL) % 31 + 1;
for (int j = 0; j < my_msg.hdr.len; ) {
j += snprintf(my_msg.data + j, my_msg.hdr.len - j, "%i", i - 1);
}
my_msg.data[my_msg.hdr.len-1] = '\0';
//printf("%d: writing %d\n", i-1, my_msg.hdr.len);
len = write(s, &my_msg, sizeof(my_msg.hdr) + my_msg.hdr.len);
if (error == -1)
err(EXIT_FAILURE, "write");
//usleep(10000);
}
close(s);
}
int kcm_init(void)
{
int kcmfd;
kcmfd = socket(AF_KCM, SOCK_DGRAM, KCMPROTO_CONNECTED);
if (kcmfd == -1)
err(EXIT_FAILURE, "socket(AF_KCM)");
return kcmfd;
}
int kcm_attach(int kcmfd, int csock, int bpf_prog_fd)
{
int error;
struct kcm_attach attach_info = {
.fd = csock,
.bpf_fd = bpf_prog_fd,
};
error = ioctl(kcmfd, SIOCKCMATTACH, &attach_info);
if (error == -1)
err(EXIT_FAILURE, "ioctl(SIOCKCMATTACH)");
}
void process(int kcmfd)
{
struct my_proto my_msg;
int error, len;
struct msghdr msg;
struct iovec iov = {
.iov_base = &my_msg,
.iov_len = sizeof(my_msg),
};
printf("server is receiving data\n");
int i =0;
while (1) {
memset(&msg, 0, sizeof(msg));
msg.msg_iov = &iov;
msg.msg_iovlen = 1;
memset(&my_msg, 0, sizeof(my_msg));
len = recvmsg(kcmfd, &msg, 0);
if (len == -1)
err(EXIT_FAILURE, "recvmsg");
if (len != my_msg.hdr.len + 4) {
printf("Got %d, expected %d on %dth message: %s; flags: %x\n", len - 4, my_msg.hdr.len, i, my_msg.data, msg.msg_flags);
exit(1);
}
i++;
}
}
void server(int tcpfd, int bpf_prog_fd)
{
int kcmfd, error;
struct sockaddr_in client;
int len, csock;
printf("server is starting\n");
kcmfd = kcm_init();
len = sizeof(client);
csock = accept(tcpfd, (struct sockaddr *)&client, &len);
if (csock == -1)
err(EXIT_FAILURE, "accept");
kcm_attach(kcmfd, csock, bpf_prog_fd);
process(kcmfd);
close(kcmfd);
}
int main(int argc, char **argv)
{
int error, tcpfd, bpf_prog_fd;
pid_t pid;
int pipefd[2];
int dummy;
int port = argc > 1 ? atoi(argv[1]) : 10000;
error = pipe(pipefd);
if (error == -1)
err(EXIT_FAILURE, "pipe");
pid = fork();
if (pid == -1)
err(EXIT_FAILURE, "fork");
if (pid == 0) {
/* wait for server's ready */
read(pipefd[0], &dummy, sizeof(dummy));
client(port);
exit(0);
}
tcpfd = servsock_init(port);
bpf_prog_fd = bpf_init();
/* tell ready */
write(pipefd[1], &dummy, sizeof(dummy));
server(tcpfd, bpf_prog_fd);
waitpid(pid, NULL, 0);
close(bpf_prog_fd);
close(tcpfd);
return 0;
}
^ permalink raw reply
* Re: [PATCH net-next] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: Doron Roberts-Kedes @ 2018-08-03 18:49 UTC (permalink / raw)
To: Vakul Garg
Cc: David S . Miller, Dave Watson, Boris Pismenny, Aviad Yehezkel,
netdev@vger.kernel.org
In-Reply-To: <DB7PR04MB42528525C8E8B39AF3FB65E48B230@DB7PR04MB4252.eurprd04.prod.outlook.com>
On Fri, Aug 03, 2018 at 01:23:33AM +0000, Vakul Garg wrote:
>
>
> > -----Original Message-----
> > From: Doron Roberts-Kedes [mailto:doronrk@fb.com]
> > Sent: Friday, August 3, 2018 6:00 AM
> > To: David S . Miller <davem@davemloft.net>
> > Cc: Dave Watson <davejwatson@fb.com>; Vakul Garg
> > <vakul.garg@nxp.com>; Boris Pismenny <borisp@mellanox.com>; Aviad
> > Yehezkel <aviadye@mellanox.com>; netdev@vger.kernel.org; Doron
> > Roberts-Kedes <doronrk@fb.com>
> > Subject: [PATCH net-next] net/tls: Calculate nsg for zerocopy path without
> > skb_cow_data.
> >
> > decrypt_skb fails if the number of sg elements required to map is greater
> > than MAX_SKB_FRAGS. As noted by Vakul Garg, nsg must always be
> > calculated, but skb_cow_data adds unnecessary memcpy's for the zerocopy
> > case.
> >
> > The new function skb_nsg calculates the number of scatterlist elements
> > required to map the skb without the extra overhead of skb_cow_data. This
> > function mimics the structure of skb_to_sgvec.
> >
> > Fixes: c46234ebb4d1 ("tls: RX path for ktls")
> > Signed-off-by: Doron Roberts-Kedes <doronrk@fb.com>
> > ---
> > net/tls/tls_sw.c | 89
> > ++++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 86 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
> > ff3a6904a722..c62793601cfc 100644
> > --- a/net/tls/tls_sw.c
> > +++ b/net/tls/tls_sw.c
> > @@ -43,6 +43,76 @@
> >
> > #define MAX_IV_SIZE TLS_CIPHER_AES_GCM_128_IV_SIZE
> >
> > +static int __skb_nsg(struct sk_buff *skb, int offset, int len,
> > + unsigned int recursion_level)
> > +{
> > + int start = skb_headlen(skb);
> > + int i, copy = start - offset;
> > + struct sk_buff *frag_iter;
> > + int elt = 0;
> > +
> > + if (unlikely(recursion_level >= 24))
> > + return -EMSGSIZE;
> > +
> > + if (copy > 0) {
> > + if (copy > len)
> > + copy = len;
> > + elt++;
> > + if ((len -= copy) == 0)
> > + return elt;
> > + offset += copy;
> > + }
> > +
> > + for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> > + int end;
> > +
> > + WARN_ON(start > offset + len);
> > +
> > + end = start + skb_frag_size(&skb_shinfo(skb)->frags[i]);
> > + if ((copy = end - offset) > 0) {
> > + if (copy > len)
> > + copy = len;
> > + elt++;
> > + if (!(len -= copy))
> > + return elt;
> > + offset += copy;
> > + }
> > + start = end;
> > + }
> > +
> > + skb_walk_frags(skb, frag_iter) {
> > + int end, ret;
> > +
> > + WARN_ON(start > offset + len);
> > +
> > + end = start + frag_iter->len;
> > + if ((copy = end - offset) > 0) {
> > +
> > + if (copy > len)
> > + copy = len;
> > + ret = __skb_nsg(frag_iter, offset - start, copy,
> > + recursion_level + 1);
> > + if (unlikely(ret < 0))
> > + return ret;
> > + elt += ret;
> > + if ((len -= copy) == 0)
> > + return elt;
> > + offset += copy;
> > + }
> > + start = end;
> > + }
> > + BUG_ON(len);
> > + return elt;
> > +}
> > +
> > +/* Return the number of scatterlist elements required to completely map
> > +the
> > + * skb, or -EMSGSIZE if the recursion depth is exceeded.
> > + */
> > +static int skb_nsg(struct sk_buff *skb, int offset, int len) {
> > + return __skb_nsg(skb, offset, len, 0); }
> > +
>
> These is generic function and useful elsewhere too.
> Should the above two functions be exported by skbuff.c?
True. Perhaps it can move into skbuff.c if/when there is a second
use case for it.
>
> > static int tls_do_decryption(struct sock *sk,
> > struct scatterlist *sgin,
> > struct scatterlist *sgout,
> > @@ -693,7 +763,7 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
> > struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
> > struct scatterlist *sgin = &sgin_arr[0];
> > struct strp_msg *rxm = strp_msg(skb);
> > - int ret, nsg = ARRAY_SIZE(sgin_arr);
> > + int ret, nsg;
> > struct sk_buff *unused;
> >
> > ret = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE, @@ -
> > 704,10 +774,23 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
> >
> > memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
> > if (!sgout) {
> > - nsg = skb_cow_data(skb, 0, &unused) + 1;
> > + nsg = skb_cow_data(skb, 0, &unused);
> > + } else {
> > + nsg = skb_nsg(skb,
> > + rxm->offset + tls_ctx->rx.prepend_size,
> > + rxm->full_len - tls_ctx->rx.prepend_size);
> > + if (nsg <= 0)
> > + return nsg;
> Comparison should be (nsg < 1). TLS forbids '0' sized records.
Yes true, v2 incoming
>
> > + }
> > +
> > + // We need one extra for ctx->rx_aad_ciphertext
> > + nsg++;
> > +
> > + if (nsg > ARRAY_SIZE(sgin_arr))
> > sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
>
> Add check for kmalloc_array returnining NULL.
Yes true, v2 incoming.
>
> > +
> > + if (!sgout)
> > sgout = sgin;
> > - }
> >
> > sg_init_table(sgin, nsg);
> > sg_set_buf(&sgin[0], ctx->rx_aad_ciphertext, TLS_AAD_SPACE_SIZE);
> > --
> > 2.17.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox