* [PATCH bpf-next 3/8] bpf: add dsack_dups/delivered{,_ce} to bpf_tcp_sock
From: Stanislav Fomichev @ 2019-07-01 20:48 UTC (permalink / raw)
To: netdev, bpf
Cc: davem, ast, daniel, Stanislav Fomichev, Eric Dumazet,
Priyaranjan Jha, Yuchung Cheng, Soheil Hassas Yeganeh
In-Reply-To: <20190701204821.44230-1-sdf@google.com>
Add more fields to bpf_tcp_sock that might be useful for debugging
congestion control issues.
Cc: Eric Dumazet <edumazet@google.com>
Cc: Priyaranjan Jha <priyarjha@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/uapi/linux/bpf.h | 5 +++++
net/core/filter.c | 11 ++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9cdd0aaeba06..bfb0b1a76684 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3073,6 +3073,11 @@ struct bpf_tcp_sock {
* sum(delta(snd_una)), or how many bytes
* were acked.
*/
+ __u32 dsack_dups; /* RFC4898 tcpEStatsStackDSACKDups
+ * total number of DSACK blocks received
+ */
+ __u32 delivered; /* Total data packets delivered incl. rexmits */
+ __u32 delivered_ce; /* Like the above but only ECE marked packets */
};
struct bpf_sock_tuple {
diff --git a/net/core/filter.c b/net/core/filter.c
index ad908526545d..3da4b6c38b46 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5544,7 +5544,7 @@ static const struct bpf_func_proto bpf_sock_addr_sk_lookup_udp_proto = {
bool bpf_tcp_sock_is_valid_access(int off, int size, enum bpf_access_type type,
struct bpf_insn_access_aux *info)
{
- if (off < 0 || off >= offsetofend(struct bpf_tcp_sock, bytes_acked))
+ if (off < 0 || off >= offsetofend(struct bpf_tcp_sock, delivered_ce))
return false;
if (off % size != 0)
@@ -5652,6 +5652,15 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
case offsetof(struct bpf_tcp_sock, bytes_acked):
BPF_TCP_SOCK_GET_COMMON(bytes_acked);
break;
+ case offsetof(struct bpf_tcp_sock, dsack_dups):
+ BPF_TCP_SOCK_GET_COMMON(dsack_dups);
+ break;
+ case offsetof(struct bpf_tcp_sock, delivered):
+ BPF_TCP_SOCK_GET_COMMON(delivered);
+ break;
+ case offsetof(struct bpf_tcp_sock, delivered_ce):
+ BPF_TCP_SOCK_GET_COMMON(delivered_ce);
+ break;
}
return insn - insn_buf;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next 2/8] bpf: split shared bpf_tcp_sock and bpf_sock_ops implementation
From: Stanislav Fomichev @ 2019-07-01 20:48 UTC (permalink / raw)
To: netdev, bpf
Cc: davem, ast, daniel, Stanislav Fomichev, Eric Dumazet,
Priyaranjan Jha, Yuchung Cheng, Soheil Hassas Yeganeh
In-Reply-To: <20190701204821.44230-1-sdf@google.com>
We've added bpf_tcp_sock member to bpf_sock_ops and don't expect
any new tcp_sock fields in bpf_sock_ops. Let's remove
CONVERT_COMMON_TCP_SOCK_FIELDS so bpf_tcp_sock can be independently
extended.
Cc: Eric Dumazet <edumazet@google.com>
Cc: Priyaranjan Jha <priyarjha@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
net/core/filter.c | 180 ++++++++++++++++++++++++++++++++--------------
1 file changed, 126 insertions(+), 54 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 4836264f82ee..ad908526545d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5194,54 +5194,6 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
};
#endif /* CONFIG_IPV6_SEG6_BPF */
-#define CONVERT_COMMON_TCP_SOCK_FIELDS(md_type, CONVERT) \
-do { \
- switch (si->off) { \
- case offsetof(md_type, snd_cwnd): \
- CONVERT(snd_cwnd); break; \
- case offsetof(md_type, srtt_us): \
- CONVERT(srtt_us); break; \
- case offsetof(md_type, snd_ssthresh): \
- CONVERT(snd_ssthresh); break; \
- case offsetof(md_type, rcv_nxt): \
- CONVERT(rcv_nxt); break; \
- case offsetof(md_type, snd_nxt): \
- CONVERT(snd_nxt); break; \
- case offsetof(md_type, snd_una): \
- CONVERT(snd_una); break; \
- case offsetof(md_type, mss_cache): \
- CONVERT(mss_cache); break; \
- case offsetof(md_type, ecn_flags): \
- CONVERT(ecn_flags); break; \
- case offsetof(md_type, rate_delivered): \
- CONVERT(rate_delivered); break; \
- case offsetof(md_type, rate_interval_us): \
- CONVERT(rate_interval_us); break; \
- case offsetof(md_type, packets_out): \
- CONVERT(packets_out); break; \
- case offsetof(md_type, retrans_out): \
- CONVERT(retrans_out); break; \
- case offsetof(md_type, total_retrans): \
- CONVERT(total_retrans); break; \
- case offsetof(md_type, segs_in): \
- CONVERT(segs_in); break; \
- case offsetof(md_type, data_segs_in): \
- CONVERT(data_segs_in); break; \
- case offsetof(md_type, segs_out): \
- CONVERT(segs_out); break; \
- case offsetof(md_type, data_segs_out): \
- CONVERT(data_segs_out); break; \
- case offsetof(md_type, lost_out): \
- CONVERT(lost_out); break; \
- case offsetof(md_type, sacked_out): \
- CONVERT(sacked_out); break; \
- case offsetof(md_type, bytes_received): \
- CONVERT(bytes_received); break; \
- case offsetof(md_type, bytes_acked): \
- CONVERT(bytes_acked); break; \
- } \
-} while (0)
-
#ifdef CONFIG_INET
static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
int dif, int sdif, u8 family, u8 proto)
@@ -5623,9 +5575,6 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
offsetof(struct tcp_sock, FIELD)); \
} while (0)
- CONVERT_COMMON_TCP_SOCK_FIELDS(struct bpf_tcp_sock,
- BPF_TCP_SOCK_GET_COMMON);
-
if (insn > insn_buf)
return insn - insn_buf;
@@ -5640,6 +5589,69 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
offsetof(struct tcp_sock, rtt_min) +
offsetof(struct minmax_sample, v));
break;
+ case offsetof(struct bpf_tcp_sock, snd_cwnd):
+ BPF_TCP_SOCK_GET_COMMON(snd_cwnd);
+ break;
+ case offsetof(struct bpf_tcp_sock, srtt_us):
+ BPF_TCP_SOCK_GET_COMMON(srtt_us);
+ break;
+ case offsetof(struct bpf_tcp_sock, snd_ssthresh):
+ BPF_TCP_SOCK_GET_COMMON(snd_ssthresh);
+ break;
+ case offsetof(struct bpf_tcp_sock, rcv_nxt):
+ BPF_TCP_SOCK_GET_COMMON(rcv_nxt);
+ break;
+ case offsetof(struct bpf_tcp_sock, snd_nxt):
+ BPF_TCP_SOCK_GET_COMMON(snd_nxt);
+ break;
+ case offsetof(struct bpf_tcp_sock, snd_una):
+ BPF_TCP_SOCK_GET_COMMON(snd_una);
+ break;
+ case offsetof(struct bpf_tcp_sock, mss_cache):
+ BPF_TCP_SOCK_GET_COMMON(mss_cache);
+ break;
+ case offsetof(struct bpf_tcp_sock, ecn_flags):
+ BPF_TCP_SOCK_GET_COMMON(ecn_flags);
+ break;
+ case offsetof(struct bpf_tcp_sock, rate_delivered):
+ BPF_TCP_SOCK_GET_COMMON(rate_delivered);
+ break;
+ case offsetof(struct bpf_tcp_sock, rate_interval_us):
+ BPF_TCP_SOCK_GET_COMMON(rate_interval_us);
+ break;
+ case offsetof(struct bpf_tcp_sock, packets_out):
+ BPF_TCP_SOCK_GET_COMMON(packets_out);
+ break;
+ case offsetof(struct bpf_tcp_sock, retrans_out):
+ BPF_TCP_SOCK_GET_COMMON(retrans_out);
+ break;
+ case offsetof(struct bpf_tcp_sock, total_retrans):
+ BPF_TCP_SOCK_GET_COMMON(total_retrans);
+ break;
+ case offsetof(struct bpf_tcp_sock, segs_in):
+ BPF_TCP_SOCK_GET_COMMON(segs_in);
+ break;
+ case offsetof(struct bpf_tcp_sock, data_segs_in):
+ BPF_TCP_SOCK_GET_COMMON(data_segs_in);
+ break;
+ case offsetof(struct bpf_tcp_sock, segs_out):
+ BPF_TCP_SOCK_GET_COMMON(segs_out);
+ break;
+ case offsetof(struct bpf_tcp_sock, data_segs_out):
+ BPF_TCP_SOCK_GET_COMMON(data_segs_out);
+ break;
+ case offsetof(struct bpf_tcp_sock, lost_out):
+ BPF_TCP_SOCK_GET_COMMON(lost_out);
+ break;
+ case offsetof(struct bpf_tcp_sock, sacked_out):
+ BPF_TCP_SOCK_GET_COMMON(sacked_out);
+ break;
+ case offsetof(struct bpf_tcp_sock, bytes_received):
+ BPF_TCP_SOCK_GET_COMMON(bytes_received);
+ break;
+ case offsetof(struct bpf_tcp_sock, bytes_acked):
+ BPF_TCP_SOCK_GET_COMMON(bytes_acked);
+ break;
}
return insn - insn_buf;
@@ -7913,9 +7925,6 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
SOCK_OPS_GET_FIELD(BPF_FIELD, OBJ_FIELD, OBJ); \
} while (0)
- CONVERT_COMMON_TCP_SOCK_FIELDS(struct bpf_sock_ops,
- SOCK_OPS_GET_TCP_SOCK_FIELD);
-
if (insn > insn_buf)
return insn - insn_buf;
@@ -8085,6 +8094,69 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
SOCK_OPS_GET_OR_SET_FIELD(sk_txhash, sk_txhash,
struct sock, type);
break;
+ case offsetof(struct bpf_sock_ops, snd_cwnd):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(snd_cwnd);
+ break;
+ case offsetof(struct bpf_sock_ops, srtt_us):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(srtt_us);
+ break;
+ case offsetof(struct bpf_sock_ops, snd_ssthresh):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(snd_ssthresh);
+ break;
+ case offsetof(struct bpf_sock_ops, rcv_nxt):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(rcv_nxt);
+ break;
+ case offsetof(struct bpf_sock_ops, snd_nxt):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(snd_nxt);
+ break;
+ case offsetof(struct bpf_sock_ops, snd_una):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(snd_una);
+ break;
+ case offsetof(struct bpf_sock_ops, mss_cache):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(mss_cache);
+ break;
+ case offsetof(struct bpf_sock_ops, ecn_flags):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(ecn_flags);
+ break;
+ case offsetof(struct bpf_sock_ops, rate_delivered):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(rate_delivered);
+ break;
+ case offsetof(struct bpf_sock_ops, rate_interval_us):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(rate_interval_us);
+ break;
+ case offsetof(struct bpf_sock_ops, packets_out):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(packets_out);
+ break;
+ case offsetof(struct bpf_sock_ops, retrans_out):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(retrans_out);
+ break;
+ case offsetof(struct bpf_sock_ops, total_retrans):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(total_retrans);
+ break;
+ case offsetof(struct bpf_sock_ops, segs_in):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(segs_in);
+ break;
+ case offsetof(struct bpf_sock_ops, data_segs_in):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(data_segs_in);
+ break;
+ case offsetof(struct bpf_sock_ops, segs_out):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(segs_out);
+ break;
+ case offsetof(struct bpf_sock_ops, data_segs_out):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(data_segs_out);
+ break;
+ case offsetof(struct bpf_sock_ops, lost_out):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(lost_out);
+ break;
+ case offsetof(struct bpf_sock_ops, sacked_out):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(sacked_out);
+ break;
+ case offsetof(struct bpf_sock_ops, bytes_received):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(bytes_received);
+ break;
+ case offsetof(struct bpf_sock_ops, bytes_acked):
+ SOCK_OPS_GET_TCP_SOCK_FIELD(bytes_acked);
+ break;
case offsetof(struct bpf_sock_ops, sk):
*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
struct bpf_sock_ops_kern,
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next 1/8] bpf: add BPF_CGROUP_SOCK_OPS callback that is executed on every RTT
From: Stanislav Fomichev @ 2019-07-01 20:48 UTC (permalink / raw)
To: netdev, bpf
Cc: davem, ast, daniel, Stanislav Fomichev, Eric Dumazet,
Priyaranjan Jha, Yuchung Cheng, Soheil Hassas Yeganeh
In-Reply-To: <20190701204821.44230-1-sdf@google.com>
Performance impact should be minimal because it's under a new
BPF_SOCK_OPS_RTT_CB_FLAG flag that has to be explicitly enabled.
Suggested-by: Eric Dumazet <edumazet@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Priyaranjan Jha <priyarjha@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/net/tcp.h | 8 ++++++++
include/uapi/linux/bpf.h | 6 +++++-
net/ipv4/tcp_input.c | 4 ++++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 9d36cc88d043..e16d8a3fd3b4 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2221,6 +2221,14 @@ static inline bool tcp_bpf_ca_needs_ecn(struct sock *sk)
return (tcp_call_bpf(sk, BPF_SOCK_OPS_NEEDS_ECN, 0, NULL) == 1);
}
+static inline void tcp_bpf_rtt(struct sock *sk)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+
+ if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTT_CB_FLAG))
+ tcp_call_bpf(sk, BPF_SOCK_OPS_RTT_CB, 0, NULL);
+}
+
#if IS_ENABLED(CONFIG_SMC)
extern struct static_key_false tcp_have_smc;
#endif
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index cffea1826a1f..9cdd0aaeba06 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1770,6 +1770,7 @@ union bpf_attr {
* * **BPF_SOCK_OPS_RTO_CB_FLAG** (retransmission time out)
* * **BPF_SOCK_OPS_RETRANS_CB_FLAG** (retransmission)
* * **BPF_SOCK_OPS_STATE_CB_FLAG** (TCP state change)
+ * * **BPF_SOCK_OPS_RTT_CB_FLAG** (every RTT)
*
* Therefore, this function can be used to clear a callback flag by
* setting the appropriate bit to zero. e.g. to disable the RTO
@@ -3314,7 +3315,8 @@ struct bpf_sock_ops {
#define BPF_SOCK_OPS_RTO_CB_FLAG (1<<0)
#define BPF_SOCK_OPS_RETRANS_CB_FLAG (1<<1)
#define BPF_SOCK_OPS_STATE_CB_FLAG (1<<2)
-#define BPF_SOCK_OPS_ALL_CB_FLAGS 0x7 /* Mask of all currently
+#define BPF_SOCK_OPS_RTT_CB_FLAG (1<<3)
+#define BPF_SOCK_OPS_ALL_CB_FLAGS 0xF /* Mask of all currently
* supported cb flags
*/
@@ -3369,6 +3371,8 @@ enum {
BPF_SOCK_OPS_TCP_LISTEN_CB, /* Called on listen(2), right after
* socket transition to LISTEN state.
*/
+ BPF_SOCK_OPS_RTT_CB, /* Called on every RTT.
+ */
};
/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b71efeb0ae5b..c21e8a22fb3b 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -778,6 +778,8 @@ static void tcp_rtt_estimator(struct sock *sk, long mrtt_us)
tp->rttvar_us -= (tp->rttvar_us - tp->mdev_max_us) >> 2;
tp->rtt_seq = tp->snd_nxt;
tp->mdev_max_us = tcp_rto_min_us(sk);
+
+ tcp_bpf_rtt(sk);
}
} else {
/* no previous measure. */
@@ -786,6 +788,8 @@ static void tcp_rtt_estimator(struct sock *sk, long mrtt_us)
tp->rttvar_us = max(tp->mdev_us, tcp_rto_min_us(sk));
tp->mdev_max_us = tp->rttvar_us;
tp->rtt_seq = tp->snd_nxt;
+
+ tcp_bpf_rtt(sk);
}
tp->srtt_us = max(1U, srtt);
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next 0/8] bpf: TCP RTT sock_ops bpf callback
From: Stanislav Fomichev @ 2019-07-01 20:48 UTC (permalink / raw)
To: netdev, bpf
Cc: davem, ast, daniel, Stanislav Fomichev, Eric Dumazet,
Priyaranjan Jha, Yuchung Cheng, Soheil Hassas Yeganeh
Congestion control team would like to have a periodic callback to
track some TCP statistics. Let's add a sock_ops callback that can be
selectively enabled on a socket by socket basis and is executed for
every RTT. BPF program frequency can be further controlled by calling
bpf_ktime_get_ns and bailing out early.
I run neper tcp_stream and tcp_rr tests with the sample program
from the last patch and didn't observe any noticeable performance
difference.
Suggested-by: Eric Dumazet <edumazet@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Priyaranjan Jha <priyarjha@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Stanislav Fomichev (8):
bpf: add BPF_CGROUP_SOCK_OPS callback that is executed on every RTT
bpf: split shared bpf_tcp_sock and bpf_sock_ops implementation
bpf: add dsack_dups/delivered{,_ce} to bpf_tcp_sock
bpf: add icsk_retransmits to bpf_tcp_sock
bpf/tools: sync bpf.h
selftests/bpf: test BPF_SOCK_OPS_RTT_CB
samples/bpf: add sample program that periodically dumps TCP stats
samples/bpf: fix tcp_bpf.readme detach command
include/net/tcp.h | 8 +
include/uapi/linux/bpf.h | 12 +-
net/core/filter.c | 207 +++++++++++-----
net/ipv4/tcp_input.c | 4 +
samples/bpf/Makefile | 1 +
samples/bpf/tcp_bpf.readme | 2 +-
samples/bpf/tcp_dumpstats_kern.c | 65 +++++
tools/include/uapi/linux/bpf.h | 12 +-
tools/testing/selftests/bpf/Makefile | 3 +-
tools/testing/selftests/bpf/progs/tcp_rtt.c | 61 +++++
tools/testing/selftests/bpf/test_tcp_rtt.c | 253 ++++++++++++++++++++
11 files changed, 570 insertions(+), 58 deletions(-)
create mode 100644 samples/bpf/tcp_dumpstats_kern.c
create mode 100644 tools/testing/selftests/bpf/progs/tcp_rtt.c
create mode 100644 tools/testing/selftests/bpf/test_tcp_rtt.c
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH net-next 3/7] net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after posting IB_WR_LOCAL_INV
From: santosh.shilimkar @ 2019-07-01 20:41 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <505e9af7-a0cd-bf75-4a72-5d883ee06bf1@oracle.com>
On 7/1/19 9:39 AM, Gerd Rausch wrote:
> In order to:
> 1) avoid a silly bouncing between "clean_list" and "drop_list"
> triggered by function "rds_ib_reg_frmr" as it is releases frmr
> regions whose state is not "FRMR_IS_FREE" right away.
>
> 2) prevent an invalid access error in a race from a pending
> "IB_WR_LOCAL_INV" operation with a teardown ("dma_unmap_sg", "put_page")
> and de-registration ("ib_dereg_mr") of the corresponding
> memory region.
>
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
> net/rds/ib_frmr.c | 89 ++++++++++++++++++++++++++++++-----------------
> net/rds/ib_mr.h | 2 ++
> 2 files changed, 59 insertions(+), 32 deletions(-)
>
> diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> index 9f8aa310c27a..3c953034dca3 100644
> --- a/net/rds/ib_frmr.c
> +++ b/net/rds/ib_frmr.c
> @@ -76,6 +76,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
>
> frmr->fr_state = FRMR_IS_FREE;
> init_waitqueue_head(&frmr->fr_inv_done);
> + init_waitqueue_head(&frmr->fr_reg_done);
> return ibmr;
>
> out_no_cigar:
> @@ -124,6 +125,7 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
> */
> ib_update_fast_reg_key(frmr->mr, ibmr->remap_count++);
> frmr->fr_state = FRMR_IS_INUSE;
> + frmr->fr_reg = true;
>
> memset(®_wr, 0, sizeof(reg_wr));
> reg_wr.wr.wr_id = (unsigned long)(void *)ibmr;
> @@ -144,7 +146,29 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
> if (printk_ratelimit())
> pr_warn("RDS/IB: %s returned error(%d)\n",
> __func__, ret);
> + goto out;
> + }
> +
> + if (!frmr->fr_reg)
> + goto out;
> +
> + /* Wait for the registration to complete in order to prevent an invalid
> + * access error resulting from a race between the memory region already
> + * being accessed while registration is still pending.
> + */
> + wait_event_timeout(frmr->fr_reg_done, !frmr->fr_reg,
> + msecs_to_jiffies(100));
> +
This arbitrary timeout in this patch as well as pacth 1/7 which
Dave pointed out has any logic ?
MR registration command issued to hardware can at times take as
much as command timeout(e.g 60 seconds in CX3) and upto that its still
legitimate operation and not necessary failure. We shouldn't add
arbitrary time outs in ULPs.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH net-next] r8169: fix ntohs/htons sparse warnings
From: Heiner Kallweit @ 2019-07-01 20:36 UTC (permalink / raw)
To: Al Viro; +Cc: Realtek linux nic maintainers, David Miller,
netdev@vger.kernel.org
In-Reply-To: <20190701195621.GC17978@ZenIV.linux.org.uk>
On 01.07.2019 21:56, Al Viro wrote:
> On Mon, Jul 01, 2019 at 09:35:28PM +0200, Heiner Kallweit wrote:
>> Sparse complains about casting to/from restricted __be16. Fix this.
>
> Fix what, exactly? Force-cast is not a fix - it's "STFU, I know
> better, it's really correct" to sparse. Which may or may not
> match the reality, but it definitely requires more in way of
> commit message than "sparse says it's wrong; shut it up".
>
>> static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
>> @@ -1537,7 +1537,7 @@ static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
>>
>> if (opts2 & RxVlanTag)
>> __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
>> - ntohs(opts2 & 0xffff));
>> + ntohs((__force __be16)(opts2 & 0xffff)));
>> }
>
> Should that be ntohs at all? What behaviour is correct on big-endian host?
>
> AFAICS, in that code opts2 comes from little-endian 32bit. It's converted to
> host-endian, lower 16 bits (i.e. the first two octets in memory) are then
> fed to ntohs. Suppose we had in-core value stored as A0, A1, A2, A3.
> On little-endian that code will yield A0 * 256 + A1, treated as host-endian.
> On big-endian the same will yield A1 * 256 + A0. Is that actually correct?
>
I think you're right and the original patch should be reverted.
> The code dealing with the value passed to __vlan_hwaccel_put_tag() as the
> third argument treats it as a host-endian integer. So... Has anyone
> tested that code on b-e host? Should that ntohs() actually be swab16(),
> yielding (on any host) the same value we currently get for l-e hosts only?
>
I haven't seen any b-e host with a Realtek network chip yet.
^ permalink raw reply
* Re: [PATCH net-next 2/7] net/rds: Get rid of "wait_clean_list_grace" and add locking
From: santosh.shilimkar @ 2019-07-01 20:33 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <5c49f180-0dbf-88b9-965d-6cb88061f31b@oracle.com>
On 7/1/19 9:39 AM, Gerd Rausch wrote:
> Waiting for activity on the "clean_list" to quiesce is no substitute
> for proper locking.
>
> We can have multiple threads competing for "llist_del_first"
> via "rds_ib_reuse_mr", and a single thread competing
> for "llist_del_all" and "llist_del_first" via "rds_ib_flush_mr_pool".
>
> Since "llist_del_first" depends on "list->first->next" not to change
> in the midst of the operation, simply waiting for all current calls
> to "rds_ib_reuse_mr" to quiesce across all CPUs is woefully inadequate:
>
> By the time "wait_clean_list_grace" is done iterating over all CPUs to see
> that there is no concurrent caller to "rds_ib_reuse_mr", a new caller may
> have just shown up on the first CPU.
>
> Furthermore, <linux/llist.h> explicitly calls out the need for locking:
> * Cases where locking is needed:
> * If we have multiple consumers with llist_del_first used in one consumer,
> * and llist_del_first or llist_del_all used in other consumers,
> * then a lock is needed.
>
> Also, while at it, drop the unused "pool" parameter
> from "list_to_llist_nodes".
>
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
Looks good.
^ permalink raw reply
* Re: [PATCH 1/4] net: dsa: Change DT bindings for Vitesse VSC73xx switches
From: Linus Walleij @ 2019-07-01 20:23 UTC (permalink / raw)
To: Florian Fainelli
Cc: Pawel Dembicki, Andrew Lunn, Vivien Didelot, David S. Miller,
Rob Herring, Mark Rutland, netdev,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <45ff597a-5090-3874-b43d-5b5f45d2d2f6@gmail.com>
On Mon, Jul 1, 2019 at 6:44 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> Take b53 for instance which supports MDIO and SPI by default, and
> optionally memory mapped and SRAB (indirect memory map) accesses, they
> all have the same compatible strings. Whether the switches will appear
> as spi_device, platform_device, or something else is entirely based on
> how the Device Tree is laid out.
That's clever.
Pawel can you restructure the series around this observation?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH net-next 4/7] net/rds: Fix NULL/ERR_PTR inconsistency
From: santosh.shilimkar @ 2019-07-01 20:14 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <9f46098a-bcc7-bc0e-20db-2cbf05fefdee@oracle.com>
On 7/1/19 9:39 AM, Gerd Rausch wrote:
> Make function "rds_ib_try_reuse_ibmr" return NULL in case
> memory region could not be allocated, since callers
> simply check if the return value is not NULL.
>
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
Looks good to me. Will add this to other fixes.
^ permalink raw reply
* Re: [PATCH net-next 7/7] net/rds: Initialize ic->i_fastreg_wrs upon allocation
From: santosh.shilimkar @ 2019-07-01 20:15 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <a97a92d4-c443-12e1-6d82-1646f9584828@oracle.com>
On 7/1/19 9:40 AM, Gerd Rausch wrote:
> Otherwise, if an IB connection is torn down before "rds_ib_setup_qp"
> is called, the value of "ic->i_fastreg_wrs" is still at zero
> (as it wasn't initialized by "rds_ib_setup_qp").
> Consequently "rds_ib_conn_path_shutdown" will spin forever,
> waiting for it to go back to "RDS_IB_DEFAULT_FR_WR",
> which of course will never happen as there are no
> outstanding work requests.
>
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
Looks good to me. Will add this to other fixes.
^ permalink raw reply
* Re: [PATCH net-next 6/7] net/rds: Keep track of and wait for FRWR segments in use upon shutdown
From: santosh.shilimkar @ 2019-07-01 20:15 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <ad27abb5-b86a-1942-e2c8-2cba00812849@oracle.com>
On 7/1/19 9:40 AM, Gerd Rausch wrote:
> Since "rds_ib_free_frmr" and "rds_ib_free_frmr_list" simply put
> the FRMR memory segments on the "drop_list" or "free_list",
> and it is the job of "rds_ib_flush_mr_pool" to reap those entries
> by ultimately issuing a "IB_WR_LOCAL_INV" work-request,
> we need to trigger and then wait for all those memory segments
> attached to a particular connection to be fully released before
> we can move on to release the QP, CQ, etc.
>
> So we make "rds_ib_conn_path_shutdown" wait for one more
> atomic_t called "i_fastreg_inuse_count" that keeps track of how
> many FRWR memory segments are out there marked "FRMR_IS_INUSE"
> (and also wake_up rds_ib_ring_empty_wait, as they go away).
>
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
Looks good to me. Will add this to other fixes.
^ permalink raw reply
* Re: [PATCH net-next 5/7] net/rds: Set fr_state only to FRMR_IS_FREE if IB_WR_LOCAL_INV had been successful
From: santosh.shilimkar @ 2019-07-01 20:14 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <eb94e6bf-cbde-8cf1-b139-66fc8351f181@oracle.com>
On 7/1/19 9:40 AM, Gerd Rausch wrote:
> Fix a bug where fr_state first goes to FRMR_IS_STALE, because of a failure
> of operation IB_WR_LOCAL_INV, but then gets set back to "FRMR_IS_FREE"
> uncoditionally, even though the operation failed.
>
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
> net/rds/ib_frmr.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> index 3c953034dca3..a5d8f4128515 100644
> --- a/net/rds/ib_frmr.c
> +++ b/net/rds/ib_frmr.c
> @@ -328,7 +328,8 @@ void rds_ib_mr_cqe_handler(struct rds_ib_connection *ic, struct ib_wc *wc)
> }
>
> if (frmr->fr_inv) {
> - frmr->fr_state = FRMR_IS_FREE;
> + if (frmr->fr_state == FRMR_IS_INUSE)
> + frmr->fr_state = FRMR_IS_FREE;
> frmr->fr_inv = false;
> wake_up(&frmr->fr_inv_done);
> }
>
Looks good to me. Will add this to other fixes.
^ permalink raw reply
* Re: [PATCH 2/3] net: phy: realtek: Enable accessing RTL8211E extension pages
From: Andrew Lunn @ 2019-07-01 20:02 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190701195225.120808-2-mka@chromium.org>
On Mon, Jul 01, 2019 at 12:52:24PM -0700, Matthias Kaehlcke wrote:
> The RTL8211E has extension pages, which can be accessed after
> selecting a page through a custom method. Add a function to
> modify bits in a register of an extension page and a few
> helpers for dealing with ext pages.
>
> rtl8211e_modify_ext_paged() and rtl821e_restore_page() are
> inspired by their counterparts phy_modify_paged() and
> phy_restore_page().
Hi Matthias
While an extended page is selected, what happens to the normal
registers in the range 0-0x1c? Are they still accessible?
Andrew
^ permalink raw reply
* Re: [PATCH 2/2] samples: pktgen: allow to specify destination port
From: Daniel T. Lee @ 2019-07-01 20:01 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: David S . Miller, netdev, Robert Olsson, Jean Hsiao
In-Reply-To: <20190701170819.548a7457@carbon>
Thanks for the review!
About the equivalent port in the same burst thing, I didn't realize it
would work in
that way. It doesn't matter in my use-case, but thank you for letting me know!
On Tue, Jul 2, 2019 at 12:08 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> On Sat, 29 Jun 2019 22:33:58 +0900
> "Daniel T. Lee" <danieltimlee@gmail.com> wrote:
>
> > Currently, kernel pktgen has the feature to specify udp destination port
> > for sending packet. (e.g. pgset "udp_dst_min 9")
> >
> > But on samples, each of the scripts doesn't have any option to achieve this.
> >
> > This commit adds the DST_PORT option to specify the target port(s) in the script.
> >
> > -p : ($DST_PORT) destination PORT range (e.g. 433-444) is also allowed
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
>
> Nice feature, this look very usable for testing. I think my QA asked
> me for something similar.
>
> One nitpick is that script named pktgen_sample03_burst_single_flow.sh
> implies this is a single flow, but by specifying a port-range this will
> be more flows. I'm okay with adding this, as the end-user specifying a
> port-range should realize this. Thus, you get my ACK.
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Another thing you should realize (but you/we cannot do anything about)
> is that when the scripts use burst or clone, then the port (UDPDST_RND)
> will be the same for all packets in the same burst. I don't know if it
> matters for your use-case.
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next] r8169: fix ntohs/htons sparse warnings
From: Al Viro @ 2019-07-01 19:56 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Realtek linux nic maintainers, David Miller,
netdev@vger.kernel.org
In-Reply-To: <1d1f9dba-1ade-7782-6cc0-3151a7086a4b@gmail.com>
On Mon, Jul 01, 2019 at 09:35:28PM +0200, Heiner Kallweit wrote:
> Sparse complains about casting to/from restricted __be16. Fix this.
Fix what, exactly? Force-cast is not a fix - it's "STFU, I know
better, it's really correct" to sparse. Which may or may not
match the reality, but it definitely requires more in way of
commit message than "sparse says it's wrong; shut it up".
> static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
> @@ -1537,7 +1537,7 @@ static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
>
> if (opts2 & RxVlanTag)
> __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> - ntohs(opts2 & 0xffff));
> + ntohs((__force __be16)(opts2 & 0xffff)));
> }
Should that be ntohs at all? What behaviour is correct on big-endian host?
AFAICS, in that code opts2 comes from little-endian 32bit. It's converted to
host-endian, lower 16 bits (i.e. the first two octets in memory) are then
fed to ntohs. Suppose we had in-core value stored as A0, A1, A2, A3.
On little-endian that code will yield A0 * 256 + A1, treated as host-endian.
On big-endian the same will yield A1 * 256 + A0. Is that actually correct?
The code dealing with the value passed to __vlan_hwaccel_put_tag() as the
third argument treats it as a host-endian integer. So... Has anyone
tested that code on b-e host? Should that ntohs() actually be swab16(),
yielding (on any host) the same value we currently get for l-e hosts only?
^ permalink raw reply
* [PATCH 1/3] dt-bindings: net: Add bindings for Realtek PHYs
From: Matthias Kaehlcke @ 2019-07-01 19:52 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
Add the 'realtek,enable-ssc' property to enable Spread Spectrum
Clocking (SSC) on Realtek PHYs that support it.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
.../devicetree/bindings/net/realtek.txt | 21 +++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/realtek.txt
diff --git a/Documentation/devicetree/bindings/net/realtek.txt b/Documentation/devicetree/bindings/net/realtek.txt
new file mode 100644
index 000000000000..9fad97e7404f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/realtek.txt
@@ -0,0 +1,21 @@
+Realtek PHY properties.
+
+This document describes properties of Realtek PHYs.
+
+Optional properties:
+- realtek,enable-ssc Enable Spread Spectrum Clocking (SSC) on this port.
+ SSC is only available on some Realtek PHYs (e.g.
+ RTL8211E).
+
+Example:
+
+mdio0 {
+ compatible = "snps,dwmac-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethphy: ethernet-phy@1 {
+ reg = <1>;
+ realtek,enable-ssc;
+ };
+};
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH 3/3] net: phy: realtek: Support SSC for the RTL8211E
From: Matthias Kaehlcke @ 2019-07-01 19:52 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
In-Reply-To: <20190701195225.120808-1-mka@chromium.org>
By default Spread-Spectrum Clocking (SSC) is disabled on the RTL8211E.
Enable it if the device tree property 'realtek,enable-ssc' exists.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
drivers/net/phy/realtek.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index dfc2e20ef335..b617169ccc8c 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -9,8 +9,10 @@
* Copyright (c) 2004 Freescale Semiconductor, Inc.
*/
#include <linux/bitops.h>
-#include <linux/phy.h>
+#include <linux/device.h>
+#include <linux/of.h>
#include <linux/module.h>
+#include <linux/phy.h>
#define RTL821x_PHYSR 0x11
#define RTL821x_PHYSR_DUPLEX BIT(13)
@@ -28,6 +30,8 @@
#define RTL8211E_EXT_PAGE 7
#define RTL8211E_EPAGSR 0x1e
+#define RTL8211E_SCR 0x1a
+#define RTL8211E_SCR_DISABLE_RXC_SSC BIT(2)
#define RTL8211F_INSR 0x1d
@@ -87,8 +91,8 @@ static int rtl821e_restore_page(struct phy_device *phydev, int oldpage, int ret)
return ret;
}
-static int __maybe_unused rtl8211e_modify_ext_paged(struct phy_device *phydev,
- int page, u32 regnum, u16 mask, u16 set)
+static int rtl8211e_modify_ext_paged(struct phy_device *phydev, int page,
+ u32 regnum, u16 mask, u16 set)
{
int ret = 0;
int oldpage;
@@ -114,6 +118,22 @@ static int __maybe_unused rtl8211e_modify_ext_paged(struct phy_device *phydev,
return rtl821e_restore_page(phydev, oldpage, ret);
}
+static int rtl8211e_probe(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ int err;
+
+ if (of_property_read_bool(dev->of_node, "realtek,enable-ssc")) {
+ err = rtl8211e_modify_ext_paged(phydev, 0xa0, RTL8211E_SCR,
+ RTL8211E_SCR_DISABLE_RXC_SSC,
+ 0);
+ if (err)
+ dev_err(dev, "failed to enable SSC on RXC: %d\n", err);
+ }
+
+ return 0;
+}
+
static int rtl8201_ack_interrupt(struct phy_device *phydev)
{
int err;
@@ -372,6 +392,7 @@ static struct phy_driver realtek_drvs[] = {
.config_init = &rtl8211e_config_init,
.ack_interrupt = &rtl821x_ack_interrupt,
.config_intr = &rtl8211e_config_intr,
+ .probe = rtl8211e_probe,
.suspend = genphy_suspend,
.resume = genphy_resume,
.read_page = rtl821x_read_page,
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH 2/3] net: phy: realtek: Enable accessing RTL8211E extension pages
From: Matthias Kaehlcke @ 2019-07-01 19:52 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
In-Reply-To: <20190701195225.120808-1-mka@chromium.org>
The RTL8211E has extension pages, which can be accessed after
selecting a page through a custom method. Add a function to
modify bits in a register of an extension page and a few
helpers for dealing with ext pages.
rtl8211e_modify_ext_paged() and rtl821e_restore_page() are
inspired by their counterparts phy_modify_paged() and
phy_restore_page().
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
This code might be applicable to other Realtek PHYs, but I don't
have access to the datasheets to confirm it, so for now it's just
for the RTL8211E.
drivers/net/phy/realtek.c | 61 +++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index a669945eb829..dfc2e20ef335 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -26,6 +26,9 @@
#define RTL821x_EXT_PAGE_SELECT 0x1e
#define RTL821x_PAGE_SELECT 0x1f
+#define RTL8211E_EXT_PAGE 7
+#define RTL8211E_EPAGSR 0x1e
+
#define RTL8211F_INSR 0x1d
#define RTL8211F_TX_DELAY BIT(8)
@@ -53,6 +56,64 @@ static int rtl821x_write_page(struct phy_device *phydev, int page)
return __phy_write(phydev, RTL821x_PAGE_SELECT, page);
}
+static int rtl821e_select_ext_page(struct phy_device *phydev, int page)
+{
+ int rc;
+
+ rc = phy_write(phydev, RTL821x_PAGE_SELECT, RTL8211E_EXT_PAGE);
+ if (rc)
+ return rc;
+
+ return phy_write(phydev, RTL8211E_EPAGSR, page);
+}
+
+static int rtl821e_restore_page(struct phy_device *phydev, int oldpage, int ret)
+{
+ int r;
+
+ if (oldpage >= 0) {
+ r = phy_write(phydev, RTL821x_PAGE_SELECT, oldpage);
+
+ /* Propagate the operation return code if the page write
+ * was successful.
+ */
+ if (ret >= 0 && r < 0)
+ ret = r;
+ } else {
+ /* Propagate the page selection error code */
+ ret = oldpage;
+ }
+
+ return ret;
+}
+
+static int __maybe_unused rtl8211e_modify_ext_paged(struct phy_device *phydev,
+ int page, u32 regnum, u16 mask, u16 set)
+{
+ int ret = 0;
+ int oldpage;
+ int new;
+
+ oldpage = phy_read(phydev, RTL821x_PAGE_SELECT);
+ if (oldpage < 0)
+ goto out;
+
+ ret = rtl821e_select_ext_page(phydev, page);
+ if (ret)
+ goto out;
+
+ ret = phy_read(phydev, regnum);
+ if (ret < 0)
+ goto out;
+
+ new = (ret & ~mask) | set;
+ if (new != ret)
+ ret = phy_write(phydev, regnum, new);
+
+out:
+ return rtl821e_restore_page(phydev, oldpage, ret);
+}
+
static int rtl8201_ack_interrupt(struct phy_device *phydev)
{
int err;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH net-next] r8169: fix ntohs/htons sparse warnings
From: Heiner Kallweit @ 2019-07-01 19:35 UTC (permalink / raw)
To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org
Sparse complains about casting to/from restricted __be16. Fix this.
Fixes: 759d09574172 ("r8169: improve handling VLAN tag")
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index a73f25321..450c74dc1 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -1528,7 +1528,7 @@ static int rtl8169_set_features(struct net_device *dev,
static inline u32 rtl8169_tx_vlan_tag(struct sk_buff *skb)
{
return (skb_vlan_tag_present(skb)) ?
- TxVlanTag | htons(skb_vlan_tag_get(skb)) : 0x00;
+ TxVlanTag | (__force u16)htons(skb_vlan_tag_get(skb)) : 0x00;
}
static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
@@ -1537,7 +1537,7 @@ static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
if (opts2 & RxVlanTag)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
- ntohs(opts2 & 0xffff));
+ ntohs((__force __be16)(opts2 & 0xffff)));
}
static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
--
2.22.0
^ permalink raw reply related
* Re: [PATCH net-next] bonding/main: fix NULL dereference in bond_select_active_slave()
From: Eric Dumazet @ 2019-07-01 19:31 UTC (permalink / raw)
To: Jay Vosburgh
Cc: David S . Miller, netdev, Eric Dumazet, John Sperbeck,
Jarod Wilson, Veaceslav Falico, Andy Gospodarek
In-Reply-To: <18508.1562008524@famine>
On Mon, Jul 1, 2019 at 9:15 PM Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>
> Eric Dumazet <edumazet@google.com> wrote:
>
> >A bonding master can be up while best_slave is NULL.
> >
> >[12105.636318] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
> >[12105.638204] mlx4_en: eth1: Linkstate event 1 -> 1
> >[12105.648984] IP: bond_select_active_slave+0x125/0x250
> >[12105.653977] PGD 0 P4D 0
> >[12105.656572] Oops: 0000 [#1] SMP PTI
> >[12105.660487] gsmi: Log Shutdown Reason 0x03
> >[12105.664620] Modules linked in: kvm_intel loop act_mirred uhaul vfat fat stg_standard_ftl stg_megablocks stg_idt stg_hdi stg elephant_dev_num stg_idt_eeprom w1_therm wire i2c_mux_pca954x i2c_mux mlx4_i2c i2c_usb cdc_acm ehci_pci ehci_hcd i2c_iimc mlx4_en mlx4_ib ib_uverbs ib_core mlx4_core [last unloaded: kvm_intel]
> >[12105.685686] mlx4_core 0000:03:00.0: dispatching link up event for port 2
> >[12105.685700] mlx4_en: eth2: Linkstate event 2 -> 1
> >[12105.685700] mlx4_en: eth2: Link Up (linkstate)
> >[12105.724452] Workqueue: bond0 bond_mii_monitor
> >[12105.728854] RIP: 0010:bond_select_active_slave+0x125/0x250
> >[12105.734355] RSP: 0018:ffffaf146a81fd88 EFLAGS: 00010246
> >[12105.739637] RAX: 0000000000000003 RBX: ffff8c62b03c6900 RCX: 0000000000000000
> >[12105.746838] RDX: 0000000000000000 RSI: ffffaf146a81fd08 RDI: ffff8c62b03c6000
> >[12105.754054] RBP: ffffaf146a81fdb8 R08: 0000000000000001 R09: ffff8c517d387600
> >[12105.761299] R10: 00000000001075d9 R11: ffffffffaceba92f R12: 0000000000000000
> >[12105.768553] R13: ffff8c8240ae4800 R14: 0000000000000000 R15: 0000000000000000
> >[12105.775748] FS: 0000000000000000(0000) GS:ffff8c62bfa40000(0000) knlGS:0000000000000000
> >[12105.783892] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >[12105.789716] CR2: 0000000000000000 CR3: 0000000d0520e001 CR4: 00000000001626f0
> >[12105.796976] Call Trace:
> >[12105.799446] [<ffffffffac31d387>] bond_mii_monitor+0x497/0x6f0
> >[12105.805317] [<ffffffffabd42643>] process_one_work+0x143/0x370
> >[12105.811225] [<ffffffffabd42c7a>] worker_thread+0x4a/0x360
> >[12105.816761] [<ffffffffabd48bc5>] kthread+0x105/0x140
> >[12105.821865] [<ffffffffabd42c30>] ? rescuer_thread+0x380/0x380
> >[12105.827757] [<ffffffffabd48ac0>] ? kthread_associate_blkcg+0xc0/0xc0
> >[12105.834266] [<ffffffffac600241>] ret_from_fork+0x51/0x60
> >
> >Fixes: e2a7420df2e0 ("bonding/main: convert to using slave printk macros")
> >Signed-off-by: Eric Dumazet <edumazet@google.com>
> >Reported-by: John Sperbeck <jsperbeck@google.com>
> >Cc: Jarod Wilson <jarod@redhat.com>
> >CC: Jay Vosburgh <j.vosburgh@gmail.com>
> >CC: Veaceslav Falico <vfalico@gmail.com>
> >CC: Andy Gospodarek <andy@greyhouse.net>
> >---
> > drivers/net/bonding/bond_main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> >index a30595955a37a485b9e045a31969313f8336b668..84168455aded96dfd85b310841dee2a0d917b580 100644
> >--- a/drivers/net/bonding/bond_main.c
> >+++ b/drivers/net/bonding/bond_main.c
> >@@ -937,7 +937,7 @@ void bond_select_active_slave(struct bonding *bond)
> > return;
> >
> > if (netif_carrier_ok(bond->dev))
> >- slave_info(bond->dev, best_slave->dev, "active interface up!\n");
> >+ netdev_info(bond->dev, "active interface up!\n");
> > else
> > netdev_info(bond->dev, "now running without any active interface!\n");
> > }
>
> What is the bonding mode and options in the failure case?
I am not sure.
Basically I only have crash dumps at this point (no idea how to repro the bug)
I guess that is :
Bonding Mode: IEEE 802.3ad Dynamic link aggregation
Transmit Hash Policy: encap3+4 (4)
Use Carrier: 1
MII Status: up
MII Polling Interval (ms): 100
Up Delay (ms): 0
Down Delay (ms): 0
MAC address: 00:1a:11:c0:65:40
Active Members: eth1 eth2
>
> I see that the fix is fine in the sense that it returns to the
> original status quo for the message.
>
> However, the code path seems odd; if best_slave is NULL, that
> means that bond_find_best_slave() saw all slaves as down, but if
> netif_carrier_ok is true, then bond_set_carrier() did not.
^ permalink raw reply
* Re: [PATCH net-next] bonding/main: fix NULL dereference in bond_select_active_slave()
From: Jay Vosburgh @ 2019-07-01 19:15 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, John Sperbeck,
Jarod Wilson, Veaceslav Falico, Andy Gospodarek
In-Reply-To: <20190701174851.70293-1-edumazet@google.com>
Eric Dumazet <edumazet@google.com> wrote:
>A bonding master can be up while best_slave is NULL.
>
>[12105.636318] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
>[12105.638204] mlx4_en: eth1: Linkstate event 1 -> 1
>[12105.648984] IP: bond_select_active_slave+0x125/0x250
>[12105.653977] PGD 0 P4D 0
>[12105.656572] Oops: 0000 [#1] SMP PTI
>[12105.660487] gsmi: Log Shutdown Reason 0x03
>[12105.664620] Modules linked in: kvm_intel loop act_mirred uhaul vfat fat stg_standard_ftl stg_megablocks stg_idt stg_hdi stg elephant_dev_num stg_idt_eeprom w1_therm wire i2c_mux_pca954x i2c_mux mlx4_i2c i2c_usb cdc_acm ehci_pci ehci_hcd i2c_iimc mlx4_en mlx4_ib ib_uverbs ib_core mlx4_core [last unloaded: kvm_intel]
>[12105.685686] mlx4_core 0000:03:00.0: dispatching link up event for port 2
>[12105.685700] mlx4_en: eth2: Linkstate event 2 -> 1
>[12105.685700] mlx4_en: eth2: Link Up (linkstate)
>[12105.724452] Workqueue: bond0 bond_mii_monitor
>[12105.728854] RIP: 0010:bond_select_active_slave+0x125/0x250
>[12105.734355] RSP: 0018:ffffaf146a81fd88 EFLAGS: 00010246
>[12105.739637] RAX: 0000000000000003 RBX: ffff8c62b03c6900 RCX: 0000000000000000
>[12105.746838] RDX: 0000000000000000 RSI: ffffaf146a81fd08 RDI: ffff8c62b03c6000
>[12105.754054] RBP: ffffaf146a81fdb8 R08: 0000000000000001 R09: ffff8c517d387600
>[12105.761299] R10: 00000000001075d9 R11: ffffffffaceba92f R12: 0000000000000000
>[12105.768553] R13: ffff8c8240ae4800 R14: 0000000000000000 R15: 0000000000000000
>[12105.775748] FS: 0000000000000000(0000) GS:ffff8c62bfa40000(0000) knlGS:0000000000000000
>[12105.783892] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>[12105.789716] CR2: 0000000000000000 CR3: 0000000d0520e001 CR4: 00000000001626f0
>[12105.796976] Call Trace:
>[12105.799446] [<ffffffffac31d387>] bond_mii_monitor+0x497/0x6f0
>[12105.805317] [<ffffffffabd42643>] process_one_work+0x143/0x370
>[12105.811225] [<ffffffffabd42c7a>] worker_thread+0x4a/0x360
>[12105.816761] [<ffffffffabd48bc5>] kthread+0x105/0x140
>[12105.821865] [<ffffffffabd42c30>] ? rescuer_thread+0x380/0x380
>[12105.827757] [<ffffffffabd48ac0>] ? kthread_associate_blkcg+0xc0/0xc0
>[12105.834266] [<ffffffffac600241>] ret_from_fork+0x51/0x60
>
>Fixes: e2a7420df2e0 ("bonding/main: convert to using slave printk macros")
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Reported-by: John Sperbeck <jsperbeck@google.com>
>Cc: Jarod Wilson <jarod@redhat.com>
>CC: Jay Vosburgh <j.vosburgh@gmail.com>
>CC: Veaceslav Falico <vfalico@gmail.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>---
> drivers/net/bonding/bond_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index a30595955a37a485b9e045a31969313f8336b668..84168455aded96dfd85b310841dee2a0d917b580 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -937,7 +937,7 @@ void bond_select_active_slave(struct bonding *bond)
> return;
>
> if (netif_carrier_ok(bond->dev))
>- slave_info(bond->dev, best_slave->dev, "active interface up!\n");
>+ netdev_info(bond->dev, "active interface up!\n");
> else
> netdev_info(bond->dev, "now running without any active interface!\n");
> }
What is the bonding mode and options in the failure case?
I see that the fix is fine in the sense that it returns to the
original status quo for the message.
However, the code path seems odd; if best_slave is NULL, that
means that bond_find_best_slave() saw all slaves as down, but if
netif_carrier_ok is true, then bond_set_carrier() did not.
-J
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [PATCH net-next 5/5] ipv4: use indirect call wrappers for {tcp,udp}_{recv,send}msg()
From: Willem de Bruijn @ 2019-07-01 19:07 UTC (permalink / raw)
To: Paolo Abeni; +Cc: Network Development, David S. Miller
In-Reply-To: <8c32b92eee12bf0725ead331e7607d8c4012d51f.1561999976.git.pabeni@redhat.com>
On Mon, Jul 1, 2019 at 1:10 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> This avoids an indirect call per syscall for common ipv4 transports
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/ipv4/af_inet.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 8421e2f5bbb3..9a2f17d0c5f5 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -797,6 +797,8 @@ int inet_send_prepare(struct sock *sk)
> }
> EXPORT_SYMBOL_GPL(inet_send_prepare);
>
> +INDIRECT_CALLABLE_DECLARE(int udp_sendmsg(struct sock *, struct msghdr *,
> + size_t));
Small nit: this is already defined in include/net/udp.h, which is
included. So like tcp_sendmsg, probably no need to declare.
If defining inet6_sendmsg and inet6_recvmsg in include/net/ipv6.h,
perhaps do the same for the other missing functions, instead of these
indirect declarations at the callsite?
Aside from that small point, patch set looks great to me. Thanks Paolo.
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: r8169 not working on 5.2.0rc6 with GPD MicroPC
From: Karsten Wiborg @ 2019-07-01 19:07 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn; +Cc: nic_swsd, romieu, netdev
In-Reply-To: <d48eb3dd-be07-1422-4649-91f3461676c4@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 611 bytes --]
Hi Heiner,
that is very unfortunately regarding the state of the device. I guess
GPD won't really react. Anyway thank you very much for trying to contact
them.
Thanks also a lot for your help and input.
Best regards,
Karsten
On 01/07/2019 20:51, Heiner Kallweit wrote:
> On 01.07.2019 20:15, Karsten Wiborg wrote:
> The information is sufficient now. Still, using a random MAC address
> is an emergency fallback. The device is simply broken.
> I contacted GPD, let's see whether they respond something.
>
> In parallel I'll add a random MAC address as fallback to the
> mainline driver.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4156 bytes --]
^ permalink raw reply
* Re: r8169 not working on 5.2.0rc6 with GPD MicroPC
From: Heiner Kallweit @ 2019-07-01 18:51 UTC (permalink / raw)
To: Karsten Wiborg, Andrew Lunn; +Cc: nic_swsd, romieu, netdev
In-Reply-To: <672d3b3f-e55d-e2bb-1d8c-a83d0a0c057a@web.de>
On 01.07.2019 20:15, Karsten Wiborg wrote:
> Hi Andrew, Heiner,
>
> the device is a really small notebook. So detaching mains still leaves
> the battery which is delicately built in. So can't currently remove
> power completely.
>
> Anyway can I deliver more debugging data to you guys which might add
> support for the r8169 for this device?
>
The information is sufficient now. Still, using a random MAC address
is an emergency fallback. The device is simply broken.
I contacted GPD, let's see whether they respond something.
In parallel I'll add a random MAC address as fallback to the
mainline driver.
> Regards,
> Karsten
>
Heiner
> On 01/07/2019 15:35, Andrew Lunn wrote:
>>> When the vendor driver assigns a random MAC address, it writes it to the
>>> chip. The related registers may be persistent (can't say exactly due to
>>> missing documentation).
>>
>> If the device supports WOL, it could be it is powered using the
>> standby supply, not the main supply. Try pulling the plug from the
>> wall to really remove all power.
>>
>> Andrew
>>
>
^ permalink raw reply
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
From: Y Song @ 2019-07-01 18:39 UTC (permalink / raw)
To: Song Liu
Cc: Jiri Benc, Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Mathieu Xhonneux
In-Reply-To: <CAPhsuW4Ric_nMGxpKf3mEJw3JDBZYpbeAQwTW_Nrsz79T2zisw@mail.gmail.com>
On Sat, Jun 29, 2019 at 11:05 AM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Sat, Jun 29, 2019 at 11:04 AM Song Liu <liu.song.a23@gmail.com> wrote:
> >
> > On Sat, Jun 29, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote:
> > >
> > > Selftests are reporting this failure in test_lwt_seg6local.sh:
> > >
> > > + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> > > Error fetching program/map!
> > > Failed to parse eBPF program: Operation not permitted
> > >
> > > The problem is __attribute__((always_inline)) alone is not enough to prevent
> > > clang from inserting those functions in .text. In that case, .text is not
> > > marked as relocateable.
> > >
> > > See the output of objdump -h test_lwt_seg6local.o:
> > >
> > > Idx Name Size VMA LMA File off Algn
> > > 0 .text 00003530 0000000000000000 0000000000000000 00000040 2**3
> > > CONTENTS, ALLOC, LOAD, READONLY, CODE
> > >
> > > This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> > > bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> > > relocateable .text section in the file.
> > >
> > > Add 'static inline' to fix this.
> > >
> > > Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> > > Signed-off-by: Jiri Benc <jbenc@redhat.com>
> >
> > Maybe use "__always_inline" as most other tests do?
>
> I meant "static __always_inline".
By default, we have
# define __always_inline inline __attribute__((always_inline))
So just use __always_inline should be less verbose in your patch.
BTW, what compiler did you use have this behavior?
Did you have issues with `static __attribute__((always_inline))`?
>
> Song
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox