* [PATCH net v9 0/7] cxgb4/cxgbi: misc. fixes for cxgb4i
From: Karen Xie @ 2014-12-11 15:25 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
This patch set fixes cxgb4i's tx credit calculation and adds handling of
additional rx message and negative advice types. It also removes the duplicate
code in cxgb4i to set the outgoing queues of a packet.
Karen Xie (7):
cxgb4i: fix tx immediate data credit check
cxgb4i: fix credit check for tx_data_wr
cxgb4/cxgb4i: set max. outgoing pdu length in the f/w
cxgb4i: add more types of negative advice
cxgb4i: handle non pdu-aligned rx data
cxgb4i: use cxgb4's set_wr_txq() for setting outgoing queues
libcxgbi: fix the debug print accessing skb after it is freed
Sending to net as the fixes are mostly in the network area and it touches
cxgb4's header file (t4fw_api.h).
v2 corrects the "CHECK"s flagged by checkpatch.pl --strict.
v3 splits the 3rd patch from v2 to two separate patches. Adds detailed commit
messages and makes subject more concise. Patch 3/6 also changes the return
value of is_neg_adv() from int to bool.
v4 -- please ignore.
v5 splits the 1st patch from v3 to two separate patches and reduces code
duplication in make_tx_data_wr().
v6 removed the code style cleanup in the 2nd patch. The style update will be
addressed in a separate patch.
v7 updates the 7th patch with more detailed commit message.
v8 removes the duplicate subject lines from the message bodies.
v9 reformatted the commit messages to be max. 80 characters per line.
^ permalink raw reply
* [PATCH net v9 5/7] cxgb4i: handle non-pdu-aligned rx data
From: Karen Xie @ 2014-12-11 15:25 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
From: Karen Xie <kxie@chelsio.com>
Abort the connection upon receiving of cpl_rx_data, which means the pdu cannot
be recovered from the tcp stream. This generally is due to pdu header
corruption.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 2880f200..dff7345 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -1034,6 +1034,27 @@ rel_skb:
__kfree_skb(skb);
}
+static void do_rx_data(struct cxgbi_device *cdev, struct sk_buff *skb)
+{
+ struct cxgbi_sock *csk;
+ struct cpl_rx_data *cpl = (struct cpl_rx_data *)skb->data;
+ unsigned int tid = GET_TID(cpl);
+ struct cxgb4_lld_info *lldi = cxgbi_cdev_priv(cdev);
+ struct tid_info *t = lldi->tids;
+
+ csk = lookup_tid(t, tid);
+ if (!csk) {
+ pr_err("can't find connection for tid %u.\n", tid);
+ } else {
+ /* not expecting this, reset the connection. */
+ pr_err("csk 0x%p, tid %u, rcv cpl_rx_data.\n", csk, tid);
+ spin_lock_bh(&csk->lock);
+ send_abort_req(csk);
+ spin_unlock_bh(&csk->lock);
+ }
+ __kfree_skb(skb);
+}
+
static void do_rx_iscsi_hdr(struct cxgbi_device *cdev, struct sk_buff *skb)
{
struct cxgbi_sock *csk;
@@ -1453,6 +1474,7 @@ cxgb4i_cplhandler_func cxgb4i_cplhandlers[NUM_CPL_CMDS] = {
[CPL_SET_TCB_RPL] = do_set_tcb_rpl,
[CPL_RX_DATA_DDP] = do_rx_data_ddp,
[CPL_RX_ISCSI_DDP] = do_rx_data_ddp,
+ [CPL_RX_DATA] = do_rx_data,
};
int cxgb4i_ofld_init(struct cxgbi_device *cdev)
^ permalink raw reply related
* [PATCH net v9 6/7] cxgb4i: use set_wr_txq() to set tx queues
From: Karen Xie @ 2014-12-11 15:25 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
From: Karen Xie <kxie@chelsio.com>
use cxgb4's set_wr_txq() for setting of the tx queue for a outgoing packet.
Remove the similar function in cxgb4i.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 16 +++++-----------
1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index dff7345..6aa50fc 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -157,12 +157,6 @@ static struct scsi_transport_template *cxgb4i_stt;
#define RCV_BUFSIZ_MASK 0x3FFU
#define MAX_IMM_TX_PKT_LEN 128
-static inline void set_queue(struct sk_buff *skb, unsigned int queue,
- const struct cxgbi_sock *csk)
-{
- skb->queue_mapping = queue;
-}
-
static int push_tx_frames(struct cxgbi_sock *, int);
/*
@@ -404,7 +398,7 @@ static void send_abort_req(struct cxgbi_sock *csk)
csk->cpl_abort_req = NULL;
req = (struct cpl_abort_req *)skb->head;
- set_queue(skb, CPL_PRIORITY_DATA, csk);
+ set_wr_txq(skb, CPL_PRIORITY_DATA, csk->port_id);
req->cmd = CPL_ABORT_SEND_RST;
t4_set_arp_err_handler(skb, csk, abort_arp_failure);
INIT_TP_WR(req, csk->tid);
@@ -430,7 +424,7 @@ static void send_abort_rpl(struct cxgbi_sock *csk, int rst_status)
csk, csk->state, csk->flags, csk->tid, rst_status);
csk->cpl_abort_rpl = NULL;
- set_queue(skb, CPL_PRIORITY_DATA, csk);
+ set_wr_txq(skb, CPL_PRIORITY_DATA, csk->port_id);
INIT_TP_WR(rpl, csk->tid);
OPCODE_TID(rpl) = cpu_to_be32(MK_OPCODE_TID(CPL_ABORT_RPL, csk->tid));
rpl->cmd = rst_status;
@@ -555,7 +549,7 @@ static inline int send_tx_flowc_wr(struct cxgbi_sock *csk)
flowc->mnemval[8].mnemonic = FW_FLOWC_MNEM_TXDATAPLEN_MAX;
flowc->mnemval[8].val = 16384;
- set_queue(skb, CPL_PRIORITY_DATA, csk);
+ set_wr_txq(skb, CPL_PRIORITY_DATA, csk->port_id);
log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
"csk 0x%p, tid 0x%x, %u,%u,%u,%u,%u,%u,%u.\n",
@@ -660,7 +654,7 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
break;
}
__skb_unlink(skb, &csk->write_queue);
- set_queue(skb, CPL_PRIORITY_DATA, csk);
+ set_wr_txq(skb, CPL_PRIORITY_DATA, csk->port_id);
skb->csum = credits_needed + flowclen16;
csk->wr_cred -= credits_needed;
csk->wr_una_cred += credits_needed;
@@ -1552,7 +1546,7 @@ static int ddp_ppod_write_idata(struct cxgbi_device *cdev, unsigned int port_id,
return -ENOMEM;
}
req = (struct ulp_mem_io *)skb->head;
- set_queue(skb, CPL_PRIORITY_CONTROL, NULL);
+ set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0);
ulp_mem_io_set_hdr(lldi, req, wr_len, dlen, pm_addr);
idata = (struct ulptx_idata *)(req + 1);
^ permalink raw reply related
* [PATCH net v9 7/7] libcxgbi: fix freeing skb prematurely
From: Karen Xie @ 2014-12-11 15:25 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
From: Karen Xie <kxie@chelsio.com>
With debug turned on the debug print would access the skb after it is freed.
Fix it to free the skb after the debug print.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/libcxgbi.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 7da59c3..eb58afc 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -2294,10 +2294,12 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task)
return err;
}
- kfree_skb(skb);
log_debug(1 << CXGBI_DBG_ISCSI | 1 << CXGBI_DBG_PDU_TX,
"itt 0x%x, skb 0x%p, len %u/%u, xmit err %d.\n",
task->itt, skb, skb->len, skb->data_len, err);
+
+ kfree_skb(skb);
+
iscsi_conn_printk(KERN_ERR, task->conn, "xmit err %d.\n", err);
iscsi_conn_failure(task->conn, ISCSI_ERR_XMIT_FAILED);
return err;
^ permalink raw reply related
* [PATCH net v9 1/7] cxgb4i: fix tx immediate data credit check
From: Karen Xie @ 2014-12-11 15:25 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
From: Karen Xie <kxie@chelsio.com>
Only data skbs need the wr header added while control skbs do not. Make sure
they are treated differently.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 22 +++++++++++++++-------
drivers/scsi/cxgbi/libcxgbi.h | 4 ++--
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 1508125..f119a67 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -171,10 +171,14 @@ static int push_tx_frames(struct cxgbi_sock *, int);
* Returns true if a packet can be sent as an offload WR with immediate
* data. We currently use the same limit as for Ethernet packets.
*/
-static inline int is_ofld_imm(const struct sk_buff *skb)
+static inline bool is_ofld_imm(const struct sk_buff *skb)
{
- return skb->len <= (MAX_IMM_TX_PKT_LEN -
- sizeof(struct fw_ofld_tx_data_wr));
+ int len = skb->len;
+
+ if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR)))
+ len += sizeof(struct fw_ofld_tx_data_wr);
+
+ return len <= MAX_IMM_TX_PKT_LEN;
}
static void send_act_open_req(struct cxgbi_sock *csk, struct sk_buff *skb,
@@ -597,11 +601,15 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
skb_reset_transport_header(skb);
if (is_ofld_imm(skb))
- credits_needed = DIV_ROUND_UP(dlen +
- sizeof(struct fw_ofld_tx_data_wr), 16);
+ credits_needed = DIV_ROUND_UP(dlen, 16);
else
- credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
- + sizeof(struct fw_ofld_tx_data_wr),
+ credits_needed = DIV_ROUND_UP(
+ 8 * calc_tx_flits_ofld(skb),
+ 16);
+
+ if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR)))
+ credits_needed += DIV_ROUND_UP(
+ sizeof(struct fw_ofld_tx_data_wr),
16);
if (csk->wr_cred < credits_needed) {
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 2c7cb1c..aba1af7 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -317,8 +317,8 @@ static inline void cxgbi_skcb_clear_flag(struct sk_buff *skb,
__clear_bit(flag, &(cxgbi_skcb_flags(skb)));
}
-static inline int cxgbi_skcb_test_flag(struct sk_buff *skb,
- enum cxgbi_skcb_flags flag)
+static inline int cxgbi_skcb_test_flag(const struct sk_buff *skb,
+ enum cxgbi_skcb_flags flag)
{
return test_bit(flag, &(cxgbi_skcb_flags(skb)));
}
^ permalink raw reply related
* [PATCH net v9 3/7] cxgb4/cxgb4i: set the max. pdu length in firmware
From: Karen Xie @ 2014-12-11 15:25 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
From: Karen Xie <kxie@chelsio.com>
Programs the firmware of the maximum outgoing iscsi pdu length per connection.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 69 +++++++++++++++++++++++++++---------
1 files changed, 51 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
index 3409756..743a350 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
@@ -529,6 +529,7 @@ enum fw_flowc_mnem {
FW_FLOWC_MNEM_RCVNXT,
FW_FLOWC_MNEM_SNDBUF,
FW_FLOWC_MNEM_MSS,
+ FW_FLOWC_MNEM_TXDATAPLEN_MAX,
};
struct fw_flowc_mnemval {
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index abee611..5f31eb7 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -75,6 +75,7 @@ typedef void (*cxgb4i_cplhandler_func)(struct cxgbi_device *, struct sk_buff *);
static void *t4_uld_add(const struct cxgb4_lld_info *);
static int t4_uld_rx_handler(void *, const __be64 *, const struct pkt_gl *);
static int t4_uld_state_change(void *, enum cxgb4_state state);
+static inline int send_tx_flowc_wr(struct cxgbi_sock *);
static const struct cxgb4_uld_info cxgb4i_uld_info = {
.name = DRV_MODULE_NAME,
@@ -391,6 +392,12 @@ static void send_abort_req(struct cxgbi_sock *csk)
if (unlikely(csk->state == CTP_ABORTING) || !skb || !csk->cdev)
return;
+
+ if (!cxgbi_sock_flag(csk, CTPF_TX_DATA_SENT)) {
+ send_tx_flowc_wr(csk);
+ cxgbi_sock_set_flag(csk, CTPF_TX_DATA_SENT);
+ }
+
cxgbi_sock_set_state(csk, CTP_ABORTING);
cxgbi_sock_set_flag(csk, CTPF_ABORT_RPL_PENDING);
cxgbi_sock_purge_write_queue(csk);
@@ -493,20 +500,40 @@ static inline unsigned int calc_tx_flits_ofld(const struct sk_buff *skb)
return flits + sgl_len(cnt);
}
-static inline void send_tx_flowc_wr(struct cxgbi_sock *csk)
+#define FLOWC_WR_NPARAMS_MIN 9
+static inline int tx_flowc_wr_credits(int *nparamsp, int *flowclenp)
+{
+ int nparams, flowclen16, flowclen;
+
+ nparams = FLOWC_WR_NPARAMS_MIN;
+ flowclen = offsetof(struct fw_flowc_wr, mnemval[nparams]);
+ flowclen16 = DIV_ROUND_UP(flowclen, 16);
+ flowclen = flowclen16 * 16;
+ /*
+ * Return the number of 16-byte credits used by the FlowC request.
+ * Pass back the nparams and actual FlowC length if requested.
+ */
+ if (nparamsp)
+ *nparamsp = nparams;
+ if (flowclenp)
+ *flowclenp = flowclen;
+
+ return flowclen16;
+}
+
+static inline int send_tx_flowc_wr(struct cxgbi_sock *csk)
{
struct sk_buff *skb;
struct fw_flowc_wr *flowc;
- int flowclen, i;
+ int nparams, flowclen16, flowclen;
- flowclen = 80;
+ flowclen16 = tx_flowc_wr_credits(&nparams, &flowclen);
skb = alloc_wr(flowclen, 0, GFP_ATOMIC);
flowc = (struct fw_flowc_wr *)skb->head;
flowc->op_to_nparams =
- htonl(FW_WR_OP(FW_FLOWC_WR) | FW_FLOWC_WR_NPARAMS(8));
+ htonl(FW_WR_OP(FW_FLOWC_WR) | FW_FLOWC_WR_NPARAMS(nparams));
flowc->flowid_len16 =
- htonl(FW_WR_LEN16(DIV_ROUND_UP(72, 16)) |
- FW_WR_FLOWID(csk->tid));
+ htonl(FW_WR_LEN16(flowclen16) | FW_WR_FLOWID(csk->tid));
flowc->mnemval[0].mnemonic = FW_FLOWC_MNEM_PFNVFN;
flowc->mnemval[0].val = htonl(csk->cdev->pfvf);
flowc->mnemval[1].mnemonic = FW_FLOWC_MNEM_CH;
@@ -525,11 +552,9 @@ static inline void send_tx_flowc_wr(struct cxgbi_sock *csk)
flowc->mnemval[7].val = htonl(csk->advmss);
flowc->mnemval[8].mnemonic = 0;
flowc->mnemval[8].val = 0;
- for (i = 0; i < 9; i++) {
- flowc->mnemval[i].r4[0] = 0;
- flowc->mnemval[i].r4[1] = 0;
- flowc->mnemval[i].r4[2] = 0;
- }
+ flowc->mnemval[8].mnemonic = FW_FLOWC_MNEM_TXDATAPLEN_MAX;
+ flowc->mnemval[8].val = 16384;
+
set_queue(skb, CPL_PRIORITY_DATA, csk);
log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
@@ -539,6 +564,8 @@ static inline void send_tx_flowc_wr(struct cxgbi_sock *csk)
csk->advmss);
cxgb4_ofld_send(csk->cdev->ports[csk->port_id], skb);
+
+ return flowclen16;
}
static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
@@ -599,6 +626,7 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
int dlen = skb->len;
int len = skb->len;
unsigned int credits_needed;
+ int flowclen16 = 0;
skb_reset_transport_header(skb);
if (is_ofld_imm(skb))
@@ -613,6 +641,17 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
sizeof(struct fw_ofld_tx_data_wr),
16);
+ /*
+ * Assumes the initial credits is large enough to support
+ * fw_flowc_wr plus largest possible first payload
+ */
+ if (!cxgbi_sock_flag(csk, CTPF_TX_DATA_SENT)) {
+ flowclen16 = send_tx_flowc_wr(csk);
+ csk->wr_cred -= flowclen16;
+ csk->wr_una_cred += flowclen16;
+ cxgbi_sock_set_flag(csk, CTPF_TX_DATA_SENT);
+ }
+
if (csk->wr_cred < credits_needed) {
log_debug(1 << CXGBI_DBG_PDU_TX,
"csk 0x%p, skb %u/%u, wr %d < %u.\n",
@@ -622,7 +661,7 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
}
__skb_unlink(skb, &csk->write_queue);
set_queue(skb, CPL_PRIORITY_DATA, csk);
- skb->csum = credits_needed;
+ skb->csum = credits_needed + flowclen16;
csk->wr_cred -= credits_needed;
csk->wr_una_cred += credits_needed;
cxgbi_sock_enqueue_wr(csk, skb);
@@ -633,12 +672,6 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
csk->wr_cred, csk->wr_una_cred);
if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR))) {
- if (!cxgbi_sock_flag(csk, CTPF_TX_DATA_SENT)) {
- send_tx_flowc_wr(csk);
- skb->csum += 5;
- csk->wr_cred -= 5;
- csk->wr_una_cred += 5;
- }
len += cxgbi_ulp_extra_len(cxgbi_skcb_ulp_mode(skb));
make_tx_data_wr(csk, skb, dlen, len, credits_needed,
req_completion);
^ permalink raw reply related
* RE: [PATCH net v8 1/7] cxgb4i: fix tx immediate data credit check
From: Karen Xie @ 2014-12-11 15:26 UTC (permalink / raw)
To: David Miller
Cc: linux-scsi@vger.kernel.org, netdev@vger.kernel.org, Hariprasad S,
Anish Bhatt, hch@infradead.org,
James.Bottomley@HansenPartnership.com, michaelc@cs.wisc.edu
In-Reply-To: <20141210.225445.458745535803327140.davem@davemloft.net>
From: David Miller [mailto:davem@davemloft.net]
Sent: Wednesday, December 10, 2014 7:55 PM
> Please text format your commit messages to 80 characters or less per line.
>
> People are going to read your commits using text tools, not necessarily GUI
> tools which will auto-format your commit message.
>
> I know this seems like we are being difficult, but you are making many
> fundamental mistakes in your efforts to merge in these changes.
Submitted v9 with reformatted commit messages. Thanks for your patience.
^ permalink raw reply
* [PATCH net v9 2/7] cxgb4i: fix credit check for tx_data_wr
From: Karen Xie @ 2014-12-11 15:25 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
From: Karen Xie <kxie@chelsio.com>
make sure any tx credit related checking is done before adding the wr header.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index f119a67..abee611 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -547,10 +547,11 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
struct fw_ofld_tx_data_wr *req;
unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
unsigned int wr_ulp_mode = 0;
+ bool imm = is_ofld_imm(skb);
req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
- if (is_ofld_imm(skb)) {
+ if (imm) {
req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
FW_WR_COMPL(1) |
FW_WR_IMMDLEN(dlen));
^ permalink raw reply related
* [PATCH net v9 4/7] cxgb4i: additional types of negative advice
From: Karen Xie @ 2014-12-11 15:25 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
From: Karen Xie <kxie@chelsio.com>
Treat both CPL_ERR_KEEPALV_NEG_ADVICE and CPL_ERR_PERSIST_NEG_ADVICE as
negative advice.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 5f31eb7..2880f200 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -846,6 +846,13 @@ static void csk_act_open_retry_timer(unsigned long data)
}
+static inline bool is_neg_adv(unsigned int status)
+{
+ return status == CPL_ERR_RTX_NEG_ADVICE ||
+ status == CPL_ERR_KEEPALV_NEG_ADVICE ||
+ status == CPL_ERR_PERSIST_NEG_ADVICE;
+}
+
static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
{
struct cxgbi_sock *csk;
@@ -867,7 +874,7 @@ static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
"csk 0x%p,%u,0x%lx. ", (&csk->saddr), (&csk->daddr),
atid, tid, status, csk, csk->state, csk->flags);
- if (status == CPL_ERR_RTX_NEG_ADVICE)
+ if (is_neg_adv(status))
goto rel_skb;
module_put(THIS_MODULE);
@@ -973,8 +980,7 @@ static void do_abort_req_rss(struct cxgbi_device *cdev, struct sk_buff *skb)
(&csk->saddr), (&csk->daddr),
csk, csk->state, csk->flags, csk->tid, req->status);
- if (req->status == CPL_ERR_RTX_NEG_ADVICE ||
- req->status == CPL_ERR_PERSIST_NEG_ADVICE)
+ if (is_neg_adv(req->status))
goto rel_skb;
cxgbi_sock_get(csk);
^ permalink raw reply related
* Re: [PATCH iproute2] ip: Simplify executing ip cmd within namespace
From: Jiri Benc @ 2014-12-11 16:09 UTC (permalink / raw)
To: Vadim Kochan; +Cc: netdev
In-Reply-To: <1418252195-2612-1-git-send-email-vadim4j@gmail.com>
On Thu, 11 Dec 2014 00:56:35 +0200, Vadim Kochan wrote:
> From: Vadim Kochan <vadim4j@gmail.com>
>
> Added new '-ns' option to simplify executing following cmd:
>
> ip netns exec NETNS ip OPTIONS COMMAND OBJECT
>
> to
>
> ip -ns NETNS OPTIONS COMMAND OBJECT
>
> e.g.:
>
> ip -ns vnet0 link add br0 type bridge
This is great! It's a thing that has been bothering me for long time
but never got high enough on my todo list. Thanks for working on this.
However,
> --- a/ip/ip.c
> +++ b/ip/ip.c
> @@ -262,6 +262,12 @@ int main(int argc, char **argv)
> rcvbuf = size;
> } else if (matches(opt, "-help") == 0) {
> usage();
> + } else if (matches(opt, "-ns") == 0) {
> + argc--;
> + argv++;
> + argv[0] = argv[1];
> + argv[1] = basename;
> + return netns_exec(argc, argv);
I very much dislike this. There's no reason to exec another ip binary.
The main reason I wanted the -n (or whatever) option was to speed up
execution of test scripts in environments with hundreds of interfaces
in different net namespaces.
Please just change to the specified netns and continue with interpreting
of the rest of the command line, there's absolutely no reason for doing
the exec.
Thanks,
Jiri
--
Jiri Benc
^ permalink raw reply
* Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration
From: Roopa Prabhu @ 2014-12-11 16:37 UTC (permalink / raw)
To: Jiri Pirko
Cc: Varlese, Marco, John Fastabend, netdev@vger.kernel.org,
stephen@networkplumber.org, Fastabend, John R, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20141211110115.GA1979@nanopsycho.lan>
On 12/11/14, 3:01 AM, Jiri Pirko wrote:
> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>> -----Original Message-----
>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>> To: Jiri Pirko
>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>> stephen@networkplumber.org; Fastabend, John R;
>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>> kernel@vger.kernel.org
>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>> configuration
>>>
>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com wrote:
>>>>> From: Marco Varlese <marco.varlese@intel.com>
>>>>>
>>>>> Switch hardware offers a list of attributes that are configurable on
>>>>> a per port basis.
>>>>> This patch provides a mechanism to configure switch ports by adding
>>>>> an NDO for setting specific values to specific attributes.
>>>>> There will be a separate patch that extends iproute2 to call the new
>>>>> NDO.
>>>>
>>>> What are these attributes? Can you give some examples. I'm asking
>>>> because there is a plan to pass generic attributes to switch ports
>>>> replacing current specific ndo_switch_port_stp_update. In this case,
>>>> bridge is setting that attribute.
>>>>
>>>> Is there need to set something directly from userspace or does it make
>>>> rather sense to use involved bridge/ovs/bond ? I think that both will
>>>> be needed.
>>> +1
>>>
>>> I think for many attributes it would be best to have both. The in kernel callers
>>> and netlink userspace can use the same driver ndo_ops.
>>>
>>> But then we don't _require_ any specific bridge/ovs/etc module. And we
>>> may have some attributes that are not specific to any existing software
>>> module. I'm guessing Marco has some examples of these.
>>>
>>> [...]
>>>
>>>
>>> --
>>> John Fastabend Intel Corporation
>> We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>
>> An example of attributes are:
>> * enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>> * internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>> * flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>
>> Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>
>> One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>
>> I hope this clarifies some points.
> It does. Makes sense. We need to expose this attr set/get for both
> in-kernel and userspace use cases.
>
> Please adjust you patch for this. Also, as a second patch, it would be
> great if you can convert ndo_switch_port_stp_update to this new ndo.
Why are we exposing generic switch attribute get/set from userspace ?.
We already have specific attributes for learning/flooding which can be
extended further.
And for in kernel api....i had a sample patch in my RFC series (Which i
was going to resubmit, until it was decided that we will use existing
api around ndo_bridge_setlink/ndo_bridge_getlink):
http://www.spinics.net/lists/netdev/msg305473.html
Thanks,
Roopa
^ permalink raw reply
* Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Hubert Sokolowski @ 2014-12-11 16:39 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: netdev, Jamal Hadi Salim
In-Reply-To: <54894850.5000603@cumulusnetworks.com>
> I think commit
> "5e6d243587990a588143b9da3974833649595587 "bridge: netlink dump
> interface at par with brctl" tried to make sure even the dflt entries
> (ie dev->uc and dev->mc) were also printed in the below case. ie the
> 'self' entries in the below output.
>
> # ./bridge fdb show brport eth1
> 02:00:00:12:01:02 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 self permanent
> 33:33:00:00:00:01 self permanent
>
> Am guessing reverting the patch is going to make the 'self' entries in
> the above output to go away.
> Can you please confirm ?.
I don't want you to revert the patch, as the main goal of the patch
was to enable filtering in the kernel. I am only proposing
to revert part of it that allows driver to implement own dump.
This does not break the filtering in the kernel.
Whether the 'self' entries will go away it depends if the driver
overrides ndo_fdb_dump callback with its own. For cases where the driver
does not implement the callback, the dflt callback is still called
showing 'self' entries:
[root@silpixa00378825 ~]# bridge fdb show
33:33:00:00:00:01 dev em1 self permanent
01:00:5e:00:00:01 dev em1 self permanent
33:33:00:00:00:01 dev p4p1 self permanent
01:00:5e:00:00:01 dev p4p1 self permanent
33:33:ff:81:56:db dev p4p1 self permanent
01:00:5e:00:00:fb dev p4p1 self permanent
33:33:00:00:00:01 dev dummy0 self permanent
>
> Also, if i hear your concern correctly, for bridge ports that implement
> ndo_fdb_dump, with commit 5e6d243587990a588143b9da3974833649595587, we
> will get two entries for each 'self' entry above.
> Can you also paste sample output for that ?.
My patch affects *only* drivers that implements own dump callback.
Implementing own dump callback means the driver want to have a control
over what is being dumped. For example you may want to dump a hardware
MAC table only (my case) where 'self' entries created by kernel make no sense.
Also there are drivers that calls dflt callback from inside own dump function.
Please see following dump callback implemented for QLogic:
static int qlcnic_fdb_dump(struct sk_buff *skb, struct netlink_callback *ncb,
struct net_device *netdev,
struct net_device *filter_dev, int idx)
{
struct qlcnic_adapter *adapter = netdev_priv(netdev);
if (!adapter->fdb_mac_learn)
return ndo_dflt_fdb_dump(skb, ncb, netdev, filter_dev, idx);
[...]
Another example of dflt being called twice is macvlan.c where ndo_fdb_dump
is actually initialized with the dflt callback:
macvlan.c:1022: .ndo_fdb_dump = ndo_dflt_fdb_dump,
Thanks,
Hubert
^ permalink raw reply
* Re: [PATCH iproute2] ip: Simplify executing ip cmd within namespace
From: vadim4j @ 2014-12-11 16:33 UTC (permalink / raw)
To: Jiri Benc; +Cc: Vadim Kochan, netdev
In-Reply-To: <20141211170928.62c8e637@griffin>
On Thu, Dec 11, 2014 at 05:09:28PM +0100, Jiri Benc wrote:
> On Thu, 11 Dec 2014 00:56:35 +0200, Vadim Kochan wrote:
> > From: Vadim Kochan <vadim4j@gmail.com>
> >
> > Added new '-ns' option to simplify executing following cmd:
> >
> > ip netns exec NETNS ip OPTIONS COMMAND OBJECT
> >
> > to
> >
> > ip -ns NETNS OPTIONS COMMAND OBJECT
> >
> > e.g.:
> >
> > ip -ns vnet0 link add br0 type bridge
>
> This is great! It's a thing that has been bothering me for long time
> but never got high enough on my todo list. Thanks for working on this.
>
> However,
>
> > --- a/ip/ip.c
> > +++ b/ip/ip.c
> > @@ -262,6 +262,12 @@ int main(int argc, char **argv)
> > rcvbuf = size;
> > } else if (matches(opt, "-help") == 0) {
> > usage();
> > + } else if (matches(opt, "-ns") == 0) {
> > + argc--;
> > + argv++;
> > + argv[0] = argv[1];
> > + argv[1] = basename;
> > + return netns_exec(argc, argv);
>
> I very much dislike this. There's no reason to exec another ip binary.
> The main reason I wanted the -n (or whatever) option was to speed up
> execution of test scripts in environments with hundreds of interfaces
> in different net namespaces.
>
> Please just change to the specified netns and continue with interpreting
> of the rest of the command line, there's absolutely no reason for doing
> the exec.
Yes, I will follow that way.
Thanks,
>
> Thanks,
>
> Jiri
>
> --
> Jiri Benc
^ permalink raw reply
* Re: tg3 broken in 3.18.0?
From: Marcelo Ricardo Leitner @ 2014-12-11 16:45 UTC (permalink / raw)
To: Nils Holland, netdev
In-Reply-To: <20141210230634.GA22884@teela.fritz.box>
On 10-12-2014 21:06, Nils Holland wrote:
> Hi everyone,
>
> I just upgraded a machine from 3.17.3 to 3.18.0 and noticed that after
> the upgrade, the machine's network interface (which is a tg3) would no
> longer run correctly (or, for that matter, run at all). During the
> upgrade, I didn't change any kernel config options or any other parts
> of the system.
Same thing here! Thanks for reporting this, Nils.
> Since the machine is remote and I don't have direct access to it, it's
> kind of hard currently to give more details, but here's what I'm
> seeing in the logs:
I have access to mine, kudos to secondary NIC.
$ ethtool -i p1p1
driver: tg3
version: 3.137
firmware-version: 5722-v3.13
bus-info: 0000:02:00.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: no
$ ethtool p1p1
Settings for p1p1:
Supported ports: [ TP ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Half 1000baseT/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Half 1000baseT/Full
Advertised pause frame use: Symmetric
Advertised auto-negotiation: Yes
Speed: Unknown!
Duplex: Unknown! (255)
Port: Twisted Pair
PHYAD: 1
Transceiver: internal
Auto-negotiation: on
MDI-X: Unknown
$ sudo ip link set p1p1 up
RTNETLINK answers: No such device
> If I see things correctly, there were only two patches affecting tg3
> between 3.17(.3) and 3.18:
>
> 2c7c9ea429ba30fe506747b7da110e2212d8fefa
> a620a6bc1c94c22d6c312892be1e0ae171523125
I'm running net-next, 395eea6ccf2b253f81b4718ffbcae67d36fe2e69.
So my diffs would be:
$ git log v3.17..origin/master --oneline -- drivers/net/ethernet/broadcom/tg3.c
892311f ethtool: Support for configurable RSS hash function
60b7379 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
a620a6b tg3: fix ring init when there are more TX than RX channels
3964835 tg3: use netdev_rss_key_fill() helper
2c7c9ea tg3: Add skb->xmit_more support
Reverting all these, issue continues.
If no one has a better shot, I'll try bissecting later.
Marcelo
^ permalink raw reply
* Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Hubert Sokolowski @ 2014-12-11 16:51 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David Miller, netdev, Roopa Prabhu, Vlad Yasevich
In-Reply-To: <548984C0.2040706@mojatatu.com>
>
> It wont work. As pointed out by Roopa in
> the other email dev->uc/mc will not get dumped with this
> change. Vlad will be in a better position to comment.
> CCing Vlad.
Jamal, this will still get dumped unless your driver implements
own dump callback. Can you please exactly tell us what wont work?
Please see my previous post where I added more explanation.
Thanks,
Hubert
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] swdevice: add new api to set and del bridge port attributes
From: Roopa Prabhu @ 2014-12-11 16:52 UTC (permalink / raw)
To: Jiri Pirko
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
vyasevic, netdev, davem, shm, gospo
In-Reply-To: <20141210093755.GB1863@nanopsycho.orion>
On 12/10/14, 1:37 AM, Jiri Pirko wrote:
> Wed, Dec 10, 2014 at 10:05:18AM CET, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch adds two new api's netdev_switch_port_bridge_setlink
>> and netdev_switch_port_bridge_dellink to offload bridge port attributes
>> to switch asic
>>
>> (The names of the apis look odd with 'switch_port_bridge',
>> but am more inclined to change the prefix of the api to something else.
>> Will take any suggestions).
>>
>> The api's look at the NETIF_F_HW_NETFUNC_OFFLOAD feature flag to
>> pass bridge port attributes to the port device.
>>
>> If the device has the NETIF_F_HW_NETFUNC_OFFLOAD, but does not support
>> the bridge port attribute offload ndo, call bridge port attribute ndo's on
>> the lowerdevs if supported. This is one way to pass bridge port attributes
>> through stacked netdevs (example when bridge port is a bond and bond slaves
>> are switch ports).
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> include/net/switchdev.h | 5 +++-
>> net/switchdev/switchdev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 74 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>> index 8a6d164..22676b6 100644
>> --- a/include/net/switchdev.h
>> +++ b/include/net/switchdev.h
>> @@ -17,7 +17,10 @@
>> int netdev_switch_parent_id_get(struct net_device *dev,
>> struct netdev_phys_item_id *psid);
>> int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>> -
>> +int netdev_switch_port_bridge_setlink(struct net_device *dev,
>> + struct nlmsghdr *nlh, u16 flags);
>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>> + struct nlmsghdr *nlh, u16 flags);
>> #else
>>
>> static inline int netdev_switch_parent_id_get(struct net_device *dev,
>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>> index d162b21..62317e1 100644
>> --- a/net/switchdev/switchdev.c
>> +++ b/net/switchdev/switchdev.c
>> @@ -50,3 +50,73 @@ int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>> return ops->ndo_switch_port_stp_update(dev, state);
>> }
>> EXPORT_SYMBOL(netdev_switch_port_stp_update);
>> +
>> +/**
>> + * netdev_switch_port_bridge_setlink - Notify switch device port of bridge
>> + * port attributes
>> + *
>> + * @dev: port device
>> + * @nlh: netlink msg with bridge port attributes
>> + *
>> + * Notify switch device port of bridge port attributes
>> + */
>> +int netdev_switch_port_bridge_setlink(struct net_device *dev,
>> + struct nlmsghdr *nlh, u16 flags)
>> +{
>> + const struct net_device_ops *ops = dev->netdev_ops;
>> + struct net_device *lower_dev;
>> + struct list_head *iter;
>> + int ret = 0, err = 0;
>> +
>> + if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>> + return err;
>> +
>> + if (ops->ndo_bridge_setlink) {
>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>> + return ops->ndo_bridge_setlink(dev, nlh, flags);
> You have to change ndo_bridge_setlink in netdevice.h first.
> Otherwise when only this patch is applied (during bisection)
> this won't compile.
ack, will fix it and keep that in mind next time.
>
>> + }
>> +
>> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
> I do not understand why to iterate over lower devices. At this
> stage we don't know a thing about this upper or its lowers. Let
> the uppers (/masters) to decide if this needs to be propagated
> or not.
Jiri, In the stacked devices case, there is no way to propagate the
bridge port attributes to switch device driver today (vlan and other
bridge port attributes). Can you tell me if there is a way ?. no,
ndo_vlan* ndo's are not useful here. Nor we should go and implement
ndo_bridge_setlink* in all devices that can be bridge ports.
And this allows a switch driver to receive these callbacks if it has
marked the switch port with an offload flag. Your way of using the
switch port to get to the switch driver does not help in these cases.
The other option is to use the 'switch device (not port)' to get to the
switch driver.
This patch shows that you can still do this with the ndo ops.
>
>> + err = netdev_switch_port_bridge_setlink(lower_dev, nlh, flags);
>> + if (err)
>> + ret = err;
>> + }
> ^^^^^ Indent is off. This should be catched by scripts/checkpatch.pl.
>
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(netdev_switch_port_bridge_setlink);
>> +
>> +/**
>> + * netdev_switch_port_bridge_dellink - Notify switch device port of bridge
>> + * attribute delete
>> + *
>> + * @dev: port device
>> + * @nlh: netlink msg with bridge port attributes
>> + *
>> + * Notify switch device port of bridge port attribute delete
>> + */
>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>> + struct nlmsghdr *nlh, u16 flags)
>> +{
>> + const struct net_device_ops *ops = dev->netdev_ops;
>> + struct net_device *lower_dev;
>> + struct list_head *iter;
>> + int ret = 0, err = 0;
>> +
>> + if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>> + return err;
>> +
>> + if (ops->ndo_bridge_dellink) {
>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>> + return ops->ndo_bridge_dellink(dev, nlh, flags);
>> + }
>> +
>> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
>> + err = netdev_switch_port_bridge_dellink(lower_dev, nlh, flags);
>> + if (err)
>> + ret = err;
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>> --
>> 1.7.10.4
>>
^ permalink raw reply
* Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-11 16:56 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Varlese, Marco, John Fastabend, netdev@vger.kernel.org,
stephen@networkplumber.org, Fastabend, John R, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <5489C85A.5020400@cumulusnetworks.com>
Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>-----Original Message-----
>>>>From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>Sent: Wednesday, December 10, 2014 5:04 PM
>>>>To: Jiri Pirko
>>>>Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>stephen@networkplumber.org; Fastabend, John R;
>>>>roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>kernel@vger.kernel.org
>>>>Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>configuration
>>>>
>>>>On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com wrote:
>>>>>>From: Marco Varlese <marco.varlese@intel.com>
>>>>>>
>>>>>>Switch hardware offers a list of attributes that are configurable on
>>>>>>a per port basis.
>>>>>>This patch provides a mechanism to configure switch ports by adding
>>>>>>an NDO for setting specific values to specific attributes.
>>>>>>There will be a separate patch that extends iproute2 to call the new
>>>>>>NDO.
>>>>>
>>>>>What are these attributes? Can you give some examples. I'm asking
>>>>>because there is a plan to pass generic attributes to switch ports
>>>>>replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>bridge is setting that attribute.
>>>>>
>>>>>Is there need to set something directly from userspace or does it make
>>>>>rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>be needed.
>>>>+1
>>>>
>>>>I think for many attributes it would be best to have both. The in kernel callers
>>>>and netlink userspace can use the same driver ndo_ops.
>>>>
>>>>But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>may have some attributes that are not specific to any existing software
>>>>module. I'm guessing Marco has some examples of these.
>>>>
>>>>[...]
>>>>
>>>>
>>>>--
>>>>John Fastabend Intel Corporation
>>>We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>
>>>An example of attributes are:
>>>* enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>* internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>* flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>
>>>Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>
>>>One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>
>>>I hope this clarifies some points.
>>It does. Makes sense. We need to expose this attr set/get for both
>>in-kernel and userspace use cases.
>>
>>Please adjust you patch for this. Also, as a second patch, it would be
>>great if you can convert ndo_switch_port_stp_update to this new ndo.
>
>Why are we exposing generic switch attribute get/set from userspace ?. We
>already have specific attributes for learning/flooding which can be extended
>further.
Yes, but that is for PF_BRIDGE and bridge specific attributes. There
might be another generic attrs, no?
>And for in kernel api....i had a sample patch in my RFC series (Which i was
>going to resubmit, until it was decided that we will use existing api around
>ndo_bridge_setlink/ndo_bridge_getlink):
>http://www.spinics.net/lists/netdev/msg305473.html
Yes, this might become handy for other generic non-bridge attrs.
>
>Thanks,
>Roopa
>
>
>
^ permalink raw reply
* Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver
From: David Miller @ 2014-12-11 17:01 UTC (permalink / raw)
To: m-karicheri2
Cc: netdev, linux-arm-kernel, linux-kernel, devicetree, robh+dt,
grant.likely
In-Reply-To: <5489A6CD.20909@ti.com>
From: Murali Karicheri <m-karicheri2@ti.com>
Date: Thu, 11 Dec 2014 09:14:37 -0500
> BTW, could you provide any suggestions that would help us merge this
> series to upstream? This has been sitting on this list for a while
> now.
You simply have to continue going through the review and revision
process until people no longer find problems with your changes.
This could take several more weeks, you simply must be patient.
^ permalink raw reply
* Re: [PATCH net-next RESEND 2/2] ipv6: fix sparse warning
From: Paul E. McKenney @ 2014-12-11 17:02 UTC (permalink / raw)
To: Ying Xue
Cc: davem, eric.dumazet, jon.maloy, erik.hugne, netdev, kbuild-all,
linux-kernel
In-Reply-To: <5488F9D1.9060109@windriver.com>
On Thu, Dec 11, 2014 at 09:56:33AM +0800, Ying Xue wrote:
> On 12/11/2014 12:04 AM, Paul E. McKenney wrote:
> > On Wed, Dec 10, 2014 at 04:46:07PM +0800, Ying Xue wrote:
> >> This fixes the following spare warning when using
> >>
> >> make C=1 CF=-D__CHECK_ENDIAN__ net/ipv6/addrconf.o
> >> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> >> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> >> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> >> net/ipv6/addrconf.c:3495:9: error: incompatible types in comparison expression (different address spaces)
> >>
> >> To silence above spare complaint, an RCU annotation should be added
> >> to "next" pointer of hlist_node structure through hlist_next_rcu()
> >> macro when iterating over a hlist with
> >> hlist_for_each_entry_continue_rcu_bh().
> >>
> >> By the way, this commit also resolves the same error appearing in
> >> hlist_for_each_entry_continue_rcu().
> >>
> >> Signed-off-by: Ying Xue <ying.xue@windriver.com>
> >
> > If you pull the rculist.h changes from the first patch into this one,
> > I will queue it up through -rcu.
>
> Please just queue this patch into your RCU tree. Some changes of the
> first patch currently only exists in net-next tree, so it cannot be
> merged into RCU tree now. Therefore, please temporarily ignore it. In
> next 3.19 development cycle, I will resubmit it to you.
Sounds good, but could you please rebase the second patch to be
independent of the first patch? If you do that, I would be happy
to queue it.
Thanx, Paul
> Thanks,
> Ying
>
> > Thanx, Paul
> >
> >> ---
> >> include/linux/rculist.h | 16 ++++++++--------
> >> 1 file changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> >> index 866d9c9..32bd4ad 100644
> >> --- a/include/linux/rculist.h
> >> +++ b/include/linux/rculist.h
> >> @@ -524,11 +524,11 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> >> * @member: the name of the hlist_node within the struct.
> >> */
> >> #define hlist_for_each_entry_continue_rcu(pos, member) \
> >> - for (pos = hlist_entry_safe(rcu_dereference((pos)->member.next),\
> >> - typeof(*(pos)), member); \
> >> + for (pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu( \
> >> + &(pos)->member)), typeof(*(pos)), member); \
> >> pos; \
> >> - pos = hlist_entry_safe(rcu_dereference((pos)->member.next),\
> >> - typeof(*(pos)), member))
> >> + pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu( \
> >> + &(pos)->member)), typeof(*(pos)), member))
> >>
> >> /**
> >> * hlist_for_each_entry_continue_rcu_bh - iterate over a hlist continuing after current point
> >> @@ -536,11 +536,11 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> >> * @member: the name of the hlist_node within the struct.
> >> */
> >> #define hlist_for_each_entry_continue_rcu_bh(pos, member) \
> >> - for (pos = hlist_entry_safe(rcu_dereference_bh((pos)->member.next),\
> >> - typeof(*(pos)), member); \
> >> + for (pos = hlist_entry_safe(rcu_dereference_bh(hlist_next_rcu( \
> >> + &(pos)->member)), typeof(*(pos)), member); \
> >> pos; \
> >> - pos = hlist_entry_safe(rcu_dereference_bh((pos)->member.next),\
> >> - typeof(*(pos)), member))
> >> + pos = hlist_entry_safe(rcu_dereference_bh(hlist_next_rcu( \
> >> + &(pos)->member)), typeof(*(pos)), member))
> >>
> >> /**
> >> * hlist_for_each_entry_from_rcu - iterate over a hlist continuing from current point
> >> --
> >> 1.7.9.5
> >>
> >
> >
> >
>
^ permalink raw reply
* Re: [PATCH net-next v9 0/3] add hisilicon hip04 ethernet driver
From: David Miller @ 2014-12-11 17:04 UTC (permalink / raw)
To: dingtianhong-hv44wF8Li93QT0dZR+AlfA
Cc: zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A, linux-lFZ/pmaqli7XmaaqVzeoHQ,
arnd-r2nGTMty4D4, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
mark.rutland-5wv7dgnIgG8, David.Laight-ZS65k/vG3HxXrIkS9f7CXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1418298150-4944-1-git-send-email-dingtianhong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
The net-next tree is closed, therefore it is not appropriate to
submit net-next changes at this time.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Driver: Vmxnet3: Make Rx ring 2 size configurable
From: David Miller @ 2014-12-11 17:05 UTC (permalink / raw)
To: skhare; +Cc: sbhatewara, pv-drivers, netdev, linux-kernel, bollar
In-Reply-To: <1418276262-4728-1-git-send-email-skhare@vmware.com>
From: Shrikrishna Khare <skhare@vmware.com>
Date: Wed, 10 Dec 2014 21:37:42 -0800
> Rx ring 2 size can be configured by adjusting rx-jumbo parameter
> of ethtool -G.
>
> Signed-off-by: Ramya Bolla <bollar@vmware.com>
> Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
> Signed-off-by: Shrikrishna Khare <skhare@vmware.com>
This is a new feature patch, and the net-next tree is closed, therefore
it is not appropriate to submit a change like this at this time.
Please resubmit when the net-next tree opens back up, some time after
the merge window is over.
^ permalink raw reply
* Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Hubert Sokolowski @ 2014-12-11 17:06 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: netdev, Jamal Hadi Salim
In-Reply-To: <54894850.5000603@cumulusnetworks.com>
My apologies for sending again, I forgot to include a sample output you asked.
> Also, if i hear your concern correctly, for bridge ports that implement
> ndo_fdb_dump, with commit 5e6d243587990a588143b9da3974833649595587, we
> will get two entries for each 'self' entry above.
> Can you also paste sample output for that ?.
>
[root@silpixa00378825 ~]# bridge fdb show brport mac0
33:33:00:00:00:01 self permanent
33:33:00:00:00:01 self permanent
Regards,
Hubert
^ permalink raw reply
* Re: [PATCH v2 1/4 net-next] net: bcmgenet: bcmgenet_init_tx_ring() cleanup
From: David Miller @ 2014-12-11 17:09 UTC (permalink / raw)
To: pgynther; +Cc: netdev, f.fainelli
In-Reply-To: <20141211102030.5E534222AAA@puck.mtv.corp.google.com>
The net-next tree is closed, as I announced last evening, therefore
submitting changes for that tree right now is not appropriate.
Please resubmit this series when the net-next tree opens back up,
and also please when you do so provide a proper cover "0/N" posting
for your patch series.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] swdevice: add new api to set and del bridge port attributes
From: Jiri Pirko @ 2014-12-11 17:11 UTC (permalink / raw)
To: Roopa Prabhu
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
vyasevic, netdev, davem, shm, gospo
In-Reply-To: <5489CBBA.7050302@cumulusnetworks.com>
Thu, Dec 11, 2014 at 05:52:10PM CET, roopa@cumulusnetworks.com wrote:
>On 12/10/14, 1:37 AM, Jiri Pirko wrote:
>>Wed, Dec 10, 2014 at 10:05:18AM CET, roopa@cumulusnetworks.com wrote:
>>>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>>This patch adds two new api's netdev_switch_port_bridge_setlink
>>>and netdev_switch_port_bridge_dellink to offload bridge port attributes
>>>to switch asic
>>>
>>>(The names of the apis look odd with 'switch_port_bridge',
>>>but am more inclined to change the prefix of the api to something else.
>>>Will take any suggestions).
>>>
>>>The api's look at the NETIF_F_HW_NETFUNC_OFFLOAD feature flag to
>>>pass bridge port attributes to the port device.
>>>
>>>If the device has the NETIF_F_HW_NETFUNC_OFFLOAD, but does not support
>>>the bridge port attribute offload ndo, call bridge port attribute ndo's on
>>>the lowerdevs if supported. This is one way to pass bridge port attributes
>>>through stacked netdevs (example when bridge port is a bond and bond slaves
>>>are switch ports).
>>>
>>>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>---
>>>include/net/switchdev.h | 5 +++-
>>>net/switchdev/switchdev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>>>2 files changed, 74 insertions(+), 1 deletion(-)
>>>
>>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>>index 8a6d164..22676b6 100644
>>>--- a/include/net/switchdev.h
>>>+++ b/include/net/switchdev.h
>>>@@ -17,7 +17,10 @@
>>>int netdev_switch_parent_id_get(struct net_device *dev,
>>> struct netdev_phys_item_id *psid);
>>>int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>>>-
>>>+int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>+ struct nlmsghdr *nlh, u16 flags);
>>>+int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>+ struct nlmsghdr *nlh, u16 flags);
>>>#else
>>>
>>>static inline int netdev_switch_parent_id_get(struct net_device *dev,
>>>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>>index d162b21..62317e1 100644
>>>--- a/net/switchdev/switchdev.c
>>>+++ b/net/switchdev/switchdev.c
>>>@@ -50,3 +50,73 @@ int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>>> return ops->ndo_switch_port_stp_update(dev, state);
>>>}
>>>EXPORT_SYMBOL(netdev_switch_port_stp_update);
>>>+
>>>+/**
>>>+ * netdev_switch_port_bridge_setlink - Notify switch device port of bridge
>>>+ * port attributes
>>>+ *
>>>+ * @dev: port device
>>>+ * @nlh: netlink msg with bridge port attributes
>>>+ *
>>>+ * Notify switch device port of bridge port attributes
>>>+ */
>>>+int netdev_switch_port_bridge_setlink(struct net_device *dev,
>>>+ struct nlmsghdr *nlh, u16 flags)
>>>+{
>>>+ const struct net_device_ops *ops = dev->netdev_ops;
>>>+ struct net_device *lower_dev;
>>>+ struct list_head *iter;
>>>+ int ret = 0, err = 0;
>>>+
>>>+ if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>+ return err;
>>>+
>>>+ if (ops->ndo_bridge_setlink) {
>>>+ WARN_ON(!ops->ndo_switch_parent_id_get);
>>>+ return ops->ndo_bridge_setlink(dev, nlh, flags);
>> You have to change ndo_bridge_setlink in netdevice.h first.
>> Otherwise when only this patch is applied (during bisection)
>> this won't compile.
>
>ack, will fix it and keep that in mind next time.
>>
>>>+ }
>>>+
>>>+ netdev_for_each_lower_dev(dev, lower_dev, iter) {
>> I do not understand why to iterate over lower devices. At this
>> stage we don't know a thing about this upper or its lowers. Let
>> the uppers (/masters) to decide if this needs to be propagated
>> or not.
>
>Jiri, In the stacked devices case, there is no way to propagate the bridge
>port attributes to switch device driver today (vlan and other bridge port
>attributes). Can you tell me if there is a way ?. no, ndo_vlan* ndo's are not
>useful here. Nor we should go and implement ndo_bridge_setlink* in all
>devices that can be bridge ports.
Hmm. I just think that is cleaner to implement ndo_bridge_setlink in
bonding for example and let it propagate the the call to slaves.
Let every "upper" to handle ndo_bridge_setlink their way. Sometimes it
might not make sense to propagate to "lowers".
>
>And this allows a switch driver to receive these callbacks if it has marked
>the switch port with an offload flag. Your way of using the switch port to
>get to the switch driver does not help in these cases.
I do not follow how this is related to this case (stacked layout).
>
>The other option is to use the 'switch device (not port)' to get to the
>switch driver.
That would not help this case (stacked layout) I believe.
>This patch shows that you can still do this with the ndo ops.
>>
>>>+ err = netdev_switch_port_bridge_setlink(lower_dev, nlh, flags);
>>>+ if (err)
>>>+ ret = err;
>>>+ }
>> ^^^^^ Indent is off. This should be catched by scripts/checkpatch.pl.
>>
>>>+
>>>+ return ret;
>>>+}
>>>+EXPORT_SYMBOL(netdev_switch_port_bridge_setlink);
>>>+
>>>+/**
>>>+ * netdev_switch_port_bridge_dellink - Notify switch device port of bridge
>>>+ * attribute delete
>>>+ *
>>>+ * @dev: port device
>>>+ * @nlh: netlink msg with bridge port attributes
>>>+ *
>>>+ * Notify switch device port of bridge port attribute delete
>>>+ */
>>>+int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>+ struct nlmsghdr *nlh, u16 flags)
>>>+{
>>>+ const struct net_device_ops *ops = dev->netdev_ops;
>>>+ struct net_device *lower_dev;
>>>+ struct list_head *iter;
>>>+ int ret = 0, err = 0;
>>>+
>>>+ if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>+ return err;
>>>+
>>>+ if (ops->ndo_bridge_dellink) {
>>>+ WARN_ON(!ops->ndo_switch_parent_id_get);
>>>+ return ops->ndo_bridge_dellink(dev, nlh, flags);
>>>+ }
>>>+
>>>+ netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>+ err = netdev_switch_port_bridge_dellink(lower_dev, nlh, flags);
>>>+ if (err)
>>>+ ret = err;
>>>+ }
>>>+
>>>+ return ret;
>>>+}
>>>+EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>>>--
>>>1.7.10.4
>>>
>
^ permalink raw reply
* Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver
From: Murali Karicheri @ 2014-12-11 17:17 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A, WingMan Kwok
In-Reply-To: <20141211.120104.312460507509497826.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On 12/11/2014 12:01 PM, David Miller wrote:
> From: Murali Karicheri<m-karicheri2-l0cyMroinI0@public.gmane.org>
> Date: Thu, 11 Dec 2014 09:14:37 -0500
>
>> BTW, could you provide any suggestions that would help us merge this
>> series to upstream? This has been sitting on this list for a while
>> now.
>
> You simply have to continue going through the review and revision
> process until people no longer find problems with your changes.
>
> This could take several more weeks, you simply must be patient.
Ok. Thanks. That is encouraging to hear!
Regards,
--
Murali Karicheri
Linux Kernel, Texas Instruments
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
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