Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 07/10] net/smc: cleanup listen worker mutex unlocking
From: Ursula Braun @ 2018-11-22  9:26 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>

For easier reading move the unlock of mutex smc_create_lgr_pending into
smc_listen_work(), i.e. into the function the mutex has been locked.
No functional change.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
 net/smc/af_smc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 93f7ff8f6e88..7657e249f526 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1182,7 +1182,6 @@ static int smc_listen_rdma_finish(struct smc_sock *new_smc,
 	return 0;
 
 decline:
-	mutex_unlock(&smc_create_lgr_pending);
 	smc_listen_decline(new_smc, reason_code, local_contact);
 	return reason_code;
 }
@@ -1282,8 +1281,10 @@ static void smc_listen_work(struct work_struct *work)
 
 	/* finish worker */
 	if (!ism_supported) {
-		if (smc_listen_rdma_finish(new_smc, &cclc, local_contact))
+		if (smc_listen_rdma_finish(new_smc, &cclc, local_contact)) {
+			mutex_unlock(&smc_create_lgr_pending);
 			return;
+		}
 	}
 	smc_conn_save_peer_info(new_smc, &cclc);
 	mutex_unlock(&smc_create_lgr_pending);
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 06/10] net/smc: short wait for late smc_clc_wait_msg
From: Ursula Braun @ 2018-11-22  9:26 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>

After sending one of the initial LLC messages CONFIRM LINK or
ADD LINK, there is already a wait for the LLC response. It does
not make sense to wait another long time for a CLC DECLINE. Thus
this patch introduces a shorter wait time for these cases.

Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
 net/smc/af_smc.c  | 15 ++++++++-------
 net/smc/smc_clc.c |  6 +++---
 net/smc/smc_clc.h |  3 ++-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 66836cfbc587..93f7ff8f6e88 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -335,7 +335,7 @@ static int smc_clnt_conf_first_link(struct smc_sock *smc)
 		struct smc_clc_msg_decline dclc;
 
 		rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
-				      SMC_CLC_DECLINE);
+				      SMC_CLC_DECLINE, CLC_WAIT_TIME_SHORT);
 		return rc == -EAGAIN ? SMC_CLC_DECL_TIMEOUT_CL : rc;
 	}
 
@@ -363,7 +363,7 @@ static int smc_clnt_conf_first_link(struct smc_sock *smc)
 		struct smc_clc_msg_decline dclc;
 
 		rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
-				      SMC_CLC_DECLINE);
+				      SMC_CLC_DECLINE, CLC_WAIT_TIME_SHORT);
 		return rc == -EAGAIN ? SMC_CLC_DECL_TIMEOUT_AL : rc;
 	}
 
@@ -533,7 +533,8 @@ static int smc_connect_clc(struct smc_sock *smc, int smc_type,
 	if (rc)
 		return rc;
 	/* receive SMC Accept CLC message */
-	return smc_clc_wait_msg(smc, aclc, sizeof(*aclc), SMC_CLC_ACCEPT);
+	return smc_clc_wait_msg(smc, aclc, sizeof(*aclc), SMC_CLC_ACCEPT,
+				CLC_WAIT_TIME);
 }
 
 /* setup for RDMA connection of client */
@@ -965,7 +966,7 @@ static int smc_serv_conf_first_link(struct smc_sock *smc)
 		struct smc_clc_msg_decline dclc;
 
 		rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
-				      SMC_CLC_DECLINE);
+				      SMC_CLC_DECLINE, CLC_WAIT_TIME_SHORT);
 		return rc == -EAGAIN ? SMC_CLC_DECL_TIMEOUT_CL : rc;
 	}
 
@@ -986,7 +987,7 @@ static int smc_serv_conf_first_link(struct smc_sock *smc)
 		struct smc_clc_msg_decline dclc;
 
 		rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
-				      SMC_CLC_DECLINE);
+				      SMC_CLC_DECLINE, CLC_WAIT_TIME_SHORT);
 		return rc == -EAGAIN ? SMC_CLC_DECL_TIMEOUT_AL : rc;
 	}
 
@@ -1222,7 +1223,7 @@ static void smc_listen_work(struct work_struct *work)
 	 */
 	pclc = (struct smc_clc_msg_proposal *)&buf;
 	reason_code = smc_clc_wait_msg(new_smc, pclc, SMC_CLC_MAX_LEN,
-				       SMC_CLC_PROPOSAL);
+				       SMC_CLC_PROPOSAL, CLC_WAIT_TIME);
 	if (reason_code) {
 		smc_listen_decline(new_smc, reason_code, 0);
 		return;
@@ -1272,7 +1273,7 @@ static void smc_listen_work(struct work_struct *work)
 
 	/* receive SMC Confirm CLC message */
 	reason_code = smc_clc_wait_msg(new_smc, &cclc, sizeof(cclc),
-				       SMC_CLC_CONFIRM);
+				       SMC_CLC_CONFIRM, CLC_WAIT_TIME);
 	if (reason_code) {
 		mutex_unlock(&smc_create_lgr_pending);
 		smc_listen_decline(new_smc, reason_code, local_contact);
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 62043d69e3a3..776e9dfc915d 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -265,7 +265,7 @@ int smc_clc_prfx_match(struct socket *clcsock,
  * clcsock error, -EINTR, -ECONNRESET, -EPROTO otherwise.
  */
 int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
-		     u8 expected_type)
+		     u8 expected_type, unsigned long timeout)
 {
 	long rcvtimeo = smc->clcsock->sk->sk_rcvtimeo;
 	struct sock *clc_sk = smc->clcsock->sk;
@@ -285,7 +285,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 	 * sizeof(struct smc_clc_msg_hdr)
 	 */
 	krflags = MSG_PEEK | MSG_WAITALL;
-	smc->clcsock->sk->sk_rcvtimeo = CLC_WAIT_TIME;
+	clc_sk->sk_rcvtimeo = timeout;
 	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1,
 			sizeof(struct smc_clc_msg_hdr));
 	len = sock_recvmsg(smc->clcsock, &msg, krflags);
@@ -351,7 +351,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 	}
 
 out:
-	smc->clcsock->sk->sk_rcvtimeo = rcvtimeo;
+	clc_sk->sk_rcvtimeo = rcvtimeo;
 	return reason_code;
 }
 
diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index 18da89b681c2..24658e8c0de4 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -27,6 +27,7 @@
 #define SMC_TYPE_D		1		/* SMC-D only		      */
 #define SMC_TYPE_B		3		/* SMC-R and SMC-D	      */
 #define CLC_WAIT_TIME		(6 * HZ)	/* max. wait time on clcsock  */
+#define CLC_WAIT_TIME_SHORT	HZ		/* short wait time on clcsock */
 #define SMC_CLC_DECL_MEM	0x01010000  /* insufficient memory resources  */
 #define SMC_CLC_DECL_TIMEOUT_CL	0x02010000  /* timeout w4 QP confirm link     */
 #define SMC_CLC_DECL_TIMEOUT_AL	0x02020000  /* timeout w4 QP add link	      */
@@ -182,7 +183,7 @@ struct smcd_dev;
 int smc_clc_prfx_match(struct socket *clcsock,
 		       struct smc_clc_msg_proposal_prefix *prop);
 int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
-		     u8 expected_type);
+		     u8 expected_type, unsigned long timeout);
 int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info);
 int smc_clc_send_proposal(struct smc_sock *smc, int smc_type,
 			  struct smc_ib_device *smcibdev, u8 ibport, u8 gid[],
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 09/10] net/smc: add infrastructure to send delete rkey messages
From: Ursula Braun @ 2018-11-22  9:26 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>

From: Karsten Graul <kgraul@linux.ibm.com>

