* [PATCH net-next 12/12] sctp: add support for the process of unordered idata
From: Xin Long @ 2017-12-05 15:16 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1512486606.git.lucien.xin@gmail.com>
Unordered idata process is more complicated than unordered data:
- It has to add mid into sctp_stream_out to save the next mid value,
which is separated from ordered idata's.
- To support pd for unordered idata, another mid and pd_mode need to
be added to save the message id and pd state in sctp_stream_in.
- To make unordered idata reasm easier, it adds a new event queue
to save frags for idata.
The patch mostly adds the samilar reasm functions for unordered idata
as ordered idata's, and also adjusts some other codes on assign_mid,
abort_pd and ulpevent_data for idata.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 14 +-
include/net/sctp/ulpqueue.h | 1 +
net/sctp/socket.c | 23 ++-
net/sctp/stream_interleave.c | 377 ++++++++++++++++++++++++++++++++++++++++---
net/sctp/ulpqueue.c | 5 +
5 files changed, 392 insertions(+), 28 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index d7da719..e43ed30 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -413,6 +413,14 @@ void sctp_stream_update(struct sctp_stream *stream, struct sctp_stream *new);
#define sctp_stream_in(asoc, sid) (&(asoc)->stream.in[sid])
+/* What is the current MID_uo number for this stream? */
+#define sctp_mid_uo_peek(stream, type, sid) \
+ ((stream)->type[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++)
+
/*
* Pointers to address related SCTP functions.
* (i.e. things that depend on the address family.)
@@ -1378,8 +1386,9 @@ struct sctp_stream_out {
__u32 mid;
__u16 ssn;
};
- __u8 state;
+ __u32 mid_uo;
struct sctp_stream_out_ext *ext;
+ __u8 state;
};
struct sctp_stream_in {
@@ -1387,8 +1396,11 @@ struct sctp_stream_in {
__u32 mid;
__u16 ssn;
};
+ __u32 mid_uo;
__u32 fsn;
+ __u32 fsn_uo;
char pd_mode;
+ char pd_mode_uo;
};
struct sctp_stream {
diff --git a/include/net/sctp/ulpqueue.h b/include/net/sctp/ulpqueue.h
index eb98c71..bb0ecba 100644
--- a/include/net/sctp/ulpqueue.h
+++ b/include/net/sctp/ulpqueue.h
@@ -45,6 +45,7 @@ struct sctp_ulpq {
char pd_mode;
struct sctp_association *asoc;
struct sk_buff_head reasm;
+ struct sk_buff_head reasm_uo;
struct sk_buff_head lobby;
};
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index fe2cab9..db8e7cc 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -201,6 +201,22 @@ static void sctp_for_each_tx_datachunk(struct sctp_association *asoc,
cb(chunk);
}
+static void sctp_for_each_rx_skb(struct sctp_association *asoc, struct sock *sk,
+ void (*cb)(struct sk_buff *, struct sock *))
+
+{
+ struct sk_buff *skb, *tmp;
+
+ sctp_skb_for_each(skb, &asoc->ulpq.lobby, tmp)
+ cb(skb, sk);
+
+ sctp_skb_for_each(skb, &asoc->ulpq.reasm, tmp)
+ cb(skb, sk);
+
+ sctp_skb_for_each(skb, &asoc->ulpq.reasm_uo, tmp)
+ cb(skb, sk);
+}
+
/* Verify that this is a valid address. */
static inline int sctp_verify_addr(struct sock *sk, union sctp_addr *addr,
int len)
@@ -1554,6 +1570,7 @@ static void sctp_close(struct sock *sk, long timeout)
if (data_was_unread || !skb_queue_empty(&asoc->ulpq.lobby) ||
!skb_queue_empty(&asoc->ulpq.reasm) ||
+ !skb_queue_empty(&asoc->ulpq.reasm_uo) ||
(sock_flag(sk, SOCK_LINGER) && !sk->sk_lingertime)) {
struct sctp_chunk *chunk;
@@ -8507,11 +8524,7 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
}
- sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp)
- sctp_skb_set_owner_r_frag(skb, newsk);
-
- sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp)
- sctp_skb_set_owner_r_frag(skb, newsk);
+ sctp_for_each_rx_skb(assoc, newsk, sctp_skb_set_owner_r_frag);
/* Set the type of socket to indicate that it is peeled off from the
* original UDP-style socket or created with the accept() call on a
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index 1014de2..c469703 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -74,12 +74,10 @@ static void sctp_chunk_assign_mid(struct sctp_chunk *chunk)
list_for_each_entry(lchunk, &chunk->msg->chunks, frag_list) {
struct sctp_idatahdr *hdr;
+ __u32 mid;
lchunk->has_mid = 1;
- if (lchunk->chunk_hdr->flags & SCTP_DATA_UNORDERED)
- continue;
-
hdr = lchunk->subh.idata_hdr;
if (lchunk->chunk_hdr->flags & SCTP_DATA_FIRST_FRAG)
@@ -87,10 +85,16 @@ static void sctp_chunk_assign_mid(struct sctp_chunk *chunk)
else
hdr->fsn = htonl(cfsn++);
- if (lchunk->chunk_hdr->flags & SCTP_DATA_LAST_FRAG)
- hdr->mid = htonl(sctp_mid_next(stream, out, sid));
- else
- hdr->mid = htonl(sctp_mid_peek(stream, out, sid));
+ if (lchunk->chunk_hdr->flags & SCTP_DATA_UNORDERED) {
+ mid = lchunk->chunk_hdr->flags & SCTP_DATA_LAST_FRAG ?
+ sctp_mid_uo_next(stream, out, sid) :
+ sctp_mid_uo_peek(stream, out, sid);
+ } else {
+ mid = lchunk->chunk_hdr->flags & SCTP_DATA_LAST_FRAG ?
+ sctp_mid_next(stream, out, sid) :
+ sctp_mid_peek(stream, out, sid);
+ }
+ hdr->mid = htonl(mid);
}
}
@@ -449,9 +453,6 @@ static struct sctp_ulpevent *sctp_intl_order(struct sctp_ulpq *ulpq,
struct sctp_stream *stream;
__u16 sid;
- if (event->msg_flags & SCTP_DATA_UNORDERED)
- return event;
-
stream = &ulpq->asoc->stream;
sid = event->stream;
@@ -512,6 +513,317 @@ static int sctp_enqueue_event(struct sctp_ulpq *ulpq,
return 0;
}
+static void sctp_intl_store_reasm_uo(struct sctp_ulpq *ulpq,
+ struct sctp_ulpevent *event)
+{
+ struct sctp_ulpevent *cevent;
+ struct sk_buff *pos;
+
+ pos = skb_peek_tail(&ulpq->reasm_uo);
+ if (!pos) {
+ __skb_queue_tail(&ulpq->reasm_uo, sctp_event2skb(event));
+ return;
+ }
+
+ cevent = sctp_skb2event(pos);
+
+ if (event->stream == cevent->stream &&
+ event->mid == cevent->mid &&
+ (cevent->msg_flags & SCTP_DATA_FIRST_FRAG ||
+ (!(event->msg_flags & SCTP_DATA_FIRST_FRAG) &&
+ event->fsn > cevent->fsn))) {
+ __skb_queue_tail(&ulpq->reasm_uo, sctp_event2skb(event));
+ return;
+ }
+
+ if ((event->stream == cevent->stream &&
+ MID_lt(cevent->mid, event->mid)) ||
+ event->stream > cevent->stream) {
+ __skb_queue_tail(&ulpq->reasm_uo, sctp_event2skb(event));
+ return;
+ }
+
+ skb_queue_walk(&ulpq->reasm_uo, pos) {
+ cevent = sctp_skb2event(pos);
+
+ if (event->stream < cevent->stream ||
+ (event->stream == cevent->stream &&
+ MID_lt(event->mid, cevent->mid)))
+ break;
+
+ if (event->stream == cevent->stream &&
+ event->mid == cevent->mid &&
+ !(cevent->msg_flags & SCTP_DATA_FIRST_FRAG) &&
+ (event->msg_flags & SCTP_DATA_FIRST_FRAG ||
+ event->fsn < cevent->fsn))
+ break;
+ }
+
+ __skb_queue_before(&ulpq->reasm_uo, pos, sctp_event2skb(event));
+}
+
+static struct sctp_ulpevent *sctp_intl_retrieve_partial_uo(
+ struct sctp_ulpq *ulpq,
+ struct sctp_ulpevent *event)
+{
+ struct sk_buff *first_frag = NULL;
+ struct sk_buff *last_frag = NULL;
+ struct sctp_ulpevent *retval;
+ struct sctp_stream_in *sin;
+ struct sk_buff *pos;
+ __u32 next_fsn = 0;
+ int is_last = 0;
+
+ sin = sctp_stream_in(ulpq->asoc, event->stream);
+
+ skb_queue_walk(&ulpq->reasm_uo, pos) {
+ struct sctp_ulpevent *cevent = sctp_skb2event(pos);
+
+ if (cevent->stream < event->stream)
+ continue;
+ if (cevent->stream > event->stream)
+ break;
+
+ if (MID_lt(cevent->mid, sin->mid_uo))
+ continue;
+ if (MID_lt(sin->mid_uo, cevent->mid))
+ break;
+
+ switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
+ case SCTP_DATA_FIRST_FRAG:
+ goto out;
+ case SCTP_DATA_MIDDLE_FRAG:
+ if (!first_frag) {
+ if (cevent->fsn == sin->fsn_uo) {
+ first_frag = pos;
+ last_frag = pos;
+ next_fsn = cevent->fsn + 1;
+ }
+ } else if (cevent->fsn == next_fsn) {
+ last_frag = pos;
+ next_fsn++;
+ } else {
+ goto out;
+ }
+ break;
+ case SCTP_DATA_LAST_FRAG:
+ if (!first_frag) {
+ if (cevent->fsn == sin->fsn_uo) {
+ first_frag = pos;
+ last_frag = pos;
+ next_fsn = 0;
+ is_last = 1;
+ }
+ } else if (cevent->fsn == next_fsn) {
+ last_frag = pos;
+ next_fsn = 0;
+ is_last = 1;
+ }
+ goto out;
+ default:
+ goto out;
+ }
+ }
+
+out:
+ if (!first_frag)
+ return NULL;
+
+ retval = sctp_make_reassembled_event(sock_net(ulpq->asoc->base.sk),
+ &ulpq->reasm_uo, first_frag,
+ last_frag);
+ if (retval) {
+ sin->fsn_uo = next_fsn;
+ if (is_last) {
+ retval->msg_flags |= MSG_EOR;
+ sin->pd_mode_uo = 0;
+ }
+ }
+
+ return retval;
+}
+
+static struct sctp_ulpevent *sctp_intl_retrieve_reassembled_uo(
+ struct sctp_ulpq *ulpq,
+ struct sctp_ulpevent *event)
+{
+ struct sctp_association *asoc = ulpq->asoc;
+ struct sk_buff *pos, *first_frag = NULL;
+ struct sctp_ulpevent *retval = NULL;
+ struct sk_buff *pd_first = NULL;
+ struct sk_buff *pd_last = NULL;
+ struct sctp_stream_in *sin;
+ __u32 next_fsn = 0;
+ __u32 pd_point = 0;
+ __u32 pd_len = 0;
+ __u32 mid = 0;
+
+ sin = sctp_stream_in(ulpq->asoc, event->stream);
+
+ skb_queue_walk(&ulpq->reasm_uo, pos) {
+ struct sctp_ulpevent *cevent = sctp_skb2event(pos);
+
+ if (cevent->stream < event->stream)
+ continue;
+ if (cevent->stream > event->stream)
+ break;
+
+ if (MID_lt(cevent->mid, event->mid))
+ continue;
+ if (MID_lt(event->mid, cevent->mid))
+ break;
+
+ switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
+ case SCTP_DATA_FIRST_FRAG:
+ if (!sin->pd_mode_uo) {
+ sin->mid_uo = cevent->mid;
+ pd_first = pos;
+ pd_last = pos;
+ pd_len = pos->len;
+ }
+
+ first_frag = pos;
+ next_fsn = 0;
+ mid = cevent->mid;
+ break;
+
+ case SCTP_DATA_MIDDLE_FRAG:
+ if (first_frag && cevent->mid == mid &&
+ cevent->fsn == next_fsn) {
+ next_fsn++;
+ if (pd_first) {
+ pd_last = pos;
+ pd_len += pos->len;
+ }
+ } else {
+ first_frag = NULL;
+ }
+ break;
+
+ case SCTP_DATA_LAST_FRAG:
+ if (first_frag && cevent->mid == mid &&
+ cevent->fsn == next_fsn)
+ goto found;
+ else
+ first_frag = NULL;
+ break;
+ }
+ }
+
+ if (!pd_first)
+ goto out;
+
+ pd_point = sctp_sk(asoc->base.sk)->pd_point;
+ if (pd_point && pd_point <= pd_len) {
+ retval = sctp_make_reassembled_event(sock_net(asoc->base.sk),
+ &ulpq->reasm_uo,
+ pd_first, pd_last);
+ if (retval) {
+ sin->fsn_uo = next_fsn;
+ sin->pd_mode_uo = 1;
+ }
+ }
+ goto out;
+
+found:
+ retval = sctp_make_reassembled_event(sock_net(asoc->base.sk),
+ &ulpq->reasm_uo,
+ first_frag, pos);
+ if (retval)
+ retval->msg_flags |= MSG_EOR;
+
+out:
+ return retval;
+}
+
+static struct sctp_ulpevent *sctp_intl_reasm_uo(struct sctp_ulpq *ulpq,
+ struct sctp_ulpevent *event)
+{
+ struct sctp_ulpevent *retval = NULL;
+ struct sctp_stream_in *sin;
+
+ if (SCTP_DATA_NOT_FRAG == (event->msg_flags & SCTP_DATA_FRAG_MASK)) {
+ event->msg_flags |= MSG_EOR;
+ return event;
+ }
+
+ sctp_intl_store_reasm_uo(ulpq, event);
+
+ sin = sctp_stream_in(ulpq->asoc, 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);
+
+ if (!retval)
+ retval = sctp_intl_retrieve_reassembled_uo(ulpq, event);
+
+ return retval;
+}
+
+static struct sctp_ulpevent *sctp_intl_retrieve_first_uo(struct sctp_ulpq *ulpq)
+{
+ struct sctp_stream_in *csin, *sin;
+ struct sk_buff *first_frag = NULL;
+ struct sk_buff *last_frag = NULL;
+ struct sctp_ulpevent *retval;
+ struct sk_buff *pos;
+ __u32 next_fsn = 0;
+ __u16 sid = 0;
+
+ skb_queue_walk(&ulpq->reasm_uo, pos) {
+ struct sctp_ulpevent *cevent = sctp_skb2event(pos);
+
+ csin = sctp_stream_in(ulpq->asoc, cevent->stream);
+ if (csin->pd_mode_uo)
+ continue;
+
+ switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
+ case SCTP_DATA_FIRST_FRAG:
+ if (first_frag)
+ goto out;
+ first_frag = pos;
+ last_frag = pos;
+ next_fsn = 0;
+ sin = csin;
+ sid = cevent->stream;
+ sin->mid_uo = cevent->mid;
+ break;
+ case SCTP_DATA_MIDDLE_FRAG:
+ if (!first_frag)
+ break;
+ if (cevent->stream == sid &&
+ cevent->mid == sin->mid_uo &&
+ cevent->fsn == next_fsn) {
+ next_fsn++;
+ last_frag = pos;
+ } else {
+ goto out;
+ }
+ break;
+ case SCTP_DATA_LAST_FRAG:
+ if (first_frag)
+ goto out;
+ break;
+ default:
+ break;
+ }
+ }
+
+ if (!first_frag)
+ return NULL;
+
+out:
+ retval = sctp_make_reassembled_event(sock_net(ulpq->asoc->base.sk),
+ &ulpq->reasm_uo, first_frag,
+ last_frag);
+ if (retval) {
+ sin->fsn_uo = next_fsn;
+ sin->pd_mode_uo = 1;
+ }
+
+ return retval;
+}
+
static int sctp_ulpevent_idata(struct sctp_ulpq *ulpq,
struct sctp_chunk *chunk, gfp_t gfp)
{
@@ -529,12 +841,16 @@ static int sctp_ulpevent_idata(struct sctp_ulpq *ulpq,
else
event->fsn = ntohl(chunk->subh.idata_hdr->fsn);
- event = sctp_intl_reasm(ulpq, event);
- if (event && event->msg_flags & MSG_EOR) {
- skb_queue_head_init(&temp);
- __skb_queue_tail(&temp, sctp_event2skb(event));
+ if (!(event->msg_flags & SCTP_DATA_UNORDERED)) {
+ event = sctp_intl_reasm(ulpq, event);
+ if (event && event->msg_flags & MSG_EOR) {
+ skb_queue_head_init(&temp);
+ __skb_queue_tail(&temp, sctp_event2skb(event));
- event = sctp_intl_order(ulpq, event);
+ event = sctp_intl_order(ulpq, event);
+ }
+ } else {
+ event = sctp_intl_reasm_uo(ulpq, event);
}
if (event) {
@@ -614,14 +930,21 @@ static void sctp_intl_start_pd(struct sctp_ulpq *ulpq, gfp_t gfp)
{
struct sctp_ulpevent *event;
- if (skb_queue_empty(&ulpq->reasm))
- return;
+ if (!skb_queue_empty(&ulpq->reasm)) {
+ do {
+ event = sctp_intl_retrieve_first(ulpq);
+ if (event)
+ sctp_enqueue_event(ulpq, event);
+ } while (event);
+ }
- do {
- event = sctp_intl_retrieve_first(ulpq);
- if (event)
- sctp_enqueue_event(ulpq, event);
- } while (event);
+ if (!skb_queue_empty(&ulpq->reasm_uo)) {
+ do {
+ event = sctp_intl_retrieve_first_uo(ulpq);
+ if (event)
+ sctp_enqueue_event(ulpq, event);
+ } while (event);
+ }
}
static void sctp_renege_events(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
@@ -643,6 +966,9 @@ static void sctp_renege_events(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
if (freed < needed)
freed += sctp_ulpq_renege_list(ulpq, &ulpq->reasm,
needed);
+ if (freed < needed)
+ freed += sctp_ulpq_renege_list(ulpq, &ulpq->reasm_uo,
+ needed);
}
if (chunk && freed >= needed)
@@ -734,6 +1060,13 @@ static void sctp_intl_abort_pd(struct sctp_ulpq *ulpq, gfp_t gfp)
struct sctp_stream_in *sin = &stream->in[sid];
__u32 mid;
+ if (sin->pd_mode_uo) {
+ sin->pd_mode_uo = 0;
+
+ mid = sin->mid_uo;
+ sctp_intl_stream_abort_pd(ulpq, sid, mid, 0x1, gfp);
+ }
+
if (sin->pd_mode) {
sin->pd_mode = 0;
diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index dd53daa..97fae53 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -60,6 +60,7 @@ struct sctp_ulpq *sctp_ulpq_init(struct sctp_ulpq *ulpq,
ulpq->asoc = asoc;
skb_queue_head_init(&ulpq->reasm);
+ skb_queue_head_init(&ulpq->reasm_uo);
skb_queue_head_init(&ulpq->lobby);
ulpq->pd_mode = 0;
@@ -83,6 +84,10 @@ void sctp_ulpq_flush(struct sctp_ulpq *ulpq)
sctp_ulpevent_free(event);
}
+ while ((skb = __skb_dequeue(&ulpq->reasm_uo)) != NULL) {
+ event = sctp_skb2event(skb);
+ sctp_ulpevent_free(event);
+ }
}
/* Dispose of a ulpqueue. */
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net-next] net: phy: meson-gxl: cleanup by defining the control registers
From: Yixun Lan @ 2017-12-05 15:23 UTC (permalink / raw)
To: Jerome Brunet, Andrew Lunn, Florian Fainelli, Neil Armstrong
Cc: yixun.lan, netdev, linux-kernel, linux-amlogic
In-Reply-To: <20171205093334.8261-1-jbrunet@baylibre.com>
HI Jerome:
On 12/05/17 17:33, Jerome Brunet wrote:
> From: Neil Armstrong <narmstrong@baylibre.com>
>
> Define registers and bits in meson-gxl PHY driver to make a bit
> more human friendly. No functional change
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> drivers/net/phy/meson-gxl.c | 111 +++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 93 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> index 1ea69b7585d9..82c11556605e 100644
> --- a/drivers/net/phy/meson-gxl.c
> +++ b/drivers/net/phy/meson-gxl.c
> @@ -22,32 +22,107 @@
> #include <linux/ethtool.h>
> #include <linux/phy.h>
> #include <linux/netdevice.h>
> +#include <linux/bitfield.h>
> +
> +#define TSTCNTL 0x14
> +#define TSTREAD1 0x15
> +#define TSTREAD2 0x16
> +#define TSTWRITE 0x17
> +
> +#define TSTCNTL_READ BIT(15)
> +#define TSTCNTL_WRITE BIT(14)
> +#define TSTCNTL_REG_BANK_SEL GENMASK(12, 11)
> +#define TSTCNTL_TEST_MODE BIT(10)
> +#define TSTCNTL_READ_ADDRESS GENMASK(9, 5)
> +#define TSTCNTL_WRITE_ADDRESS GENMASK(4, 0)
> +
> +#define BANK_ANALOG_DSP 0
> +#define BANK_BIST 3
> +
> +/* Analog/DSP Registers */
> +#define A6_CONFIG_REG 0x17
> +
> +/* BIST Registers */
> +#define FR_PLL_CONTROL 0x1b
> +#define FR_PLL_DIV0 0x1c
> +#define FR_PLL_DIV1 0x1d
> +
> +#define A6_CONFIG_PLLMULX4ICH BIT(15)
> +#define A6_CONFIG_PLLBIASSEL BIT(14)
> +#define A6_CONFIG_PLLINTRATIO GENMASK(13, 12)
> +#define A6_CONFIG_PLLBUFITRIM GENMASK(11, 9)
> +#define A6_CONFIG_PLLCHTRIM GENMASK(8, 5)
> +#define A6_CONFIG_PLLCHBIASSEL BIT(4)
> +#define A6_CONFIG_PLLRSTVCOPD BIT(3)
> +#define A6_CONFIG_PLLCPOFF BIT(2)
> +#define A6_CONFIG_PLLPD BIT(1)
> +#define A6_CONFIG_PLL_SRC BIT(0)
> +
> +static inline int meson_gxl_write_reg(struct phy_device *phydev,
> + unsigned int bank, unsigned int reg,
> + uint16_t value)
> +{
> + int ret;
> +
> + /* Enable Analog and DSP register Bank access by
> + * toggling TSTCNTL_TEST_MODE bit in the TSTCNTL register
> + */
> + ret = phy_write(phydev, TSTCNTL, 0);
> + if (ret)
> + goto out;
> + ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
> + if (ret)
> + goto out;
> + ret = phy_write(phydev, TSTCNTL, 0);
> + if (ret)
> + goto out;
> + ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
> + if (ret)
> + goto out;
> +
how about just do the above enable procedure once?
from the datasheet, the access won't be disabled if don't reset, or
write to register TSTCNTL with TEST_MODE=0
> + ret = phy_write(phydev, TSTWRITE, value);
> + if (ret)
> + goto out;
> +
> + ret = phy_write(phydev, TSTCNTL, TSTCNTL_WRITE |
> + FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
> + TSTCNTL_TEST_MODE |
> + FIELD_PREP(TSTCNTL_WRITE_ADDRESS, reg));
> +
> +out:
> + /* Close the bank access on our way out */
> + phy_write(phydev, TSTCNTL, 0);
> + return ret;
> +}
> +
>
> static int meson_gxl_config_init(struct phy_device *phydev)
> {
> - /* Enable Analog and DSP register Bank access by */
> - phy_write(phydev, 0x14, 0x0000);
> - phy_write(phydev, 0x14, 0x0400);
> - phy_write(phydev, 0x14, 0x0000);
> - phy_write(phydev, 0x14, 0x0400);
> + int ret;
>
> - /* Write Analog register 23 */
> - phy_write(phydev, 0x17, 0x8E0D);
> - phy_write(phydev, 0x14, 0x4417);
> + /* Write PLL Configuration 1 */
> + ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
> + A6_CONFIG_PLLMULX4ICH |
> + FIELD_PREP(A6_CONFIG_PLLBUFITRIM, 7) |
> + A6_CONFIG_PLLRSTVCOPD |
> + A6_CONFIG_PLLCPOFF |
> + A6_CONFIG_PLL_SRC);
> + if (ret)
> + return ret;
>
> - /* Enable fractional PLL */
> - phy_write(phydev, 0x17, 0x0005);
> - phy_write(phydev, 0x14, 0x5C1B);
> + /* Enable fractional PLL configuration */
> + ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
> + if (ret)
> + return ret;
>
> - /* Program fraction FR_PLL_DIV1 */
> - phy_write(phydev, 0x17, 0x029A);
> - phy_write(phydev, 0x14, 0x5C1D);
> + ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV1, 0x029a);
> + if (ret)
> + return ret;
>
> - /* Program fraction FR_PLL_DIV1 */
> - phy_write(phydev, 0x17, 0xAAAA);
> - phy_write(phydev, 0x14, 0x5C1C);
> + /* Program fraction FR_PLL_DIV0 */
> + ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV0, 0xaaaa);
>
> - return 0;
> + return ret;
> }
>
> static struct phy_driver meson_gxl_phy[] = {
>
^ permalink raw reply
* Re: Linux ECN Handling
From: Neal Cardwell @ 2017-12-05 15:23 UTC (permalink / raw)
To: Steve Ibanez
Cc: Eric Dumazet, Yuchung Cheng, Daniel Borkmann, Netdev,
Florian Westphal, Mohammad Alizadeh, Lawrence Brakmo
In-Reply-To: <CACJspmKP4PKkZ2ywUeta9SuEk73RQ0cMLJWDfgJzQn9XkwjEbA@mail.gmail.com>
On Tue, Dec 5, 2017 at 12:22 AM, Steve Ibanez <sibanez@stanford.edu> wrote:
> Hi Neal,
>
> Happy to help out :) And thanks for the tip!
>
> I was able to track down where the missing bytes that you pointed out
> are being lost. It turns out the destination host seems to be
> misbehaving. I performed a packet capture at the destination host
> interface (a snapshot of the trace is attached). I see the following
> sequence of events when a timeout occurs (note that I have NIC
> offloading enabled so wireshark captures packets larger than the MTU):
>
> 1. The destination receives a data packet of length X with seqNo = Y
> from the src with the CWR bit set and does not send back a
> corresponding ACK.
> 2. The source times out and sends a retransmission packet of length Z
> (where Z < X) with seqNo = Y
> 3. The destination sends back an ACK with AckNo = Y + X
>
> So in other words, the packet which the destination host does not
> initially ACK (causing the timeout) does not actually get lost because
> after receiving the retransmission the AckNo moves forward all the way
> past the bytes in the initial unACKed CWR packet. In the attached
> screenshot, I've marked the unACKed CWR packet with a red box.
>
> Have you seen this behavior before? And do you know what might be
> causing the destination host not to ACK the CWR packet? In most cases
> the CWR marked packets are ACKed properly, it's just occasionally they
> are not.
Thanks for the detailed report!
I have not heard of an incoming CWR causing the receiver to fail to
ACK. And in re-reading the code, I don't see an obvious way in which a
CWR bit should cause the receiver to fail to ACK.
That screen shot is a bit hard to parse. Would you be able to post a
tcpdump .pcap of that particular section, or post a screen shot of a
time-sequence plot of that section?
To extract that segment and take screen shot, you could use something like:
editcap -A "2017-12-04 11:22:27" -B "2017-12-04 11:22:30" all.pcap
slice.pcap
tcptrace -S -xy -zy slice.pcap
xplot.org a2b_tsg.xpl &
# take screenshot
Or, alternatively, would you be able to post the slice.pcap on a web
server or public drive?
thanks,
neal
^ permalink raw reply
* Re: [crypto 4/8] chtls: CPL handler definition
From: Stefano Brivio @ 2017-12-05 15:23 UTC (permalink / raw)
To: Atul Gupta
Cc: herbert, linux-crypto, netdev, davem, davejwatson, ganeshgr,
harsh
In-Reply-To: <1512474000-6681-1-git-send-email-atul.gupta@chelsio.com>
On Tue, 5 Dec 2017 17:10:00 +0530
Atul Gupta <atul.gupta@chelsio.com> wrote:
> CPL handlers for TLS session, record transmit and receive
>
> Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
> ---
> drivers/crypto/chelsio/chtls/chtls_cm.c | 2048 +++++++++++++++++++++++++++++++
> 1 file changed, 2048 insertions(+)
> create mode 100644 drivers/crypto/chelsio/chtls/chtls_cm.c
>
> diff --git a/drivers/crypto/chelsio/chtls/chtls_cm.c b/drivers/crypto/chelsio/chtls/chtls_cm.c
> new file mode 100644
> index 0000000..ea1c301
> --- /dev/null
> +++ b/drivers/crypto/chelsio/chtls/chtls_cm.c
> @@ -0,0 +1,2048 @@
> +/*
> + * Copyright (c) 2017 Chelsio Communications, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Written by: Atul Gupta (atul.gupta@chelsio.com)
> + */
> +
> +#include <linux/module.h>
> +#include <linux/list.h>
> +#include <linux/workqueue.h>
> +#include <linux/skbuff.h>
> +#include <linux/timer.h>
> +#include <linux/notifier.h>
> +#include <linux/inetdevice.h>
> +#include <linux/ip.h>
> +#include <linux/tcp.h>
> +#include <linux/sched/signal.h>
> +#include <linux/kallsyms.h>
> +#include <linux/kprobes.h>
> +#include <linux/if_vlan.h>
> +#include <net/tcp.h>
> +#include <net/dst.h>
> +
> +#include "chtls.h"
> +#include "chtls_cm.h"
> +
> +extern struct request_sock_ops chtls_rsk_ops;
> +static void (*tcp_time_wait_p)(struct sock *sk, int state, int timeo);
> +
> +/*
> + * State transitions and actions for close. Note that if we are in SYN_SENT
> + * we remain in that state as we cannot control a connection while it's in
> + * SYN_SENT; such connections are allowed to establish and are then aborted.
> + */
> +static unsigned char new_state[16] = {
> + /* current state: new state: action: */
> + /* (Invalid) */ TCP_CLOSE,
> + /* TCP_ESTABLISHED */ TCP_FIN_WAIT1 | TCP_ACTION_FIN,
> + /* TCP_SYN_SENT */ TCP_SYN_SENT,
> + /* TCP_SYN_RECV */ TCP_FIN_WAIT1 | TCP_ACTION_FIN,
> + /* TCP_FIN_WAIT1 */ TCP_FIN_WAIT1,
> + /* TCP_FIN_WAIT2 */ TCP_FIN_WAIT2,
> + /* TCP_TIME_WAIT */ TCP_CLOSE,
> + /* TCP_CLOSE */ TCP_CLOSE,
> + /* TCP_CLOSE_WAIT */ TCP_LAST_ACK | TCP_ACTION_FIN,
> + /* TCP_LAST_ACK */ TCP_LAST_ACK,
> + /* TCP_LISTEN */ TCP_CLOSE,
> + /* TCP_CLOSING */ TCP_CLOSING,
> +};
> +
> +static struct chtls_sock *chtls_sock_create(struct chtls_dev *cdev)
> +{
> + struct chtls_sock *csk = kzalloc(sizeof(*csk), GFP_NOIO);
> +
> + if (!csk)
> + return NULL;
> +
> + csk->txdata_skb_cache = alloc_skb(TXDATA_SKB_LEN, GFP_ATOMIC);
Excess whitespace.
> + if (!csk->txdata_skb_cache) {
> + kfree(csk);
> + return NULL;
> + }
> +
> + kref_init(&csk->kref);
> + csk->cdev = cdev;
> + skb_queue_head_init(&csk->txq);
> + csk->wr_skb_head = NULL;
> + csk->wr_skb_tail = NULL;
> + csk->mss = MAX_MSS;
> + csk->tlshws.ofld = 1;
> + csk->tlshws.txkey = -1;
> + csk->tlshws.rxkey = -1;
> + csk->tlshws.mfs = TLS_MFS;
> + skb_queue_head_init(&csk->tlshws.sk_recv_queue);
> + return csk;
> +}
> +
> +void chtls_sock_release(struct kref *ref)
> +{
> + struct chtls_sock *csk =
> + container_of(ref, struct chtls_sock, kref);
> +
> + kfree(csk);
> +}
> +
> +void get_tcp_symbol(void)
> +{
> + tcp_time_wait_p = (void *)kallsyms_lookup_name("tcp_time_wait");
> + if (!tcp_time_wait_p)
> + pr_info("could not locate tcp_time_wait");
Probably not something that should be used here. Why do you need this?
> +}
> +
> +static struct net_device *chtls_ipv4_netdev(struct chtls_dev *cdev,
> + struct sock *sk)
> +{
> + struct net_device *ndev = cdev->ports[0];
> +
> + if (likely(!inet_sk(sk)->inet_rcv_saddr))
> + return ndev;
> +
> + ndev = ip_dev_find(&init_net, inet_sk(sk)->inet_rcv_saddr);
> + if (!ndev)
> + return NULL;
> +
> + if (is_vlan_dev(ndev))
> + return vlan_dev_real_dev(ndev);
> + return ndev;
> +}
> +
> +static void assign_rxopt(struct sock *sk, unsigned int opt)
> +{
> + const struct chtls_dev *cdev;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
Reverse christmas tree format?
> +
> + cdev = csk->cdev;
> + tp->tcp_header_len = sizeof(struct tcphdr);
> + tp->rx_opt.mss_clamp = cdev->mtus[TCPOPT_MSS_G(opt)] - 40;
> + tp->mss_cache = tp->rx_opt.mss_clamp;
> + tp->rx_opt.tstamp_ok = TCPOPT_TSTAMP_G(opt);
> + tp->rx_opt.snd_wscale = TCPOPT_SACK_G(opt);
> + tp->rx_opt.wscale_ok = TCPOPT_WSCALE_OK_G(opt);
> + SND_WSCALE(tp) = TCPOPT_SND_WSCALE_G(opt);
> + if (!tp->rx_opt.wscale_ok)
> + tp->rx_opt.rcv_wscale = 0;
> + if (tp->rx_opt.tstamp_ok) {
> + tp->tcp_header_len += TCPOLEN_TSTAMP_ALIGNED;
> + tp->rx_opt.mss_clamp -= TCPOLEN_TSTAMP_ALIGNED;
> + } else if (csk->opt2 & TSTAMPS_EN_F) {
> + csk->opt2 &= ~TSTAMPS_EN_F;
> + csk->mtu_idx = TCPOPT_MSS_G(opt);
> + }
> +}
> +
> +static void chtls_purge_rcv_queue(struct sock *sk)
> +{
> + struct sk_buff *skb;
> +
> + while ((skb = __skb_dequeue(&sk->sk_receive_queue)) != NULL) {
> + skb_dst_set(skb, (void *)NULL);
> + kfree_skb(skb);
> + }
> +}
> +
> +static void chtls_purge_write_queue(struct sock *sk)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct sk_buff *skb;
> +
> + while ((skb = __skb_dequeue(&csk->txq))) {
> + sk->sk_wmem_queued -= skb->truesize;
> + __kfree_skb(skb);
> + }
> +}
> +
> +static void chtls_purge_receive_queue(struct sock *sk)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_hws *tlsk = &csk->tlshws;
> + struct sk_buff *skb;
> +
> + while ((skb = __skb_dequeue(&tlsk->sk_recv_queue)) != NULL) {
> + skb_dst_set(skb, NULL);
> + kfree_skb(skb);
> + }
> +}
> +
> +static void abort_arp_failure(void *handle, struct sk_buff *skb)
> +{
> + struct cpl_abort_req *req = cplhdr(skb);
> + struct chtls_dev *cdev = (struct chtls_dev *)handle;
Reverse christmas tree format?
> +
> + req->cmd = CPL_ABORT_NO_RST;
> + cxgb4_ofld_send(cdev->lldi->ports[0], skb);
> +}
> +
> +struct sk_buff *alloc_ctrl_skb(struct sk_buff *skb, int len)
> +{
> + if (likely(skb && !skb_shared(skb) && !skb_cloned(skb))) {
> + __skb_trim(skb, 0);
> + refcount_add(2, &skb->users);
> + } else {
> + skb = alloc_skb(len, GFP_KERNEL | __GFP_NOFAIL);
> + }
> + return skb;
> +}
> +
> +static void chtls_send_abort(struct sock *sk, int mode, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_abort_req *req;
> +
> + if (!skb)
> + skb = alloc_ctrl_skb(csk->txdata_skb_cache, sizeof(*req));
What if alloc_ctrl_skb() fails?
> + req = (struct cpl_abort_req *)skb_put(skb, sizeof(*req));
> + INIT_TP_WR_CPL(req, CPL_ABORT_REQ, csk->tid);
> + set_queue(skb, (csk->txq_idx << 1) | CPL_PRIORITY_DATA, sk);
> + req->rsvd0 = htonl(tp->snd_nxt);
> + req->rsvd1 = !csk_flag_nochk(csk, CSK_TX_DATA_SENT);
> + req->cmd = mode;
> + t4_set_arp_err_handler(skb, csk->cdev, abort_arp_failure);
> + send_or_defer(sk, tp, skb, mode == CPL_ABORT_SEND_RST);
> +}
> +
> +int chtls_send_reset(struct sock *sk, int mode, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> +
> + if (unlikely(csk_flag_nochk(csk, CSK_ABORT_SHUTDOWN) ||
> + !csk->cdev)) {
> + if (sk->sk_state == TCP_SYN_RECV)
> + csk_set_flag(csk, CSK_RST_ABORTED);
> + goto out;
> + }
> +
> + if (!csk_flag_nochk(csk, CSK_TX_DATA_SENT)) {
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + if (send_tx_flowc_wr(sk, 0, tp->snd_nxt, tp->rcv_nxt) < 0)
> + WARN_ONCE(1, "send tx flowc error");
> + csk_set_flag(csk, CSK_TX_DATA_SENT);
> + }
> +
> + csk_set_flag(csk, CSK_ABORT_RPL_PENDING);
> + chtls_purge_write_queue(sk);
> +
> + csk_set_flag(csk, CSK_ABORT_SHUTDOWN);
> + if (sk->sk_state != TCP_SYN_RECV)
> + chtls_send_abort(sk, mode, skb);
If sk->sk_state == TCP_SYN_RECV, aren't we leaking skb, coming e.g.
from reset_listen_child()?
> +
> + return 0;
> +out:
> + if (skb)
> + kfree_skb(skb);
> + return 1;
> +}
> +
> +static void release_tcp_port(struct sock *sk)
> +{
> + if (inet_csk(sk)->icsk_bind_hash)
> + inet_put_port(sk);
> +}
> +
> +static void tcp_uncork(struct sock *sk)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + if (tp->nonagle & TCP_NAGLE_CORK) {
> + tp->nonagle &= ~TCP_NAGLE_CORK;
> + chtls_tcp_push(sk, 0);
> + }
> +}
> +
> +static void chtls_close_conn(struct sock *sk)
> +{
> + struct sk_buff *skb;
> + struct cpl_close_con_req *req;
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + unsigned int tid = csk->tid;
> + unsigned int len = roundup(sizeof(struct cpl_close_con_req), 16);
> +
> + skb = alloc_skb(len, GFP_KERNEL | __GFP_NOFAIL);
> + req = (struct cpl_close_con_req *)__skb_put(skb, len);
> + memset(req, 0, len);
> + req->wr.wr_hi = htonl(FW_WR_OP_V(FW_TP_WR) |
> + FW_WR_IMMDLEN_V(sizeof(*req) -
> + sizeof(req->wr)));
> + req->wr.wr_mid = htonl(FW_WR_LEN16_V(DIV_ROUND_UP(sizeof(*req), 16)) |
> + FW_WR_FLOWID_V(tid));
> +
> + OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_CLOSE_CON_REQ, tid));
> +
> + tcp_uncork(sk);
> + skb_entail(sk, skb, ULPCB_FLAG_NO_HDR | ULPCB_FLAG_NO_APPEND);
> + if (sk->sk_state != TCP_SYN_SENT)
> + chtls_push_frames(csk, 1);
> +}
> +
> +/*
> + * Perform a state transition during close and return the actions indicated
> + * for the transition. Do not make this function inline, the main reason
> + * it exists at all is to avoid multiple inlining of tcp_set_state.
> + */
> +static int make_close_transition(struct sock *sk)
> +{
> + int next = (int)new_state[sk->sk_state];
> +
> + tcp_set_state(sk, next & TCP_STATE_MASK);
> + return next & TCP_ACTION_FIN;
> +}
> +
> +void chtls_close(struct sock *sk, long timeout)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + int data_lost, prev_state;
> +
> + lock_sock(sk);
> + if (sk->sk_prot->close != chtls_close) {
> + release_sock(sk);
> + return sk->sk_prot->close(sk, timeout);
> + }
> +
> + sk->sk_shutdown |= SHUTDOWN_MASK;
> +
> + data_lost = skb_queue_len(&sk->sk_receive_queue);
> + data_lost |= skb_queue_len(&csk->tlshws.sk_recv_queue);
> + chtls_purge_receive_queue(sk);
> + chtls_purge_rcv_queue(sk);
> +
> + if (sk->sk_state == TCP_CLOSE) {
> + goto wait;
> + } else if (data_lost || sk->sk_state == TCP_SYN_SENT) {
> + chtls_send_reset(sk, CPL_ABORT_SEND_RST, NULL);
> + release_tcp_port(sk);
> + goto unlock;
> + } else if (sock_flag(sk, SOCK_LINGER) && !sk->sk_lingertime) {
> + sk->sk_prot->disconnect(sk, 0);
> + } else if (make_close_transition(sk)) {
> + chtls_close_conn(sk);
> + }
> +wait:
> + if (timeout)
> + sk_stream_wait_close(sk, timeout);
> +
> +unlock:
> + prev_state = sk->sk_state;
> + sock_hold(sk);
> + sock_orphan(sk);
> +
> + release_sock(sk);
> +
> + local_bh_disable();
> + bh_lock_sock(sk);
> +
> + if (prev_state != TCP_CLOSE && sk->sk_state == TCP_CLOSE)
> + goto out;
> +
> + if (sk->sk_state == TCP_FIN_WAIT2 && tcp_sk(sk)->linger2 < 0 &&
> + !csk_flag(sk, CSK_ABORT_SHUTDOWN)) {
> + struct sk_buff *skb;
> +
> + skb = alloc_skb(sizeof(struct cpl_abort_req), GFP_ATOMIC);
> + if (skb)
> + chtls_send_reset(sk, CPL_ABORT_SEND_RST, skb);
> + }
> +
> + if (sk->sk_state == TCP_CLOSE)
> + inet_csk_destroy_sock(sk);
> +
> +out:
> + bh_unlock_sock(sk);
> + local_bh_enable();
> + sock_put(sk);
> +}
> +
> +/*
> + * Wait until a socket enters on of the given states.
> + */
> +static int wait_for_states(struct sock *sk, unsigned int states)
> +{
> + struct socket_wq _sk_wq;
> + long current_timeo = 200;
> + DECLARE_WAITQUEUE(wait, current);
> + int err = 0;
> +
> + /*
> + * We want this to work even when there's no associated struct socket.
> + * In that case we provide a temporary wait_queue_head_t.
> + */
> + if (!sk->sk_wq) {
> + init_waitqueue_head(&_sk_wq.wait);
> + _sk_wq.fasync_list = NULL;
> + init_rcu_head_on_stack(&_sk_wq.rcu);
> + sk->sk_wq = &_sk_wq;
> + }
> +
> + add_wait_queue(sk_sleep(sk), &wait);
> + while (!sk_in_state(sk, states)) {
> + if (!current_timeo) {
> + err = -EBUSY;
> + break;
> + }
> + if (signal_pending(current)) {
> + err = sock_intr_errno(current_timeo);
> + break;
> + }
> + set_current_state(TASK_UNINTERRUPTIBLE);
> + release_sock(sk);
> + if (!sk_in_state(sk, states))
> + current_timeo = schedule_timeout(current_timeo);
> + __set_current_state(TASK_RUNNING);
> + lock_sock(sk);
> + }
> + remove_wait_queue(sk_sleep(sk), &wait);
> +
> + if (sk->sk_wq == &_sk_wq)
> + sk->sk_wq = NULL;
> + return err;
> +}
> +
> +int chtls_disconnect(struct sock *sk, int flags)
> +{
> + struct chtls_sock *csk;
> + struct tcp_sock *tp = tcp_sk(sk);
> + int err;
> +
> + if (sk->sk_prot->disconnect != chtls_disconnect)
> + return sk->sk_prot->disconnect(sk, flags);
> +
> + csk = rcu_dereference_sk_user_data(sk);
> + chtls_purge_receive_queue(sk);
> + chtls_purge_rcv_queue(sk);
> + chtls_purge_write_queue(sk);
> +
> + if (sk->sk_state != TCP_CLOSE) {
> + sk->sk_err = ECONNRESET;
> + chtls_send_reset(sk, CPL_ABORT_SEND_RST, NULL);
> + err = wait_for_states(sk, TCPF_CLOSE);
> + if (err)
> + return err;
> + }
> + if (sk->sk_prot->disconnect != chtls_disconnect)
> + return sk->sk_prot->disconnect(sk, flags);
> +
> + chtls_purge_receive_queue(sk);
> + chtls_purge_rcv_queue(sk);
> + tp->max_window = 0xFFFF << (tp->rx_opt.snd_wscale);
> + return tcp_disconnect(sk, flags);
> +}
> +
> +#define SHUTDOWN_ELIGIBLE_STATE (TCPF_ESTABLISHED | \
> + TCPF_SYN_RECV | TCPF_CLOSE_WAIT)
> +void chtls_shutdown(struct sock *sk, int how)
> +{
> + if (sk->sk_prot->shutdown != chtls_shutdown)
> + return sk->sk_prot->shutdown(sk, how);
> +
> + if ((how & SEND_SHUTDOWN) &&
> + sk_in_state(sk, SHUTDOWN_ELIGIBLE_STATE) &&
> + make_close_transition(sk))
> + chtls_close_conn(sk);
> +}
> +
> +void chtls_destroy_sock(struct sock *sk)
> +{
> + struct chtls_sock *csk;
> +
> + if (sk->sk_prot->destroy != chtls_destroy_sock)
> + return sk->sk_prot->destroy(sk);
> +
> + csk = rcu_dereference_sk_user_data(sk);
> + chtls_purge_receive_queue(sk);
> + csk->ulp_mode = ULP_MODE_NONE;
> + chtls_purge_write_queue(sk);
> + free_tls_keyid(sk);
> + kref_put(&csk->kref, chtls_sock_release);
> +
> + sk->sk_prot = &tcp_prot;
> + sk->sk_prot->destroy(sk);
> +}
> +
> +static void reset_listen_child(struct sock *child)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(child);
> + struct sk_buff *skb;
> +
> + skb = alloc_ctrl_skb(csk->txdata_skb_cache,
> + sizeof(struct cpl_abort_req));
> +
> + chtls_send_reset(child, CPL_ABORT_SEND_RST, skb);
> + sock_orphan(child);
> + INC_ORPHAN_COUNT(child);
> + if (child->sk_state == TCP_CLOSE)
> + inet_csk_destroy_sock(child);
> +}
> +
> +void chtls_disconnect_acceptq(struct sock *listen_sk)
> +{
> + struct request_sock **pprev;
> +
> + pprev = ACCEPT_QUEUE(listen_sk);
> + while (*pprev) {
> + struct request_sock *req = *pprev;
> +
> + if (req->rsk_ops == &chtls_rsk_ops) {
> + struct sock *child = req->sk;
> +
> + *pprev = req->dl_next;
> + sk_acceptq_removed(listen_sk);
> + reqsk_put(req);
> + sock_hold(child);
> + local_bh_disable();
> + bh_lock_sock(child);
> + release_tcp_port(child);
> + reset_listen_child(child);
> + bh_unlock_sock(child);
> + local_bh_enable();
> + sock_put(child);
> + } else {
> + pprev = &req->dl_next;
> + }
> + }
> +}
> +
> +static int listen_hashfn(const struct sock *sk)
> +{
> + return ((unsigned long)sk >> 10) & (LISTEN_INFO_HASH_SIZE - 1);
> +}
> +
> +static struct listen_info *listen_hash_add(struct chtls_dev *cdev,
> + struct sock *sk,
> + unsigned int stid)
> +{
> + struct listen_info *p = kmalloc(sizeof(*p), GFP_KERNEL);
> +
> + if (p) {
> + int key = listen_hashfn(sk);
> +
> + p->sk = sk;
> + p->stid = stid;
> + spin_lock(&cdev->listen_lock);
> + p->next = cdev->listen_hash_tab[key];
> + cdev->listen_hash_tab[key] = p;
> + spin_unlock(&cdev->listen_lock);
> + }
> + return p;
> +}
> +
> +static int listen_hash_find(struct chtls_dev *cdev,
> + struct sock *sk)
> +{
> + struct listen_info *p;
> + int key = listen_hashfn(sk);
> + int stid = -1;
Reverse christmas tree format?
> +
> + spin_lock(&cdev->listen_lock);
> + for (p = cdev->listen_hash_tab[key]; p; p = p->next)
> + if (p->sk == sk) {
> + stid = p->stid;
> + break;
> + }
> + spin_unlock(&cdev->listen_lock);
> + return stid;
> +}
> +
> +static int listen_hash_del(struct chtls_dev *cdev,
> + struct sock *sk)
> +{
> + int key = listen_hashfn(sk);
> + struct listen_info *p, **prev = &cdev->listen_hash_tab[key];
> + int stid = -1;
> +
> + spin_lock(&cdev->listen_lock);
> + for (p = *prev; p; prev = &p->next, p = p->next)
> + if (p->sk == sk) {
> + stid = p->stid;
> + *prev = p->next;
> + kfree(p);
> + break;
> + }
> + spin_unlock(&cdev->listen_lock);
> + return stid;
> +}
> +
> +int chtls_listen_start(struct chtls_dev *cdev, struct sock *sk)
> +{
> + struct net_device *ndev;
> + struct port_info *pi;
> + struct adapter *adap;
> + struct listen_ctx *ctx;
> + int stid;
> + int ret;
Reverse christmas tree format?
> +
> + if (sk->sk_family != PF_INET)
> + return -EAGAIN;
> +
> + rcu_read_lock();
> + ndev = chtls_ipv4_netdev(cdev, sk);
> + rcu_read_unlock();
> + if (!ndev)
> + return -EBADF;
> +
> + pi = netdev_priv(ndev);
> + adap = pi->adapter;
> + if (!(adap->flags & FULL_INIT_DONE))
> + return -EBADF;
> +
> + if (listen_hash_find(cdev, sk) >= 0) /* already have it */
> + return -EADDRINUSE;
> +
> + ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + __module_get(THIS_MODULE);
> + ctx->lsk = sk;
> + ctx->cdev = cdev;
> + ctx->state = T4_LISTEN_START_PENDING;
> +
> + if (cdev->lldi->enable_fw_ofld_conn &&
> + sk->sk_family == PF_INET)
> + stid = cxgb4_alloc_sftid(cdev->tids, sk->sk_family, ctx);
> + else
> + stid = cxgb4_alloc_stid(cdev->tids, sk->sk_family, ctx);
> +
> + if (stid < 0)
> + goto free_ctx;
> +
> + sock_hold(sk);
> + if (!listen_hash_add(cdev, sk, stid))
> + goto free_stid;
> +
> + if (cdev->lldi->enable_fw_ofld_conn) {
> + ret = cxgb4_create_server_filter(ndev, stid,
> + inet_sk(sk)->inet_rcv_saddr,
> + inet_sk(sk)->inet_sport, 0,
> + cdev->lldi->rxq_ids[0], 0, 0);
> + } else {
> + ret = cxgb4_create_server(ndev, stid,
> + inet_sk(sk)->inet_rcv_saddr,
> + inet_sk(sk)->inet_sport, 0,
> + cdev->lldi->rxq_ids[0]);
> + }
> + if (ret > 0)
> + ret = net_xmit_errno(ret);
> + if (ret)
> + goto del_hash;
> +
> + if (!ret)
Not needed I guess?
> + return 0;
> +del_hash:
> + listen_hash_del(cdev, sk);
> +free_stid:
> + cxgb4_free_stid(cdev->tids, stid, sk->sk_family);
> + sock_put(sk);
> +free_ctx:
> + kfree(ctx);
> + module_put(THIS_MODULE);
> + return -EBADF;
> +}
> +
> +void chtls_listen_stop(struct chtls_dev *cdev, struct sock *sk)
> +{
> + int stid;
> +
> + stid = listen_hash_del(cdev, sk);
> + if (stid < 0)
> + return;
> +
> + if (cdev->lldi->enable_fw_ofld_conn) {
> + cxgb4_remove_server_filter(cdev->lldi->ports[0], stid,
> + cdev->lldi->rxq_ids[0], 0);
> + } else {
> + cxgb4_remove_server(cdev->lldi->ports[0], stid,
> + cdev->lldi->rxq_ids[0], 0);
> + }
> + chtls_disconnect_acceptq(sk);
> +}
> +
> +static int chtls_pass_open_rpl(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_pass_open_rpl *rpl = cplhdr(skb) + RSS_HDR;
> + unsigned int stid = GET_TID(rpl);
> + struct listen_ctx *listen_ctx;
> +
> + listen_ctx = (struct listen_ctx *)lookup_stid(cdev->tids, stid);
> + if (!listen_ctx)
> + return 1;
> +
> + if (listen_ctx->state == T4_LISTEN_START_PENDING) {
> + listen_ctx->state = T4_LISTEN_STARTED;
> + return 1;
> + }
> +
> + if (rpl->status != CPL_ERR_NONE) {
> + pr_info("Unexpected PASS_OPEN_RPL status %u for STID %u\n",
> + rpl->status, stid);
Why do we return 0? How is this error handled?
> + } else {
> + cxgb4_free_stid(cdev->tids, stid, listen_ctx->lsk->sk_family);
> + sock_put(listen_ctx->lsk);
> + kfree(listen_ctx);
> + module_put(THIS_MODULE);
> + }
> +
> + return 0;
> +}
> +
> +static int chtls_close_listsrv_rpl(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_close_listsvr_rpl *rpl = cplhdr(skb) + RSS_HDR;
> + unsigned int stid = GET_TID(rpl);
> + void *data = lookup_stid(cdev->tids, stid);
> +
> + if (rpl->status != CPL_ERR_NONE) {
> + pr_info("Unexpected CLOSE_LISTSRV_RPL status %u for STID %u\n",
> + rpl->status, stid);
> + } else {
> + struct listen_ctx *listen_ctx = (struct listen_ctx *)data;
> +
> + cxgb4_free_stid(cdev->tids, stid, listen_ctx->lsk->sk_family);
> + sock_put(listen_ctx->lsk);
> + kfree(listen_ctx);
> + module_put(THIS_MODULE);
> + }
> +
> + return 0;
> +}
> +
> +static void conn_remove_handle(struct chtls_dev *cdev,
> + int tid)
> +{
> + spin_lock_bh(&cdev->aidr_lock);
> + idr_remove(&cdev->aidr, tid);
> + spin_unlock_bh(&cdev->aidr_lock);
> +}
> +
> +void free_atid(struct chtls_sock *csk, struct chtls_dev *cdev,
> + unsigned int atid)
> +{
> + struct tid_info *tids = cdev->tids;
> +
> + conn_remove_handle(cdev, atid);
> + cxgb4_free_atid(tids, atid);
> + sock_put(csk->sk);
> + kref_put(&csk->kref, chtls_sock_release);
> +}
> +
> +static void chtls_release_resources(struct sock *sk)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_dev *cdev = csk->cdev;
> + struct tid_info *tids;
> + unsigned int tid = csk->tid;
> +
> + if (!cdev)
> + return;
> +
> + tids = cdev->tids;
> + kfree_skb(csk->txdata_skb_cache);
> + csk->txdata_skb_cache = NULL;
> +
> + if (csk->l2t_entry) {
> + cxgb4_l2t_release(csk->l2t_entry);
> + csk->l2t_entry = NULL;
> + }
> +
> + if (sk->sk_state == TCP_SYN_SENT) {
> + free_atid(csk, cdev, tid);
> + __skb_queue_purge(&csk->ooo_queue);
> + } else {
> + cxgb4_remove_tid(tids, csk->port_id, tid, sk->sk_family);
> + sock_put(sk);
> + }
> +}
> +
> +static void cleanup_syn_rcv_conn(struct sock *child, struct sock *parent)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(child);
> + struct request_sock *req = csk->passive_reap_next;
> +
> + reqsk_queue_removed(&inet_csk(parent)->icsk_accept_queue, req);
> + chtls_reqsk_free(req);
> + csk->passive_reap_next = NULL;
> +}
> +
> +static void chtls_conn_done(struct sock *sk)
> +{
> + if (sock_flag(sk, SOCK_DEAD))
> + chtls_purge_rcv_queue(sk);
> + sk_wakeup_sleepers(sk, 0);
> + tcp_done(sk);
> +}
> +
> +static void do_abort_syn_rcv(struct sock *child, struct sock *parent)
> +{
> + /*
> + * If the server is still open we clean up the child connection,
> + * otherwise the server already did the clean up as it was purging
> + * its SYN queue and the skb was just sitting in its backlog.
> + */
> + if (likely(parent->sk_state == TCP_LISTEN)) {
> + cleanup_syn_rcv_conn(child, parent);
> + /* Without the below call to sock_orphan,
> + * we leak the socket resource with syn_flood test
> + * as inet_csk_destroy_sock will not be called
> + * in tcp_done since SOCK_DEAD flag is not set.
> + * Kernel handles this differently where new socket is
> + * created only after 3 way handshake is done.
> + */
> + sock_orphan(child);
> + percpu_counter_inc((child)->sk_prot->orphan_count);
> + chtls_release_resources(child);
> + chtls_conn_done(child);
> + } else {
> + if (csk_flag(child, CSK_RST_ABORTED)) {
> + chtls_release_resources(child);
> + chtls_conn_done(child);
> + }
> + }
> +}
> +
> +static void pass_open_abort(struct sock *child, struct sock *parent,
> + struct sk_buff *skb)
> +{
> + do_abort_syn_rcv(child, parent);
> + kfree_skb(skb);
> +}
> +
> +static void bl_pass_open_abort(struct sock *lsk, struct sk_buff *skb)
> +{
> + pass_open_abort(skb->sk, lsk, skb);
> +}
> +
> +static void chtls_pass_open_arp_failure(struct sock *sk,
> + struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_dev *cdev = csk->cdev;
> + struct sock *parent;
> + const struct request_sock *oreq;
> + void *data;
> +
> + /*
> + * If the connection is being aborted due to the parent listening
> + * socket going away there's nothing to do, the ABORT_REQ will close
> + * the connection.
> + */
> + if (csk_flag(sk, CSK_ABORT_RPL_PENDING)) {
> + kfree_skb(skb);
> + return;
> + }
> +
> + oreq = csk->passive_reap_next;
> + data = lookup_stid(cdev->tids, oreq->ts_recent);
> + parent = ((struct listen_ctx *)data)->lsk;
> +
> + bh_lock_sock(parent);
> + if (!sock_owned_by_user(parent)) {
> + pass_open_abort(sk, parent, skb);
> + } else {
> + BLOG_SKB_CB(skb)->backlog_rcv = bl_pass_open_abort;
> + __sk_add_backlog(parent, skb);
> + }
> + bh_unlock_sock(parent);
> +}
> +
> +static void chtls_accept_rpl_arp_failure(void *handle,
> + struct sk_buff *skb)
> +{
> + struct sock *sk = (struct sock *)handle;
> +
> + sock_hold(sk);
> + process_cpl_msg(chtls_pass_open_arp_failure, sk, skb);
> + sock_put(sk);
> +}
> +
> +static unsigned int chtls_select_mss(const struct chtls_sock *csk,
> + unsigned int pmtu,
> + struct cpl_pass_accept_req *req)
> +{
> + struct sock *sk = csk->sk;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct dst_entry *dst = __sk_dst_get(sk);
> + struct chtls_dev *cdev = csk->cdev;
> + unsigned int iphdrsz;
> + unsigned int tcpoptsz = 0;
> + unsigned int mtu_idx;
> + unsigned int mss = ntohs(req->tcpopt.mss);
> +
> + iphdrsz = sizeof(struct iphdr) + sizeof(struct tcphdr);
> + if (req->tcpopt.tstamp)
> + tcpoptsz += round_up(TCPOLEN_TIMESTAMP, 4);
> +
> + tp->advmss = dst_metric_advmss(dst);
> + if (USER_MSS(tp) && tp->advmss > USER_MSS(tp))
> + tp->advmss = USER_MSS(tp);
> + if (tp->advmss > pmtu - iphdrsz)
> + tp->advmss = pmtu - iphdrsz;
> + if (mss && tp->advmss > mss)
> + tp->advmss = mss;
> +
> + tp->advmss = cxgb4_best_aligned_mtu(cdev->lldi->mtus,
> + iphdrsz + tcpoptsz,
> + tp->advmss - tcpoptsz,
> + 8, &mtu_idx);
> + tp->advmss -= iphdrsz;
> +
> + inet_csk(sk)->icsk_pmtu_cookie = pmtu;
> + return mtu_idx;
> +}
> +
> +static unsigned int select_rcv_wnd(struct chtls_sock *csk)
> +{
> + struct sock *sk = csk->sk;
> + unsigned int wnd = tcp_full_space(sk);
> + unsigned int rcvwnd;
Reverse christmas tree format?
> + if (wnd < MIN_RCV_WND)
> + wnd = MIN_RCV_WND;
> +
> + rcvwnd = MAX_RCV_WND;
> +
> + csk_set_flag(csk, CSK_UPDATE_RCV_WND);
> + return min(wnd, rcvwnd);
> +}
> +
> +static void chtls_pass_accept_rpl(struct sk_buff *skb,
> + struct cpl_pass_accept_req *req,
> + unsigned int tid)
> +
> +{
> + struct sock *sk;
> + struct chtls_sock *csk;
> + const struct tcphdr *tcph;
> + struct cxgb4_lld_info *lldi;
> + struct cpl_t5_pass_accept_rpl *rpl5;
> + unsigned int len = roundup(sizeof(*rpl5), 16);
> + const struct tcp_sock *tp;
> + u64 opt0;
> + u32 opt2, hlen;
Reverse christmas tree format?
> +
> + sk = skb->sk;
> + tp = tcp_sk(sk);
> + csk = sk->sk_user_data;
> + csk->tid = tid;
> + lldi = csk->cdev->lldi;
> +
> + rpl5 = __skb_put_zero(skb, len);
> + INIT_TP_WR(rpl5, tid);
> +
> + OPCODE_TID(rpl5) = cpu_to_be32(MK_OPCODE_TID(CPL_PASS_ACCEPT_RPL,
> + csk->tid));
> + csk->mtu_idx = chtls_select_mss(csk, dst_mtu(__sk_dst_get(sk)),
> + req);
> + opt0 = TCAM_BYPASS_F |
Excess whitespace.
> + WND_SCALE_V((tp)->rx_opt.rcv_wscale) |
> + MSS_IDX_V(csk->mtu_idx) |
> + L2T_IDX_V(csk->l2t_entry->idx) |
> + NAGLE_V(!(tp->nonagle & TCP_NAGLE_OFF)) |
> + TX_CHAN_V(csk->tx_chan) |
> + SMAC_SEL_V(csk->smac_idx) |
> + DSCP_V(csk->tos >> 2) |
> + ULP_MODE_V(ULP_MODE_TLS) |
> + RCV_BUFSIZ_V(min(tp->rcv_wnd >> 10, RCV_BUFSIZ_M));
> +
> + opt2 = RX_CHANNEL_V(0) |
> + RSS_QUEUE_VALID_F | RSS_QUEUE_V(csk->rss_qid);
> +
> + if (!is_t5(lldi->adapter_type))
> + opt2 |= RX_FC_DISABLE_F;
> + if (req->tcpopt.tstamp)
> + opt2 |= TSTAMPS_EN_F;
> + if (req->tcpopt.sack)
> + opt2 |= SACK_EN_F;
> + hlen = ntohl(req->hdr_len);
> +
> + tcph = (struct tcphdr *)((u8 *)(req + 1) +
> + T6_ETH_HDR_LEN_G(hlen) + T6_IP_HDR_LEN_G(hlen));
> + if (tcph->ece && tcph->cwr)
> + opt2 |= CCTRL_ECN_V(1);
> + opt2 |= CONG_CNTRL_V(CONG_ALG_NEWRENO);
> + opt2 |= T5_ISS_F;
> + opt2 |= T5_OPT_2_VALID_F;
> + rpl5->opt0 = cpu_to_be64(opt0);
> + rpl5->opt2 = cpu_to_be32(opt2);
> + rpl5->iss = cpu_to_be32((prandom_u32() & ~7UL) - 1);
> + set_wr_txq(skb, CPL_PRIORITY_SETUP, csk->port_id);
> + t4_set_arp_err_handler(skb, sk, chtls_accept_rpl_arp_failure);
> + cxgb4_l2t_send(csk->egress_dev, skb, csk->l2t_entry);
> +}
> +
> +static void inet_inherit_port(struct inet_hashinfo *hash_info,
> + struct sock *lsk, struct sock *newsk)
> +{
> + local_bh_disable();
> + __inet_inherit_port(lsk, newsk);
> + local_bh_enable();
> +}
> +
> +static int chtls_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> +{
> + if (skb->protocol) {
> + kfree_skb(skb);
> + return 0;
> + }
> + BLOG_SKB_CB(skb)->backlog_rcv(sk, skb);
> + return 0;
> +}
> +
> +static struct sock *chtls_recv_sock(struct sock *lsk,
> + struct request_sock *oreq,
> + void *network_hdr,
> + const struct cpl_pass_accept_req *req,
> + struct chtls_dev *cdev)
> +
> +{
> + struct sock *newsk;
> + struct dst_entry *dst = NULL;
> + const struct tcphdr *tcph;
> + struct neighbour *n;
> + struct net_device *ndev;
> + struct chtls_sock *csk;
> + struct tcp_sock *tp;
> + struct inet_sock *newinet;
> + u16 port_id;
> + int step;
> + int rxq_idx;
> + const struct iphdr *iph = (const struct iphdr *)network_hdr;
Reverse christmas tree format?
> +
> + newsk = tcp_create_openreq_child(lsk, oreq, cdev->askb);
> + if (!newsk)
> + goto free_oreq;
> +
> + dst = inet_csk_route_child_sock(lsk, newsk, oreq);
> + if (!dst)
> + goto free_sk;
> +
> + tcph = (struct tcphdr *)(iph + 1);
> + n = dst_neigh_lookup(dst, &iph->saddr);
> + if (!n)
> + goto free_sk;
> +
> + ndev = n->dev;
> + if (!ndev)
> + goto free_sk;
> + port_id = cxgb4_port_idx(ndev);
> +
> + csk = chtls_sock_create(cdev);
> + if (!csk)
> + goto free_sk;
> +
> + csk->l2t_entry = cxgb4_l2t_get(cdev->lldi->l2t, n, ndev, 0);
> + if (!csk->l2t_entry)
> + goto free_csk;
> +
> + newsk->sk_user_data = csk;
> + newsk->sk_backlog_rcv = chtls_backlog_rcv;
> +
> + tp = tcp_sk(newsk);
> + newinet = inet_sk(newsk);
> +
> + newinet->inet_daddr = iph->saddr;
> + newinet->inet_rcv_saddr = iph->daddr;
> + newinet->inet_saddr = iph->daddr;
> +
> + oreq->ts_recent = PASS_OPEN_TID_G(ntohl(req->tos_stid));
> + sk_setup_caps(newsk, dst);
> + csk->sk = newsk;
> + csk->passive_reap_next = oreq;
> + csk->tx_chan = cxgb4_port_chan(ndev);
> + csk->port_id = port_id;
> + csk->egress_dev = ndev;
> + csk->tos = PASS_OPEN_TOS_G(ntohl(req->tos_stid));
> + csk->ulp_mode = ULP_MODE_TLS;
> + step = cdev->lldi->nrxq / cdev->lldi->nchan;
> + csk->rss_qid = cdev->lldi->rxq_ids[port_id * step];
> + rxq_idx = port_id * step;
> + csk->txq_idx = (rxq_idx < cdev->lldi->ntxq) ? rxq_idx :
> + port_id * step;
> + csk->sndbuf = newsk->sk_sndbuf;
> + csk->smac_idx = cxgb4_tp_smt_idx(cdev->lldi->adapter_type,
> + cxgb4_port_viid(ndev));
> + tp->rcv_wnd = select_rcv_wnd(csk);
> +
> + neigh_release(n);
> + lsk->sk_prot->hash(newsk);
> + inet_inherit_port(&tcp_hashinfo, lsk, newsk);
> + bh_unlock_sock(newsk);
Where is this locked?
> +
> + return newsk;
> +free_csk:
> + chtls_sock_release(&csk->kref);
> +free_sk:
> + dst_release(dst);
> +free_oreq:
> + chtls_reqsk_free(oreq);
> + return NULL;
> +}
> +
> +/*
> + * Populate a TID_RELEASE WR. The skb must be already propely sized.
> + */
> +static void mk_tid_release(struct sk_buff *skb,
> + unsigned int chan, unsigned int tid)
> +{
> + struct cpl_tid_release *req;
> + unsigned int len = roundup(sizeof(struct cpl_tid_release), 16);
Reverse christmas tree format?
> + req = (struct cpl_tid_release *)__skb_put(skb, len);
> + memset(req, 0, len);
> + set_wr_txq(skb, CPL_PRIORITY_SETUP, chan);
> + INIT_TP_WR_CPL(req, CPL_TID_RELEASE, tid);
> +}
> +
> +static int chtls_get_module(struct sock *sk)
> +{
> + struct inet_connection_sock *icsk = inet_csk(sk);
> +
> + if (!try_module_get(icsk->icsk_ulp_ops->owner))
> + return -1;
> +
> + return 0;
> +}
> +
> +static void chtls_pass_accept_request(struct sock *sk,
> + struct sk_buff *skb)
> +{
> + struct sock *newsk;
> + struct sk_buff *reply_skb;
> + struct cpl_t5_pass_accept_rpl *rpl;
> + struct cpl_pass_accept_req *req = cplhdr(skb) + RSS_HDR;
> + unsigned int tid = GET_TID(req);
> + struct ethhdr *eh;
> + struct iphdr *iph;
> + struct tcphdr *tcph;
> + struct request_sock *oreq = NULL;
> + void *network_hdr;
> + unsigned int len = roundup(sizeof(*rpl), 16);
> + struct chtls_dev *cdev = BLOG_SKB_CB(skb)->cdev;
Reverse christmas tree format?
> +
> + newsk = lookup_tid(cdev->tids, tid);
> + if (newsk) {
> + pr_info("tid (%d) already in use\n", tid);
> + return;
> + }
> +
> + reply_skb = alloc_skb(len, GFP_ATOMIC);
> + if (!reply_skb) {
> + cxgb4_remove_tid(cdev->tids, 0, tid, sk->sk_family);
> + kfree_skb(skb);
> + return;
> + }
> +
> + if (sk->sk_state != TCP_LISTEN)
> + goto reject;
> +
> + if (inet_csk_reqsk_queue_is_full(sk))
> + goto reject;
> +
> + if (sk_acceptq_is_full(sk))
> + goto reject;
> +
> + oreq = inet_reqsk_alloc(&chtls_rsk_ops, sk, true);
> + if (!oreq)
> + goto reject;
> +
> + oreq->rsk_rcv_wnd = 0;
> + oreq->rsk_window_clamp = 0;
> + oreq->cookie_ts = 0;
> + oreq->mss = 0;
> + oreq->ts_recent = 0;
> +
> + eh = (struct ethhdr *)(req + 1);
> + iph = (struct iphdr *)(eh + 1);
> + if (iph->version != 0x4)
> + goto reject;
> +
> + network_hdr = (void *)(eh + 1);
> + tcph = (struct tcphdr *)(iph + 1);
> +
> + tcp_rsk(oreq)->tfo_listener = false;
> + tcp_rsk(oreq)->rcv_isn = ntohl(tcph->seq);
> + chtls_set_req_port(oreq, tcph->source, tcph->dest);
> + inet_rsk(oreq)->ecn_ok = 0;
> + chtls_set_req_addr(oreq, iph->daddr, iph->saddr);
> + chtls_set_req_opt(oreq, NULL);
> + if (req->tcpopt.wsf <= 14) {
> + inet_rsk(oreq)->wscale_ok = 1;
> + inet_rsk(oreq)->snd_wscale = req->tcpopt.wsf;
> + }
> + inet_rsk(oreq)->ir_iif = sk->sk_bound_dev_if;
> +
> + newsk = chtls_recv_sock(sk, oreq, network_hdr, req, cdev);
> + if (!newsk)
> + goto reject;
> +
> + if (chtls_get_module(newsk))
> + goto reject;
> + inet_csk_reqsk_queue_added(sk);
> + reply_skb->sk = newsk;
> + chtls_install_cpl_ops(newsk);
> + cxgb4_insert_tid(cdev->tids, newsk, tid, newsk->sk_family);
> + chtls_pass_accept_rpl(reply_skb, req, tid);
> + kfree_skb(skb);
> + return;
> +
> +reject:
Is this leaking oreq from inet_reqsk_alloc() above?
> + mk_tid_release(reply_skb, 0, tid);
> + cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
> + kfree_skb(skb);
> +}
> +
> +/*
> + * Handle a CPL_PASS_ACCEPT_REQ message.
> + */
> +static int chtls_pass_accept_req(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_pass_accept_req *req = cplhdr(skb) + RSS_HDR;
> + unsigned int stid = PASS_OPEN_TID_G(ntohl(req->tos_stid));
> + unsigned int tid = GET_TID(req);
> + void *data;
> + struct listen_ctx *ctx;
> + struct sock *lsk;
Reverse christmas tree format?
> +
> + data = lookup_stid(cdev->tids, stid);
> + if (!data)
> + return 1;
> +
> + ctx = (struct listen_ctx *)data;
> + lsk = ctx->lsk;
> +
> + if (unlikely(tid >= cdev->tids->ntids)) {
> + pr_info("passive open TID %u too large\n", tid);
> + return 1;
> + }
> +
> + BLOG_SKB_CB(skb)->cdev = cdev;
> + process_cpl_msg(chtls_pass_accept_request, lsk, skb);
> + return 0;
> +}
> +
> +/*
> + * Completes some final bits of initialization for just established connections
> + * and changes their state to TCP_ESTABLISHED.
> + *
> + * snd_isn here is the ISN after the SYN, i.e., the true ISN + 1.
> + */
> +static void make_established(struct sock *sk, u32 snd_isn, unsigned int opt)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + tp->pushed_seq = snd_isn;
> + tp->write_seq = snd_isn;
> + tp->snd_nxt = snd_isn;
> + tp->snd_una = snd_isn;
> + inet_sk(sk)->inet_id = tp->write_seq ^ jiffies;
What is the purpose of xor'ing this with jiffies?
> + assign_rxopt(sk, opt);
Reverse christmas tree format?
> +
> + if (tp->rcv_wnd > (RCV_BUFSIZ_M << 10))
> + tp->rcv_wup -= tp->rcv_wnd - (RCV_BUFSIZ_M << 10);
> +
> + dst_confirm(sk->sk_dst_cache);
> +
> + smp_mb();
> + tcp_set_state(sk, TCP_ESTABLISHED);
> +}
> +
> +static void chtls_abort_conn(struct sock *sk, struct sk_buff *skb)
> +{
> + struct sk_buff *abort_skb;
> +
> + abort_skb = alloc_skb(sizeof(struct cpl_abort_req), GFP_ATOMIC);
> + if (abort_skb)
> + chtls_send_reset(sk, CPL_ABORT_SEND_RST, abort_skb);
> +}
> +
> +static struct sock *reap_list;
> +static DEFINE_SPINLOCK(reap_list_lock);
> +
> +/*
> + * Process the reap list.
> + */
> +DECLARE_TASK_FUNC(process_reap_list, task_param)
> +{
> + spin_lock_bh(&reap_list_lock);
> + while (reap_list) {
> + struct sock *sk = reap_list;
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> +
> + reap_list = csk->passive_reap_next;
> + csk->passive_reap_next = NULL;
> + spin_unlock(&reap_list_lock);
> + sock_hold(sk);
> +
> + bh_lock_sock(sk);
> + chtls_abort_conn(sk, NULL);
> + sock_orphan(sk);
> + if (sk->sk_state == TCP_CLOSE)
> + inet_csk_destroy_sock(sk);
> + bh_unlock_sock(sk);
> + sock_put(sk);
> + spin_lock(&reap_list_lock);
Could it be that you are missing a spin_unlock() here?
> + }
> + spin_unlock_bh(&reap_list_lock);
> +}
> +
> +static DECLARE_WORK(reap_task, process_reap_list);
> +
> +static void add_to_reap_list(struct sock *sk)
> +{
> + struct chtls_sock *csk = sk->sk_user_data;
> +
> + local_bh_disable();
> + bh_lock_sock(sk);
> + release_tcp_port(sk); /* release the port immediately */
> +
> + spin_lock(&reap_list_lock);
> + csk->passive_reap_next = reap_list;
> + reap_list = sk;
> + if (!csk->passive_reap_next)
> + schedule_work(&reap_task);
> + spin_unlock(&reap_list_lock);
> + bh_unlock_sock(sk);
> + local_bh_enable();
> +}
> +
> +static void add_pass_open_to_parent(struct sock *child, struct sock *lsk,
> + struct chtls_dev *cdev)
> +{
> + struct chtls_sock *csk = child->sk_user_data;
> + struct request_sock *oreq;
> +
> + if (lsk->sk_state != TCP_LISTEN)
> + return;
> +
> + oreq = csk->passive_reap_next;
> + csk->passive_reap_next = NULL;
> +
> + reqsk_queue_removed(&inet_csk(lsk)->icsk_accept_queue, oreq);
> +
> + if (sk_acceptq_is_full(lsk)) {
> + chtls_reqsk_free(oreq);
> + add_to_reap_list(child);
> + } else {
> + refcount_set(&oreq->rsk_refcnt, 1);
> + inet_csk_reqsk_queue_add(lsk, oreq, child);
> + lsk->sk_data_ready(lsk);
> + }
> +}
> +
> +static void bl_add_pass_open_to_parent(struct sock *lsk, struct sk_buff *skb)
> +{
> + struct sock *child = skb->sk;
> +
> + skb->sk = NULL;
> + add_pass_open_to_parent(child, lsk, BLOG_SKB_CB(skb)->cdev);
> + kfree_skb(skb);
> +}
> +
> +static int chtls_pass_establish(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_pass_establish *req = cplhdr(skb) + RSS_HDR;
> + struct chtls_sock *csk;
> + struct sock *lsk, *sk;
> + unsigned int hwtid = GET_TID(req);
Reverse christmas tree format?
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + if (!sk)
> + return 1;
> +
> + bh_lock_sock(sk);
> + if (unlikely(sock_owned_by_user(sk))) {
> + kfree_skb(skb);
> + } else {
> + void *data;
> + unsigned int stid;
> +
> + csk = sk->sk_user_data;
> + csk->wr_max_credits = 64;
> + csk->wr_credits = 64;
> + csk->wr_unacked = 0;
> + make_established(sk, ntohl(req->snd_isn), ntohs(req->tcp_opt));
> + stid = PASS_OPEN_TID_G(ntohl(req->tos_stid));
> + sk->sk_state_change(sk);
> + if (unlikely(sk->sk_socket))
> + sk_wake_async(sk, 0, POLL_OUT);
> +
> + data = lookup_stid(cdev->tids, stid);
> + lsk = ((struct listen_ctx *)data)->lsk;
> +
> + bh_lock_sock(lsk);
> + if (likely(!sock_owned_by_user(lsk))) {
> + kfree_skb(skb);
> + add_pass_open_to_parent(sk, lsk, cdev);
> + } else {
> + skb->sk = sk;
> + BLOG_SKB_CB(skb)->cdev = cdev;
> + BLOG_SKB_CB(skb)->backlog_rcv =
> + bl_add_pass_open_to_parent;
> + __sk_add_backlog(lsk, skb);
> + }
> + bh_unlock_sock(lsk);
> + }
> + bh_unlock_sock(sk);
> + return 0;
> +}
> +
> +/*
> + * Handle receipt of an urgent pointer.
> + */
> +static void handle_urg_ptr(struct sock *sk, u32 urg_seq)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + urg_seq--;
> + if (tp->urg_data && !after(urg_seq, tp->urg_seq))
> + return; /* duplicate pointer */
> +
> + sk_send_sigurg(sk);
> + if (tp->urg_seq == tp->copied_seq && tp->urg_data &&
> + !sock_flag(sk, SOCK_URGINLINE) &&
> + tp->copied_seq != tp->rcv_nxt) {
> + struct sk_buff *skb = skb_peek(&sk->sk_receive_queue);
> +
> + tp->copied_seq++;
> + if (skb && tp->copied_seq - ULP_SKB_CB(skb)->seq >= skb->len)
> + chtls_free_skb(sk, skb);
> + }
> +
> + tp->urg_data = TCP_URG_NOTYET;
> + tp->urg_seq = urg_seq;
> +}
> +
> +static void check_sk_callbacks(struct chtls_sock *csk)
> +{
> + struct sock *sk = csk->sk;
> +
> + if (unlikely(sk->sk_user_data &&
> + !csk_flag_nochk(csk, CSK_CALLBACKS_CHKD)))
> + csk_set_flag(csk, CSK_CALLBACKS_CHKD);
> +}
> +
> +/*
> + * Handles Rx data that arrives in a state where the socket isn't accepting
> + * new data.
> + */
> +static void handle_excess_rx(struct sock *sk, struct sk_buff *skb)
> +{
> + if (!csk_flag(sk, CSK_ABORT_SHUTDOWN))
> + chtls_abort_conn(sk, skb);
> +
> + kfree_skb(skb);
> +}
> +
> +static void chtls_recv_data(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_rx_data *hdr = cplhdr(skb) + RSS_HDR;
Reverse christmas tree format?
> +
> + if (unlikely(sk->sk_shutdown & RCV_SHUTDOWN)) {
> + handle_excess_rx(sk, skb);
> + return;
> + }
> +
> + ULP_SKB_CB(skb)->seq = ntohl(hdr->seq);
> + ULP_SKB_CB(skb)->psh = hdr->psh;
> + skb_ulp_mode(skb) = ULP_MODE_NONE;
> +
> + skb_reset_transport_header(skb);
> + __skb_pull(skb, sizeof(*hdr) + RSS_HDR);
> + if (!skb->data_len)
> + __skb_trim(skb, ntohs(hdr->len));
> +
> + if (unlikely(hdr->urg))
> + handle_urg_ptr(sk, tp->rcv_nxt + ntohs(hdr->urg));
> + if (unlikely(tp->urg_data == TCP_URG_NOTYET &&
> + tp->urg_seq - tp->rcv_nxt < skb->len))
> + tp->urg_data = TCP_URG_VALID |
> + skb->data[tp->urg_seq - tp->rcv_nxt];
> +
> + if (unlikely(hdr->dack_mode != csk->delack_mode)) {
> + csk->delack_mode = hdr->dack_mode;
> + csk->delack_seq = tp->rcv_nxt;
> + }
> +
> + tcp_hdr(skb)->fin = 0;
> + tp->rcv_nxt += skb->len;
> +
> + __skb_queue_tail(&sk->sk_receive_queue, skb);
> +
> + if (!sock_flag(sk, SOCK_DEAD)) {
> + check_sk_callbacks(csk);
> + sk->sk_data_ready(sk);
> + }
> +}
> +
> +static int chtls_rx_data(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sock *sk;
> + struct cpl_rx_data *req = cplhdr(skb) + RSS_HDR;
> + unsigned int hwtid = GET_TID(req);
Reverse christmas tree format?
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + skb_dst_set(skb, NULL);
> + process_cpl_msg(chtls_recv_data, sk, skb);
> + return 0;
> +}
> +
> +static void chtls_recv_pdu(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_hws *tlsk = &csk->tlshws;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_tls_data *hdr = cplhdr(skb);
Reverse christmas tree format?
> +
> + if (unlikely(sk->sk_shutdown & RCV_SHUTDOWN)) {
> + handle_excess_rx(sk, skb);
> + return;
> + }
> +
> + ULP_SKB_CB(skb)->seq = ntohl(hdr->seq);
> + ULP_SKB_CB(skb)->flags = 0;
> + skb_ulp_mode(skb) = ULP_MODE_TLS;
> +
> + skb_reset_transport_header(skb);
> + __skb_pull(skb, sizeof(*hdr));
> + if (!skb->data_len)
> + __skb_trim(skb,
> + CPL_TLS_DATA_LENGTH_G(ntohl(hdr->length_pkd)));
> +
> + if (unlikely(tp->urg_data == TCP_URG_NOTYET && tp->urg_seq -
> + tp->rcv_nxt < skb->len))
> + tp->urg_data = TCP_URG_VALID |
> + skb->data[tp->urg_seq - tp->rcv_nxt];
> +
> + tcp_hdr(skb)->fin = 0;
> + tlsk->pldlen = CPL_TLS_DATA_LENGTH_G(ntohl(hdr->length_pkd));
> + __skb_queue_tail(&tlsk->sk_recv_queue, skb);
> +}
> +
> +static int chtls_rx_pdu(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sock *sk;
> + struct cpl_tls_data *req = cplhdr(skb);
> + unsigned int hwtid = GET_TID(req);
Reverse christmas tree format?
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + skb_dst_set(skb, NULL);
> + process_cpl_msg(chtls_recv_pdu, sk, skb);
> + return 0;
> +}
> +
> +static void chtls_set_hdrlen(struct sk_buff *skb, unsigned int nlen)
> +{
> + struct tlsrx_cmp_hdr *tls_cmp_hdr = cplhdr(skb);
> +
> + skb->hdr_len = ntohs(tls_cmp_hdr->length);
> + tls_cmp_hdr->length = ntohs(nlen);
> +}
> +
> +static void chtls_rx_hdr(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct chtls_hws *tlsk = &csk->tlshws;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_rx_tls_cmp *cmp_cpl = cplhdr(skb);
> + struct sk_buff *skb_rec = NULL;
Reverse christmas tree format?
> +
> + ULP_SKB_CB(skb)->seq = ntohl(cmp_cpl->seq);
> + ULP_SKB_CB(skb)->flags = 0;
> +
> + skb_reset_transport_header(skb);
> + __skb_pull(skb, sizeof(*cmp_cpl));
> + if (!skb->data_len)
> + __skb_trim(skb, CPL_RX_TLS_CMP_LENGTH_G
> + (ntohl(cmp_cpl->pdulength_length)));
> +
> + tp->rcv_nxt +=
> + CPL_RX_TLS_CMP_PDULENGTH_G(ntohl(cmp_cpl->pdulength_length));
> +
> + skb_rec = __skb_dequeue(&tlsk->sk_recv_queue);
> + if (!skb_rec) {
> + ULP_SKB_CB(skb)->flags |= ULPCB_FLAG_TLS_ND;
> + __skb_queue_tail(&sk->sk_receive_queue, skb);
> + } else {
> + chtls_set_hdrlen(skb, tlsk->pldlen);
> + tlsk->pldlen = 0;
> + __skb_queue_tail(&sk->sk_receive_queue, skb);
> + __skb_queue_tail(&sk->sk_receive_queue, skb_rec);
> + }
> +
> + if (!sock_flag(sk, SOCK_DEAD)) {
> + check_sk_callbacks(csk);
> + sk->sk_data_ready(sk);
> + }
> +}
> +
> +static int chtls_rx_cmp(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sock *sk;
> + struct cpl_rx_tls_cmp *req = cplhdr(skb);
> + unsigned int hwtid = GET_TID(req);
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + skb_dst_set(skb, NULL);
> + process_cpl_msg(chtls_rx_hdr, sk, skb);
> +
> + return 0;
> +}
> +
> +static void chtls_timewait(struct sock *sk)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + tp->rcv_nxt++;
> + tp->rx_opt.ts_recent_stamp = get_seconds();
> + tp->srtt_us = 0;
> + tcp_time_wait_p(sk, TCP_TIME_WAIT, 0);
> +}
> +
> +static void chtls_peer_close(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> +
> + sk->sk_shutdown |= RCV_SHUTDOWN;
> + sock_set_flag(sk, SOCK_DONE);
> +
> + switch (sk->sk_state) {
> + case TCP_SYN_RECV:
> + case TCP_ESTABLISHED:
> + tcp_set_state(sk, TCP_CLOSE_WAIT);
> + break;
> + case TCP_FIN_WAIT1:
> + tcp_set_state(sk, TCP_CLOSING);
> + break;
> + case TCP_FIN_WAIT2:
> + chtls_release_resources(sk);
> + if (csk_flag_nochk(csk, CSK_ABORT_RPL_PENDING))
> + chtls_conn_done(sk);
> + else
> + chtls_timewait(sk);
> + break;
> + default:
> + pr_info("cpl_peer_close in bad state %d\n", sk->sk_state);
> + }
> +
> + if (!sock_flag(sk, SOCK_DEAD)) {
> + sk->sk_state_change(sk);
> + /* Do not send POLL_HUP for half duplex close. */
> +
> + if ((sk->sk_shutdown & SEND_SHUTDOWN) ||
> + sk->sk_state == TCP_CLOSE)
> + sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_HUP);
> + else
> + sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
> + }
> +}
> +
> +static void chtls_close_con_rpl(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_close_con_rpl *rpl = cplhdr(skb) + RSS_HDR;
Reverse christmas tree format?
> +
> + tp->snd_una = ntohl(rpl->snd_nxt) - 1; /* exclude FIN */
> +
> + switch (sk->sk_state) {
> + case TCP_CLOSING:
> + chtls_release_resources(sk);
> + if (csk_flag_nochk(csk, CSK_ABORT_RPL_PENDING))
> + chtls_conn_done(sk);
> + else
> + chtls_timewait(sk);
> + break;
> + case TCP_LAST_ACK:
> + chtls_release_resources(sk);
> + chtls_conn_done(sk);
> + break;
> + case TCP_FIN_WAIT1:
> + tcp_set_state(sk, TCP_FIN_WAIT2);
> + sk->sk_shutdown |= SEND_SHUTDOWN;
> + dst_confirm(sk->sk_dst_cache);
> +
> + if (!sock_flag(sk, SOCK_DEAD))
> + sk->sk_state_change(sk);
> + else if (tcp_sk(sk)->linger2 < 0 &&
> + !csk_flag_nochk(csk, CSK_ABORT_SHUTDOWN))
> + chtls_abort_conn(sk, skb);
> + break;
> + default:
> + pr_info("close_con_rpl in bad state %d\n", sk->sk_state);
> + }
> + kfree_skb(skb);
> +}
> +
> +static struct sk_buff *get_cpl_skb(struct sk_buff *skb,
> + size_t len, gfp_t gfp)
> +{
> + if (likely(!skb_is_nonlinear(skb) && !skb_cloned(skb))) {
> + WARN_ONCE(skb->len < len, "skb alloc error");
> + __skb_trim(skb, len);
> + skb_get(skb);
> + } else {
> + skb = alloc_skb(len, gfp);
> + if (skb)
> + __skb_put(skb, len);
> + }
> + return skb;
> +}
> +
> +static void set_abort_rpl_wr(struct sk_buff *skb, unsigned int tid,
> + int cmd)
> +{
> + struct cpl_abort_rpl *rpl = cplhdr(skb);
> +
> + INIT_TP_WR_CPL(rpl, CPL_ABORT_RPL, tid);
> + rpl->cmd = cmd;
> +}
> +
> +static void send_defer_abort_rpl(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sk_buff *reply_skb;
> + struct cpl_abort_req_rss *req = cplhdr(skb);
Reverse christmas tree format?
> +
> + reply_skb = alloc_skb(sizeof(struct cpl_abort_rpl),
> + GFP_KERNEL | __GFP_NOFAIL);
> + if (!reply_skb)
> + return;
> +
> + __skb_put(reply_skb, sizeof(struct cpl_abort_rpl));
> + set_abort_rpl_wr(reply_skb, GET_TID(req),
> + (req->status & CPL_ABORT_NO_RST));
> + set_wr_txq(reply_skb, CPL_PRIORITY_DATA, req->status >> 1);
> + cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
> + kfree_skb(skb);
> +}
> +
> +static void send_abort_rpl(struct sock *sk, struct sk_buff *skb,
> + struct chtls_dev *cdev, int status, int queue)
> +{
> + struct sk_buff *reply_skb;
> + struct cpl_abort_req_rss *req = cplhdr(skb);
Reverse christmas tree format?
> +
> + reply_skb = alloc_skb(sizeof(struct cpl_abort_rpl),
> + GFP_KERNEL);
> +
> + if (!reply_skb) {
> + req->status = (queue << 1);
> + send_defer_abort_rpl(cdev, skb);
> + return;
> + }
> +
> + set_abort_rpl_wr(reply_skb, GET_TID(req), status);
> + kfree_skb(skb);
> +
> + set_wr_txq(reply_skb, CPL_PRIORITY_DATA, queue);
> + if (sock_flag(sk, SOCK_INLINE)) {
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct l2t_entry *e = csk->l2t_entry;
> +
> + if (e && sk->sk_state != TCP_SYN_RECV) {
> + cxgb4_l2t_send(csk->egress_dev, reply_skb, e);
> + return;
> + }
> + }
> + cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
> +}
> +
> +/*
> + * Add an skb to the deferred skb queue for processing from process context.
> + */
> +void t4_defer_reply(struct sk_buff *skb, struct chtls_dev *cdev,
> + defer_handler_t handler)
> +{
> + DEFERRED_SKB_CB(skb)->handler = handler;
> + spin_lock_bh(&cdev->deferq.lock);
> + __skb_queue_tail(&cdev->deferq, skb);
> + if (skb_queue_len(&cdev->deferq) == 1)
> + schedule_work(&cdev->deferq_task);
> + spin_unlock_bh(&cdev->deferq.lock);
> +}
> +
> +static void chtls_send_abort_rpl(struct sock *sk, struct sk_buff *skb,
> + struct chtls_dev *cdev,
> + int status, int queue)
> +{
> + struct sk_buff *reply_skb;
> + struct cpl_abort_req_rss *req = cplhdr(skb) + RSS_HDR;
> + unsigned int tid = GET_TID(req);
Reverse christmas tree format?
> +
> + reply_skb = get_cpl_skb(skb, sizeof(struct cpl_abort_rpl), gfp_any());
> + if (!reply_skb) {
> + req->status = (queue << 1) | status;
> + t4_defer_reply(skb, cdev, send_defer_abort_rpl);
> + return;
> + }
> +
> + set_abort_rpl_wr(reply_skb, tid, status);
> + set_wr_txq(reply_skb, CPL_PRIORITY_DATA, queue);
> + if (sock_flag(sk, SOCK_INLINE)) {
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct l2t_entry *e = csk->l2t_entry;
> +
> + if (e && sk->sk_state != TCP_SYN_RECV) {
> + cxgb4_l2t_send(csk->egress_dev, reply_skb, e);
> + return;
> + }
> + }
> + cxgb4_ofld_send(cdev->lldi->ports[0], reply_skb);
> + kfree_skb(skb);
> +}
> +
> +/*
> + * This is run from a listener's backlog to abort a child connection in
> + * SYN_RCV state (i.e., one on the listener's SYN queue).
> + */
> +static void bl_abort_syn_rcv(struct sock *lsk, struct sk_buff *skb)
> +{
> + struct sock *child = skb->sk;
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(child);
> + int queue = csk->txq_idx;
> +
> + skb->sk = NULL;
> + do_abort_syn_rcv(child, lsk);
> + send_abort_rpl(child, skb, BLOG_SKB_CB(skb)->cdev,
> + CPL_ABORT_NO_RST, queue);
> +}
> +
> +static int abort_syn_rcv(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = sk->sk_user_data;
> + struct chtls_dev *cdev = csk->cdev;
> + const struct request_sock *oreq = csk->passive_reap_next;
> + struct listen_ctx *listen_ctx;
> + struct sock *psk;
> + void *ctx;
> +
> + if (!oreq)
> + return -1;
> +
> + ctx = lookup_stid(cdev->tids, oreq->ts_recent);
> + if (!ctx)
> + return -1;
> +
> + listen_ctx = (struct listen_ctx *)ctx;
> + psk = listen_ctx->lsk;
> +
> + bh_lock_sock(psk);
> + if (!sock_owned_by_user(psk)) {
> + int queue = csk->txq_idx;
> +
> + do_abort_syn_rcv(sk, psk);
> + send_abort_rpl(sk, skb, cdev, CPL_ABORT_NO_RST, queue);
> + } else {
> + skb->sk = sk;
> + BLOG_SKB_CB(skb)->backlog_rcv = bl_abort_syn_rcv;
> + __sk_add_backlog(psk, skb);
> + }
> + bh_unlock_sock(psk);
> + return 0;
> +}
> +
> +static void chtls_abort_req_rss(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = sk->sk_user_data;
> + const struct cpl_abort_req_rss *req = cplhdr(skb) + RSS_HDR;
> + int queue = csk->txq_idx;
> + int rst_status = CPL_ABORT_NO_RST;
Reverse christmas tree format?
> +
> + if (is_neg_adv(req->status)) {
> + if (sk->sk_state == TCP_SYN_RECV)
> + chtls_set_tcb_tflag(sk, 0, 0);
> +
> + kfree_skb(skb);
> + return;
> + }
> +
> + csk_reset_flag(csk, CSK_ABORT_REQ_RCVD);
> +
> + if (!csk_flag_nochk(csk, CSK_ABORT_SHUTDOWN) &&
> + !csk_flag_nochk(csk, CSK_TX_DATA_SENT)) {
> + struct tcp_sock *tp = tcp_sk(sk);
> +
> + if (send_tx_flowc_wr(sk, 0, tp->snd_nxt, tp->rcv_nxt) < 0)
> + WARN_ONCE(1, "send_tx_flowc error");
> + csk_set_flag(csk, CSK_TX_DATA_SENT);
> + }
> +
> + csk_set_flag(csk, CSK_ABORT_SHUTDOWN);
> +
> + if (!csk_flag_nochk(csk, CSK_ABORT_RPL_PENDING)) {
> + sk->sk_err = ETIMEDOUT;
> +
> + if (!sock_flag(sk, SOCK_DEAD))
> + sk->sk_error_report(sk);
> +
> + if (sk->sk_state == TCP_SYN_RECV && !abort_syn_rcv(sk, skb))
> + return;
> +
> + chtls_release_resources(sk);
> + chtls_conn_done(sk);
> + }
> +
> + chtls_send_abort_rpl(sk, skb, csk->cdev, rst_status, queue);
> +}
> +
> +static void chtls_abort_rpl_rss(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct cpl_abort_rpl_rss *rpl = cplhdr(skb) + RSS_HDR;
> + struct chtls_dev *cdev = csk->cdev;
> +
> + if (csk_flag_nochk(csk, CSK_ABORT_RPL_PENDING)) {
> + csk_reset_flag(csk, CSK_ABORT_RPL_PENDING);
> + if (!csk_flag_nochk(csk, CSK_ABORT_REQ_RCVD)) {
> + if (sk->sk_state == TCP_SYN_SENT) {
> + cxgb4_remove_tid(cdev->tids,
> + csk->port_id,
> + GET_TID(rpl),
> + sk->sk_family);
> + sock_put(sk);
> + }
> + chtls_release_resources(sk);
> + chtls_conn_done(sk);
> + }
> + }
> + kfree_skb(skb);
> +}
> +
> +static int chtls_conn_cpl(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct sock *sk;
> + struct cpl_peer_close *req = cplhdr(skb) + RSS_HDR;
> + unsigned int hwtid = GET_TID(req);
> + void (*fn)(struct sock *sk, struct sk_buff *skb);
> + u8 opcode = ((const struct rss_header *)cplhdr(skb))->opcode;
Reverse christmas tree format?
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + if (!sk)
> + goto rel_skb;
> +
> + switch (opcode) {
> + case CPL_PEER_CLOSE:
> + fn = chtls_peer_close;
> + break;
> + case CPL_CLOSE_CON_RPL:
> + fn = chtls_close_con_rpl;
> + break;
> + case CPL_ABORT_REQ_RSS:
> + fn = chtls_abort_req_rss;
> + break;
> + case CPL_ABORT_RPL_RSS:
> + fn = chtls_abort_rpl_rss;
> + break;
> + default:
> + goto rel_skb;
> + }
> +
> + process_cpl_msg(fn, sk, skb);
> + return 0;
> +
> +rel_skb:
> + kfree_skb(skb);
> + return 0;
> +}
> +
> +static struct sk_buff *dequeue_wr(struct sock *sk)
> +{
> + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
> + struct sk_buff *skb = csk->wr_skb_head;
> +
> + if (likely(skb)) {
> + /* Don't bother clearing the tail */
> + csk->wr_skb_head = WR_SKB_CB(skb)->next_wr;
> + WR_SKB_CB(skb)->next_wr = NULL;
> + }
> + return skb;
> +}
> +
> +static void chtls_rx_ack(struct sock *sk, struct sk_buff *skb)
> +{
> + struct chtls_sock *csk = sk->sk_user_data;
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct cpl_fw4_ack *hdr = cplhdr(skb) + RSS_HDR;
> + u8 credits = hdr->credits;
> + u32 snd_una = ntohl(hdr->snd_una);
Reverse christmas tree format?
> +
> + csk->wr_credits += credits;
> +
> + if (csk->wr_unacked > csk->wr_max_credits - csk->wr_credits)
> + csk->wr_unacked = csk->wr_max_credits - csk->wr_credits;
> +
> + while (credits) {
> + struct sk_buff *pskb = csk->wr_skb_head;
> +
> + if (unlikely(!pskb)) {
> + if (csk->wr_nondata)
> + csk->wr_nondata -= credits;
> + break;
> + }
> + if (unlikely(credits < pskb->csum)) {
> + pskb->csum -= credits;
> + break;
> + }
> + dequeue_wr(sk);
> + credits -= pskb->csum;
> + kfree_skb(pskb);
> + }
> + if (hdr->seq_vld & CPL_FW4_ACK_FLAGS_SEQVAL) {
> + if (unlikely(before(snd_una, tp->snd_una))) {
> + kfree_skb(skb);
> + return;
> + }
> +
> + if (tp->snd_una != snd_una) {
> + tp->snd_una = snd_una;
> + dst_confirm(sk->sk_dst_cache);
> + tp->rcv_tstamp = tcp_time_stamp(tp);
> + if (tp->snd_una == tp->snd_nxt &&
> + !csk_flag_nochk(csk, CSK_TX_FAILOVER))
> + csk_reset_flag(csk, CSK_TX_WAIT_IDLE);
> + }
> + }
> +
> + if (hdr->seq_vld & CPL_FW4_ACK_FLAGS_CH) {
> + unsigned int fclen16 = roundup(failover_flowc_wr_len, 16);
> +
> + csk->wr_credits -= fclen16;
> + csk_reset_flag(csk, CSK_TX_WAIT_IDLE);
> + csk_reset_flag(csk, CSK_TX_FAILOVER);
> + }
> + if (skb_queue_len(&csk->txq) && chtls_push_frames(csk, 0))
> + sk->sk_write_space(sk);
> + kfree_skb(skb);
I guess you actually always want to kfree_skb(skb) here, right?
> +}
> +
> +static int chtls_wr_ack(struct chtls_dev *cdev, struct sk_buff *skb)
> +{
> + struct cpl_fw4_ack *rpl = cplhdr(skb) + RSS_HDR;
> + unsigned int hwtid = GET_TID(rpl);
> + struct sock *sk;
> +
> + sk = lookup_tid(cdev->tids, hwtid);
> + process_cpl_msg(chtls_rx_ack, sk, skb);
> +
> + return 0;
> +}
> +
> +chtls_handler_func chtls_handlers[NUM_CPL_CMDS] = {
> + [CPL_PASS_OPEN_RPL] = chtls_pass_open_rpl,
> + [CPL_CLOSE_LISTSRV_RPL] = chtls_close_listsrv_rpl,
> + [CPL_PASS_ACCEPT_REQ] = chtls_pass_accept_req,
> + [CPL_PASS_ESTABLISH] = chtls_pass_establish,
> + [CPL_RX_DATA] = chtls_rx_data,
> + [CPL_TLS_DATA] = chtls_rx_pdu,
> + [CPL_RX_TLS_CMP] = chtls_rx_cmp,
> + [CPL_PEER_CLOSE] = chtls_conn_cpl,
> + [CPL_CLOSE_CON_RPL] = chtls_conn_cpl,
> + [CPL_ABORT_REQ_RSS] = chtls_conn_cpl,
> + [CPL_ABORT_RPL_RSS] = chtls_conn_cpl,
> + [CPL_FW4_ACK] = chtls_wr_ack,
> +};
--
Stefano
^ permalink raw reply
* Re: [crypto 4/8] chtls: CPL handler definition
From: Hannes Frederic Sowa @ 2017-12-05 15:49 UTC (permalink / raw)
To: Atul Gupta, herbert, linux-crypto
Cc: netdev, davem, davejwatson, ganeshgr, harsh
In-Reply-To: <1512474000-6681-1-git-send-email-atul.gupta@chelsio.com>
Hello,
On Tue, Dec 5, 2017, at 12:40, Atul Gupta wrote:
> CPL handlers for TLS session, record transmit and receive
This does very much looks like full TCP offload with TLS on top? It
would be nice if you could give a few more details in the patch
descriptions.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH 4/7 v2] net: ethernet: i825xx: Fix platform_get_irq's error checking
From: David Miller @ 2017-12-05 15:49 UTC (permalink / raw)
To: arvind.yadav.cs
Cc: wg, mkl, michal.simek, opendmb, f.fainelli, linux-kernel,
linux-arm-kernel, netdev
In-Reply-To: <39363858-995c-eea4-070e-b21a7ac49bec@gmail.com>
From: Arvind Yadav <arvind.yadav.cs@gmail.com>
Date: Tue, 5 Dec 2017 11:04:55 +0530
> Hi David,
>
>
> On Monday 04 December 2017 11:55 PM, David Miller wrote:
>> From: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> Date: Mon, 4 Dec 2017 23:18:20 +0530
>>
>>> @@ -120,9 +120,10 @@ static int sni_82596_probe(struct platform_device
>>> *dev)
>>> netdevice->dev_addr[5] = readb(eth_addr + 0x06);
>>> iounmap(eth_addr);
>>> - if (!netdevice->irq) {
>>> + if (netdevice->irq <= 0) {
>>> printk(KERN_ERR "%s: IRQ not found for i82596 at 0x%lx\n",
>>> __FILE__, netdevice->base_addr);
>>> + retval = netdevice->irq ? netdevice->irq : -ENODEV;
>>> goto probe_failed;
>>> }
>> Ok, thinking about this some more...
>>
>> It is impossible to use platform_get_irq() without every single call
>> site having this funny:
>>
>> ret = val ? val : -ENODEV;
>>
>> sequence.
>>
>> This is unnecessary duplication and it is also error prone, so I
>> really think this logic belongs in platform_get_irq() itself. It can
>> convert '0' to -ENODEV and that way we need no special logic in the
>> callers at all.
> platform_get_irq() will return 0 only for sparc, If sparc initialize
> platform
> data irq[PROMINTR_MAX] as zero. Otherwise platform_get_irq() will
> never return
> 0. It will return either IRQ number or error (as negative number). But
> I am getting
> review comment by reviewer/maintainer in other subsystem to add check
> for
> zero. So I have done same changes here. Please correct me if i am
> wrong.
If you make the change that I suggest, you instead can check for
'-ENODEV' to mean no IRQ.
^ permalink raw reply
* Re: [PATCH v2 0/6] bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type
From: Daniel Borkmann @ 2017-12-05 15:53 UTC (permalink / raw)
To: Hendrik Brueckner, Arnaldo Carvalho de Melo, Alexei Starovoitov,
Martin Schwidefsky, Will Deacon, Ingo Molnar
Cc: Arnd Bergmann, Peter Zijlstra, David S. Miller, linux-kernel,
linux-s390, netdev
In-Reply-To: <1512381409-19775-1-git-send-email-brueckner@linux.vnet.ibm.com>
On 12/04/2017 10:56 AM, Hendrik Brueckner wrote:
> Perf tool bpf selftests revealed a broken uapi for s390 and arm64.
> With the BPF_PROG_TYPE_PERF_EVENT program type the bpf_perf_event
> structure exports the pt_regs structure for all architectures.
>
> This fails for s390 and arm64 because pt_regs are not part of the
> user api and kept in-kernel only. To mitigate the broken uapi,
> introduce a wrapper that exports pt_regs in an asm-generic way.
> For arm64, export the exising user_pt_regs structure. For s390,
> introduce a user_pt_regs structure that exports the beginning of
> pt_regs.
>
> Note that user_pt_regs must export from the beginning of pt_regs
> as BPF_PROG_TYPE_PERF_EVENT program type is not the only type for
> running BPF programs.
>
> Some more background:
> For the bpf_perf_event, there is a uapi definition that is
> passed to the BPF program. For other "probe" points like
> trace points, kprobes, and uprobes, there is no uapi and the
> BPF program is always passed pt_regs (which is OK as the BPF
> program runs in the kernel context). The perf tool can attach
> BPF programs to all of these "probe" points and, optionally,
> can create a BPF prologue to access particular arguments
> (passed as registers). For this, it uses DWARF/CFI
> information to obtain the register and calls a perf-arch
> backend function, regs_query_register_offset(). This function
> returns the index into (user_)pt_regs for a particular
> register. Then, perf creates a BPF prologue that accesses
> this register based on the passed stucture from the "probe"
> point.
>
> Part of this series, are also updates to the testing and bpf selftest
> to deal with asm-specifics. To complete the bpf support in perf, the
> the regs_query_register_offset function is added for s390 to support
> BPF prologue creation.
>
> Changelog v1 -> v2:
> - Correct kbuild test bot issues by including
> asm-generic/bpf_perf_event.h for archictectures that do not have
> their own asm version.
> - Added patch to clean-up whitespace and coding style issues in s390
> asm/ptrace.h (#4/6) as suggested by Alexei.
>
>
> Hendrik Brueckner (6):
> bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type
> s390/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program
> type
> arm64/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program
> type
> s390/uapi: correct whitespace & coding style in asm/ptrace.h
> selftests/bpf: sync kernel headers and introduce arch support in
> Makefile
> perf s390: add regs_query_register_offset()
Series looks good to me, thanks for working on this Hendrik! If nobody
hollers, I would take the fixes via bpf tree later tonight.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next] net: phy: meson-gxl: cleanup by defining the control registers
From: Jerome Brunet @ 2017-12-05 15:57 UTC (permalink / raw)
To: Yixun Lan, Andrew Lunn, Florian Fainelli, Neil Armstrong
Cc: netdev, linux-kernel, linux-amlogic
In-Reply-To: <970919cd-cf0f-1782-22b8-46746a92fc43@amlogic.com>
On Tue, 2017-12-05 at 23:23 +0800, Yixun Lan wrote:
> > +static inline int meson_gxl_write_reg(struct phy_device *phydev,
> > + unsigned int bank, unsigned int reg,
> > + uint16_t value)
> > +{
> > + int ret;
> > +
> > + /* Enable Analog and DSP register Bank access by
> > + * toggling TSTCNTL_TEST_MODE bit in the TSTCNTL register
> > + */
> > + ret = phy_write(phydev, TSTCNTL, 0);
> > + if (ret)
> > + goto out;
> > + ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
> > + if (ret)
> > + goto out;
> > + ret = phy_write(phydev, TSTCNTL, 0);
> > + if (ret)
> > + goto out;
> > + ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
> > + if (ret)
> > + goto out;
> > +
>
> how about just do the above enable procedure once?
> from the datasheet, the access won't be disabled if don't reset, or
> write to register TSTCNTL with TEST_MODE=0
It just seems more clean. This is way, the write function is self sufficient and
we can use it w/o making assumption about what happened out of it.
^ permalink raw reply
* Re: [PATCH net-next V3] tun: add eBPF based queue selection method
From: Willem de Bruijn @ 2017-12-05 16:13 UTC (permalink / raw)
To: Jason Wang
Cc: Network Development, LKML, Michael S. Tsirkin, Tom Herbert,
aconole, wexu
In-Reply-To: <d53e8ed1-33a7-a29a-c53a-0be9e1a97901@redhat.com>
On Tue, Dec 5, 2017 at 2:29 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2017年12月05日 08:16, Willem de Bruijn wrote:
>>
>> On Mon, Dec 4, 2017 at 4:31 AM, Jason Wang <jasowang@redhat.com> wrote:
>>>
>>> This patch introduces an eBPF based queue selection method. With this,
>>> the policy could be offloaded to userspace completely through a new
>>> ioctl TUNSETSTEERINGEBPF.
>>>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>> +static u16 tun_ebpf_select_queue(struct tun_struct *tun, struct sk_buff
>>> *skb)
>>> +{
>>> + struct tun_steering_prog *prog;
>>> + u16 ret = 0;
>>> +
>>> + prog = rcu_dereference(tun->steering_prog);
>>> + if (prog)
>>> + ret = bpf_prog_run_clear_cb(prog->prog, skb);
>>
>> This dereferences tun->steering_prog for a second time. It is safe
>> in this load balancing case to assign a few extra packets to queue 0.
>> But the issue can also be avoided by replacing the function with a
>> direct call in tun_net_xmit:
>>
>> struct tun_steering_prog *s = rcu_dereference(tun->steering_prog);
>> if (s)
>> ret = bpf_prog_run_clear_cb(s->prog, skb) %
>> tun->numqueues;
>
>
> Right.
>
>
>>
>>> /* Net device start xmit */
>>> -static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device
>>> *dev)
>>> +static void tun_automq_xmit(struct tun_struct *tun, struct sk_buff *skb)
>>> {
>>> - struct tun_struct *tun = netdev_priv(dev);
>>> - int txq = skb->queue_mapping;
>>> - struct tun_file *tfile;
>>> - u32 numqueues = 0;
>>> -
>>> - rcu_read_lock();
>>> - tfile = rcu_dereference(tun->tfiles[txq]);
>>> - numqueues = READ_ONCE(tun->numqueues);
>>> -
>>> - /* Drop packet if interface is not attached */
>>> - if (txq >= numqueues)
>>> - goto drop;
>>> -
>>> #ifdef CONFIG_RPS
>>> - if (numqueues == 1 && static_key_false(&rps_needed)) {
>>> + if (tun->numqueues == 1 && static_key_false(&rps_needed)) {
>>> /* Select queue was not called for the skbuff, so we
>>> extract the
>>> * RPS hash and save it into the flow_table here.
>>> */
>>> @@ -969,6 +986,26 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb,
>>> struct net_device *dev)
>>> }
>>> }
>>> #endif
>>> +}
>>> +
>>> +/* Net device start xmit */
>>> +static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device
>>> *dev)
>>> +{
>>> + struct tun_struct *tun = netdev_priv(dev);
>>> + int txq = skb->queue_mapping;
>>> + struct tun_file *tfile;
>>> + u32 numqueues = 0;
>>> +
>>> + rcu_read_lock();
>>> + tfile = rcu_dereference(tun->tfiles[txq]);
>>> + numqueues = READ_ONCE(tun->numqueues);
>>
>> Now tun->numqueues is read twice, reversing commit fa35864e0bb7
>> ("tuntap: Fix for a race in accessing numqueues"). I don't see anything
>> left that would cause a divide by zero after the relevant code was
>> converted from divide to multiple and subsequently even removed.
>>
>> But if it's safe to read multiple times, might as well remove the
>> READ_ONCE.
>
>
> Good point, but READ_ONCE() is not something new, we'd better change this in
> another patch.
Sounds good. It's a simple follow-up. I can also send that.
>
>
>>
>>> @@ -1551,7 +1588,7 @@ static ssize_t tun_get_user(struct tun_struct *tun,
>>> struct tun_file *tfile,
>>> int copylen;
>>> bool zerocopy = false;
>>> int err;
>>> - u32 rxhash;
>>> + u32 rxhash = 0;
>>> int skb_xdp = 1;
>>> bool frags = tun_napi_frags_enabled(tun);
>>>
>>> @@ -1739,7 +1776,10 @@ static ssize_t tun_get_user(struct tun_struct
>>> *tun, struct tun_file *tfile,
>>> rcu_read_unlock();
>>> }
>>>
>>> - rxhash = __skb_get_hash_symmetric(skb);
>>> + rcu_read_lock();
>>> + if (!rcu_dereference(tun->steering_prog))
>>> + rxhash = __skb_get_hash_symmetric(skb);
>>> + rcu_read_unlock();
>>>
>>> if (frags) {
>>> /* Exercise flow dissector code path. */
>>> @@ -1783,7 +1823,9 @@ static ssize_t tun_get_user(struct tun_struct *tun,
>>> struct tun_file *tfile,
>>> u64_stats_update_end(&stats->syncp);
>>> put_cpu_ptr(stats);
>>>
>>> - tun_flow_update(tun, rxhash, tfile);
>>> + if (rxhash)
>>> + tun_flow_update(tun, rxhash, tfile);
>>> +
>>
>> Nit: zero is a valid hash? In which case, an int64_t initialized to -1 is
>> the
>> safer check.
>
>
> Looks not? E.g looking at __flow_hash_from_keys() it did:
>
> static inline u32 __flow_hash_from_keys(struct flow_keys *keys, u32 keyval)
> {
> u32 hash;
>
> __flow_hash_consistentify(keys);
>
> hash = __flow_hash_words(flow_keys_hash_start(keys),
> flow_keys_hash_length(keys), keyval);
> if (!hash)
> hash = 1;
>
> return hash;
> }
>
> Thanks
Interesting, thanks. In that case
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net-next] rtnetlink: fix rtnl_link msghandler rcu annotations
From: David Miller @ 2017-12-05 16:15 UTC (permalink / raw)
To: fw; +Cc: netdev
In-Reply-To: <20171204214230.15474-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Mon, 4 Dec 2017 22:42:30 +0100
> Incorrect/missing annotations caused a few sparse warnings:
>
> rtnetlink.c:155:15: incompatible types .. (different address spaces)
> rtnetlink.c:157:23: incompatible types .. (different address spaces)
> rtnetlink.c:185:15: incompatible types .. (different address spaces)
> rtnetlink.c:285:15: incompatible types .. (different address spaces)
> rtnetlink.c:317:9: incompatible types .. (different address spaces)
> rtnetlink.c:3054:23: incompatible types .. (different address spaces)
>
> no change in generated code.
>
> Fixes: addf9b90de22f7 ("net: rtnetlink: use rcu to free rtnl message handlers")
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied, thanks for fixing this so quickly.
^ permalink raw reply
* Re: [PATCH net-next 00/11] SFP/phylink updates
From: David Miller @ 2017-12-05 16:16 UTC (permalink / raw)
To: linux; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>
From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Fri, 1 Dec 2017 10:23:07 +0000
> This series, which follows on from the fixes posted earlier, improves
> the phylink/sfp support. Changes included here are:
...
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH v3] net: sched: crash on blocks with goto chain action
From: David Miller @ 2017-12-05 16:22 UTC (permalink / raw)
To: code; +Cc: netdev, xiyou.wangcong, jiri
In-Reply-To: <20171129192014.20026-1-code@rkapl.cz>
From: Roman Kapl <code@rkapl.cz>
Date: Wed, 29 Nov 2017 20:20:14 +0100
> tcf_block_put_ext has assumed that all filters (and thus their goto
> actions) are destroyed in RCU callback and so can not race with our
> list iteration. However, that is not true during netns cleanup (see
> tcf_exts_get_net comment). The assumption was broken by the patch series
> c7e460ce5572..623859ae06b8 ("Merge branch 'net-sched-race-fix'").
>
> Prevent the user after free by holding all chains (except 0, that one is
> already held) as it was done before
> 822e86d997e4 ("net_sched: remove tcf_block_put_deferred()").
>
> To reproduce, run the following in a netns and then delete the ns:
> ip link add dtest type dummy
> tc qdisc add dev dtest ingress
> tc filter add dev dtest chain 1 parent ffff: handle 1 prio 1 flower action goto chain 2
>
> Fixes: 623859ae06b8 ("Merge branch 'net-sched-race-fix'")
> Signed-off-by: Roman Kapl <code@rkapl.cz>
This doesn't apply cleanly to 'net'.
^ permalink raw reply
* Re: [Intel-wired-lan] [next-queue 03/10] ixgbe: add ipsec engine start and stop routines
From: Alexander Duyck @ 2017-12-05 16:22 UTC (permalink / raw)
To: Shannon Nelson
Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Sowmini Varadhan,
Netdev
In-Reply-To: <1512452116-14795-4-git-send-email-shannon.nelson@oracle.com>
On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> Add in the code for running and stopping the hardware ipsec
> encryption/decryption engine. It is good to keep the engine
> off when not in use in order to save on the power draw.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 140 +++++++++++++++++++++++++
> 1 file changed, 140 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> index 14dd011..38a1a16 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -148,10 +148,150 @@ void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter)
> }
>
> /**
> + * ixgbe_ipsec_stop_data
> + * @adapter: board private structure
> + **/
> +static void ixgbe_ipsec_stop_data(struct ixgbe_adapter *adapter)
> +{
> + struct ixgbe_hw *hw = &adapter->hw;
> + bool link = adapter->link_up;
> + u32 t_rdy, r_rdy;
> + u32 reg;
> +
> + /* halt data paths */
> + reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
> + reg |= IXGBE_SECTXCTRL_TX_DIS;
> + IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, reg);
> +
> + reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
> + reg |= IXGBE_SECRXCTRL_RX_DIS;
> + IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, reg);
> +
> + IXGBE_WRITE_FLUSH(hw);
> +
> + /* If the tx fifo doesn't have link, but still has data,
> + * we can't clear the tx sec block. Set the MAC loopback
> + * before block clear
> + */
> + if (!link) {
> + reg = IXGBE_READ_REG(hw, IXGBE_MACC);
> + reg |= IXGBE_MACC_FLU;
> + IXGBE_WRITE_REG(hw, IXGBE_MACC, reg);
> +
> + reg = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> + reg |= IXGBE_HLREG0_LPBK;
> + IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
> +
> + IXGBE_WRITE_FLUSH(hw);
> + mdelay(3);
> + }
> +
> + /* wait for the paths to empty */
> + do {
> + mdelay(10);
> + t_rdy = IXGBE_READ_REG(hw, IXGBE_SECTXSTAT) &
> + IXGBE_SECTXSTAT_SECTX_RDY;
> + r_rdy = IXGBE_READ_REG(hw, IXGBE_SECRXSTAT) &
> + IXGBE_SECRXSTAT_SECRX_RDY;
> + } while (!t_rdy && !r_rdy);
This piece seems buggy to me. There should be some sort of limit on
how long you are willing to delay. Otherwise a surprise remove can
cause this to spin forever when the register reads return all 1's.
> +
> + /* undo loopback if we played with it earlier */
> + if (!link) {
> + reg = IXGBE_READ_REG(hw, IXGBE_MACC);
> + reg &= ~IXGBE_MACC_FLU;
> + IXGBE_WRITE_REG(hw, IXGBE_MACC, reg);
> +
> + reg = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> + reg &= ~IXGBE_HLREG0_LPBK;
> + IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
> +
> + IXGBE_WRITE_FLUSH(hw);
> + }
> +}
> +
> +/**
> + * ixgbe_ipsec_stop_engine
> + * @adapter: board private structure
> + **/
> +static void ixgbe_ipsec_stop_engine(struct ixgbe_adapter *adapter)
> +{
> + struct ixgbe_hw *hw = &adapter->hw;
> + u32 reg;
> +
> + ixgbe_ipsec_stop_data(adapter);
> +
> + /* disable Rx and Tx SA lookup */
> + IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, 0);
> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, 0);
> +
> + /* disable the Rx and Tx engines and full packet store-n-forward */
> + reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
> + reg |= IXGBE_SECTXCTRL_SECTX_DIS;
> + reg &= ~IXGBE_SECTXCTRL_STORE_FORWARD;
> + IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, reg);
> +
> + reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
> + reg |= IXGBE_SECRXCTRL_SECRX_DIS;
> + IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, reg);
> +
> + /* restore the "tx security buffer almost full threshold" to 0x250 */
> + IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, 0x250);
> +
> + /* Set minimum IFG between packets back to the default 0x1 */
> + reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
> + reg = (reg & 0xfffffff0) | 0x1;
> + IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
> +
> + /* final set for normal (no ipsec offload) processing */
> + IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, IXGBE_SECTXCTRL_SECTX_DIS);
> + IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, IXGBE_SECRXCTRL_SECRX_DIS);
> +
> + IXGBE_WRITE_FLUSH(hw);
> +}
> +
> +/**
> + * ixgbe_ipsec_start_engine
> + * @adapter: board private structure
> + *
> + * NOTE: this increases power consumption whether being used or not
> + **/
> +static void ixgbe_ipsec_start_engine(struct ixgbe_adapter *adapter)
> +{
> + struct ixgbe_hw *hw = &adapter->hw;
> + u32 reg;
> +
> + ixgbe_ipsec_stop_data(adapter);
> +
> + /* Set minimum IFG between packets to 3 */
> + reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
> + reg = (reg & 0xfffffff0) | 0x3;
> + IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
> +
> + /* Set "tx security buffer almost full threshold" to 0x15 so that the
> + * almost full indication is generated only after buffer contains at
> + * least an entire jumbo packet.
> + */
> + reg = IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF);
> + reg = (reg & 0xfffffc00) | 0x15;
> + IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, reg);
> +
> + /* restart the data paths by clearing the DISABLE bits */
> + IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, 0);
> + IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, IXGBE_SECTXCTRL_STORE_FORWARD);
> +
> + /* enable Rx and Tx SA lookup */
> + IXGBE_WRITE_REG(hw, IXGBE_IPSTXIDX, IXGBE_RXTXIDX_IPS_EN);
> + IXGBE_WRITE_REG(hw, IXGBE_IPSRXIDX, IXGBE_RXTXIDX_IPS_EN);
> +
> + IXGBE_WRITE_FLUSH(hw);
> +}
> +
It would probably make sense to add a data member to the hardware
structure that tracks if you have IPsec enabled or not. Then you don't
have to track the IPS_EN bits in patch 2 like you currently are and
could instead either not do IPsec SA updates if IPsec is not enabled,
or use the enable value to determine what you write for IPS_EN instead
of having to read registers.
> +/**
> * ixgbe_init_ipsec_offload - initialize security registers for IPSec operation
> * @adapter: board private structure
> **/
> void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter)
> {
> ixgbe_ipsec_clear_hw_tables(adapter);
> + ixgbe_ipsec_stop_engine(adapter);
> }
> --
> 2.7.4
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [Intel-wired-lan] [next-queue 02/10] ixgbe: add ipsec register access routines
From: Rustad, Mark D @ 2017-12-05 16:24 UTC (permalink / raw)
To: Shannon Nelson
Cc: intel-wired-lan, Kirsher, Jeffrey T, steffen.klassert@secunet.com,
sowmini.varadhan@oracle.com, netdev@vger.kernel.org
In-Reply-To: <1512452116-14795-3-git-send-email-shannon.nelson@oracle.com>
> On Dec 4, 2017, at 9:35 PM, Shannon Nelson <shannon.nelson@oracle.com> wrote:
>
> Add a few routines to make access to the ipsec registers just a little
> easier, and throw in the beginnings of an initialization.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
> drivers/net/ethernet/intel/ixgbe/Makefile | 1 +
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 6 +
> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 157 +++++++++++++++++++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h | 50 ++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 1 +
<snip>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> new file mode 100644
> index 0000000..14dd011
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -0,0 +1,157 @@
> +/*******************************************************************************
> + *
> + * Intel 10 Gigabit PCI Express Linux driver
> + * Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
I don't think that it really makes sense to assert "All rights reserved" in something that is GPL. It makes it seem like something is being asserted that is counter to the GPL.
<snip>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
> new file mode 100644
> index 0000000..017b13f
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
> @@ -0,0 +1,50 @@
> +/*******************************************************************************
> +
> + Intel 10 Gigabit PCI Express Linux driver
> + Copyright(c) 2017 Oracle and/or its affiliates. All rights reserved.
Likewise here.
--
Mark Rustad, Networking Division, Intel Corporation
^ permalink raw reply
* Re: Linux 4.14 - regression: broken tun/tap / bridge network with virtio - bisected
From: Andreas Hartmann @ 2017-12-05 16:23 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, john.fastabend, Michal Kubecek
In-Reply-To: <73b7a7b0-4264-2bd0-9e65-69841377f09f@redhat.com>
On 12/05/2017 at 04:50 AM Jason Wang wrote:
>
>
> On 2017年12月05日 00:28, Andreas Hartmann wrote:
>> On 12/03/2017 at 12:35 PM Andreas Hartmann wrote:
>>> On 12/01/2017 at 11:11 AM Andreas Hartmann wrote:
>>>> Hello!
>>>>
>>>> I hopefully could get rid of both of my problems (hanging network w/
>>>> virtio) and endless hanging qemu-process on VM shutdown by upgrading
>>>> qemu from 2.6.2 to 2.10.1. I hope it will persist.
>>> It didn't persist. 10h later - same problems happened again. It's just
>>> much harder to trigger the problems.
>>>
>>> I'm now trying it with
>>>
>>> CONFIG_RCU_NOCB_CPU=y and
>>> rcu_nocbs=0-15
>>>
>>> Since then, I didn't see any problem any more. But this doesn't mean
>>> anything until now ... .
>> Didn't work ether. Disabling vhost_net's zcopy hadn't any effect, too.
>>
>> => It's just finally broken since
>>
>> 2ddf71e23cc246e95af72a6deed67b4a50a7b81c
>> net: add notifier hooks for devmap bpf map
>
> Hi:
>
> Did you use XDP devmap in host? If not, please double check it was the
> first bad commit since the patch should only work when XDP/devmap is
> used on host.
How do I know if XDP/devmap is enabled / used? Could you please give
some hint?
Thanks,
Andreas
^ permalink raw reply
* Re: [PATCH v2] net: macb: change GFP_KERNEL to GFP_ATOMIC
From: David Miller @ 2017-12-05 16:27 UTC (permalink / raw)
To: julia.lawall; +Cc: rafalo, nicolas.ferre, netdev, linux-kernel, kbuild-all
In-Reply-To: <alpine.DEB.2.20.1712020800010.2168@hadrien>
From: Julia Lawall <julia.lawall@lip6.fr>
Date: Sat, 2 Dec 2017 08:01:21 +0100 (CET)
> Function gem_add_flow_filter called on line 2958 inside lock on line 2949
> but uses GFP_KERNEL
>
> Generated by: scripts/coccinelle/locks/call_kern.cocci
>
> Fixes: ae8223de3df5 ("net: macb: Added support for RX filtering")
> CC: Rafal Ozieblo <rafalo@cadence.com>
> Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH net] nfp: fix port stats for mac representors
From: David Miller @ 2017-12-05 16:27 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers, pieter.jansenvanvuuren
In-Reply-To: <20171202053723.32660-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 1 Dec 2017 21:37:23 -0800
> From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
>
> Previously we swapped the tx_packets, tx_bytes and tx_dropped counters
> with rx_packets, rx_bytes and rx_dropped counters, respectively. This
> behaviour is correct and expected for VF representors but it should not
> be swapped for physical port mac representors.
>
> Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] Revert "tcp: must block bh in __inet_twsk_hashdance()"
From: David Miller @ 2017-12-05 16:27 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1512170924.19682.50.camel@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 01 Dec 2017 15:28:44 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> We had to disable BH _before_ calling __inet_twsk_hashdance() in commit
> cfac7f836a71 ("tcp/dccp: block bh before arming time_wait timer").
>
> This means we can revert 614bdd4d6e61 ("tcp: must block bh in
> __inet_twsk_hashdance()").
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] Revert "net: core: maybe return -EEXIST in __dev_alloc_name"
From: David Miller @ 2017-12-05 16:27 UTC (permalink / raw)
To: johannes; +Cc: netdev, j, linux, johannes.berg
In-Reply-To: <20171202074155.29146-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Sat, 2 Dec 2017 08:41:55 +0100
> From: Johannes Berg <johannes.berg@intel.com>
>
> This reverts commit d6f295e9def0; some userspace (in the case
> we noticed it's wpa_supplicant), is relying on the current
> error code to determine that a fixed name interface already
> exists.
>
> Reported-by: Jouni Malinen <j@w1.fi>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] nfp: fix port stats for mac representors
From: David Miller @ 2017-12-05 16:30 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers, pieter.jansenvanvuuren
In-Reply-To: <CAJpBn1x_-oO4_fesjtr5Le7Ooma7ptLcQNqC6KP0Y-2bFZ2rdA@mail.gmail.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Sat, 2 Dec 2017 02:03:31 -0800
> On Fri, Dec 1, 2017 at 9:37 PM, Jakub Kicinski
> <jakub.kicinski@netronome.com> wrote:
>> From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
>>
>> Previously we swapped the tx_packets, tx_bytes and tx_dropped counters
>> with rx_packets, rx_bytes and rx_dropped counters, respectively. This
>> behaviour is correct and expected for VF representors but it should not
>> be swapped for physical port mac representors.
>
> Ah, I forgot to point the finger. Should I repost?
>
> Fixes: eadfa4c3be99 ("nfp: add stats and xmit helpers for representors")
I added this to the commit message, thanks.
^ permalink raw reply
* Re: [PATCH net 1/2] netlink: add NLA_U8_BUGGY attribute type
From: David Miller @ 2017-12-05 16:31 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, netdev, j, dsahern, johannes.berg
In-Reply-To: <20171202202332.10205-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Sat, 2 Dec 2017 21:23:31 +0100
> From: Johannes Berg <johannes.berg@intel.com>
>
> This netlink type is used only for backwards compatibility
> with broken userspace that used the wrong size for a given
> u8 attribute, which is now rejected. It would've been wrong
> before already, since on big endian the wrong value (always
> zero) would be used by the kernel, but we can't break the
> existing deployed userspace - hostapd for example now fails
> to initialize entirely.
>
> We could try to fix up the big endian problem here, but we
> don't know *how* userspace misbehaved - if using nla_put_u32
> then we could, but we also found a debug tool (which we'll
> ignore for the purposes of this regression) that was putting
> the padding into the length.
>
> Fixes: 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
We're stuck with this thing forever... I'd like to consider other
options.
I've seen this problem at least one time before, therefore I
suggest when we see a U8 attribute with a U32's length:
1) We access it as a u32, this takes care of all endianness
issues.
2) We emit a warning so that the app gets fixes.
Thanks.
^ permalink raw reply
* Re: [PATCH net 1/2] netlink: add NLA_U8_BUGGY attribute type
From: Johannes Berg @ 2017-12-05 16:34 UTC (permalink / raw)
To: David Miller
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, j, dsahern-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20171205.113145.172521292247335321.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Tue, 2017-12-05 at 11:31 -0500, David Miller wrote:
>
> > We could try to fix up the big endian problem here, but we
> > don't know *how* userspace misbehaved - if using nla_put_u32
> > then we could, but we also found a debug tool (which we'll
> > ignore for the purposes of this regression) that was putting
> > the padding into the length.
> We're stuck with this thing forever... I'd like to consider other
> options.
>
> I've seen this problem at least one time before, therefore I
> suggest when we see a U8 attribute with a U32's length:
>
> 1) We access it as a u32, this takes care of all endianness
> issues.
Possible, but as I said above, I've seen at least one tool (a debug
only script) now that will actually emit a U8 followed by 3 bytes of
padding to make it netlink-aligned, but set the length to 4. That would
be broken by making this change.
I'm not saying this is bad - but there are different levels of
compatibility and I'd probably go for "bug compatibility" here rather
than "fix-it-up compatibility".
Your call, ultimately - I've already fixed the tool I had found :-)
> 2) We emit a warning so that the app gets fixes.
For sure.
johannes
^ permalink raw reply
* Re: [PATCH net-next 1/8] net: hns3: Add HNS3 VF IMP(Integrated Management Proc) cmd interface
From: David Miller @ 2017-12-05 16:37 UTC (permalink / raw)
To: salil.mehta-hv44wF8Li93QT0dZR+AlfA
Cc: yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
lipeng321-hv44wF8Li93QT0dZR+AlfA,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <20171203123307.19820-2-salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
From: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Date: Sun, 3 Dec 2017 12:33:00 +0000
> +static int hclgevf_ring_space(struct hclgevf_cmq_ring *ring)
> +{
> + int ntu = ring->next_to_use;
> + int ntc = ring->next_to_clean;
> + int used = (ntu - ntc + ring->desc_num) % ring->desc_num;
Order local variables from longest to shortest line, please.
Audit your entire submission for this issue.
> +static int hclgevf_cmd_csq_done(struct hclgevf_hw *hw)
> +{
> + u32 head = hclgevf_read_dev(hw, HCLGEVF_NIC_CSQ_HEAD_REG);
> + return head == hw->cmq.csq.next_to_use;
> +}
Please return bool from this function.
> +void hclgevf_cmd_setup_basic_desc(struct hclgevf_desc *desc,
> + enum hclgevf_opcode_type opcode, bool is_read)
> +{
> + memset((void *)desc, 0, sizeof(struct hclgevf_desc));
You never need casts like this, when the functions argument is void any
pointer can be passed in as-is.
> +
> + /* If the command is sync, wait for the firmware to write back,
> + * if multi descriptors to be sent, use the first one to check
> + */
> + if (HCLGEVF_SEND_SYNC(le16_to_cpu(desc->flag))) {
> + do {
> + if (hclgevf_cmd_csq_done(hw))
> + break;
> + udelay(1);
> + timeout++;
> + } while (timeout < hw->cmq.tx_timeout);
> + }
This is potentially a long timeout with a spinlock held. Consider using
sleeping and completions.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net 1/2] netlink: add NLA_U8_BUGGY attribute type
From: David Miller @ 2017-12-05 16:41 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, netdev, j, dsahern
In-Reply-To: <1512491661.26976.19.camel@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 05 Dec 2017 17:34:21 +0100
> On Tue, 2017-12-05 at 11:31 -0500, David Miller wrote:
>>
>> > We could try to fix up the big endian problem here, but we
>> > don't know *how* userspace misbehaved - if using nla_put_u32
>> > then we could, but we also found a debug tool (which we'll
>> > ignore for the purposes of this regression) that was putting
>> > the padding into the length.
>
>> We're stuck with this thing forever... I'd like to consider other
>> options.
>>
>> I've seen this problem at least one time before, therefore I
>> suggest when we see a U8 attribute with a U32's length:
>>
>> 1) We access it as a u32, this takes care of all endianness
>> issues.
>
> Possible, but as I said above, I've seen at least one tool (a debug
> only script) now that will actually emit a U8 followed by 3 bytes of
> padding to make it netlink-aligned, but set the length to 4. That would
> be broken by making this change.
There is no reasonable interpretation for what that application is
doing, so I think we can safely call that case as buggy.
We are only trying to handle the situation where a U8 attribute
is presented as a bonafide U32 or a correct U8.
Does this make sense?
^ permalink raw reply
* Re: [PATCH net 1/2] netlink: add NLA_U8_BUGGY attribute type
From: David Ahern @ 2017-12-05 16:41 UTC (permalink / raw)
To: Johannes Berg, David Miller; +Cc: linux-wireless, netdev, j
In-Reply-To: <1512491661.26976.19.camel@sipsolutions.net>
[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]
On 12/5/17 9:34 AM, Johannes Berg wrote:
> On Tue, 2017-12-05 at 11:31 -0500, David Miller wrote:
>>
>>> We could try to fix up the big endian problem here, but we
>>> don't know *how* userspace misbehaved - if using nla_put_u32
>>> then we could, but we also found a debug tool (which we'll
>>> ignore for the purposes of this regression) that was putting
>>> the padding into the length.
>
>> We're stuck with this thing forever... I'd like to consider other
>> options.
>>
>> I've seen this problem at least one time before, therefore I
>> suggest when we see a U8 attribute with a U32's length:
>>
>> 1) We access it as a u32, this takes care of all endianness
>> issues.
>
> Possible, but as I said above, I've seen at least one tool (a debug
> only script) now that will actually emit a U8 followed by 3 bytes of
> padding to make it netlink-aligned, but set the length to 4. That would
> be broken by making this change.
>
> I'm not saying this is bad - but there are different levels of
> compatibility and I'd probably go for "bug compatibility" here rather
> than "fix-it-up compatibility".
>
> Your call, ultimately - I've already fixed the tool I had found :-)
>
>> 2) We emit a warning so that the app gets fixes.
>
The attached is my proposal. Basically, allow it the length mismatch but
print a warning. This restores previous behavior and tells users of bad
commands.
[-- Attachment #2: 0001-netlink-Relax-attr-validation-for-fixed-length-types.patch --]
[-- Type: text/plain, Size: 2222 bytes --]
From 29a504c8de553c17d4aafd5a2cb9384a28672fe4 Mon Sep 17 00:00:00 2001
From: David Ahern <dsahern@gmail.com>
Date: Sun, 3 Dec 2017 07:22:20 -0800
Subject: [PATCH] netlink: Relax attr validation for fixed length types
Commit 28033ae4e0f5 ("net: netlink: Update attr validation to require
exact length for some types") requires attributes using types NLA_U* and
NLA_S* to have an exact length. This change is exposing bugs in various
userspace commands that are sending attributes with an invalid length
(e.g., attribute has type NLA_U8 and userspace sends NLA_U32). While
the commands are clearly broken and need to be fixed, users are arguing
that the sudden change in enforcement is breaking older commands on
newer kernels for use cases that otherwise "worked".
Relax the validation to print a warning mesage similar to what is done
for messages containing extra bytes after parsing.
Fixes: 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
lib/nlattr.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 8bf78b4b78f0..6122662906c8 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -28,8 +28,16 @@ static const u8 nla_attr_len[NLA_TYPE_MAX+1] = {
};
static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = {
+ [NLA_U8] = sizeof(u8),
+ [NLA_U16] = sizeof(u16),
+ [NLA_U32] = sizeof(u32),
+ [NLA_U64] = sizeof(u64),
[NLA_MSECS] = sizeof(u64),
[NLA_NESTED] = NLA_HDRLEN,
+ [NLA_S8] = sizeof(s8),
+ [NLA_S16] = sizeof(s16),
+ [NLA_S32] = sizeof(s32),
+ [NLA_S64] = sizeof(s64),
};
static int validate_nla_bitfield32(const struct nlattr *nla,
@@ -70,10 +78,9 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
BUG_ON(pt->type > NLA_TYPE_MAX);
/* for data types NLA_U* and NLA_S* require exact length */
- if (nla_attr_len[pt->type]) {
- if (attrlen != nla_attr_len[pt->type])
- return -ERANGE;
- return 0;
+ if (nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) {
+ pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",
+ current->comm, type);
}
switch (pt->type) {
--
2.11.0
^ permalink raw reply related
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