Add the infrastructure to send LLC messages of type DELETE RKEY to
unregister a shared memory region at the peer.

Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
 net/smc/smc_core.h |  3 +++
 net/smc/smc_llc.c  | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 net/smc/smc_llc.h  |  2 ++
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 07364c0b41a1..bce39d6df45a 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -109,6 +109,9 @@ struct smc_link {
 	int			llc_testlink_time; /* testlink interval */
 	struct completion	llc_confirm_rkey; /* wait 4 rx of cnf rkey */
 	int			llc_confirm_rkey_rc; /* rc from cnf rkey msg */
+	struct completion	llc_delete_rkey; /* wait 4 rx of del rkey */
+	int			llc_delete_rkey_rc; /* rc from del rkey msg */
+	struct mutex		llc_delete_rkey_mutex; /* serialize usage */
 };
 
 /* For now we just allow one parallel link per link group. The SMC protocol
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 132c6a8e49f8..a6d3623d06f4 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -238,6 +238,29 @@ static int smc_llc_send_confirm_rkey(struct smc_link *link,
 	return rc;
 }
 
+/* send LLC delete rkey request */
+static int smc_llc_send_delete_rkey(struct smc_link *link,
+				    struct smc_buf_desc *rmb_desc)
+{
+	struct smc_llc_msg_delete_rkey *rkeyllc;
+	struct smc_wr_tx_pend_priv *pend;
+	struct smc_wr_buf *wr_buf;
+	int rc;
+
+	rc = smc_llc_add_pending_send(link, &wr_buf, &pend);
+	if (rc)
+		return rc;
+	rkeyllc = (struct smc_llc_msg_delete_rkey *)wr_buf;
+	memset(rkeyllc, 0, sizeof(*rkeyllc));
+	rkeyllc->hd.common.type = SMC_LLC_DELETE_RKEY;
+	rkeyllc->hd.length = sizeof(struct smc_llc_msg_delete_rkey);
+	rkeyllc->num_rkeys = 1;
+	rkeyllc->rkey[0] = htonl(rmb_desc->mr_rx[SMC_SINGLE_LINK]->rkey);
+	/* send llc message */
+	rc = smc_wr_tx_send(link, pend);
+	return rc;
+}
+
 /* prepare an add link message */
 static void smc_llc_prep_add_link(struct smc_llc_msg_add_link *addllc,
 				  struct smc_link *link, u8 mac[], u8 gid[],
@@ -509,7 +532,9 @@ static void smc_llc_rx_delete_rkey(struct smc_link *link,
 	int i, max;
 
 	if (llc->hd.flags & SMC_LLC_FLAG_RESP) {
-		/* unused as long as we don't send this type of msg */
+		link->llc_delete_rkey_rc = llc->hd.flags &
+					    SMC_LLC_FLAG_RKEY_NEG;
+		complete(&link->llc_delete_rkey);
 	} else {
 		max = min_t(u8, llc->num_rkeys, SMC_LLC_DEL_RKEY_MAX);
 		for (i = 0; i < max; i++) {
@@ -610,6 +635,8 @@ int smc_llc_link_init(struct smc_link *link)
 	init_completion(&link->llc_add);
 	init_completion(&link->llc_add_resp);
 	init_completion(&link->llc_confirm_rkey);
+	init_completion(&link->llc_delete_rkey);
+	mutex_init(&link->llc_delete_rkey_mutex);
 	init_completion(&link->llc_testlink_resp);
 	INIT_DELAYED_WORK(&link->llc_testlink_wrk, smc_llc_testlink_work);
 	return 0;
@@ -650,6 +677,7 @@ int smc_llc_do_confirm_rkey(struct smc_link *link,
 {
 	int rc;
 
+	/* protected by mutex smc_create_lgr_pending */
 	reinit_completion(&link->llc_confirm_rkey);
 	rc = smc_llc_send_confirm_rkey(link, rmb_desc);
 	if (rc)
@@ -662,6 +690,29 @@ int smc_llc_do_confirm_rkey(struct smc_link *link,
 	return 0;
 }
 
+/* unregister an rtoken at the remote peer */
+int smc_llc_do_delete_rkey(struct smc_link *link,
+			   struct smc_buf_desc *rmb_desc)
+{
+	int rc;
+
+	mutex_lock(&link->llc_delete_rkey_mutex);
+	reinit_completion(&link->llc_delete_rkey);
+	rc = smc_llc_send_delete_rkey(link, rmb_desc);
+	if (rc)
+		goto out;
+	/* receive DELETE RKEY response from server over RoCE fabric */
+	rc = wait_for_completion_interruptible_timeout(&link->llc_delete_rkey,
+						       SMC_LLC_WAIT_TIME);
+	if (rc <= 0 || link->llc_delete_rkey_rc)
+		rc = -EFAULT;
+	else
+		rc = 0;
+out:
+	mutex_unlock(&link->llc_delete_rkey_mutex);
+	return rc;
+}
+
 /***************************** init, exit, misc ******************************/
 
 static struct smc_wr_rx_handler smc_llc_rx_handlers[] = {
diff --git a/net/smc/smc_llc.h b/net/smc/smc_llc.h
index 9e2ff088e301..461c0c3ef76e 100644
--- a/net/smc/smc_llc.h
+++ b/net/smc/smc_llc.h
@@ -49,6 +49,8 @@ void smc_llc_link_inactive(struct smc_link *link);
 void smc_llc_link_clear(struct smc_link *link);
 int smc_llc_do_confirm_rkey(struct smc_link *link,
 			    struct smc_buf_desc *rmb_desc);
+int smc_llc_do_delete_rkey(struct smc_link *link,
+			   struct smc_buf_desc *rmb_desc);
 int smc_llc_init(void) __init;
 
 #endif /* SMC_LLC_H */
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 08/10] net/smc: avoid a delay by waiting for nothing
From: Ursula Braun @ 2018-11-22  9:26 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>

From: Karsten Graul <kgraul@linux.ibm.com>

When a send failed then don't start to wait for a response in
smc_llc_do_confirm_rkey.

Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
 net/smc/smc_llc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 9c916c709ca7..132c6a8e49f8 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -651,7 +651,9 @@ int smc_llc_do_confirm_rkey(struct smc_link *link,
 	int rc;
 
 	reinit_completion(&link->llc_confirm_rkey);
-	smc_llc_send_confirm_rkey(link, rmb_desc);
+	rc = smc_llc_send_confirm_rkey(link, rmb_desc);
+	if (rc)
+		return rc;
 	/* receive CONFIRM RKEY response from server over RoCE fabric */
 	rc = wait_for_completion_interruptible_timeout(&link->llc_confirm_rkey,
 						       SMC_LLC_WAIT_TIME);
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 10/10] net/smc: unregister rkeys of unused buffer
From: Ursula Braun @ 2018-11-22  9:26 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181122092643.26820-1-ubraun@linux.ibm.com>

From: Karsten Graul <kgraul@linux.ibm.com>

When an rmb is no longer in use by a connection, unregister its rkey at
the remote peer with an LLC DELETE RKEY message. With this change,
unused buffers held in the buffer pool are no longer registered at the
remote peer. They are registered before the buffer is actually used and
unregistered when they are no longer used by a connection.

Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
 net/smc/af_smc.c   | 22 +++++++++++-----------
 net/smc/smc_core.c |  7 ++++++-
 net/smc/smc_core.h |  2 +-
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 7657e249f526..4b865250e238 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -299,14 +299,17 @@ static void smc_copy_sock_settings_to_smc(struct smc_sock *smc)
 	smc_copy_sock_settings(&smc->sk, smc->clcsock->sk, SK_FLAGS_CLC_TO_SMC);
 }
 
-/* register a new rmb, optionally send confirm_rkey msg to register with peer */
+/* register a new rmb, send confirm_rkey msg to register with peer */
 static int smc_reg_rmb(struct smc_link *link, struct smc_buf_desc *rmb_desc,
 		       bool conf_rkey)
 {
-	/* register memory region for new rmb */
-	if (smc_wr_reg_send(link, rmb_desc->mr_rx[SMC_SINGLE_LINK])) {
-		rmb_desc->regerr = 1;
-		return -EFAULT;
+	if (!rmb_desc->wr_reg) {
+		/* register memory region for new rmb */
+		if (smc_wr_reg_send(link, rmb_desc->mr_rx[SMC_SINGLE_LINK])) {
+			rmb_desc->regerr = 1;
+			return -EFAULT;
+		}
+		rmb_desc->wr_reg = 1;
 	}
 	if (!conf_rkey)
 		return 0;
@@ -581,8 +584,7 @@ static int smc_connect_rdma(struct smc_sock *smc,
 			return smc_connect_abort(smc, SMC_CLC_DECL_ERR_RDYLNK,
 						 local_contact);
 	} else {
-		if (!smc->conn.rmb_desc->reused &&
-		    smc_reg_rmb(link, smc->conn.rmb_desc, true))
+		if (smc_reg_rmb(link, smc->conn.rmb_desc, true))
 			return smc_connect_abort(smc, SMC_CLC_DECL_ERR_REGRMB,
 						 local_contact);
 	}
@@ -1143,10 +1145,8 @@ static int smc_listen_rdma_reg(struct smc_sock *new_smc, int local_contact)
 	struct smc_link *link = &new_smc->conn.lgr->lnk[SMC_SINGLE_LINK];
 
 	if (local_contact != SMC_FIRST_CONTACT) {
-		if (!new_smc->conn.rmb_desc->reused) {
-			if (smc_reg_rmb(link, new_smc->conn.rmb_desc, true))
-				return SMC_CLC_DECL_ERR_REGRMB;
-		}
+		if (smc_reg_rmb(link, new_smc->conn.rmb_desc, true))
+			return SMC_CLC_DECL_ERR_REGRMB;
 	}
 	smc_rmb_sync_sg_for_device(&new_smc->conn);
 
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index ec7a7ed3b968..1382ddae591e 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -298,8 +298,13 @@ static void smc_buf_unuse(struct smc_connection *conn,
 		conn->sndbuf_desc->used = 0;
 	if (conn->rmb_desc) {
 		if (!conn->rmb_desc->regerr) {
-			conn->rmb_desc->reused = 1;
 			conn->rmb_desc->used = 0;
+			if (!lgr->is_smcd) {
+				/* unregister rmb with peer */
+				smc_llc_do_delete_rkey(
+						&lgr->lnk[SMC_SINGLE_LINK],
+						conn->rmb_desc);
+			}
 		} else {
 			/* buf registration failed, reuse not possible */
 			write_lock_bh(&lgr->rmbs_lock);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index bce39d6df45a..e177c6675038 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -130,7 +130,7 @@ struct smc_buf_desc {
 	struct page		*pages;
 	int			len;		/* length of buffer */
 	u32			used;		/* currently used / unused */
-	u8			reused	: 1;	/* new created / reused */
+	u8			wr_reg	: 1;	/* mem region registered */
 	u8			regerr	: 1;	/* err during registration */
 	union {
 		struct { /* SMC-R */
-- 
2.16.4

^ permalink raw reply related

* Re: [PATCH bpf-next v2 1/1] bpf, lpm: make longest_prefix_match() faster
From: Daniel Borkmann @ 2018-11-22 10:10 UTC (permalink / raw)
  To: Eric Dumazet, Alexei Starovoitov; +Cc: netdev, Eric Dumazet, Vlad Dumitrescu
In-Reply-To: <20181122053952.18244-1-edumazet@google.com>

On 11/22/2018 06:39 AM, Eric Dumazet wrote:
> At LPC 2018 in Vancouver, Vlad Dumitrescu mentioned that longest_prefix_match()
> has a high cost [1].
> 
> One reason for that cost is a loop handling one byte at a time.
> 
> We can handle more bytes at a time, if enough attention is paid
> to endianness.
> 
> I was able to remove ~55 % of longest_prefix_match() cpu costs.
> 
> [1] https://linuxplumbersconf.org/event/2/contributions/88/attachments/76/87/lpc-bpf-2018-shaping.pdf
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Vlad Dumitrescu <vladum@google.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>

Looks good, applied to bpf-next, thanks!

^ permalink raw reply

* Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
From: Daniel Borkmann @ 2018-11-22 10:27 UTC (permalink / raw)
  To: Alexei Starovoitov, Yonghong Song, Andrey Ignatov,
	netdev@vger.kernel.org
  Cc: ast@kernel.org, Kernel Team
In-Reply-To: <45175828-13df-6cf1-6817-ddffa381f0ec@fb.com>

On 11/21/2018 11:22 PM, Alexei Starovoitov wrote:
> On 11/21/18 12:18 PM, Yonghong Song wrote:
>> On 11/21/18 9:40 AM, Andrey Ignatov wrote:
>>> More and more projects use libbpf and one day it'll likely be packaged
>>> and distributed as DSO and that requires ABI versioning so that both
>>> compatible and incompatible changes to ABI can be introduced in a safe
>>> way in the future without breaking executables dynamically linked with a
>>> previous version of the library.
>>>
>>> Usual way to do ABI versioning is version script for the linker. Add
>>> such a script for libbpf. All global symbols currently exported via
>>> LIBBPF_API macro are added to the version script libbpf.map.
>>>
>>> The version name LIBBPF_0.0.1 is constructed from the name of the
>>> library + version specified by $(LIBBPF_VERSION) in Makefile.
>>>
>>> Version script does not duplicate the work done by LIBBPF_API macro, it
>>> rather complements it. The macro is used at compile time and can be used
>>> by compiler to do optimization that can't be done at link time, it is
>>> purely about global symbol visibility. The version script, in turn, is
>>> used at link time and takes care of ABI versioning. Both techniques are
>>> described in details in [1].
>>>
>>> Whenever ABI is changed in the future, version script should be changed
>>> appropriately.
>>
>> Maybe we should clarify the policy of how version numbers should be
>> change? Each commit which changes default global symbol ABI? Each kernel
>> release?

+1, could you add a documentation file into tools/lib/bpf/ where we
keep note on this process?

>>> [1] https://www.akkadia.org/drepper/dsohowto.pdf
>>>
>>> Signed-off-by: Andrey Ignatov <rdna@fb.com>
>>> ---
>>>    tools/lib/bpf/Makefile   |   4 +-
>>>    tools/lib/bpf/libbpf.map | 120 +++++++++++++++++++++++++++++++++++++++
>>>    2 files changed, 123 insertions(+), 1 deletion(-)
>>>    create mode 100644 tools/lib/bpf/libbpf.map
>>>
>>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>>> index 425b480bda75..d76c41fa2d39 100644
>>> --- a/tools/lib/bpf/Makefile
>>> +++ b/tools/lib/bpf/Makefile
>>> @@ -145,6 +145,7 @@ include $(srctree)/tools/build/Makefile.include
>>>    
>>>    BPF_IN    := $(OUTPUT)libbpf-in.o
>>>    LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
>>> +VERSION_SCRIPT := libbpf.map
>>>    
>>>    CMD_TARGETS = $(LIB_FILE)
>>>    
>>> @@ -170,7 +171,8 @@ $(BPF_IN): force elfdep bpfdep
>>>    	$(Q)$(MAKE) $(build)=libbpf
>>>    
>>>    $(OUTPUT)libbpf.so: $(BPF_IN)
>>> -	$(QUIET_LINK)$(CC) --shared $^ -o $@
>>> +	$(QUIET_LINK)$(CC) --shared -Wl,--version-script=$(VERSION_SCRIPT) \
>>> +		$^ -o $@
>>>    
>>>    $(OUTPUT)libbpf.a: $(BPF_IN)
>>>    	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>>> new file mode 100644
>>> index 000000000000..9fe416b68c7d
>>> --- /dev/null
>>> +++ b/tools/lib/bpf/libbpf.map
>>> @@ -0,0 +1,120 @@
>>> +LIBBPF_0.0.1 {
>>> +	global:
>>> +		bpf_btf_get_fd_by_id;
>>
>> Do you think we could use this opportunities to
>> make naming more consistent? For example,
>> bpf_btf_get_fd_by_id => btf__get_fd_by_id?
> 
> I think this one is fine since it matches
> bpf_[map|prog]_get_fd_by_id()
> and it's a wrapper.
> 
>>> +		bpf_create_map;
>>> +		bpf_create_map_in_map;
>>> +		bpf_create_map_in_map_node;
>>> +		bpf_create_map_name;
>>> +		bpf_create_map_node;
>>> +		bpf_create_map_xattr;
>>> +		bpf_load_btf;
>>> +		bpf_load_program;
>>> +		bpf_load_program_xattr;
>>> +		bpf_map__btf_key_type_id;
>>> +		bpf_map__btf_value_type_id;
>>> +		bpf_map__def;
>>> +		bpf_map_delete_elem; > +		bpf_map__fd;
>>> +		bpf_map_get_fd_by_id;
>>> +		bpf_map_get_next_id;
>>> +		bpf_map_get_next_key; > +		bpf_map__is_offload_neutral;
>>> +		bpf_map_lookup_and_delete_elem;
>>> +		bpf_map_lookup_elem;
>>> +		bpf_map__name;
>>> +		bpf_map__next;
>>> +		bpf_map__pin;
>>> +		bpf_map__prev;
>>> +		bpf_map__priv;
>>> +		bpf_map__reuse_fd;
>>> +		bpf_map__set_ifindex;
>>> +		bpf_map__set_priv;
>>> +		bpf_map__unpin;
>>> +		bpf_map_update_elem;
>>> +		bpf_object__btf_fd;
>>> +		bpf_object__close;
>>> +		bpf_object__find_map_by_name;
>>> +		bpf_object__find_map_by_offset;
>>> +		bpf_object__find_program_by_title;
>>> +		bpf_object__kversion;
>>> +		bpf_object__load;
>>> +		bpf_object__name;
>>> +		bpf_object__next;
>>> +		bpf_object__open;
>>> +		bpf_object__open_buffer;
>>> +		bpf_object__open_xattr;
>>> +		bpf_object__pin;
>>> +		bpf_object__pin_maps;
>>> +		bpf_object__pin_programs;
>>> +		bpf_object__priv;
>>> +		bpf_object__set_priv;
>>> +		bpf_object__unload;
>>> +		bpf_object__unpin_maps;
>>> +		bpf_object__unpin_programs;
>>> +		bpf_obj_get;
>>> +		bpf_obj_get_info_by_fd;
>>> +		bpf_obj_pin;
>>> +		bpf_perf_event_read_simple;
>>> +		bpf_prog_attach;
>>> +		bpf_prog_detach;
>>> +		bpf_prog_detach2;
>>> +		bpf_prog_get_fd_by_id;
>>> +		bpf_prog_get_next_id;
>>> +		bpf_prog_load;
>>> +		bpf_prog_load_xattr;
>>> +		bpf_prog_query;
>>> +		bpf_program__fd;
>>> +		bpf_program__is_kprobe;
>>> +		bpf_program__is_perf_event;
>>> +		bpf_program__is_raw_tracepoint;
>>> +		bpf_program__is_sched_act;
>>> +		bpf_program__is_sched_cls;
>>> +		bpf_program__is_socket_filter;
>>> +		bpf_program__is_tracepoint;
>>> +		bpf_program__is_xdp;
>>> +		bpf_program__load;
>>> +		bpf_program__next;
>>> +		bpf_program__nth_fd;
>>> +		bpf_program__pin;
>>> +		bpf_program__pin_instance;
>>> +		bpf_program__prev;
>>> +		bpf_program__priv;
>>> +		bpf_program__set_expected_attach_type;
>>> +		bpf_program__set_ifindex;
>>> +		bpf_program__set_kprobe;
>>> +		bpf_program__set_perf_event;
>>> +		bpf_program__set_prep;
>>> +		bpf_program__set_priv;
>>> +		bpf_program__set_raw_tracepoint;
>>> +		bpf_program__set_sched_act;
>>> +		bpf_program__set_sched_cls;
>>> +		bpf_program__set_socket_filter;
>>> +		bpf_program__set_tracepoint;
>>> +		bpf_program__set_type;
>>> +		bpf_program__set_xdp;
>>> +		bpf_program__title;
>>> +		bpf_program__unload;
>>> +		bpf_program__unpin;
>>> +		bpf_program__unpin_instance;
>>> +		bpf_prog_test_run;
>>> +		bpf_raw_tracepoint_open;
>>> +		bpf_set_link_xdp_fd;
>>> +		bpf_task_fd_query;
>>> +		bpf_verify_program;
>>> +		btf__fd;
>>> +		btf__find_by_name;
>>> +		btf__free;
>>> +		btf_get_from_id;
>> btf_get_from_id => btf__get_from_id?
> 
> this one makes sense indeed. Martin, thoughts?
> 
>>> +		btf__name_by_offset;
>>> +		btf__new;
>>> +		btf__resolve_size;
>>> +		btf__resolve_type;
>>> +		btf__type_by_id;
>>> +		libbpf_attach_type_by_name;
>>> +		libbpf_get_error;
>>> +		libbpf_prog_type_by_name;
>>> +		libbpf_set_print;
>>> +		libbpf_strerror;
>>
>> Anything else? We have btf__, bpf_program__ prefixes with double "_"
>> while with libbpf_, bpf_<wrapper> as single "_". Not sure whether they
>> need change or not.
> 
> the convention is that syscall wrappers like bpf_map_lookup_elem()
> have single _ and map one-to-one to syscall commands.
> Double _ is for objects. That's a coding style of perf.
> Today btf, bpf_program, bpf_map, bpf_objects are objects.
> libbpf_ is a prefix for auxiliary funcs.

With that in mind, should we also change prototype for things like
bpf_set_link_xdp_fd() and bpf_perf_event_read_simple()? It's not a
syscall wrapper mapping one-to-one either.

Would libbpf_ prefix be a better fit in here, or some new bpf_misc__
prefix for everything else which is not directly related to libbpf
internals and neither to the existing objects? (bpf_misc__ would
probably be my slight preference fwiw.)

> We need to document it in a readme file.

Fully agree, this should probably go to that same document so that
this convention is clear to everyone extending libbpf.

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH v2 1/2] bpf: add __weak hook for allocating executable memory
From: Ard Biesheuvel @ 2018-11-22 21:08 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, linux-arm-kernel, Daniel Borkmann, Alexei Starovoitov,
	Rick Edgecombe, Eric Dumazet, Jann Horn, Kees Cook, Jessica Yu,
	Arnd Bergmann, Catalin Marinas, Will Deacon, Mark Rutland,
	David S. Miller, linux-kernel, netdev
In-Reply-To: <201811230404.yavKPUOn%fengguang.wu@intel.com>



> On 22 Nov 2018, at 21:01, kbuild test robot <lkp@intel.com> wrote:
> 
> Hi Ard,
> 
> I love your patch! Yet something to improve:
> 

Ugh, apologies for this. I’m sure i build tested /something/ but clearly not what i should have tested.

In any case, following up on the discussion I’ll have to respin this in any case. I’ll triple check next time that what i send out was tested properly.


> [auto build test ERROR on bpf-next/master]
> [also build test ERROR on v4.20-rc3 next-20181122]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Ard-Biesheuvel/bpf-permit-JIT-allocations-to-be-served-outside-the-module-region/20181123-033144
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> config: x86_64-randconfig-x002-201846 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
>        # save the attached .config to linux build tree
>        make ARCH=x86_64 
> 
> All error/warnings (new ones prefixed by >>):
> 
>   kernel//bpf/core.c: In function 'bpf_jit_free_exec':
>>> kernel//bpf/core.c:632:24: error: 'size' undeclared (first use in this function); did you mean 'ksize'?
>     return module_memfree(size);
>                           ^~~~
>                           ksize
>   kernel//bpf/core.c:632:24: note: each undeclared identifier is reported only once for each function it appears in
>>> kernel//bpf/core.c:632:9: warning: 'return' with a value, in function returning void
>     return module_memfree(size);
>            ^~~~~~~~~~~~~~
>   kernel//bpf/core.c:630:13: note: declared here
>    void __weak bpf_jit_free_exec(const void *addr)
>                ^~~~~~~~~~~~~~~~~
> 
> vim +632 kernel//bpf/core.c
> 
>   629    
>   630    void __weak bpf_jit_free_exec(const void *addr)
>   631    {
>> 632        return module_memfree(size);
>   633    }
>   634    
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> <.config.gz>

^ permalink raw reply

* Re: [PATCH v3 net-next 04/12] net: ethernet: Use phy_set_max_speed() to limit advertised speed
From: Anssi Hannula @ 2018-11-22 10:40 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli
In-Reply-To: <1536709999-13420-5-git-send-email-andrew@lunn.ch>

Hi,

On 12.9.2018 2:53, Andrew Lunn wrote:
> Many Ethernet MAC drivers want to limit the PHY to only advertise a
> maximum speed of 100Mbs or 1Gbps. Rather than using a mask, make use
> of the helper function phy_set_max_speed().

But what if the PHY does not support 1Gbps in the first place?

This now adds 1Gbps to ->supported and ->advertising even on PHYs that
do not support 1Gbps or have the max speed limited to e.g. 100M via the
"max-speed" device tree property, breaking things.

Unless I'm missing something on how this is supposed to work now :)

> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/ethernet/8390/ax88796.c               |  4 +---
>  drivers/net/ethernet/aeroflex/greth.c             |  4 ++--
>  drivers/net/ethernet/agere/et131x.c               | 12 ++----------
>  drivers/net/ethernet/allwinner/sun4i-emac.c       |  3 +--
>  drivers/net/ethernet/altera/altera_tse_main.c     |  5 +----
>  drivers/net/ethernet/amd/au1000_eth.c             | 12 +-----------
>  drivers/net/ethernet/broadcom/bcm63xx_enet.c      | 10 ++--------
>  drivers/net/ethernet/broadcom/genet/bcmmii.c      |  2 +-
>  drivers/net/ethernet/broadcom/sb1250-mac.c        | 11 ++---------
>  drivers/net/ethernet/broadcom/tg3.c               |  8 ++++----
>  drivers/net/ethernet/cadence/macb_main.c          |  4 ++--
>  drivers/net/ethernet/cortina/gemini.c             |  2 +-
>  drivers/net/ethernet/dnet.c                       |  4 ++--
>  drivers/net/ethernet/ethoc.c                      |  5 +----
>  drivers/net/ethernet/freescale/fec_main.c         |  4 ++--
>  drivers/net/ethernet/freescale/ucc_geth.c         |  7 +------
>  drivers/net/ethernet/lantiq_etop.c                | 11 ++---------
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c       |  4 ++--
>  drivers/net/ethernet/nxp/lpc_eth.c                |  3 +--
>  drivers/net/ethernet/rdc/r6040.c                  | 12 ++----------
>  drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c   |  4 ++--
>  drivers/net/ethernet/smsc/smsc911x.c              |  5 +++--
>  drivers/net/ethernet/smsc/smsc9420.c              |  5 +++--
>  drivers/net/ethernet/socionext/sni_ave.c          |  6 ++----
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  3 +--
>  drivers/net/ethernet/toshiba/tc35815.c            |  2 +-
>  drivers/net/ethernet/xilinx/xilinx_emaclite.c     |  3 +--
>  drivers/staging/mt7621-eth/mdio.c                 |  2 +-
>  28 files changed, 47 insertions(+), 110 deletions(-)
[...]
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 16e4ef7d7185..bd4095c3a031 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -544,9 +544,9 @@ static int macb_mii_probe(struct net_device *dev)
>  
>  	/* mask with MAC supported features */
>  	if (macb_is_gem(bp) && bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
> -		phydev->supported &= PHY_GBIT_FEATURES;
> +		phy_set_max_speed(phydev, SPEED_1000);
>  	else
> -		phydev->supported &= PHY_BASIC_FEATURES;
> +		phy_set_max_speed(phydev, SPEED_100);
>  
>  	if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
>  		phydev->supported &= ~SUPPORTED_1000baseT_Half;

[...]

-- 
Anssi Hannula / Bitwise Oy

^ permalink raw reply

* Re: [PATCH v2 1/2] bpf: add __weak hook for allocating executable memory
From: kbuild test robot @ 2018-11-22 21:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Mark Rutland, Eric Dumazet, Daniel Borkmann, Ard Biesheuvel,
	Catalin Marinas, Jann Horn, Will Deacon, Alexei Starovoitov,
	linux-kernel, netdev, Arnd Bergmann, kbuild-all, Jessica Yu,
	Rick Edgecombe, David S. Miller, linux-arm-kernel, Kees Cook
In-Reply-To: <20181121131733.14910-2-ard.biesheuvel@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

Hi Ard,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]
[also build test ERROR on v4.20-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ard-Biesheuvel/bpf-permit-JIT-allocations-to-be-served-outside-the-module-region/20181123-033144
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: i386-randconfig-s0-11191736 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/bpf/core.c: In function 'bpf_jit_free_exec':
>> kernel/bpf/core.c:632:24: error: 'size' undeclared (first use in this function)
     return module_memfree(size);
                           ^~~~
   kernel/bpf/core.c:632:24: note: each undeclared identifier is reported only once for each function it appears in
   kernel/bpf/core.c:632:9: warning: 'return' with a value, in function returning void
     return module_memfree(size);
            ^~~~~~~~~~~~~~
   kernel/bpf/core.c:630:13: note: declared here
    void __weak bpf_jit_free_exec(const void *addr)
                ^~~~~~~~~~~~~~~~~

vim +/size +632 kernel/bpf/core.c

   629	
   630	void __weak bpf_jit_free_exec(const void *addr)
   631	{
 > 632		return module_memfree(size);
   633	}
   634	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36733 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] samples: bpf: fix: error handling regarding kprobe_events
From: Daniel Borkmann @ 2018-11-22 11:09 UTC (permalink / raw)
  To: Daniel T. Lee, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <20181121160751.581-1-danieltimlee@gmail.com>

On 11/21/2018 05:07 PM, Daniel T. Lee wrote:
> Currently, kprobe_events failure won't be handled properly.
> Due to calling system() indirectly to write to kprobe_events,
> it can't be identified whether an error is derived from kprobe or system.
> 
>     // buf = "echo '%c:%s %s' >> /s/k/d/t/kprobe_events"
>     err = system(buf);
>     if (err < 0) {
>         printf("failed to create kprobe ..");
>         return -1;
>     }
> 
> For example, running ./tracex7 sample in ext4 partition,
> "echo p:open_ctree open_ctree >> /s/k/d/t/kprobe_events"
> gets 256 error code system() failure.
> => The error comes from kprobe, but it's not handled correctly.
> 
> According to man of system(3), it's return value
> just passes the termination status of the child shell
> rather than treating the error as -1. (don't care success)
> 
> Which means, currently it's not working as desired.
> (According to the upper code snippet)
> 
>     ex) running ./tracex7 with ext4 env.
>     # Current Output
>     sh: echo: I/O error
>     failed to open event open_ctree
> 
>     # Desired Output
>     failed to create kprobe 'open_ctree' error 'No such file or directory'
> 
> The problem is, error can't be verified whether from child ps or system.
> 
> But using write() directly can verify the command failure,
> and it will treat all error as -1.
> 
> So I suggest using write() directly to 'kprobe_events'
> rather than calling system().
> 
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>

Looks reasonable to me, thanks for the patch! One tiny nit below:

> ---
>  samples/bpf/bpf_load.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> index e6d7e0fe155b..3e34d733f5f8 100644
> --- a/samples/bpf/bpf_load.c
> +++ b/samples/bpf/bpf_load.c
> @@ -54,6 +54,23 @@ static int populate_prog_array(const char *event, int prog_fd)
>  	return 0;
>  }
>  
> +static int write_kprobe_events(const char *val)
> +{
> +	int ret, flags;
> +
> +	if ((val != NULL) && (val[0] == '\0'))
> +		flags = O_WRONLY | O_TRUNC;
> +	else
> +		flags = O_WRONLY | O_APPEND;
> +
> +	int fd = open("/sys/kernel/debug/tracing/kprobe_events", flags);

Nit: could you move fd declaration to above where you have ret and
flags instead of mixing with the code in order to match kernel style?

> +	ret = write(fd, val, strlen(val));
> +	close(fd);
> +
> +	return ret;
> +}
> +
>  static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
>  {
>  	bool is_socket = strncmp(event, "socket", 6) == 0;
> @@ -165,10 +182,9 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
>  
>  #ifdef __x86_64__
>  		if (strncmp(event, "sys_", 4) == 0) {
> -			snprintf(buf, sizeof(buf),
> -				 "echo '%c:__x64_%s __x64_%s' >> /sys/kernel/debug/tracing/kprobe_events",
> -				 is_kprobe ? 'p' : 'r', event, event);
> -			err = system(buf);
> +			snprintf(buf, sizeof(buf), "%c:__x64_%s __x64_%s",
> +				is_kprobe ? 'p' : 'r', event, event);
> +			err = write_kprobe_events(buf);
>  			if (err >= 0) {
>  				need_normal_check = false;
>  				event_prefix = "__x64_";
> @@ -176,10 +192,9 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
>  		}
>  #endif
>  		if (need_normal_check) {
> -			snprintf(buf, sizeof(buf),
> -				 "echo '%c:%s %s' >> /sys/kernel/debug/tracing/kprobe_events",
> -				 is_kprobe ? 'p' : 'r', event, event);
> -			err = system(buf);
> +			snprintf(buf, sizeof(buf), "%c:%s %s",
> +				is_kprobe ? 'p' : 'r', event, event);
> +			err = write_kprobe_events(buf);
>  			if (err < 0) {
>  				printf("failed to create kprobe '%s' error '%s'\n",
>  				       event, strerror(errno));
> @@ -519,7 +534,7 @@ static int do_load_bpf_file(const char *path, fixup_map_cb fixup_map)
>  		return 1;
>  
>  	/* clear all kprobes */
> -	i = system("echo \"\" > /sys/kernel/debug/tracing/kprobe_events");
> +	i = write_kprobe_events("");
>  
>  	/* scan over all elf sections to get license and map info */
>  	for (i = 1; i < ehdr.e_shnum; i++) {
> 

Thanks,
Daniel

^ permalink raw reply

* Re: selftests/bpf :get_cgroup_id_user: File not found: /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id
From: Daniel Borkmann @ 2018-11-22 11:11 UTC (permalink / raw)
  To: Y Song, naresh.kamboju; +Cc: linux-kselftest, netdev, Yonghong Song
In-Reply-To: <CAH3MdRVOFSQYYYNL_43o5DuXuShqQWhVutzQ5rpytenhObPWxQ@mail.gmail.com>

Hi Naresh,

On 11/21/2018 06:53 PM, Y Song wrote:
> On Wed, Nov 21, 2018 at 3:44 AM Naresh Kamboju
> <naresh.kamboju@linaro.org> wrote:
>>
>> Kselftest bpf get_cgroup_id_user is failed on all devices.
>>
>> selftests: bpf: get_cgroup_id_user
>> main:PASS:setup_cgroup_environment
>> main:PASS:create_and_get_cgroup
>> main:PASS:join_cgroup
>> main:PASS:bpf_prog_load
>> main:PASS:bpf_find_map
>> main:PASS:bpf_find_map
>> main:FAIL:open err -1 errno 2
>> not ok 1..15 selftests: bpf: get_cgroup_id_user [FAIL]
>>
>> The strace output shows,
>> This expected file not found,
>> "/sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id"
>>
>> bpf(BPF_MAP_UPDATE_ELEM, {map_fd=5, key=0x7fff0c68c138,
>> value=0x7fff0c68c13c, flags=BPF_ANY}, 72) = 0
>> open(\"/sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id\",
>> O_RDONLY) = -1 ENOENT (No such file or directory)
>> write(1, \"main:FAIL:open err -1 errno 2\n\", 30main:FAIL:open err -1 errno 2
>>
>> Am I missing any pre-requirement ?
> 
> You probably miss kernel config CONFIG_FTRACE_SYSCALLS.

If that does the trick for you, could you add that to the BPF kselftest
config snippet?

Thanks,
Daniel

^ permalink raw reply

* [PATCH net-next 0/4] octeontx2-af: CGX LMAC link bringup and cleanups
From: Linu Cherian @ 2018-11-22 11:48 UTC (permalink / raw)
  To: netdev, davem; +Cc: Sunil.Goutham, Linu Cherian

From: Linu Cherian <lcherian@marvell.com>

Patch 1: Code cleanup
Patch 2: Adds support for an unhandled hardware configuration 
Patch 3: Preparatory patch for enabling cgx lmac links 
Patch 4: Support for enabling cgx lmac links 

Linu Cherian (4):
  octeontx2-af: Misc cleanups in cgx driver
  octeontx2-af: Handle non-contiguous CGX LMAC interfaces
  octeontx2-af: Unregister cgx event callbacks gracefully
  octeontx2-af: Bringup CGX LMAC links by default

 .../net/ethernet/marvell/octeontx2/af/cgx.c   | 108 ++++++++++++++++--
 .../net/ethernet/marvell/octeontx2/af/cgx.h   |  34 +++---
 .../ethernet/marvell/octeontx2/af/cgx_fw_if.h |   2 -
 .../net/ethernet/marvell/octeontx2/af/rvu.c   |  26 +++--
 .../net/ethernet/marvell/octeontx2/af/rvu.h   |   6 +-
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   |  97 ++++++++++++----
 .../ethernet/marvell/octeontx2/af/rvu_nix.c   |   6 +-
 7 files changed, 214 insertions(+), 65 deletions(-)

-- 
2.19.1

^ permalink raw reply

* [PATCH net-next 1/4] octeontx2-af: Misc cleanups in cgx driver
From: Linu Cherian @ 2018-11-22 11:48 UTC (permalink / raw)
  To: netdev, davem; +Cc: Sunil.Goutham, Linu Cherian
In-Reply-To: <20181122114837.1030-1-linuc.decode@gmail.com>

From: Linu Cherian <lcherian@marvell.com>

* Do CGX init before NIX init
  This would add consistency in NIX code that depends on cgx ports

* Few other misc cleanups
  - rvu_cgx_probe renamed as rvu_cgx_init for consistency
  - rvu_cgx_exit wrapper added to take care of the exit path
  - Added error check on cgx_lmac_event_handler_init
  - Minor cleanups in cgx.h related to tab alignment
  - Removed redundant ids from enum cgx_cmd_id

Signed-off-by: Linu Cherian <lcherian@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/cgx.h   | 28 +++++++++----------
 .../ethernet/marvell/octeontx2/af/cgx_fw_if.h |  2 --
 .../net/ethernet/marvell/octeontx2/af/rvu.c   | 26 +++++++++--------
 .../net/ethernet/marvell/octeontx2/af/rvu.h   |  4 +--
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   | 27 ++++++++++++------
 5 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
index 3bd38ed6d68b..8d7d77b3035c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
@@ -27,34 +27,34 @@
 
 /* Registers */
 #define CGXX_CMRX_CFG			0x00
-#define  CMR_EN					BIT_ULL(55)
-#define  DATA_PKT_TX_EN				BIT_ULL(53)
-#define  DATA_PKT_RX_EN				BIT_ULL(54)
-#define  CGX_LMAC_TYPE_SHIFT			40
-#define  CGX_LMAC_TYPE_MASK			0xF
+#define CMR_EN				BIT_ULL(55)
+#define DATA_PKT_TX_EN			BIT_ULL(53)
+#define DATA_PKT_RX_EN			BIT_ULL(54)
+#define CGX_LMAC_TYPE_SHIFT		40
+#define CGX_LMAC_TYPE_MASK		0xF
 #define CGXX_CMRX_INT			0x040
-#define  FW_CGX_INT				BIT_ULL(1)
+#define FW_CGX_INT			BIT_ULL(1)
 #define CGXX_CMRX_INT_ENA_W1S		0x058
 #define CGXX_CMRX_RX_ID_MAP		0x060
 #define CGXX_CMRX_RX_STAT0		0x070
 #define CGXX_CMRX_RX_LMACS		0x128
 #define CGXX_CMRX_RX_DMAC_CTL0		0x1F8
-#define  CGX_DMAC_CTL0_CAM_ENABLE		BIT_ULL(3)
-#define  CGX_DMAC_CAM_ACCEPT			BIT_ULL(3)
-#define  CGX_DMAC_MCAST_MODE			BIT_ULL(1)
-#define  CGX_DMAC_BCAST_MODE			BIT_ULL(0)
+#define CGX_DMAC_CTL0_CAM_ENABLE	BIT_ULL(3)
+#define CGX_DMAC_CAM_ACCEPT		BIT_ULL(3)
+#define CGX_DMAC_MCAST_MODE		BIT_ULL(1)
+#define CGX_DMAC_BCAST_MODE		BIT_ULL(0)
 #define CGXX_CMRX_RX_DMAC_CAM0		0x200
-#define  CGX_DMAC_CAM_ADDR_ENABLE		BIT_ULL(48)
+#define CGX_DMAC_CAM_ADDR_ENABLE	BIT_ULL(48)
 #define CGXX_CMRX_RX_DMAC_CAM1		0x400
-#define CGX_RX_DMAC_ADR_MASK			GENMASK_ULL(47, 0)
+#define CGX_RX_DMAC_ADR_MASK		GENMASK_ULL(47, 0)
 #define CGXX_CMRX_TX_STAT0		0x700
 #define CGXX_SCRATCH0_REG		0x1050
 #define CGXX_SCRATCH1_REG		0x1058
 #define CGX_CONST			0x2000
 #define CGXX_SPUX_CONTROL1		0x10000
-#define  CGXX_SPUX_CONTROL1_LBK			BIT_ULL(14)
+#define CGXX_SPUX_CONTROL1_LBK		BIT_ULL(14)
 #define CGXX_GMP_PCS_MRX_CTL		0x30000
-#define  CGXX_GMP_PCS_MRX_CTL_LBK		BIT_ULL(14)
+#define CGXX_GMP_PCS_MRX_CTL_LBK	BIT_ULL(14)
 
 #define CGX_COMMAND_REG			CGXX_SCRATCH1_REG
 #define CGX_EVENT_REG			CGXX_SCRATCH0_REG
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
index fa17af3f4ba7..2d9fe51c6616 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx_fw_if.h
@@ -78,8 +78,6 @@ enum cgx_cmd_id {
 	CGX_CMD_LINK_STATE_CHANGE,
 	CGX_CMD_MODE_CHANGE,		/* hot plug support */
 	CGX_CMD_INTF_SHUTDOWN,
-	CGX_CMD_IRQ_ENABLE,
-	CGX_CMD_IRQ_DISABLE,
 };
 
 /* async event ids */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index 40eb3ad725f5..4d061d971956 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -811,17 +811,26 @@ static int rvu_setup_hw_resources(struct rvu *rvu)
 
 	err = rvu_npc_init(rvu);
 	if (err)
-		return err;
+		goto exit;
+
+	err = rvu_cgx_init(rvu);
+	if (err)
+		goto exit;
 
 	err = rvu_npa_init(rvu);
 	if (err)
-		return err;
+		goto cgx_err;
 
 	err = rvu_nix_init(rvu);
 	if (err)
-		return err;
+		goto cgx_err;
 
 	return 0;
+
+cgx_err:
+	rvu_cgx_exit(rvu);
+exit:
+	return err;
 }
 
 /* NPA and NIX admin queue APIs */
@@ -2419,13 +2428,9 @@ static int rvu_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (err)
 		goto err_hwsetup;
 
-	err = rvu_cgx_probe(rvu);
-	if (err)
-		goto err_mbox;
-
 	err = rvu_flr_init(rvu);
 	if (err)
-		goto err_cgx;
+		goto err_mbox;
 
 	err = rvu_register_interrupts(rvu);
 	if (err)
@@ -2441,11 +2446,10 @@ static int rvu_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	rvu_unregister_interrupts(rvu);
 err_flr:
 	rvu_flr_wq_destroy(rvu);
-err_cgx:
-	rvu_cgx_wq_destroy(rvu);
 err_mbox:
 	rvu_mbox_destroy(&rvu->afpf_wq_info);
 err_hwsetup:
+	rvu_cgx_exit(rvu);
 	rvu_reset_all_blocks(rvu);
 	rvu_free_hw_resources(rvu);
 err_release_regions:
@@ -2465,7 +2469,7 @@ static void rvu_remove(struct pci_dev *pdev)
 
 	rvu_unregister_interrupts(rvu);
 	rvu_flr_wq_destroy(rvu);
-	rvu_cgx_wq_destroy(rvu);
+	rvu_cgx_exit(rvu);
 	rvu_mbox_destroy(&rvu->afpf_wq_info);
 	rvu_disable_sriov(rvu);
 	rvu_reset_all_blocks(rvu);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
index c8409bc5d9c3..96049e275335 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
@@ -316,8 +316,8 @@ static inline void rvu_get_cgx_lmac_id(u8 map, u8 *cgx_id, u8 *lmac_id)
 	*lmac_id = (map & 0xF);
 }
 
-int rvu_cgx_probe(struct rvu *rvu);
-void rvu_cgx_wq_destroy(struct rvu *rvu);
+int rvu_cgx_init(struct rvu *rvu);
+int rvu_cgx_exit(struct rvu *rvu);
 void *rvu_cgx_pdata(u8 cgx_id, struct rvu *rvu);
 int rvu_cgx_config_rxtx(struct rvu *rvu, u16 pcifunc, bool start);
 int rvu_mbox_handler_cgx_start_rxtx(struct rvu *rvu, struct msg_req *req,
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index 1de6eb528d08..1f1bc837bcd6 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -216,7 +216,7 @@ static void cgx_evhandler_task(struct work_struct *work)
 	} while (1);
 }
 
-static void cgx_lmac_event_handler_init(struct rvu *rvu)
+static int cgx_lmac_event_handler_init(struct rvu *rvu)
 {
 	struct cgx_event_cb cb;
 	int cgx, lmac, err;
@@ -228,7 +228,7 @@ static void cgx_lmac_event_handler_init(struct rvu *rvu)
 	rvu->cgx_evh_wq = alloc_workqueue("rvu_evh_wq", 0, 0);
 	if (!rvu->cgx_evh_wq) {
 		dev_err(rvu->dev, "alloc workqueue failed");
-		return;
+		return -ENOMEM;
 	}
 
 	cb.notify_link_chg = cgx_lmac_postevent; /* link change call back */
@@ -244,9 +244,11 @@ static void cgx_lmac_event_handler_init(struct rvu *rvu)
 					cgx, lmac);
 		}
 	}
+
+	return 0;
 }
 
-void rvu_cgx_wq_destroy(struct rvu *rvu)
+static void rvu_cgx_wq_destroy(struct rvu *rvu)
 {
 	if (rvu->cgx_evh_wq) {
 		flush_workqueue(rvu->cgx_evh_wq);
@@ -255,9 +257,9 @@ void rvu_cgx_wq_destroy(struct rvu *rvu)
 	}
 }
 
-int rvu_cgx_probe(struct rvu *rvu)
+int rvu_cgx_init(struct rvu *rvu)
 {
-	int i, err;
+	int cgx, err;
 
 	/* find available cgx ports */
 	rvu->cgx_cnt = cgx_get_cgx_cnt();
@@ -272,8 +274,8 @@ int rvu_cgx_probe(struct rvu *rvu)
 		return -ENOMEM;
 
 	/* Initialize the cgxdata table */
-	for (i = 0; i < rvu->cgx_cnt; i++)
-		rvu->cgx_idmap[i] = cgx_get_pdata(i);
+	for (cgx = 0; cgx < rvu->cgx_cnt; cgx++)
+		rvu->cgx_idmap[cgx] = cgx_get_pdata(cgx);
 
 	/* Map CGX LMAC interfaces to RVU PFs */
 	err = rvu_map_cgx_lmac_pf(rvu);
@@ -281,7 +283,16 @@ int rvu_cgx_probe(struct rvu *rvu)
 		return err;
 
 	/* Register for CGX events */
-	cgx_lmac_event_handler_init(rvu);
+	err = cgx_lmac_event_handler_init(rvu);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+int rvu_cgx_exit(struct rvu *rvu)
+{
+	rvu_cgx_wq_destroy(rvu);
 	return 0;
 }
 
-- 
2.19.1

^ permalink raw reply related

* [PATCH net-next 2/4] octeontx2-af: Handle non-contiguous CGX LMAC interfaces
From: Linu Cherian @ 2018-11-22 11:48 UTC (permalink / raw)
  To: netdev, davem; +Cc: Sunil.Goutham, Linu Cherian
In-Reply-To: <20181122114837.1030-1-linuc.decode@gmail.com>

From: Linu Cherian <lcherian@marvell.com>

For this, cgx_id(struct cgx) definition has been changed to
reflect cgx port id instead of device instance id.
Now cgx_id can be directly used as channel offset for NPC configuration.
Assumptions on contiguous cgx port ids has been removed from
nix_calibrate_x2p as well.

As a side effect, allocation of conversion tables that were based
on cgx count are changed to cgx port id max value.
Tables would return NULL for invalid cgx ports.

Signed-off-by: Linu Cherian <lcherian@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/cgx.c   | 18 ++++++---
 .../net/ethernet/marvell/octeontx2/af/cgx.h   |  4 +-
 .../net/ethernet/marvell/octeontx2/af/rvu.h   |  2 +-
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   | 38 ++++++++++---------
 .../ethernet/marvell/octeontx2/af/rvu_nix.c   |  6 ++-
 5 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index 12db256c8c9f..5c9736d7571e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -92,17 +92,21 @@ static inline struct lmac *lmac_pdata(u8 lmac_id, struct cgx *cgx)
 	return cgx->lmac_idmap[lmac_id];
 }
 
-int cgx_get_cgx_cnt(void)
+int cgx_get_cgxcnt_max(void)
 {
 	struct cgx *cgx_dev;
-	int count = 0;
+	int idmax = -ENODEV;
 
 	list_for_each_entry(cgx_dev, &cgx_list, cgx_list)
-		count++;
+		if (cgx_dev->cgx_id > idmax)
+			idmax = cgx_dev->cgx_id;
 
-	return count;
+	if (idmax < 0)
+		return 0;
+
+	return idmax + 1;
 }
-EXPORT_SYMBOL(cgx_get_cgx_cnt);
+EXPORT_SYMBOL(cgx_get_cgxcnt_max);
 
 int cgx_get_lmac_cnt(void *cgxd)
 {
@@ -679,8 +683,10 @@ static int cgx_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto err_release_regions;
 	}
 
+	cgx->cgx_id = (pci_resource_start(pdev, PCI_CFG_REG_BAR_NUM) >> 24)
+		& CGX_ID_MASK;
+
 	list_add(&cgx->cgx_list, &cgx_list);
-	cgx->cgx_id = cgx_get_cgx_cnt() - 1;
 
 	cgx_link_usertable_init();
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
index 8d7d77b3035c..5f186e4307b1 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
@@ -20,7 +20,7 @@
 /* PCI BAR nos */
 #define PCI_CFG_REG_BAR_NUM		0
 
-#define MAX_CGX				3
+#define CGX_ID_MASK			0x7
 #define MAX_LMAC_PER_CGX		4
 #define CGX_FIFO_LEN			65536 /* 64K for both Rx & Tx */
 #define CGX_OFFSET(x)			((x) * MAX_LMAC_PER_CGX)
@@ -95,7 +95,7 @@ struct cgx_event_cb {
 
 extern struct pci_driver cgx_driver;
 
-int cgx_get_cgx_cnt(void);
+int cgx_get_cgxcnt_max(void);
 int cgx_get_lmac_cnt(void *cgxd);
 void *cgx_get_pdata(int cgx_id);
 int cgx_set_pkind(void *cgxd, u8 lmac_id, int pkind);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
index 96049e275335..ae8e2e206c87 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
@@ -226,7 +226,7 @@ struct rvu {
 	/* CGX */
 #define PF_CGXMAP_BASE		1 /* PF 0 is reserved for RVU PF */
 	u8			cgx_mapped_pfs;
-	u8			cgx_cnt; /* available cgx ports */
+	u8			cgx_cnt_max;	 /* CGX port count max */
 	u8			*pf2cgxlmac_map; /* pf to cgx_lmac map */
 	u16			*cgxlmac2pf_map; /* bitmap of mapped pfs for
 						  * every cgx lmac port
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index 1f1bc837bcd6..d5cca78d5d47 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -52,7 +52,7 @@ static inline u8 cgxlmac_id_to_bmap(u8 cgx_id, u8 lmac_id)
 
 void *rvu_cgx_pdata(u8 cgx_id, struct rvu *rvu)
 {
-	if (cgx_id >= rvu->cgx_cnt)
+	if (cgx_id >= rvu->cgx_cnt_max)
 		return NULL;
 
 	return rvu->cgx_idmap[cgx_id];
@@ -61,38 +61,40 @@ void *rvu_cgx_pdata(u8 cgx_id, struct rvu *rvu)
 static int rvu_map_cgx_lmac_pf(struct rvu *rvu)
 {
 	struct npc_pkind *pkind = &rvu->hw->pkind;
-	int cgx_cnt = rvu->cgx_cnt;
+	int cgx_cnt_max = rvu->cgx_cnt_max;
 	int cgx, lmac_cnt, lmac;
 	int pf = PF_CGXMAP_BASE;
 	int size, free_pkind;
 
-	if (!cgx_cnt)
+	if (!cgx_cnt_max)
 		return 0;
 
-	if (cgx_cnt > 0xF || MAX_LMAC_PER_CGX > 0xF)
+	if (cgx_cnt_max > 0xF || MAX_LMAC_PER_CGX > 0xF)
 		return -EINVAL;
 
 	/* Alloc map table
 	 * An additional entry is required since PF id starts from 1 and
 	 * hence entry at offset 0 is invalid.
 	 */
-	size = (cgx_cnt * MAX_LMAC_PER_CGX + 1) * sizeof(u8);
-	rvu->pf2cgxlmac_map = devm_kzalloc(rvu->dev, size, GFP_KERNEL);
+	size = (cgx_cnt_max * MAX_LMAC_PER_CGX + 1) * sizeof(u8);
+	rvu->pf2cgxlmac_map = devm_kmalloc(rvu->dev, size, GFP_KERNEL);
 	if (!rvu->pf2cgxlmac_map)
 		return -ENOMEM;
 
-	/* Initialize offset 0 with an invalid cgx and lmac id */
-	rvu->pf2cgxlmac_map[0] = 0xFF;
+	/* Initialize all entries with an invalid cgx and lmac id */
+	memset(rvu->pf2cgxlmac_map, 0xFF, size);
 
 	/* Reverse map table */
 	rvu->cgxlmac2pf_map = devm_kzalloc(rvu->dev,
-				  cgx_cnt * MAX_LMAC_PER_CGX * sizeof(u16),
+				  cgx_cnt_max * MAX_LMAC_PER_CGX * sizeof(u16),
 				  GFP_KERNEL);
 	if (!rvu->cgxlmac2pf_map)
 		return -ENOMEM;
 
 	rvu->cgx_mapped_pfs = 0;
-	for (cgx = 0; cgx < cgx_cnt; cgx++) {
+	for (cgx = 0; cgx < cgx_cnt_max; cgx++) {
+		if (!rvu_cgx_pdata(cgx, rvu))
+			continue;
 		lmac_cnt = cgx_get_lmac_cnt(rvu_cgx_pdata(cgx, rvu));
 		for (lmac = 0; lmac < lmac_cnt; lmac++, pf++) {
 			rvu->pf2cgxlmac_map[pf] = cgxlmac_id_to_bmap(cgx, lmac);
@@ -234,7 +236,7 @@ static int cgx_lmac_event_handler_init(struct rvu *rvu)
 	cb.notify_link_chg = cgx_lmac_postevent; /* link change call back */
 	cb.data = rvu;
 
-	for (cgx = 0; cgx < rvu->cgx_cnt; cgx++) {
+	for (cgx = 0; cgx <= rvu->cgx_cnt_max; cgx++) {
 		cgxd = rvu_cgx_pdata(cgx, rvu);
 		for (lmac = 0; lmac < cgx_get_lmac_cnt(cgxd); lmac++) {
 			err = cgx_lmac_evh_register(&cb, cgxd, lmac);
@@ -261,20 +263,22 @@ int rvu_cgx_init(struct rvu *rvu)
 {
 	int cgx, err;
 
-	/* find available cgx ports */
-	rvu->cgx_cnt = cgx_get_cgx_cnt();
-	if (!rvu->cgx_cnt) {
+	/* CGX port id starts from 0 and are not necessarily contiguous
+	 * Hence we allocate resources based on the maximum port id value.
+	 */
+	rvu->cgx_cnt_max = cgx_get_cgxcnt_max();
+	if (!rvu->cgx_cnt_max) {
 		dev_info(rvu->dev, "No CGX devices found!\n");
 		return -ENODEV;
 	}
 
-	rvu->cgx_idmap = devm_kzalloc(rvu->dev, rvu->cgx_cnt * sizeof(void *),
-				      GFP_KERNEL);
+	rvu->cgx_idmap = devm_kzalloc(rvu->dev, rvu->cgx_cnt_max *
+				      sizeof(void *), GFP_KERNEL);
 	if (!rvu->cgx_idmap)
 		return -ENOMEM;
 
 	/* Initialize the cgxdata table */
-	for (cgx = 0; cgx < rvu->cgx_cnt; cgx++)
+	for (cgx = 0; cgx < rvu->cgx_cnt_max; cgx++)
 		rvu->cgx_idmap[cgx] = cgx_get_pdata(cgx);
 
 	/* Map CGX LMAC interfaces to RVU PFs */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index b7998f6be386..962a82f7d141 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -2107,8 +2107,10 @@ static int nix_calibrate_x2p(struct rvu *rvu, int blkaddr)
 
 	status = rvu_read64(rvu, blkaddr, NIX_AF_STATUS);
 	/* Check if CGX devices are ready */
-	for (idx = 0; idx < cgx_get_cgx_cnt(); idx++) {
-		if (status & (BIT_ULL(16 + idx)))
+	for (idx = 0; idx < rvu->cgx_cnt_max; idx++) {
+		/* Skip when cgx port is not available */
+		if (!rvu_cgx_pdata(idx, rvu) ||
+		    (status & (BIT_ULL(16 + idx))))
 			continue;
 		dev_err(rvu->dev,
 			"CGX%d didn't respond to NIX X2P calibration\n", idx);
-- 
2.19.1

^ permalink raw reply related

* [PATCH net-next 3/4] octeontx2-af: Unregister cgx event callbacks gracefully
From: Linu Cherian @ 2018-11-22 11:48 UTC (permalink / raw)
  To: netdev, davem; +Cc: Sunil.Goutham, Linu Cherian
In-Reply-To: <20181122114837.1030-1-linuc.decode@gmail.com>

From: Linu Cherian <lcherian@marvell.com>

Added provision to unregister cgx event callbacks.
This enables the exit path to ensure event callbacks are
unregistered before workqueues get destroyed.

Signed-off-by: Linu Cherian <lcherian@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/cgx.c   | 29 ++++++++++++++++++-
 .../net/ethernet/marvell/octeontx2/af/cgx.h   |  1 +
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   | 14 +++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index 5c9736d7571e..5776978a4647 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -31,6 +31,7 @@
  * @resp:		command response
  * @link_info:		link related information
  * @event_cb:		callback for linkchange events
+ * @event_cb_lock:	lock for serializing callback with unregister
  * @cmd_pend:		flag set before new command is started
  *			flag cleared after command response is received
  * @cgx:		parent cgx port
@@ -43,6 +44,7 @@ struct lmac {
 	u64 resp;
 	struct cgx_link_user_info link_info;
 	struct cgx_event_cb event_cb;
+	spinlock_t event_cb_lock;
 	bool cmd_pend;
 	struct cgx *cgx;
 	u8 lmac_id;
@@ -449,6 +451,9 @@ static inline void cgx_link_change_handler(u64 lstat,
 	lmac->link_info = event.link_uinfo;
 	linfo = &lmac->link_info;
 
+	/* Ensure callback doesn't get unregistered until we finish it */
+	spin_lock(&lmac->event_cb_lock);
+
 	if (!lmac->event_cb.notify_link_chg) {
 		dev_dbg(dev, "cgx port %d:%d Link change handler null",
 			cgx->cgx_id, lmac->lmac_id);
@@ -459,11 +464,13 @@ static inline void cgx_link_change_handler(u64 lstat,
 		dev_info(dev, "cgx port %d:%d Link is %s %d Mbps\n",
 			 cgx->cgx_id, lmac->lmac_id,
 			 linfo->link_up ? "UP" : "DOWN", linfo->speed);
-		return;
+		goto err;
 	}
 
 	if (lmac->event_cb.notify_link_chg(&event, lmac->event_cb.data))
 		dev_err(dev, "event notification failure\n");
+err:
+	spin_unlock(&lmac->event_cb_lock);
 }
 
 static inline bool cgx_cmdresp_is_linkevent(u64 event)
@@ -552,6 +559,25 @@ int cgx_lmac_evh_register(struct cgx_event_cb *cb, void *cgxd, int lmac_id)
 }
 EXPORT_SYMBOL(cgx_lmac_evh_register);
 
+int cgx_lmac_evh_unregister(void *cgxd, int lmac_id)
+{
+	struct lmac *lmac;
+	unsigned long flags;
+	struct cgx *cgx = cgxd;
+
+	lmac = lmac_pdata(lmac_id, cgx);
+	if (!lmac)
+		return -ENODEV;
+
+	spin_lock_irqsave(&lmac->event_cb_lock, flags);
+	lmac->event_cb.notify_link_chg = NULL;
+	lmac->event_cb.data = NULL;
+	spin_unlock_irqrestore(&lmac->event_cb_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL(cgx_lmac_evh_unregister);
+
 static inline int cgx_fwi_read_version(u64 *resp, struct cgx *cgx)
 {
 	u64 req = 0;
@@ -606,6 +632,7 @@ static int cgx_lmac_init(struct cgx *cgx)
 		lmac->cgx = cgx;
 		init_waitqueue_head(&lmac->wq_cmd_cmplt);
 		mutex_init(&lmac->cmd_lock);
+		spin_lock_init(&lmac->event_cb_lock);
 		err = request_irq(pci_irq_vector(cgx->pdev,
 						 CGX_LMAC_FWI + i * 9),
 				   cgx_fwi_event_handler, 0, lmac->name, lmac);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
index 5f186e4307b1..a2bff00b17cf 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
@@ -100,6 +100,7 @@ int cgx_get_lmac_cnt(void *cgxd);
 void *cgx_get_pdata(int cgx_id);
 int cgx_set_pkind(void *cgxd, u8 lmac_id, int pkind);
 int cgx_lmac_evh_register(struct cgx_event_cb *cb, void *cgxd, int lmac_id);
+int cgx_lmac_evh_unregister(void *cgxd, int lmac_id);
 int cgx_get_tx_stats(void *cgxd, int lmac_id, int idx, u64 *tx_stat);
 int cgx_get_rx_stats(void *cgxd, int lmac_id, int idx, u64 *rx_stat);
 int cgx_lmac_rx_tx_enable(void *cgxd, int lmac_id, bool enable);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index d5cca78d5d47..66d94df9b8a2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -296,6 +296,20 @@ int rvu_cgx_init(struct rvu *rvu)
 
 int rvu_cgx_exit(struct rvu *rvu)
 {
+	int cgx, lmac;
+	void *cgxd;
+
+	for (cgx = 0; cgx <= rvu->cgx_cnt_max; cgx++) {
+		cgxd = rvu_cgx_pdata(cgx, rvu);
+		if (!cgxd)
+			continue;
+		for (lmac = 0; lmac < cgx_get_lmac_cnt(cgxd); lmac++)
+			cgx_lmac_evh_unregister(cgxd, lmac);
+	}
+
+	/* Ensure event handler unregister is completed */
+	mb();
+
 	rvu_cgx_wq_destroy(rvu);
 	return 0;
 }
-- 
2.19.1

^ permalink raw reply related

* [PATCH net-next 4/4] octeontx2-af: Bringup CGX LMAC links by default
From: Linu Cherian @ 2018-11-22 11:48 UTC (permalink / raw)
  To: netdev, davem; +Cc: Sunil.Goutham, Linu Cherian
In-Reply-To: <20181122114837.1030-1-linuc.decode@gmail.com>

From: Linu Cherian <lcherian@marvell.com>

- Added new CGX firmware interface API for sending link up/down
  commands

- Do link up for cgx lmac ports by default at the time of CGX
  driver probe. Since cgx link up in driver probe affects the
  Linux boot time, linkup procedure is kept threaded using
  workqueues.
  For this, a new cgx API cgx_lmac_linkup_start has been added.

Signed-off-by: Linu Cherian <lcherian@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/cgx.c   | 61 +++++++++++++++++++
 .../net/ethernet/marvell/octeontx2/af/cgx.h   |  1 +
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   | 20 ++++++
 3 files changed, 82 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index 5776978a4647..4c94571e03eb 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -57,6 +57,8 @@ struct cgx {
 	u8			cgx_id;
 	u8			lmac_count;
 	struct lmac		*lmac_idmap[MAX_LMAC_PER_CGX];
+	struct			work_struct cgx_cmd_work;
+	struct			workqueue_struct *cgx_cmd_workq;
 	struct list_head	cgx_list;
 };
 
@@ -68,6 +70,9 @@ static u32 cgx_speed_mbps[CGX_LINK_SPEED_MAX];
 /* Convert firmware lmac type encoding to string */
 static char *cgx_lmactype_string[LMAC_MODE_MAX];
 
+/* CGX PHY management internal APIs */
+static int cgx_fwi_link_change(struct cgx *cgx, int lmac_id, bool en);
+
 /* Supported devices */
 static const struct pci_device_id cgx_id_table[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_OCTEONTX2_CGX) },
@@ -578,6 +583,19 @@ int cgx_lmac_evh_unregister(void *cgxd, int lmac_id)
 }
 EXPORT_SYMBOL(cgx_lmac_evh_unregister);
 
+static int cgx_fwi_link_change(struct cgx *cgx, int lmac_id, bool enable)
+{
+	u64 req = 0;
+	u64 resp;
+
+	if (enable)
+		req = FIELD_SET(CMDREG_ID, CGX_CMD_LINK_BRING_UP, req);
+	else
+		req = FIELD_SET(CMDREG_ID, CGX_CMD_LINK_BRING_DOWN, req);
+
+	return cgx_fwi_cmd_generic(req, &resp, cgx, lmac_id);
+}
+
 static inline int cgx_fwi_read_version(u64 *resp, struct cgx *cgx)
 {
 	u64 req = 0;
@@ -611,6 +629,34 @@ static int cgx_lmac_verify_fwi_version(struct cgx *cgx)
 		return 0;
 }
 
+static void cgx_lmac_linkup_work(struct work_struct *work)
+{
+	struct cgx *cgx = container_of(work, struct cgx, cgx_cmd_work);
+	struct device *dev = &cgx->pdev->dev;
+	int i, err;
+
+	/* Do Link up for all the lmacs */
+	for (i = 0; i < cgx->lmac_count; i++) {
+		err = cgx_fwi_link_change(cgx, i, true);
+		if (err)
+			dev_info(dev, "cgx port %d:%d Link up command failed\n",
+				 cgx->cgx_id, i);
+	}
+}
+
+int cgx_lmac_linkup_start(void *cgxd)
+{
+	struct cgx *cgx = cgxd;
+
+	if (!cgx)
+		return -ENODEV;
+
+	queue_work(cgx->cgx_cmd_workq, &cgx->cgx_cmd_work);
+
+	return 0;
+}
+EXPORT_SYMBOL(cgx_lmac_linkup_start);
+
 static int cgx_lmac_init(struct cgx *cgx)
 {
 	struct lmac *lmac;
@@ -655,6 +701,12 @@ static int cgx_lmac_exit(struct cgx *cgx)
 	struct lmac *lmac;
 	int i;
 
+	if (cgx->cgx_cmd_workq) {
+		flush_workqueue(cgx->cgx_cmd_workq);
+		destroy_workqueue(cgx->cgx_cmd_workq);
+		cgx->cgx_cmd_workq = NULL;
+	}
+
 	/* Free all lmac related resources */
 	for (i = 0; i < cgx->lmac_count; i++) {
 		lmac = cgx->lmac_idmap[i];
@@ -713,6 +765,15 @@ static int cgx_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	cgx->cgx_id = (pci_resource_start(pdev, PCI_CFG_REG_BAR_NUM) >> 24)
 		& CGX_ID_MASK;
 
+	/* init wq for processing linkup requests */
+	INIT_WORK(&cgx->cgx_cmd_work, cgx_lmac_linkup_work);
+	cgx->cgx_cmd_workq = alloc_workqueue("cgx_cmd_workq", 0, 0);
+	if (!cgx->cgx_cmd_workq) {
+		dev_err(dev, "alloc workqueue failed for cgx cmd");
+		err = -ENOMEM;
+		goto err_release_regions;
+	}
+
 	list_add(&cgx->cgx_list, &cgx_list);
 
 	cgx_link_usertable_init();
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
index a2bff00b17cf..8c2be8493321 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
@@ -110,4 +110,5 @@ void cgx_lmac_promisc_config(int cgx_id, int lmac_id, bool enable);
 int cgx_lmac_internal_loopback(void *cgxd, int lmac_id, bool enable);
 int cgx_get_link_info(void *cgxd, int lmac_id,
 		      struct cgx_link_user_info *linfo);
+int cgx_lmac_linkup_start(void *cgxd);
 #endif /* CGX_H */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index 66d94df9b8a2..7d7133c5f799 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -238,6 +238,8 @@ static int cgx_lmac_event_handler_init(struct rvu *rvu)
 
 	for (cgx = 0; cgx <= rvu->cgx_cnt_max; cgx++) {
 		cgxd = rvu_cgx_pdata(cgx, rvu);
+		if (!cgxd)
+			continue;
 		for (lmac = 0; lmac < cgx_get_lmac_cnt(cgxd); lmac++) {
 			err = cgx_lmac_evh_register(&cb, cgxd, lmac);
 			if (err)
@@ -262,6 +264,7 @@ static void rvu_cgx_wq_destroy(struct rvu *rvu)
 int rvu_cgx_init(struct rvu *rvu)
 {
 	int cgx, err;
+	void *cgxd;
 
 	/* CGX port id starts from 0 and are not necessarily contiguous
 	 * Hence we allocate resources based on the maximum port id value.
@@ -291,6 +294,23 @@ int rvu_cgx_init(struct rvu *rvu)
 	if (err)
 		return err;
 
+	/* Ensure event handler registration is completed, before
+	 * we turn on the links
+	 */
+	mb();
+
+	/* Do link up for all CGX ports */
+	for (cgx = 0; cgx <= rvu->cgx_cnt_max; cgx++) {
+		cgxd = rvu_cgx_pdata(cgx, rvu);
+		if (!cgxd)
+			continue;
+		err = cgx_lmac_linkup_start(cgxd);
+		if (err)
+			dev_err(rvu->dev,
+				"Link up process failed to start on cgx %d\n",
+				cgx);
+	}
+
 	return 0;
 }
 
-- 
2.19.1

^ permalink raw reply related

* Re: [PATCH v2 2/2] arm64/bpf: don't allocate BPF JIT programs in module memory
From: Daniel Borkmann @ 2018-11-22 22:49 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Mark Rutland, Kees Cook, Eric Dumazet, Jann Horn, Catalin Marinas,
	Will Deacon, Alexei Starovoitov, Linux Kernel Mailing List,
	<netdev@vger.kernel.org>, Arnd Bergmann, Jessica Yu,
	Rick Edgecombe, David S. Miller, linux-arm-kernel
In-Reply-To: <CAKv+Gu-UE0hR-iEkhUieGn+UO_PFs+cD535W1Rq9iyRkgEj=qA@mail.gmail.com>

On 11/22/2018 09:02 AM, Ard Biesheuvel wrote:
> On Thu, 22 Nov 2018 at 00:20, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 11/21/2018 02:17 PM, Ard Biesheuvel wrote:
>>> The arm64 module region is a 128 MB region that is kept close to
>>> the core kernel, in order to ensure that relative branches are
>>> always in range. So using the same region for programs that do
>>> not have this restriction is wasteful, and preferably avoided.
>>>
>>> Now that the core BPF JIT code permits the alloc/free routines to
>>> be overridden, implement them by simple vmalloc_exec()/vfree()
>>> calls, which can be served from anywere. This also solves an
>>> issue under KASAN, where shadow memory is needlessly allocated for
>>> all BPF programs (which don't require KASAN shadow pages since
>>> they are not KASAN instrumented)
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  arch/arm64/net/bpf_jit_comp.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
>>> index a6fdaea07c63..f91b7c157841 100644
>>> --- a/arch/arm64/net/bpf_jit_comp.c
>>> +++ b/arch/arm64/net/bpf_jit_comp.c
>>> @@ -940,3 +940,13 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>>                                          tmp : orig_prog);
>>>       return prog;
>>>  }
>>> +
>>> +void *bpf_jit_alloc_exec(unsigned long size)
>>> +{
>>> +     return vmalloc_exec(size);
>>> +}
>>> +
>>> +void bpf_jit_free_exec(const void *addr)
>>> +{
>>> +     return vfree(size);
>>> +}
>>
>> Hmm, could you elaborate in the commit log on the potential performance
>> regression for JITed progs on arm64 after this change?
> 
> This does not affect the generated code, so I don't anticipate a
> performance hit. Did you have anything in particular in mind?

We do optimize immediate emission in the JIT, I was mostly wondering that
once the code is much further away from core kernel how much more insns we
might need to emit in some worst case for each BPF helper call, but then
unlike some other archs we always use absolute addresses so nothing would
change here, so never mind. (And BPF to BPF calls emits unoptimized 64
immediates since this is needed as we pass through the JIT several times
so we need this as place holder for later once the address is actually
known.)

>> I think this change would also break JITing of BPF to BPF calls. You might
>> have the same issue as ppc64 folks where the offset might not fit into imm
>> anymore and would have to transfer it via fp->aux->func[off]->bpf_func
>> instead.
> 
> If we are relying on BPF programs to remain within 128 MB of each
> other, then we already have a potential problem, given that the
> module_alloc() spills over into a 4 GB window if the 128 MB window is
> exhausted. Perhaps we should do something like

Hmm, good point, presumably you mean this one here fd045f6cd98e ("arm64:
add support for module PLTs"). Agree that this needs fixing.

> void *bpf_jit_alloc_exec(unsigned long size) {
>   return __vmalloc_node_range(size, MODULE_ALIGN,
>     BPF_REGION_START, BPF_REGION_END,
>     GFP_KERNEL, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
>     __builtin_return_address(0));
> }
> 
> and make [BPF_REGION_START, BPF_REGION_END) a separate 128 MB window
> at the top of the vmalloc space. That way, it is guaranteed that BPF
> programs are within branching range of each other, and we still solve
> the original problem. I also like that it becomes impossible to infer
> anything about the state of the vmalloc space, placement of the kernel
> and modules etc from the placement of the BPF programs (in case it
> leaks this information in one way or the other)
> 
> That would only give you space for 128M/4K == 32768 programs (or
> 128M/64K == 2048 on 64k pages kernels). So I guess we'd still need a

Note that it's 4k BPF insns which do not map 1:1, if possible I'd actually
prefer if we could enlarge this space a bit.

> spillover window as well, in which case we'd need a fix for the
> BPF-to-BPF branching issue (but we need that at the moment anyway)

Yeah, or spillover window instead. I think as a fix starting out with
its own region without the spillover window would be less complex and
better suited for stable?

Thanks,
Daniel

^ permalink raw reply

* Re: DSA support for Marvell 88e6065 switch
From: Pavel Machek @ 2018-11-22 13:21 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, f.fainelli, buytenh, buytenh, nico
In-Reply-To: <20181119032535.GA15561@lunn.ch>

[-- Attachment #1: Type: text/plain, Size: 1626 bytes --]

On Mon 2018-11-19 04:25:35, Andrew Lunn wrote:
> > If I wanted it to work, what do I need to do? AFAICT phy autoprobing
> > should just attach it as soon as it is compiled in?
> 
> Nope. It is a switch, not a PHY. Switches are never auto-probed
> because they are not guaranteed to have ID registers.
> 
> You need to use the legacy device tree binding. Look in
> Documentation/devicetree/bindings/net/dsa/dsa.txt, section Deprecated
> Binding. You can get more examples if you checkout old kernels. Or
> kirkwood-rd88f6281.dtsi, the dsa { } node which is disabled.

Thanks; I ported code from mv88e66xx in the meantime, and switch
appears to be detected.

But I'm running into problems with tagging code, and I guess I'd like
some help understanding.

tag_trailer: allocates new skb, then copies data around.

tag_qca: does dev->stats.tx_packets++, and reuses existing skb.

tag_brcm: reuses existing skb.

Is qca wrong in adjusting the statistics? Why does trailer allocate
new skb?

6065 seems to use 2-byte header between "SFD" and "Destination
address" in the ethernet frame. That's ... strange place to put
header, as addresses are now shifted. I need to put ethernet in
promisc mode (by running tcpdump) to get data moving.. and can not
figure out what to do in tag_...

Can I model it after tag_qca.c (simplified, because addresses will not
need to be memmoved) or is there some special consideration I am
missing?

Thanks,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* maximum number of cpu cores for interrupt handling
From: Oleg @ 2018-11-22 13:21 UTC (permalink / raw)
  To: netdev

  Hi, all.

I found info that IO APIC limit the number of cpu cores for
interrupt handling to 8(because of just 4 bits using for cpu number
or something like this). But now MSI/MSI-X is used for interrupts
and i don't know is this limit is actual now.

For example, i have 4 nic with 8 rx/tx queues each and 32 cpu cores.
Can i scale nic interrupt handling to all this cores with smp_affinity?

Thanks.

-- 
Олег Неманов (Oleg Nemanov)

^ permalink raw reply

* [PATCH v3 0/4] Fix unsafe BPF_PROG_TEST_RUN interface
From: Lorenz Bauer @ 2018-11-22 14:09 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181116125329.3974-1-lmb@cloudflare.com>

Right now, there is no safe way to use BPF_PROG_TEST_RUN with data_out.
This is because bpf_test_finish copies the output buffer to user space
without checking its size. This can lead to the kernel overwriting
data in user space after the buffer if xdp_adjust_head and friends are
in play.

Changes in v3:
* Introduce bpf_prog_test_run_xattr instead of modifying the existing
  function

Changes in v2:
* Make the syscall return ENOSPC if data_size_out is too small
* Make bpf_prog_test_run return EINVAL if size_out is missing
* Document the new behaviour of data_size_out

Lorenz Bauer (4):
  bpf: respect size hint to BPF_PROG_TEST_RUN if present
  tools: sync uapi/linux/bpf.h
  libbpf: add bpf_prog_test_run_xattr
  selftests: add a test for bpf_prog_test_run_xattr

 include/uapi/linux/bpf.h                 |  7 +++-
 net/bpf/test_run.c                       | 15 +++++++-
 tools/include/uapi/linux/bpf.h           |  7 +++-
 tools/lib/bpf/bpf.c                      | 27 +++++++++++++
 tools/lib/bpf/bpf.h                      | 13 +++++++
 tools/testing/selftests/bpf/test_progs.c | 49 ++++++++++++++++++++++++
 6 files changed, 112 insertions(+), 6 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH v3 1/4] bpf: respect size hint to BPF_PROG_TEST_RUN if present
From: Lorenz Bauer @ 2018-11-22 14:09 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181122140910.1079-1-lmb@cloudflare.com>

Use data_size_out as a size hint when copying test output to user space.
ENOSPC is returned if the output buffer is too small.
Callers which so far did not set data_size_out are not affected.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 include/uapi/linux/bpf.h |  7 +++++--
 net/bpf/test_run.c       | 15 +++++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 05d95290b848..7f5a7d032cd1 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -356,8 +356,11 @@ union bpf_attr {
 	struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
 		__u32		prog_fd;
 		__u32		retval;
-		__u32		data_size_in;
-		__u32		data_size_out;
+		__u32		data_size_in;	/* input: len of data_in */
+		__u32		data_size_out;	/* input/output: len of data_out
+						 *   returns ENOSPC if data_out
+						 *   is too small.
+						 */
 		__aligned_u64	data_in;
 		__aligned_u64	data_out;
 		__u32		repeat;
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index c89c22c49015..7663e6a57280 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -74,8 +74,18 @@ static int bpf_test_finish(const union bpf_attr *kattr,
 {
 	void __user *data_out = u64_to_user_ptr(kattr->test.data_out);
 	int err = -EFAULT;
+	u32 copy_size = size;
 
-	if (data_out && copy_to_user(data_out, data, size))
+	/* Clamp copy if the user has provided a size hint, but copy the full
+	 * buffer if not to retain old behaviour.
+	 */
+	if (kattr->test.data_size_out &&
+	    copy_size > kattr->test.data_size_out) {
+		copy_size = kattr->test.data_size_out;
+		err = -ENOSPC;
+	}
+
+	if (data_out && copy_to_user(data_out, data, copy_size))
 		goto out;
 	if (copy_to_user(&uattr->test.data_size_out, &size, sizeof(size)))
 		goto out;
@@ -83,7 +93,8 @@ static int bpf_test_finish(const union bpf_attr *kattr,
 		goto out;
 	if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration)))
 		goto out;
-	err = 0;
+	if (err != -ENOSPC)
+		err = 0;
 out:
 	return err;
 }
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 2/4] tools: sync uapi/linux/bpf.h
From: Lorenz Bauer @ 2018-11-22 14:09 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181122140910.1079-1-lmb@cloudflare.com>

Pull changes from "bpf: respect size hint to BPF_PROG_TEST_RUN if present".

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/include/uapi/linux/bpf.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 05d95290b848..7f5a7d032cd1 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -356,8 +356,11 @@ union bpf_attr {
 	struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
 		__u32		prog_fd;
 		__u32		retval;
-		__u32		data_size_in;
-		__u32		data_size_out;
+		__u32		data_size_in;	/* input: len of data_in */
+		__u32		data_size_out;	/* input/output: len of data_out
+						 *   returns ENOSPC if data_out
+						 *   is too small.
+						 */
 		__aligned_u64	data_in;
 		__aligned_u64	data_out;
 		__u32		repeat;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 3/4] libbpf: add bpf_prog_test_run_xattr
From: Lorenz Bauer @ 2018-11-22 14:09 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181122140910.1079-1-lmb@cloudflare.com>

Add a new function, which encourages safe usage of the test interface.
bpf_prog_test_run continues to work as before, but should be considered
unsafe.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/lib/bpf/bpf.c | 27 +++++++++++++++++++++++++++
 tools/lib/bpf/bpf.h | 13 +++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 961e1b9fc592..f8518bef6886 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -424,6 +424,33 @@ int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
 	return ret;
 }
 
+int bpf_prog_test_run_xattr(const struct bpf_prog_test_run_attr *test_attr,
+			    __u32 *size_out, __u32 *retval, __u32 *duration)
+{
+	union bpf_attr attr;
+	int ret;
+
+	if (!test_attr->data_out && test_attr->size_out > 0)
+		return -EINVAL;
+
+	bzero(&attr, sizeof(attr));
+	attr.test.prog_fd = test_attr->prog_fd;
+	attr.test.data_in = ptr_to_u64(test_attr->data);
+	attr.test.data_out = ptr_to_u64(test_attr->data_out);
+	attr.test.data_size_in = test_attr->size;
+	attr.test.data_size_out = test_attr->size_out;
+	attr.test.repeat = test_attr->repeat;
+
+	ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, sizeof(attr));
+	if (size_out)
+		*size_out = attr.test.data_size_out;
+	if (retval)
+		*retval = attr.test.retval;
+	if (duration)
+		*duration = attr.test.duration;
+	return ret;
+}
+
 int bpf_prog_get_next_id(__u32 start_id, __u32 *next_id)
 {
 	union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 26a51538213c..570f19f77f42 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -110,6 +110,19 @@ LIBBPF_API int bpf_prog_attach(int prog_fd, int attachable_fd,
 LIBBPF_API int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
 LIBBPF_API int bpf_prog_detach2(int prog_fd, int attachable_fd,
 				enum bpf_attach_type type);
+
+struct bpf_prog_test_run_attr {
+	int prog_fd;
+	int repeat;
+	const void *data;
+	__u32 size;
+	void *data_out; /* optional */
+	__u32 size_out;
+};
+
+LIBBPF_API int bpf_prog_test_run_xattr(const struct bpf_prog_test_run_attr *test_attr,
+				       __u32 *size_out, __u32 *retval,
+				       __u32 *duration);
 LIBBPF_API int bpf_prog_test_run(int prog_fd, int repeat, void *data,
 				 __u32 size, void *data_out, __u32 *size_out,
 				 __u32 *retval, __u32 *duration);
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 4/4] selftests: add a test for bpf_prog_test_run_xattr
From: Lorenz Bauer @ 2018-11-22 14:09 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181122140910.1079-1-lmb@cloudflare.com>

Make sure that bpf_prog_test_run_xattr returns the correct length
and that the kernel respects the output size hint. Also check
that errno indicates ENOSPC if there is a short output buffer given.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/testing/selftests/bpf/test_progs.c | 49 ++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index c1e688f61061..f9f5b1dbcc83 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -124,6 +124,54 @@ static void test_pkt_access(void)
 	bpf_object__close(obj);
 }
 
+static void test_prog_run_xattr(void)
+{
+	const char *file = "./test_pkt_access.o";
+	__u32 duration, retval, size_out;
+	struct bpf_object *obj;
+	char buf[10];
+	int err;
+	struct bpf_prog_test_run_attr tattr = {
+		.repeat = 1,
+		.data = &pkt_v4,
+		.size = sizeof(pkt_v4),
+		.data_out = buf,
+		.size_out = 5,
+	};
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj,
+			    &tattr.prog_fd);
+	if (CHECK(err, "load", "err %d errno %d\n", err, errno))
+		return;
+
+	memset(buf, 0, sizeof(buf));
+
+	err = bpf_prog_test_run_xattr(&tattr, &size_out, &retval, &duration);
+	CHECK(err != -1 || errno != ENOSPC || retval, "run",
+	      "err %d errno %d retval %d\n", err, errno, retval);
+
+	CHECK(size_out != sizeof(pkt_v4), "output_size",
+	      "incorrect output size, want %lu have %u\n",
+	      sizeof(pkt_v4), size_out);
+
+	CHECK(buf[5] != 0, "overflow",
+	      "BPF_PROG_TEST_RUN ignored size hint\n");
+
+	tattr.data_out = NULL;
+	tattr.size_out = 0;
+	errno = 0;
+
+	err = bpf_prog_test_run_xattr(&tattr, NULL, &retval, &duration);
+	CHECK(err || errno || retval, "run_no_output",
+	      "err %d errno %d retval %d\n", err, errno, retval);
+
+	tattr.size_out = 1;
+	err = bpf_prog_test_run_xattr(&tattr, NULL, NULL, &duration);
+	CHECK(err != -EINVAL, "run_wrong_size_out", "err %d\n", err);
+
+	bpf_object__close(obj);
+}
+
 static void test_xdp(void)
 {
 	struct vip key4 = {.protocol = 6, .family = AF_INET};
@@ -1837,6 +1885,7 @@ int main(void)
 	jit_enabled = is_jit_enabled();
 
 	test_pkt_access();
+	test_prog_run_xattr();
 	test_xdp();
 	test_xdp_adjust_tail();
 	test_l4lb_all();
-- 
2.17.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox