* [PATCH net-next 1/3] cxgb4: add mirror action to TC-MATCHALL offload
2020-06-25 11:58 [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL Rahul Lakkireddy
@ 2020-06-25 11:58 ` Rahul Lakkireddy
2020-06-27 4:18 ` Jakub Kicinski
2020-06-25 11:58 ` [PATCH net-next 2/3] cxgb4: add support for mirror Rxqs Rahul Lakkireddy
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Rahul Lakkireddy @ 2020-06-25 11:58 UTC (permalink / raw)
To: netdev; +Cc: davem, nirranjan, vishal, dt
Add mirror Virtual Interface (VI) support to receive all ingress
mirror traffic from the underlying device. The mirror VI is
created dynamically, if the TC-MATCHALL rule has a corresponding
mirror action. Also request MSI-X vectors needed for the mirror VI
Rxqs. If no vectors are available, then disable mirror VI support.
Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 10 ++
.../net/ethernet/chelsio/cxgb4/cxgb4_main.c | 94 ++++++++++++++++++-
.../ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 16 +++-
.../ethernet/chelsio/cxgb4/cxgb4_tc_flower.h | 3 +-
.../chelsio/cxgb4/cxgb4_tc_matchall.c | 57 ++++++++++-
.../chelsio/cxgb4/cxgb4_tc_matchall.h | 1 +
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 16 ++++
7 files changed, 187 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index d811df1b93d9..14ef48e82cde 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -679,6 +679,11 @@ struct port_info {
u8 rx_cchan;
bool tc_block_shared;
+
+ /* Mirror VI information */
+ u16 viid_mirror;
+ refcount_t vi_mirror_refcnt;
+ u16 nmirrorqsets;
};
struct dentry;
@@ -960,6 +965,7 @@ struct sge {
u16 ofldqsets; /* # of active ofld queue sets */
u16 nqs_per_uld; /* # of Rx queues per ULD */
u16 eoqsets; /* # of ETHOFLD queues */
+ u16 mirrorqsets; /* # of Mirror queues */
u16 timer_val[SGE_NTIMERS];
u8 counter_val[SGE_NCOUNTERS];
@@ -1857,6 +1863,8 @@ int t4_init_rss_mode(struct adapter *adap, int mbox);
int t4_init_portinfo(struct port_info *pi, int mbox,
int port, int pf, int vf, u8 mac[]);
int t4_port_init(struct adapter *adap, int mbox, int pf, int vf);
+int t4_init_port_mirror(struct port_info *pi, u8 mbox, u8 port, u8 pf, u8 vf,
+ u16 *mirror_viid);
void t4_fatal_err(struct adapter *adapter);
unsigned int t4_chip_rss_size(struct adapter *adapter);
int t4_config_rss_range(struct adapter *adapter, int mbox, unsigned int viid,
@@ -2141,6 +2149,8 @@ int cxgb_open(struct net_device *dev);
int cxgb_close(struct net_device *dev);
void cxgb4_enable_rx(struct adapter *adap, struct sge_rspq *q);
void cxgb4_quiesce_rx(struct sge_rspq *q);
+int cxgb4_port_mirror_alloc(struct net_device *dev);
+void cxgb4_port_mirror_free(struct net_device *dev);
#ifdef CONFIG_CHELSIO_TLS_DEVICE
int cxgb4_set_ktls_feature(struct adapter *adap, bool enable);
#endif
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 87505a0d906a..cb4cb2d70e6d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -1285,6 +1285,47 @@ static int setup_debugfs(struct adapter *adap)
return 0;
}
+int cxgb4_port_mirror_alloc(struct net_device *dev)
+{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
+ int ret;
+
+ if (!pi->nmirrorqsets)
+ return -EOPNOTSUPP;
+
+ if (pi->viid_mirror) {
+ refcount_inc(&pi->vi_mirror_refcnt);
+ return 0;
+ }
+
+ ret = t4_init_port_mirror(pi, adap->mbox, pi->port_id, adap->pf, 0,
+ &pi->viid_mirror);
+ if (ret)
+ return ret;
+
+ refcount_set(&pi->vi_mirror_refcnt, 1);
+ return 0;
+}
+
+void cxgb4_port_mirror_free(struct net_device *dev)
+{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
+
+ if (!pi->viid_mirror)
+ return;
+
+ if (refcount_read(&pi->vi_mirror_refcnt) > 1) {
+ refcount_dec(&pi->vi_mirror_refcnt);
+ return;
+ }
+
+ refcount_set(&pi->vi_mirror_refcnt, 0);
+ t4_free_vi(adap, adap->mbox, adap->pf, 0, pi->viid_mirror);
+ pi->viid_mirror = 0;
+}
+
/*
* upper-layer driver support
*/
@@ -5503,6 +5544,19 @@ static int cfg_queues(struct adapter *adap)
avail_qsets -= s->eoqsets;
}
+ /* Mirror queues must follow same scheme as normal Ethernet
+ * Queues, when there are enough queues available. Otherwise,
+ * allocate at least 1 queue per port. If even 1 queue is not
+ * available, then disable mirror queues support.
+ */
+ if (avail_qsets >= s->max_ethqsets)
+ s->mirrorqsets = s->max_ethqsets;
+ else if (avail_qsets >= adap->params.nports)
+ s->mirrorqsets = adap->params.nports;
+ else
+ s->mirrorqsets = 0;
+ avail_qsets -= s->mirrorqsets;
+
for (i = 0; i < ARRAY_SIZE(s->ethrxq); i++) {
struct sge_eth_rxq *r = &s->ethrxq[i];
@@ -5616,8 +5670,8 @@ void cxgb4_free_msix_idx_in_bmap(struct adapter *adap,
static int enable_msix(struct adapter *adap)
{
- u32 eth_need, uld_need = 0, ethofld_need = 0;
- u32 ethqsets = 0, ofldqsets = 0, eoqsets = 0;
+ u32 eth_need, uld_need = 0, ethofld_need = 0, mirror_need = 0;
+ u32 ethqsets = 0, ofldqsets = 0, eoqsets = 0, mirrorqsets = 0;
u8 num_uld = 0, nchan = adap->params.nports;
u32 i, want, need, num_vec;
struct sge *s = &adap->sge;
@@ -5648,6 +5702,12 @@ static int enable_msix(struct adapter *adap)
need += ethofld_need;
}
+ if (s->mirrorqsets) {
+ want += s->mirrorqsets;
+ mirror_need = nchan;
+ need += mirror_need;
+ }
+
want += EXTRA_VECS;
need += EXTRA_VECS;
@@ -5681,8 +5741,10 @@ static int enable_msix(struct adapter *adap)
adap->params.ethofld = 0;
s->ofldqsets = 0;
s->eoqsets = 0;
+ s->mirrorqsets = 0;
uld_need = 0;
ethofld_need = 0;
+ mirror_need = 0;
}
num_vec = allocated;
@@ -5696,6 +5758,8 @@ static int enable_msix(struct adapter *adap)
ofldqsets = nchan;
if (is_ethofld(adap))
eoqsets = ethofld_need;
+ if (s->mirrorqsets)
+ mirrorqsets = mirror_need;
num_vec -= need;
while (num_vec) {
@@ -5727,12 +5791,25 @@ static int enable_msix(struct adapter *adap)
num_vec -= uld_need;
}
}
+
+ if (s->mirrorqsets) {
+ while (num_vec) {
+ if (num_vec < mirror_need ||
+ mirrorqsets > s->mirrorqsets)
+ break;
+
+ mirrorqsets++;
+ num_vec -= mirror_need;
+ }
+ }
} else {
ethqsets = s->max_ethqsets;
if (is_uld(adap))
ofldqsets = s->ofldqsets;
if (is_ethofld(adap))
eoqsets = s->eoqsets;
+ if (s->mirrorqsets)
+ mirrorqsets = s->mirrorqsets;
}
if (ethqsets < s->max_ethqsets) {
@@ -5748,6 +5825,14 @@ static int enable_msix(struct adapter *adap)
if (is_ethofld(adap))
s->eoqsets = eoqsets;
+ if (s->mirrorqsets) {
+ s->mirrorqsets = mirrorqsets;
+ for_each_port(adap, i) {
+ pi = adap2pinfo(adap, i);
+ pi->nmirrorqsets = s->mirrorqsets / nchan;
+ }
+ }
+
/* map for msix */
ret = alloc_msix_info(adap, allocated);
if (ret)
@@ -5759,8 +5844,9 @@ static int enable_msix(struct adapter *adap)
}
dev_info(adap->pdev_dev,
- "%d MSI-X vectors allocated, nic %d eoqsets %d per uld %d\n",
- allocated, s->max_ethqsets, s->eoqsets, s->nqs_per_uld);
+ "%d MSI-X vectors allocated, nic %d eoqsets %d per uld %d mirrorqsets %d\n",
+ allocated, s->max_ethqsets, s->eoqsets, s->nqs_per_uld,
+ s->mirrorqsets);
kfree(entries);
return 0;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index ccc7aab9a8be..4e8bcc51253d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -382,6 +382,7 @@ void cxgb4_process_flow_actions(struct net_device *in,
case FLOW_ACTION_DROP:
fs->action = FILTER_DROP;
break;
+ case FLOW_ACTION_MIRRED:
case FLOW_ACTION_REDIRECT: {
struct net_device *out = act->dev;
struct port_info *pi = netdev_priv(out);
@@ -539,7 +540,8 @@ static bool valid_pedit_action(struct net_device *dev,
int cxgb4_validate_flow_actions(struct net_device *dev,
struct flow_action *actions,
- struct netlink_ext_ack *extack)
+ struct netlink_ext_ack *extack,
+ u8 matchall_filter)
{
struct flow_action_entry *act;
bool act_redir = false;
@@ -556,11 +558,19 @@ int cxgb4_validate_flow_actions(struct net_device *dev,
case FLOW_ACTION_DROP:
/* Do nothing */
break;
+ case FLOW_ACTION_MIRRED:
case FLOW_ACTION_REDIRECT: {
struct adapter *adap = netdev2adap(dev);
struct net_device *n_dev, *target_dev;
- unsigned int i;
bool found = false;
+ unsigned int i;
+
+ if (act->id == FLOW_ACTION_MIRRED &&
+ !matchall_filter) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Egress mirror action is only supported for tc-matchall");
+ return -EOPNOTSUPP;
+ }
target_dev = act->dev;
for_each_port(adap, i) {
@@ -699,7 +709,7 @@ int cxgb4_flow_rule_replace(struct net_device *dev, struct flow_rule *rule,
u8 inet_family;
int fidx, ret;
- if (cxgb4_validate_flow_actions(dev, &rule->action, extack))
+ if (cxgb4_validate_flow_actions(dev, &rule->action, extack, 0))
return -EOPNOTSUPP;
if (cxgb4_validate_flow_match(dev, rule))
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.h
index befa459324fb..6296e1d5a12b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.h
@@ -113,7 +113,8 @@ void cxgb4_process_flow_actions(struct net_device *in,
struct ch_filter_specification *fs);
int cxgb4_validate_flow_actions(struct net_device *dev,
struct flow_action *actions,
- struct netlink_ext_ack *extack);
+ struct netlink_ext_ack *extack,
+ u8 matchall_filter);
int cxgb4_tc_flower_replace(struct net_device *dev,
struct flow_cls_offload *cls);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
index c439b5bce9c9..e377e50c2492 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
@@ -188,6 +188,49 @@ static void cxgb4_matchall_free_tc(struct net_device *dev)
tc_port_matchall->egress.state = CXGB4_MATCHALL_STATE_DISABLED;
}
+static int cxgb4_matchall_mirror_alloc(struct net_device *dev,
+ struct tc_cls_matchall_offload *cls)
+{
+ struct netlink_ext_ack *extack = cls->common.extack;
+ struct cxgb4_tc_port_matchall *tc_port_matchall;
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
+ struct flow_action_entry *act;
+ int ret;
+ u32 i;
+
+ tc_port_matchall = &adap->tc_matchall->port_matchall[pi->port_id];
+ flow_action_for_each(i, act, &cls->rule->action) {
+ if (act->id == FLOW_ACTION_MIRRED) {
+ ret = cxgb4_port_mirror_alloc(dev);
+ if (ret) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Couldn't allocate mirror");
+ return ret;
+ }
+
+ tc_port_matchall->ingress.viid_mirror = pi->viid_mirror;
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static void cxgb4_matchall_mirror_free(struct net_device *dev)
+{
+ struct cxgb4_tc_port_matchall *tc_port_matchall;
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
+
+ tc_port_matchall = &adap->tc_matchall->port_matchall[pi->port_id];
+ if (!tc_port_matchall->ingress.viid_mirror)
+ return;
+
+ cxgb4_port_mirror_free(dev);
+ tc_port_matchall->ingress.viid_mirror = 0;
+}
+
static int cxgb4_matchall_alloc_filter(struct net_device *dev,
struct tc_cls_matchall_offload *cls)
{
@@ -211,6 +254,10 @@ static int cxgb4_matchall_alloc_filter(struct net_device *dev,
return -ENOMEM;
}
+ ret = cxgb4_matchall_mirror_alloc(dev, cls);
+ if (ret)
+ return ret;
+
tc_port_matchall = &adap->tc_matchall->port_matchall[pi->port_id];
fs = &tc_port_matchall->ingress.fs;
memset(fs, 0, sizeof(*fs));
@@ -229,11 +276,15 @@ static int cxgb4_matchall_alloc_filter(struct net_device *dev,
ret = cxgb4_set_filter(dev, fidx, fs);
if (ret)
- return ret;
+ goto out_free;
tc_port_matchall->ingress.tid = fidx;
tc_port_matchall->ingress.state = CXGB4_MATCHALL_STATE_ENABLED;
return 0;
+
+out_free:
+ cxgb4_matchall_mirror_free(dev);
+ return ret;
}
static int cxgb4_matchall_free_filter(struct net_device *dev)
@@ -250,6 +301,8 @@ static int cxgb4_matchall_free_filter(struct net_device *dev)
if (ret)
return ret;
+ cxgb4_matchall_mirror_free(dev);
+
tc_port_matchall->ingress.packets = 0;
tc_port_matchall->ingress.bytes = 0;
tc_port_matchall->ingress.last_used = 0;
@@ -279,7 +332,7 @@ int cxgb4_tc_matchall_replace(struct net_device *dev,
ret = cxgb4_validate_flow_actions(dev,
&cls_matchall->rule->action,
- extack);
+ extack, 1);
if (ret)
return ret;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.h
index ab6b5683dfd3..e264b6e606c4 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.h
@@ -21,6 +21,7 @@ struct cxgb4_matchall_ingress_entry {
enum cxgb4_matchall_state state; /* Current MATCHALL offload state */
u32 tid; /* Index to hardware filter entry */
struct ch_filter_specification fs; /* Filter entry */
+ u16 viid_mirror; /* Identifier for allocated Mirror VI */
u64 bytes; /* # of bytes hitting the filter */
u64 packets; /* # of packets hitting the filter */
u64 last_used; /* Last updated jiffies time */
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index 8f2b7c9aa384..a2343631c951 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -9712,6 +9712,22 @@ int t4_port_init(struct adapter *adap, int mbox, int pf, int vf)
return 0;
}
+int t4_init_port_mirror(struct port_info *pi, u8 mbox, u8 port, u8 pf, u8 vf,
+ u16 *mirror_viid)
+{
+ int ret;
+
+ ret = t4_alloc_vi(pi->adapter, mbox, port, pf, vf, 1, NULL, NULL,
+ NULL, NULL);
+ if (ret < 0)
+ return ret;
+
+ if (mirror_viid)
+ *mirror_viid = ret;
+
+ return 0;
+}
+
/**
* t4_read_cimq_cfg - read CIM queue configuration
* @adap: the adapter
--
2.24.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next 1/3] cxgb4: add mirror action to TC-MATCHALL offload
2020-06-25 11:58 ` [PATCH net-next 1/3] cxgb4: add mirror action to TC-MATCHALL offload Rahul Lakkireddy
@ 2020-06-27 4:18 ` Jakub Kicinski
2020-06-27 6:48 ` Rahul Lakkireddy
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-06-27 4:18 UTC (permalink / raw)
To: Rahul Lakkireddy; +Cc: netdev, davem, nirranjan, vishal, dt
On Thu, 25 Jun 2020 17:28:41 +0530 Rahul Lakkireddy wrote:
> + if (refcount_read(&pi->vi_mirror_refcnt) > 1) {
> + refcount_dec(&pi->vi_mirror_refcnt);
> + return;
> + }
FWIW this looks very dodgy. If you know nothing changes the count
between the read and the dec here, you probably don't need atomic
refcounts at all..
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 1/3] cxgb4: add mirror action to TC-MATCHALL offload
2020-06-27 4:18 ` Jakub Kicinski
@ 2020-06-27 6:48 ` Rahul Lakkireddy
0 siblings, 0 replies; 13+ messages in thread
From: Rahul Lakkireddy @ 2020-06-27 6:48 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, nirranjan, vishal, dt
On Friday, June 06/26/20, 2020 at 21:18:44 -0700, Jakub Kicinski wrote:
> On Thu, 25 Jun 2020 17:28:41 +0530 Rahul Lakkireddy wrote:
> > + if (refcount_read(&pi->vi_mirror_refcnt) > 1) {
> > + refcount_dec(&pi->vi_mirror_refcnt);
> > + return;
> > + }
>
> FWIW this looks very dodgy. If you know nothing changes the count
> between the read and the dec here, you probably don't need atomic
> refcounts at all..
Currently, all the callers accessing this refcount and its related
data is having the RTNL lock held by the stack. Perhaps this is a
false sense of security, especially if the stack API may change in
the future.
I'll add a proper lock to protect this data in v2 to be on the safer
side.
Thanks,
Rahul
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 2/3] cxgb4: add support for mirror Rxqs
2020-06-25 11:58 [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL Rahul Lakkireddy
2020-06-25 11:58 ` [PATCH net-next 1/3] cxgb4: add mirror action to TC-MATCHALL offload Rahul Lakkireddy
@ 2020-06-25 11:58 ` Rahul Lakkireddy
2020-06-25 11:58 ` [PATCH net-next 3/3] cxgb4: add main VI to mirror VI config replication Rahul Lakkireddy
2020-06-25 22:55 ` [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL Jakub Kicinski
3 siblings, 0 replies; 13+ messages in thread
From: Rahul Lakkireddy @ 2020-06-25 11:58 UTC (permalink / raw)
To: netdev; +Cc: davem, nirranjan, vishal, dt
When mirror VI is enabled, allocate the mirror Rxqs and setup the
mirror VI RSS table. The mirror Rxqs are allocated/freed when
the mirror VI is created/destroyed or when underlying port is
brought up/down, respectively.
Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 11 +
.../ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 69 ++++-
.../net/ethernet/chelsio/cxgb4/cxgb4_main.c | 238 ++++++++++++++++--
3 files changed, 296 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 14ef48e82cde..b4616e8cea2d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -710,6 +710,13 @@ enum {
ULP_CRYPTO_KTLS_INLINE = 1 << 3,
};
+#define CXGB4_MIRROR_RXQ_DEFAULT_DESC_NUM 1024
+#define CXGB4_MIRROR_RXQ_DEFAULT_DESC_SIZE 64
+#define CXGB4_MIRROR_RXQ_DEFAULT_INTR_USEC 5
+#define CXGB4_MIRROR_RXQ_DEFAULT_PKT_CNT 8
+
+#define CXGB4_MIRROR_FLQ_DEFAULT_DESC_NUM 72
+
struct rx_sw_desc;
struct sge_fl { /* SGE free-buffer queue state */
@@ -959,6 +966,8 @@ struct sge {
struct sge_eohw_txq *eohw_txq;
struct sge_ofld_rxq *eohw_rxq;
+ struct sge_eth_rxq *mirror_rxq[NCHAN];
+
u16 max_ethqsets; /* # of available Ethernet queue sets */
u16 ethqsets; /* # of active Ethernet queue sets */
u16 ethtxq_rover; /* Tx queue to clean up next */
@@ -992,6 +1001,8 @@ struct sge {
int fwevtq_msix_idx; /* Index to firmware event queue MSI-X info */
int nd_msix_idx; /* Index to non-data interrupts MSI-X info */
+
+ struct mutex queue_mutex; /* Sync access to dynamic queue info */
};
#define for_each_ethrxq(sge, i) for (i = 0; i < (sge)->ethqsets; i++)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index 828499256004..1e1605576368 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -2743,6 +2743,61 @@ do { \
}
r -= eth_entries;
+ mutex_lock(&s->queue_mutex);
+ for_each_port(adap, j) {
+ struct port_info *pi = adap2pinfo(adap, j);
+ const struct sge_eth_rxq *rx;
+
+ if (!refcount_read(&pi->vi_mirror_refcnt))
+ continue;
+
+ if (r >= DIV_ROUND_UP(pi->nmirrorqsets, 4)) {
+ r -= DIV_ROUND_UP(pi->nmirrorqsets, 4);
+ continue;
+ }
+
+ if (!s->mirror_rxq[j]) {
+ mutex_unlock(&s->queue_mutex);
+ goto out;
+ }
+
+ rx = &s->mirror_rxq[j][r * 4];
+ n = min(4, pi->nmirrorqsets - 4 * r);
+
+ S("QType:", "Mirror-Rxq");
+ S("Interface:",
+ rx[i].rspq.netdev ? rx[i].rspq.netdev->name : "N/A");
+ R("RspQ ID:", rspq.abs_id);
+ R("RspQ size:", rspq.size);
+ R("RspQE size:", rspq.iqe_len);
+ R("RspQ CIDX:", rspq.cidx);
+ R("RspQ Gen:", rspq.gen);
+ S3("u", "Intr delay:", qtimer_val(adap, &rx[i].rspq));
+ S3("u", "Intr pktcnt:", s->counter_val[rx[i].rspq.pktcnt_idx]);
+ R("FL ID:", fl.cntxt_id);
+ R("FL size:", fl.size - 8);
+ R("FL pend:", fl.pend_cred);
+ R("FL avail:", fl.avail);
+ R("FL PIDX:", fl.pidx);
+ R("FL CIDX:", fl.cidx);
+ RL("RxPackets:", stats.pkts);
+ RL("RxCSO:", stats.rx_cso);
+ RL("VLANxtract:", stats.vlan_ex);
+ RL("LROmerged:", stats.lro_merged);
+ RL("LROpackets:", stats.lro_pkts);
+ RL("RxDrops:", stats.rx_drops);
+ RL("RxBadPkts:", stats.bad_rx_pkts);
+ RL("FLAllocErr:", fl.alloc_failed);
+ RL("FLLrgAlcErr:", fl.large_alloc_failed);
+ RL("FLMapErr:", fl.mapping_err);
+ RL("FLLow:", fl.low);
+ RL("FLStarving:", fl.starving);
+
+ mutex_unlock(&s->queue_mutex);
+ goto out;
+ }
+ mutex_unlock(&s->queue_mutex);
+
if (!adap->tc_mqprio)
goto skip_mqprio;
@@ -3099,9 +3154,10 @@ do { \
return 0;
}
-static int sge_queue_entries(const struct adapter *adap)
+static int sge_queue_entries(struct adapter *adap)
{
int i, tot_uld_entries = 0, eohw_entries = 0, eosw_entries = 0;
+ int mirror_rxq_entries = 0;
if (adap->tc_mqprio) {
struct cxgb4_tc_port_mqprio *port_mqprio;
@@ -3124,6 +3180,15 @@ static int sge_queue_entries(const struct adapter *adap)
mutex_unlock(&adap->tc_mqprio->mqprio_mutex);
}
+ mutex_lock(&adap->sge.queue_mutex);
+ for_each_port(adap, i) {
+ struct port_info *pi = adap2pinfo(adap, i);
+
+ if (refcount_read(&pi->vi_mirror_refcnt))
+ mirror_rxq_entries += DIV_ROUND_UP(pi->nmirrorqsets, 4);
+ }
+ mutex_unlock(&adap->sge.queue_mutex);
+
if (!is_uld(adap))
goto lld_only;
@@ -3138,7 +3203,7 @@ static int sge_queue_entries(const struct adapter *adap)
mutex_unlock(&uld_mutex);
lld_only:
- return DIV_ROUND_UP(adap->sge.ethqsets, 4) +
+ return DIV_ROUND_UP(adap->sge.ethqsets, 4) + mirror_rxq_entries +
eohw_entries + eosw_entries + tot_uld_entries +
DIV_ROUND_UP(MAX_CTRL_QUEUES, 4) + 1;
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index cb4cb2d70e6d..0517eac4c8a5 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -822,6 +822,31 @@ static void adap_config_hpfilter(struct adapter *adapter)
"HP filter region isn't supported by FW\n");
}
+static int cxgb4_config_rss(const struct port_info *pi, u16 *rss,
+ u16 rss_size, u16 viid)
+{
+ struct adapter *adap = pi->adapter;
+ int ret;
+
+ ret = t4_config_rss_range(adap, adap->mbox, viid, 0, rss_size, rss,
+ rss_size);
+ if (ret)
+ return ret;
+
+ /* If Tunnel All Lookup isn't specified in the global RSS
+ * Configuration, then we need to specify a default Ingress
+ * Queue for any ingress packets which aren't hashed. We'll
+ * use our first ingress queue ...
+ */
+ return t4_config_vi_rss(adap, adap->mbox, viid,
+ FW_RSS_VI_CONFIG_CMD_IP6FOURTUPEN_F |
+ FW_RSS_VI_CONFIG_CMD_IP6TWOTUPEN_F |
+ FW_RSS_VI_CONFIG_CMD_IP4FOURTUPEN_F |
+ FW_RSS_VI_CONFIG_CMD_IP4TWOTUPEN_F |
+ FW_RSS_VI_CONFIG_CMD_UDPEN_F,
+ rss[0]);
+}
+
/**
* cxgb4_write_rss - write the RSS table for a given port
* @pi: the port
@@ -833,10 +858,10 @@ static void adap_config_hpfilter(struct adapter *adapter)
*/
int cxgb4_write_rss(const struct port_info *pi, const u16 *queues)
{
- u16 *rss;
- int i, err;
struct adapter *adapter = pi->adapter;
const struct sge_eth_rxq *rxq;
+ int i, err;
+ u16 *rss;
rxq = &adapter->sge.ethrxq[pi->first_qset];
rss = kmalloc_array(pi->rss_size, sizeof(u16), GFP_KERNEL);
@@ -847,21 +872,7 @@ int cxgb4_write_rss(const struct port_info *pi, const u16 *queues)
for (i = 0; i < pi->rss_size; i++, queues++)
rss[i] = rxq[*queues].rspq.abs_id;
- err = t4_config_rss_range(adapter, adapter->pf, pi->viid, 0,
- pi->rss_size, rss, pi->rss_size);
- /* If Tunnel All Lookup isn't specified in the global RSS
- * Configuration, then we need to specify a default Ingress
- * Queue for any ingress packets which aren't hashed. We'll
- * use our first ingress queue ...
- */
- if (!err)
- err = t4_config_vi_rss(adapter, adapter->mbox, pi->viid,
- FW_RSS_VI_CONFIG_CMD_IP6FOURTUPEN_F |
- FW_RSS_VI_CONFIG_CMD_IP6TWOTUPEN_F |
- FW_RSS_VI_CONFIG_CMD_IP4FOURTUPEN_F |
- FW_RSS_VI_CONFIG_CMD_IP4TWOTUPEN_F |
- FW_RSS_VI_CONFIG_CMD_UDPEN_F,
- rss[0]);
+ err = cxgb4_config_rss(pi, rss, pi->rss_size, pi->viid);
kfree(rss);
return err;
}
@@ -1285,6 +1296,162 @@ static int setup_debugfs(struct adapter *adap)
return 0;
}
+static void cxgb4_port_mirror_free_rxq(struct adapter *adap,
+ struct sge_eth_rxq *mirror_rxq)
+{
+ if ((adap->flags & CXGB4_FULL_INIT_DONE) &&
+ !(adap->flags & CXGB4_SHUTTING_DOWN))
+ cxgb4_quiesce_rx(&mirror_rxq->rspq);
+
+ if (adap->flags & CXGB4_USING_MSIX) {
+ cxgb4_clear_msix_aff(mirror_rxq->msix->vec,
+ mirror_rxq->msix->aff_mask);
+ free_irq(mirror_rxq->msix->vec, &mirror_rxq->rspq);
+ cxgb4_free_msix_idx_in_bmap(adap, mirror_rxq->msix->idx);
+ }
+
+ free_rspq_fl(adap, &mirror_rxq->rspq, &mirror_rxq->fl);
+}
+
+static int cxgb4_port_mirror_alloc_queues(struct net_device *dev)
+{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
+ struct sge_eth_rxq *mirror_rxq;
+ struct sge *s = &adap->sge;
+ int ret = 0, msix = 0;
+ u16 i, rxqid;
+ u16 *rss;
+
+ if (!refcount_read(&pi->vi_mirror_refcnt))
+ return 0;
+
+ mutex_lock(&s->queue_mutex);
+ if (s->mirror_rxq[pi->port_id])
+ goto out_unlock;
+
+ mirror_rxq = kcalloc(pi->nmirrorqsets, sizeof(*mirror_rxq), GFP_KERNEL);
+ if (!mirror_rxq) {
+ ret = -ENOMEM;
+ goto out_unlock;
+ }
+
+ s->mirror_rxq[pi->port_id] = mirror_rxq;
+
+ if (!(adap->flags & CXGB4_USING_MSIX))
+ msix = -((int)adap->sge.intrq.abs_id + 1);
+
+ for (i = 0, rxqid = 0; i < pi->nmirrorqsets; i++, rxqid++) {
+ mirror_rxq = &s->mirror_rxq[pi->port_id][i];
+
+ /* Allocate Mirror Rxqs */
+ if (msix >= 0) {
+ msix = cxgb4_get_msix_idx_from_bmap(adap);
+ if (msix < 0) {
+ ret = msix;
+ goto out_free_queues;
+ }
+
+ mirror_rxq->msix = &adap->msix_info[msix];
+ snprintf(mirror_rxq->msix->desc,
+ sizeof(mirror_rxq->msix->desc),
+ "%s-mirrorrxq%d", dev->name, i);
+ }
+
+ init_rspq(adap, &mirror_rxq->rspq,
+ CXGB4_MIRROR_RXQ_DEFAULT_INTR_USEC,
+ CXGB4_MIRROR_RXQ_DEFAULT_PKT_CNT,
+ CXGB4_MIRROR_RXQ_DEFAULT_DESC_NUM,
+ CXGB4_MIRROR_RXQ_DEFAULT_DESC_SIZE);
+
+ mirror_rxq->fl.size = CXGB4_MIRROR_FLQ_DEFAULT_DESC_NUM;
+
+ ret = t4_sge_alloc_rxq(adap, &mirror_rxq->rspq, false,
+ dev, msix, &mirror_rxq->fl,
+ t4_ethrx_handler, NULL, 0);
+ if (ret)
+ goto out_free_msix_idx;
+
+ /* Setup MSI-X vectors for Mirror Rxqs */
+ if (adap->flags & CXGB4_USING_MSIX) {
+ ret = request_irq(mirror_rxq->msix->vec,
+ t4_sge_intr_msix, 0,
+ mirror_rxq->msix->desc,
+ &mirror_rxq->rspq);
+ if (ret)
+ goto out_free_rxq;
+
+ cxgb4_set_msix_aff(adap, mirror_rxq->msix->vec,
+ &mirror_rxq->msix->aff_mask, i);
+ }
+
+ /* Start NAPI for Mirror Rxqs */
+ cxgb4_enable_rx(adap, &mirror_rxq->rspq);
+ }
+
+ /* Setup RSS for Mirror Rxqs */
+ rss = kcalloc(pi->rss_size, sizeof(u16), GFP_KERNEL);
+ if (!rss) {
+ ret = -ENOMEM;
+ goto out_free_queues;
+ }
+
+ mirror_rxq = &s->mirror_rxq[pi->port_id][0];
+ for (i = 0; i < pi->rss_size; i++)
+ rss[i] = mirror_rxq[i % pi->nmirrorqsets].rspq.abs_id;
+
+ ret = cxgb4_config_rss(pi, rss, pi->rss_size, pi->viid_mirror);
+ kfree(rss);
+ if (ret)
+ goto out_free_queues;
+
+ mutex_unlock(&s->queue_mutex);
+ return 0;
+
+out_free_rxq:
+ free_rspq_fl(adap, &mirror_rxq->rspq, &mirror_rxq->fl);
+
+out_free_msix_idx:
+ cxgb4_free_msix_idx_in_bmap(adap, mirror_rxq->msix->idx);
+
+out_free_queues:
+ while (rxqid-- > 0)
+ cxgb4_port_mirror_free_rxq(adap,
+ &s->mirror_rxq[pi->port_id][rxqid]);
+
+ kfree(s->mirror_rxq[pi->port_id]);
+ s->mirror_rxq[pi->port_id] = NULL;
+
+out_unlock:
+ mutex_unlock(&s->queue_mutex);
+ return ret;
+}
+
+static void cxgb4_port_mirror_free_queues(struct net_device *dev)
+{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
+ struct sge *s = &adap->sge;
+ u16 i;
+
+ if (!refcount_read(&pi->vi_mirror_refcnt))
+ return;
+
+ mutex_lock(&s->queue_mutex);
+ if (!s->mirror_rxq[pi->port_id])
+ goto out_unlock;
+
+ for (i = 0; i < pi->nmirrorqsets; i++)
+ cxgb4_port_mirror_free_rxq(adap,
+ &s->mirror_rxq[pi->port_id][i]);
+
+ kfree(s->mirror_rxq[pi->port_id]);
+ s->mirror_rxq[pi->port_id] = NULL;
+
+out_unlock:
+ mutex_unlock(&s->queue_mutex);
+}
+
int cxgb4_port_mirror_alloc(struct net_device *dev)
{
struct port_info *pi = netdev2pinfo(dev);
@@ -1305,7 +1472,20 @@ int cxgb4_port_mirror_alloc(struct net_device *dev)
return ret;
refcount_set(&pi->vi_mirror_refcnt, 1);
+
+ if (adap->flags & CXGB4_FULL_INIT_DONE) {
+ ret = cxgb4_port_mirror_alloc_queues(dev);
+ if (ret < 0)
+ goto out_free_vi;
+ }
+
return 0;
+
+out_free_vi:
+ refcount_set(&pi->vi_mirror_refcnt, 0);
+ t4_free_vi(adap, adap->mbox, adap->pf, 0, pi->viid_mirror);
+ pi->viid_mirror = 0;
+ return ret;
}
void cxgb4_port_mirror_free(struct net_device *dev)
@@ -1321,6 +1501,8 @@ void cxgb4_port_mirror_free(struct net_device *dev)
return;
}
+ cxgb4_port_mirror_free_queues(dev);
+
refcount_set(&pi->vi_mirror_refcnt, 0);
t4_free_vi(adap, adap->mbox, adap->pf, 0, pi->viid_mirror);
pi->viid_mirror = 0;
@@ -2589,16 +2771,26 @@ int cxgb_open(struct net_device *dev)
return err;
}
+ err = cxgb4_port_mirror_alloc_queues(dev);
+ if (err < 0)
+ return err;
+
/* It's possible that the basic port information could have
* changed since we first read it.
*/
err = t4_update_port_info(pi);
if (err < 0)
- return err;
+ goto out_free;
err = link_start(dev);
- if (!err)
- netif_tx_start_all_queues(dev);
+ if (err)
+ goto out_free;
+
+ netif_tx_start_all_queues(dev);
+ return err;
+
+out_free:
+ cxgb4_port_mirror_free_queues(dev);
return err;
}
@@ -2616,6 +2808,10 @@ int cxgb_close(struct net_device *dev)
cxgb4_dcb_reset(dev);
dcb_tx_queue_prio_enable(dev, false);
#endif
+ if (ret)
+ return ret;
+
+ cxgb4_port_mirror_free_queues(dev);
return ret;
}
@@ -6365,6 +6561,8 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
spin_lock_init(&adapter->tid_release_lock);
spin_lock_init(&adapter->win0_lock);
+ mutex_init(&adapter->sge.queue_mutex);
+
INIT_WORK(&adapter->tid_release_task, process_tid_release_list);
INIT_WORK(&adapter->db_full_task, process_db_full);
INIT_WORK(&adapter->db_drop_task, process_db_drop);
--
2.24.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 3/3] cxgb4: add main VI to mirror VI config replication
2020-06-25 11:58 [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL Rahul Lakkireddy
2020-06-25 11:58 ` [PATCH net-next 1/3] cxgb4: add mirror action to TC-MATCHALL offload Rahul Lakkireddy
2020-06-25 11:58 ` [PATCH net-next 2/3] cxgb4: add support for mirror Rxqs Rahul Lakkireddy
@ 2020-06-25 11:58 ` Rahul Lakkireddy
2020-06-25 22:55 ` [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL Jakub Kicinski
3 siblings, 0 replies; 13+ messages in thread
From: Rahul Lakkireddy @ 2020-06-25 11:58 UTC (permalink / raw)
To: netdev; +Cc: davem, nirranjan, vishal, dt
When mirror VI is enabled, replicate various VI config params
enabled on main VI to mirror VI. These include replicating MTU,
promiscuous mode, all-multicast mode, and enabled netdev Rx
feature offloads.
Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
.../net/ethernet/chelsio/cxgb4/cxgb4_main.c | 155 ++++++++++++++++--
1 file changed, 143 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 0517eac4c8a5..7e7fd34c04a8 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -431,14 +431,32 @@ static int set_rxmode(struct net_device *dev, int mtu, bool sleep_ok)
{
struct port_info *pi = netdev_priv(dev);
struct adapter *adapter = pi->adapter;
+ int ret;
__dev_uc_sync(dev, cxgb4_mac_sync, cxgb4_mac_unsync);
__dev_mc_sync(dev, cxgb4_mac_sync, cxgb4_mac_unsync);
- return t4_set_rxmode(adapter, adapter->mbox, pi->viid, mtu,
- (dev->flags & IFF_PROMISC) ? 1 : 0,
- (dev->flags & IFF_ALLMULTI) ? 1 : 0, 1, -1,
- sleep_ok);
+ ret = t4_set_rxmode(adapter, adapter->mbox, pi->viid, mtu,
+ (dev->flags & IFF_PROMISC) ? 1 : 0,
+ (dev->flags & IFF_ALLMULTI) ? 1 : 0, 1, -1,
+ sleep_ok);
+ if (ret)
+ return ret;
+
+ if (refcount_read(&pi->vi_mirror_refcnt)) {
+ ret = t4_set_rxmode(adapter, adapter->mbox, pi->viid_mirror,
+ mtu, (dev->flags & IFF_PROMISC) ? 1 : 0,
+ (dev->flags & IFF_ALLMULTI) ? 1 : 0, 1, -1,
+ sleep_ok);
+ if (ret) {
+ dev_err(adapter->pdev_dev,
+ "Failed setting Rx Mode for Mirror VI 0x%x, ret: %d\n",
+ pi->viid_mirror, ret);
+ return ret;
+ }
+ }
+
+ return ret;
}
/**
@@ -1270,18 +1288,36 @@ int cxgb4_set_rspq_intr_params(struct sge_rspq *q,
static int cxgb_set_features(struct net_device *dev, netdev_features_t features)
{
- const struct port_info *pi = netdev_priv(dev);
netdev_features_t changed = dev->features ^ features;
+ const struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
int err;
if (!(changed & NETIF_F_HW_VLAN_CTAG_RX))
return 0;
- err = t4_set_rxmode(pi->adapter, pi->adapter->pf, pi->viid, -1,
- -1, -1, -1,
+ err = t4_set_rxmode(adap, adap->mbox, pi->viid, -1, -1, -1, -1,
!!(features & NETIF_F_HW_VLAN_CTAG_RX), true);
- if (unlikely(err))
+ if (err)
+ goto out;
+
+ if (refcount_read(&pi->vi_mirror_refcnt)) {
+ err = t4_set_rxmode(adap, adap->mbox, pi->viid_mirror,
+ -1, -1, -1, -1,
+ !!(features & NETIF_F_HW_VLAN_CTAG_RX),
+ true);
+ if (err) {
+ dev_err(adap->pdev_dev,
+ "Failed setting VLAN Rx mode for Mirror VI 0x%x, ret: %d\n",
+ pi->viid_mirror, err);
+ goto out;
+ }
+ }
+
+out:
+ if (err)
dev->features = features ^ NETIF_F_HW_VLAN_CTAG_RX;
+
return err;
}
@@ -1452,6 +1488,74 @@ static void cxgb4_port_mirror_free_queues(struct net_device *dev)
mutex_unlock(&s->queue_mutex);
}
+static int cxgb4_port_mirror_start(struct net_device *dev)
+{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
+ int ret, idx = -1;
+
+ if (!refcount_read(&pi->vi_mirror_refcnt))
+ return 0;
+
+ /* Mirror VIs can be created dynamically after stack had
+ * already setup Rx modes like MTU, promisc, allmulti, etc.
+ * on main VI. So, parse what the stack had setup on the
+ * main VI and update the same on the mirror VI.
+ */
+ ret = t4_set_rxmode(adap, adap->mbox, pi->viid_mirror, dev->mtu,
+ (dev->flags & IFF_PROMISC) ? 1 : 0,
+ (dev->flags & IFF_ALLMULTI) ? 1 : 0, 1,
+ !!(dev->features & NETIF_F_HW_VLAN_CTAG_RX), true);
+ if (ret) {
+ dev_err(adap->pdev_dev,
+ "Failed start up Rx mode for Mirror VI 0x%x, ret: %d\n",
+ pi->viid_mirror, ret);
+ return ret;
+ }
+
+ /* Enable replication bit for the device's MAC address
+ * in MPS TCAM, so that the packets for the main VI are
+ * replicated to mirror VI.
+ */
+ ret = cxgb4_update_mac_filt(pi, pi->viid_mirror, &idx,
+ dev->dev_addr, true, NULL);
+ if (ret) {
+ dev_err(adap->pdev_dev,
+ "Failed updating MAC filter for Mirror VI 0x%x, ret: %d\n",
+ pi->viid_mirror, ret);
+ return ret;
+ }
+
+ /* Enabling a Virtual Interface can result in an interrupt
+ * during the processing of the VI Enable command and, in some
+ * paths, result in an attempt to issue another command in the
+ * interrupt context. Thus, we disable interrupts during the
+ * course of the VI Enable command ...
+ */
+ local_bh_disable();
+ ret = t4_enable_vi_params(adap, adap->mbox, pi->viid_mirror, true, true,
+ false);
+ local_bh_enable();
+ if (ret)
+ dev_err(adap->pdev_dev,
+ "Failed starting Mirror VI 0x%x, ret: %d\n",
+ pi->viid_mirror, ret);
+
+ return ret;
+}
+
+static void cxgb4_port_mirror_stop(struct net_device *dev)
+{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
+
+ if (!refcount_read(&pi->vi_mirror_refcnt))
+ return;
+
+ t4_enable_vi_params(adap, adap->mbox, pi->viid_mirror, false, false,
+ false);
+}
+
int cxgb4_port_mirror_alloc(struct net_device *dev)
{
struct port_info *pi = netdev2pinfo(dev);
@@ -1477,10 +1581,17 @@ int cxgb4_port_mirror_alloc(struct net_device *dev)
ret = cxgb4_port_mirror_alloc_queues(dev);
if (ret < 0)
goto out_free_vi;
+
+ ret = cxgb4_port_mirror_start(dev);
+ if (ret < 0)
+ goto out_free_queues;
}
return 0;
+out_free_queues:
+ cxgb4_port_mirror_free_queues(dev);
+
out_free_vi:
refcount_set(&pi->vi_mirror_refcnt, 0);
t4_free_vi(adap, adap->mbox, adap->pf, 0, pi->viid_mirror);
@@ -1501,6 +1612,7 @@ void cxgb4_port_mirror_free(struct net_device *dev)
return;
}
+ cxgb4_port_mirror_stop(dev);
cxgb4_port_mirror_free_queues(dev);
refcount_set(&pi->vi_mirror_refcnt, 0);
@@ -2786,6 +2898,10 @@ int cxgb_open(struct net_device *dev)
if (err)
goto out_free;
+ err = cxgb4_port_mirror_start(dev);
+ if (err)
+ goto out_free;
+
netif_tx_start_all_queues(dev);
return err;
@@ -2811,6 +2927,7 @@ int cxgb_close(struct net_device *dev)
if (ret)
return ret;
+ cxgb4_port_mirror_stop(dev);
cxgb4_port_mirror_free_queues(dev);
return ret;
}
@@ -3078,13 +3195,27 @@ static void cxgb_set_rxmode(struct net_device *dev)
static int cxgb_change_mtu(struct net_device *dev, int new_mtu)
{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct adapter *adap = netdev2adap(dev);
int ret;
- struct port_info *pi = netdev_priv(dev);
- ret = t4_set_rxmode(pi->adapter, pi->adapter->pf, pi->viid, new_mtu, -1,
+ ret = t4_set_rxmode(adap, adap->mbox, pi->viid, new_mtu, -1,
-1, -1, -1, true);
- if (!ret)
- dev->mtu = new_mtu;
+ if (ret)
+ return ret;
+
+ if (refcount_read(&pi->vi_mirror_refcnt)) {
+ ret = t4_set_rxmode(adap, adap->mbox, pi->viid_mirror,
+ new_mtu, -1, -1, -1, -1, true);
+ if (ret) {
+ dev_err(adap->pdev_dev,
+ "MTU change for Mirror VI 0x%x error: %d\n",
+ pi->viid_mirror, ret);
+ return ret;
+ }
+ }
+
+ dev->mtu = new_mtu;
return ret;
}
--
2.24.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL
2020-06-25 11:58 [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL Rahul Lakkireddy
` (2 preceding siblings ...)
2020-06-25 11:58 ` [PATCH net-next 3/3] cxgb4: add main VI to mirror VI config replication Rahul Lakkireddy
@ 2020-06-25 22:55 ` Jakub Kicinski
2020-06-26 10:06 ` Rahul Lakkireddy
3 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-06-25 22:55 UTC (permalink / raw)
To: Rahul Lakkireddy; +Cc: netdev, davem, nirranjan, vishal, dt
On Thu, 25 Jun 2020 17:28:40 +0530 Rahul Lakkireddy wrote:
> This series of patches add support to mirror all ingress traffic
> for TC-MATCHALL ingress offload.
>
> Patch 1 adds support to dynamically create a mirror Virtual Interface
> (VI) that accepts all mirror ingress traffic when mirror action is
> set in TC-MATCHALL offload.
>
> Patch 2 adds support to allocate mirror Rxqs and setup RSS for the
> mirror VI.
>
> Patch 3 adds support to replicate all the main VI configuration to
> mirror VI. This includes replicating MTU, promiscuous mode,
> all-multicast mode, and enabled netdev Rx feature offloads.
Could you say more about this mirror VI? Is this an internal object
within the NIC or something visible to the user?
Also looking at the implementation of redirect:
case FLOW_ACTION_REDIRECT: {
struct net_device *out = act->dev;
struct port_info *pi = netdev_priv(out);
fs->action = FILTER_SWITCH;
fs->eport = pi->port_id;
}
How do you know the output interface is controlled by your driver, and
therefore it's sage to cast netdev_priv() to port_info?
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL
2020-06-25 22:55 ` [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL Jakub Kicinski
@ 2020-06-26 10:06 ` Rahul Lakkireddy
2020-06-26 16:55 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Rahul Lakkireddy @ 2020-06-26 10:06 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, nirranjan, vishal, dt
On Thursday, June 06/25/20, 2020 at 15:55:10 -0700, Jakub Kicinski wrote:
> On Thu, 25 Jun 2020 17:28:40 +0530 Rahul Lakkireddy wrote:
> > This series of patches add support to mirror all ingress traffic
> > for TC-MATCHALL ingress offload.
> >
> > Patch 1 adds support to dynamically create a mirror Virtual Interface
> > (VI) that accepts all mirror ingress traffic when mirror action is
> > set in TC-MATCHALL offload.
> >
> > Patch 2 adds support to allocate mirror Rxqs and setup RSS for the
> > mirror VI.
> >
> > Patch 3 adds support to replicate all the main VI configuration to
> > mirror VI. This includes replicating MTU, promiscuous mode,
> > all-multicast mode, and enabled netdev Rx feature offloads.
>
> Could you say more about this mirror VI? Is this an internal object
> within the NIC or something visible to the user?
>
The Virtual Interface (VI) is an internal object managed by firmware
and Multi Port Switch (MPS) module in hardware. Each VI can be
programmed with a unique MAC address in the MPS TCAM. So, 1 physical
port can have multiple VIs, each with their own MAC address. It's
also possible for VIs to share the same MAC address, which would
result in MPS setting the replication mode for that entry in the
TCAM. In this case, the incoming packet would get replicated and
sent to all the VIs sharing the MAC address. When MPS is able to
classify the destination MAC in the incoming packet with an entry
in the MPS TCAM, it forwards the packet to the corresponding VI(s).
In case of Mirror VI, we program the same MAC as the existing main
VI. This will result in MPS setting the replication mode for that
existing entry in the MPS TCAM. So, the MPS would replicate the
incoming packet and send it to both the main VI and mirror VI.
Note that for the main VI, we also programmed the flow Lookup Engine
(LE) module to switch the packet back out on one of the underlying
ports. So, when this rule hits in the LE, the main VI's packet would
get switched back out in hardware to one of the underlying ports and
will not reach driver. The mirror VI's packet will not hit any rule
in the LE and will be received by the driver and will be sent up to
Linux networking stack.
> Also looking at the implementation of redirect:
>
> case FLOW_ACTION_REDIRECT: {
> struct net_device *out = act->dev;
> struct port_info *pi = netdev_priv(out);
>
> fs->action = FILTER_SWITCH;
> fs->eport = pi->port_id;
> }
>
> How do you know the output interface is controlled by your driver, and
> therefore it's sage to cast netdev_priv() to port_info?
We're validating it earlier in cxgb4_validate_flow_actions().
Here's the code snippet. We're saving the netdevice pointer returned
by alloc_etherdev_mq() during PCI probe in cxgb4_main.c::init_one()
and using it to compare the netdevice given by redirect action. If
the redirect action's netdevice doesn't match any of our underlying
ports, then we fail offloading this rule.
case FLOW_ACTION_REDIRECT: {
struct adapter *adap = netdev2adap(dev);
struct net_device *n_dev, *target_dev;
unsigned int i;
bool found = false;
target_dev = act->dev;
for_each_port(adap, i) {
n_dev = adap->port[i];
if (target_dev == n_dev) {
found = true;
break;
}
}
/* If interface doesn't belong to our hw, then
* the provided output port is not valid
*/
if (!found) {
netdev_err(dev, "%s: Out port invalid\n",
__func__);
return -EINVAL;
}
act_redir = true;
}
break;
Thanks,
Rahul
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL
2020-06-26 10:06 ` Rahul Lakkireddy
@ 2020-06-26 16:55 ` Jakub Kicinski
2020-06-26 20:50 ` Rahul Lakkireddy
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-06-26 16:55 UTC (permalink / raw)
To: Rahul Lakkireddy; +Cc: netdev, davem, nirranjan, vishal, dt
On Fri, 26 Jun 2020 15:36:15 +0530 Rahul Lakkireddy wrote:
> On Thursday, June 06/25/20, 2020 at 15:55:10 -0700, Jakub Kicinski wrote:
> > On Thu, 25 Jun 2020 17:28:40 +0530 Rahul Lakkireddy wrote:
> > > This series of patches add support to mirror all ingress traffic
> > > for TC-MATCHALL ingress offload.
> > >
> > > Patch 1 adds support to dynamically create a mirror Virtual Interface
> > > (VI) that accepts all mirror ingress traffic when mirror action is
> > > set in TC-MATCHALL offload.
> > >
> > > Patch 2 adds support to allocate mirror Rxqs and setup RSS for the
> > > mirror VI.
> > >
> > > Patch 3 adds support to replicate all the main VI configuration to
> > > mirror VI. This includes replicating MTU, promiscuous mode,
> > > all-multicast mode, and enabled netdev Rx feature offloads.
> >
> > Could you say more about this mirror VI? Is this an internal object
> > within the NIC or something visible to the user?
> >
>
> The Virtual Interface (VI) is an internal object managed by firmware
> and Multi Port Switch (MPS) module in hardware. Each VI can be
> programmed with a unique MAC address in the MPS TCAM. So, 1 physical
> port can have multiple VIs, each with their own MAC address. It's
> also possible for VIs to share the same MAC address, which would
> result in MPS setting the replication mode for that entry in the
> TCAM. In this case, the incoming packet would get replicated and
> sent to all the VIs sharing the MAC address. When MPS is able to
> classify the destination MAC in the incoming packet with an entry
> in the MPS TCAM, it forwards the packet to the corresponding VI(s).
>
> In case of Mirror VI, we program the same MAC as the existing main
> VI. This will result in MPS setting the replication mode for that
> existing entry in the MPS TCAM. So, the MPS would replicate the
> incoming packet and send it to both the main VI and mirror VI.
So far sounds good.
> Note that for the main VI, we also programmed the flow Lookup Engine
> (LE) module to switch the packet back out on one of the underlying
> ports. So, when this rule hits in the LE, the main VI's packet would
> get switched back out in hardware to one of the underlying ports and
> will not reach driver. The mirror VI's packet will not hit any rule
> in the LE and will be received by the driver and will be sent up to
> Linux networking stack.
This I'm not sure I'm following. Are you saying that if there is another
(flower, not matchall) rule programmed it will be ignored as far
as the matchall filter is concerned? I assume you ensure the matchall
rule is at higher prio in that case?
> > Also looking at the implementation of redirect:
> >
> > case FLOW_ACTION_REDIRECT: {
> > struct net_device *out = act->dev;
> > struct port_info *pi = netdev_priv(out);
> >
> > fs->action = FILTER_SWITCH;
> > fs->eport = pi->port_id;
> > }
> >
> > How do you know the output interface is controlled by your driver, and
> > therefore it's sage to cast netdev_priv() to port_info?
>
> We're validating it earlier in cxgb4_validate_flow_actions().
> Here's the code snippet. We're saving the netdevice pointer returned
> by alloc_etherdev_mq() during PCI probe in cxgb4_main.c::init_one()
> and using it to compare the netdevice given by redirect action. If
> the redirect action's netdevice doesn't match any of our underlying
> ports, then we fail offloading this rule.
>
> case FLOW_ACTION_REDIRECT: {
> struct adapter *adap = netdev2adap(dev);
> struct net_device *n_dev, *target_dev;
> unsigned int i;
> bool found = false;
>
> target_dev = act->dev;
> for_each_port(adap, i) {
> n_dev = adap->port[i];
> if (target_dev == n_dev) {
> found = true;
> break;
> }
> }
>
> /* If interface doesn't belong to our hw, then
> * the provided output port is not valid
> */
> if (!found) {
> netdev_err(dev, "%s: Out port invalid\n",
> __func__);
> return -EINVAL;
> }
> act_redir = true;
> }
> break;
Thanks, FWIW this is what netdev_port_same_parent_id() is for.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL
2020-06-26 16:55 ` Jakub Kicinski
@ 2020-06-26 20:50 ` Rahul Lakkireddy
2020-06-27 4:17 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Rahul Lakkireddy @ 2020-06-26 20:50 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, nirranjan, vishal, dt
On Friday, June 06/26/20, 2020 at 09:55:49 -0700, Jakub Kicinski wrote:
> On Fri, 26 Jun 2020 15:36:15 +0530 Rahul Lakkireddy wrote:
> > On Thursday, June 06/25/20, 2020 at 15:55:10 -0700, Jakub Kicinski wrote:
> > > On Thu, 25 Jun 2020 17:28:40 +0530 Rahul Lakkireddy wrote:
> > > > This series of patches add support to mirror all ingress traffic
> > > > for TC-MATCHALL ingress offload.
> > > >
> > > > Patch 1 adds support to dynamically create a mirror Virtual Interface
> > > > (VI) that accepts all mirror ingress traffic when mirror action is
> > > > set in TC-MATCHALL offload.
> > > >
> > > > Patch 2 adds support to allocate mirror Rxqs and setup RSS for the
> > > > mirror VI.
> > > >
> > > > Patch 3 adds support to replicate all the main VI configuration to
> > > > mirror VI. This includes replicating MTU, promiscuous mode,
> > > > all-multicast mode, and enabled netdev Rx feature offloads.
> > >
> > > Could you say more about this mirror VI? Is this an internal object
> > > within the NIC or something visible to the user?
> > >
> >
> > The Virtual Interface (VI) is an internal object managed by firmware
> > and Multi Port Switch (MPS) module in hardware. Each VI can be
> > programmed with a unique MAC address in the MPS TCAM. So, 1 physical
> > port can have multiple VIs, each with their own MAC address. It's
> > also possible for VIs to share the same MAC address, which would
> > result in MPS setting the replication mode for that entry in the
> > TCAM. In this case, the incoming packet would get replicated and
> > sent to all the VIs sharing the MAC address. When MPS is able to
> > classify the destination MAC in the incoming packet with an entry
> > in the MPS TCAM, it forwards the packet to the corresponding VI(s).
> >
> > In case of Mirror VI, we program the same MAC as the existing main
> > VI. This will result in MPS setting the replication mode for that
> > existing entry in the MPS TCAM. So, the MPS would replicate the
> > incoming packet and send it to both the main VI and mirror VI.
>
> So far sounds good.
>
> > Note that for the main VI, we also programmed the flow Lookup Engine
> > (LE) module to switch the packet back out on one of the underlying
> > ports. So, when this rule hits in the LE, the main VI's packet would
> > get switched back out in hardware to one of the underlying ports and
> > will not reach driver. The mirror VI's packet will not hit any rule
> > in the LE and will be received by the driver and will be sent up to
> > Linux networking stack.
>
> This I'm not sure I'm following. Are you saying that if there is another
> (flower, not matchall) rule programmed it will be ignored as far
> as the matchall filter is concerned? I assume you ensure the matchall
> rule is at higher prio in that case?
>
The order is still maintained. If there's a higher priority
flower rule, then that rule will be considered first before
considering the matchall rule.
For example, let's say we have 2 rules like below:
# tc filter add dev enp2s0f4 ingress protocol ip pref 1 \
flower skip_sw src_ip 10.1.3.3 action drop
# tc filter add dev enp2s0f4 ingress pref 100 \
matchall skip_sw action mirred egress mirror dev enp2s0f4d1
If we're receiving a packet with src_ip 10.1.3.3, then rule prio 1
will hit first and the lower prio 100 matchall rule will never hit
for that packet. For all other packets, the matchall rule prio 100
will always hit.
I had tried to explain that some special care must be taken to make
sure that the redirect action of the mirror rule must only be performed
for the main VI's packet, so that it gets switched out to enp2s0f4d1 and
_must not_ reach the driver. The same redirect action must not be
performed for the mirror VI's replicated packet and the packet _must_
reach the driver. We're ensuring this by explicitly programming the
main VI's index for the filter entry in hardware. This way the hardware
will redirect out only the main VI's packet to enp2s0f4d1. It will not
perform redirect on the replicated packet coming from mirror VI.
If no VI index is programmed, then hardware will redirect out
both the main and mirror VI packets to enp2s0f4d1 and none of
the replicated packets will reach the driver, which is unexpected.
I hope I'm making sense... :)
> > > Also looking at the implementation of redirect:
> > >
> > > case FLOW_ACTION_REDIRECT: {
> > > struct net_device *out = act->dev;
> > > struct port_info *pi = netdev_priv(out);
> > >
> > > fs->action = FILTER_SWITCH;
> > > fs->eport = pi->port_id;
> > > }
> > >
> > > How do you know the output interface is controlled by your driver, and
> > > therefore it's sage to cast netdev_priv() to port_info?
> >
> > We're validating it earlier in cxgb4_validate_flow_actions().
> > Here's the code snippet. We're saving the netdevice pointer returned
> > by alloc_etherdev_mq() during PCI probe in cxgb4_main.c::init_one()
> > and using it to compare the netdevice given by redirect action. If
> > the redirect action's netdevice doesn't match any of our underlying
> > ports, then we fail offloading this rule.
> >
> > case FLOW_ACTION_REDIRECT: {
> > struct adapter *adap = netdev2adap(dev);
> > struct net_device *n_dev, *target_dev;
> > unsigned int i;
> > bool found = false;
> >
> > target_dev = act->dev;
> > for_each_port(adap, i) {
> > n_dev = adap->port[i];
> > if (target_dev == n_dev) {
> > found = true;
> > break;
> > }
> > }
> >
> > /* If interface doesn't belong to our hw, then
> > * the provided output port is not valid
> > */
> > if (!found) {
> > netdev_err(dev, "%s: Out port invalid\n",
> > __func__);
> > return -EINVAL;
> > }
> > act_redir = true;
> > }
> > break;
>
> Thanks, FWIW this is what netdev_port_same_parent_id() is for.
Looks like I need to add ndo_get_port_parent_id() support in the
driver for this to work. I'll look into adding this support in
follow up patches.
Thanks,
Rahul
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL
2020-06-26 20:50 ` Rahul Lakkireddy
@ 2020-06-27 4:17 ` Jakub Kicinski
2020-06-27 6:44 ` Rahul Lakkireddy
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-06-27 4:17 UTC (permalink / raw)
To: Rahul Lakkireddy; +Cc: netdev, davem, nirranjan, vishal, dt
On Sat, 27 Jun 2020 02:20:12 +0530 Rahul Lakkireddy wrote:
> On Friday, June 06/26/20, 2020 at 09:55:49 -0700, Jakub Kicinski wrote:
> > On Fri, 26 Jun 2020 15:36:15 +0530 Rahul Lakkireddy wrote:
> > > On Thursday, June 06/25/20, 2020 at 15:55:10 -0700, Jakub Kicinski wrote:
> > > > On Thu, 25 Jun 2020 17:28:40 +0530 Rahul Lakkireddy wrote:
> > > > > This series of patches add support to mirror all ingress traffic
> > > > > for TC-MATCHALL ingress offload.
> > > > >
> > > > > Patch 1 adds support to dynamically create a mirror Virtual Interface
> > > > > (VI) that accepts all mirror ingress traffic when mirror action is
> > > > > set in TC-MATCHALL offload.
> > > > >
> > > > > Patch 2 adds support to allocate mirror Rxqs and setup RSS for the
> > > > > mirror VI.
> > > > >
> > > > > Patch 3 adds support to replicate all the main VI configuration to
> > > > > mirror VI. This includes replicating MTU, promiscuous mode,
> > > > > all-multicast mode, and enabled netdev Rx feature offloads.
> > > >
> > > > Could you say more about this mirror VI? Is this an internal object
> > > > within the NIC or something visible to the user?
> > > >
> > >
> > > The Virtual Interface (VI) is an internal object managed by firmware
> > > and Multi Port Switch (MPS) module in hardware. Each VI can be
> > > programmed with a unique MAC address in the MPS TCAM. So, 1 physical
> > > port can have multiple VIs, each with their own MAC address. It's
> > > also possible for VIs to share the same MAC address, which would
> > > result in MPS setting the replication mode for that entry in the
> > > TCAM. In this case, the incoming packet would get replicated and
> > > sent to all the VIs sharing the MAC address. When MPS is able to
> > > classify the destination MAC in the incoming packet with an entry
> > > in the MPS TCAM, it forwards the packet to the corresponding VI(s).
> > >
> > > In case of Mirror VI, we program the same MAC as the existing main
> > > VI. This will result in MPS setting the replication mode for that
> > > existing entry in the MPS TCAM. So, the MPS would replicate the
> > > incoming packet and send it to both the main VI and mirror VI.
> >
> > So far sounds good.
> >
> > > Note that for the main VI, we also programmed the flow Lookup Engine
> > > (LE) module to switch the packet back out on one of the underlying
> > > ports. So, when this rule hits in the LE, the main VI's packet would
> > > get switched back out in hardware to one of the underlying ports and
> > > will not reach driver. The mirror VI's packet will not hit any rule
> > > in the LE and will be received by the driver and will be sent up to
> > > Linux networking stack.
> >
> > This I'm not sure I'm following. Are you saying that if there is another
> > (flower, not matchall) rule programmed it will be ignored as far
> > as the matchall filter is concerned? I assume you ensure the matchall
> > rule is at higher prio in that case?
> >
>
> The order is still maintained. If there's a higher priority
> flower rule, then that rule will be considered first before
> considering the matchall rule.
>
> For example, let's say we have 2 rules like below:
>
> # tc filter add dev enp2s0f4 ingress protocol ip pref 1 \
> flower skip_sw src_ip 10.1.3.3 action drop
>
> # tc filter add dev enp2s0f4 ingress pref 100 \
> matchall skip_sw action mirred egress mirror dev enp2s0f4d1
>
>
> If we're receiving a packet with src_ip 10.1.3.3, then rule prio 1
> will hit first and the lower prio 100 matchall rule will never hit
> for that packet. For all other packets, the matchall rule prio 100
> will always hit.
>
> I had tried to explain that some special care must be taken to make
> sure that the redirect action of the mirror rule must only be performed
> for the main VI's packet, so that it gets switched out to enp2s0f4d1 and
> _must not_ reach the driver. The same redirect action must not be
> performed for the mirror VI's replicated packet and the packet _must_
> reach the driver. We're ensuring this by explicitly programming the
> main VI's index for the filter entry in hardware. This way the hardware
> will redirect out only the main VI's packet to enp2s0f4d1. It will not
> perform redirect on the replicated packet coming from mirror VI.
> If no VI index is programmed, then hardware will redirect out
> both the main and mirror VI packets to enp2s0f4d1 and none of
> the replicated packets will reach the driver, which is unexpected.
>
> I hope I'm making sense... :)
What's the main use case for this feature? It appears that you're
allocating queues in patch 2. At the same time I don't see SWITCHDEV
mode / representors in this driver. So the use case is to redirect a
packet out to another port while still receiving a copy?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL
2020-06-27 4:17 ` Jakub Kicinski
@ 2020-06-27 6:44 ` Rahul Lakkireddy
2020-06-29 22:53 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Rahul Lakkireddy @ 2020-06-27 6:44 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, nirranjan, vishal, dt
On Friday, June 06/26/20, 2020 at 21:17:50 -0700, Jakub Kicinski wrote:
> On Sat, 27 Jun 2020 02:20:12 +0530 Rahul Lakkireddy wrote:
> > On Friday, June 06/26/20, 2020 at 09:55:49 -0700, Jakub Kicinski wrote:
> > > On Fri, 26 Jun 2020 15:36:15 +0530 Rahul Lakkireddy wrote:
> > > > On Thursday, June 06/25/20, 2020 at 15:55:10 -0700, Jakub Kicinski wrote:
> > > > > On Thu, 25 Jun 2020 17:28:40 +0530 Rahul Lakkireddy wrote:
> > > > > > This series of patches add support to mirror all ingress traffic
> > > > > > for TC-MATCHALL ingress offload.
> > > > > >
> > > > > > Patch 1 adds support to dynamically create a mirror Virtual Interface
> > > > > > (VI) that accepts all mirror ingress traffic when mirror action is
> > > > > > set in TC-MATCHALL offload.
> > > > > >
> > > > > > Patch 2 adds support to allocate mirror Rxqs and setup RSS for the
> > > > > > mirror VI.
> > > > > >
> > > > > > Patch 3 adds support to replicate all the main VI configuration to
> > > > > > mirror VI. This includes replicating MTU, promiscuous mode,
> > > > > > all-multicast mode, and enabled netdev Rx feature offloads.
> > > > >
> > > > > Could you say more about this mirror VI? Is this an internal object
> > > > > within the NIC or something visible to the user?
> > > > >
> > > >
> > > > The Virtual Interface (VI) is an internal object managed by firmware
> > > > and Multi Port Switch (MPS) module in hardware. Each VI can be
> > > > programmed with a unique MAC address in the MPS TCAM. So, 1 physical
> > > > port can have multiple VIs, each with their own MAC address. It's
> > > > also possible for VIs to share the same MAC address, which would
> > > > result in MPS setting the replication mode for that entry in the
> > > > TCAM. In this case, the incoming packet would get replicated and
> > > > sent to all the VIs sharing the MAC address. When MPS is able to
> > > > classify the destination MAC in the incoming packet with an entry
> > > > in the MPS TCAM, it forwards the packet to the corresponding VI(s).
> > > >
> > > > In case of Mirror VI, we program the same MAC as the existing main
> > > > VI. This will result in MPS setting the replication mode for that
> > > > existing entry in the MPS TCAM. So, the MPS would replicate the
> > > > incoming packet and send it to both the main VI and mirror VI.
> > >
> > > So far sounds good.
> > >
> > > > Note that for the main VI, we also programmed the flow Lookup Engine
> > > > (LE) module to switch the packet back out on one of the underlying
> > > > ports. So, when this rule hits in the LE, the main VI's packet would
> > > > get switched back out in hardware to one of the underlying ports and
> > > > will not reach driver. The mirror VI's packet will not hit any rule
> > > > in the LE and will be received by the driver and will be sent up to
> > > > Linux networking stack.
> > >
> > > This I'm not sure I'm following. Are you saying that if there is another
> > > (flower, not matchall) rule programmed it will be ignored as far
> > > as the matchall filter is concerned? I assume you ensure the matchall
> > > rule is at higher prio in that case?
> > >
> >
> > The order is still maintained. If there's a higher priority
> > flower rule, then that rule will be considered first before
> > considering the matchall rule.
> >
> > For example, let's say we have 2 rules like below:
> >
> > # tc filter add dev enp2s0f4 ingress protocol ip pref 1 \
> > flower skip_sw src_ip 10.1.3.3 action drop
> >
> > # tc filter add dev enp2s0f4 ingress pref 100 \
> > matchall skip_sw action mirred egress mirror dev enp2s0f4d1
> >
> >
> > If we're receiving a packet with src_ip 10.1.3.3, then rule prio 1
> > will hit first and the lower prio 100 matchall rule will never hit
> > for that packet. For all other packets, the matchall rule prio 100
> > will always hit.
> >
> > I had tried to explain that some special care must be taken to make
> > sure that the redirect action of the mirror rule must only be performed
> > for the main VI's packet, so that it gets switched out to enp2s0f4d1 and
> > _must not_ reach the driver. The same redirect action must not be
> > performed for the mirror VI's replicated packet and the packet _must_
> > reach the driver. We're ensuring this by explicitly programming the
> > main VI's index for the filter entry in hardware. This way the hardware
> > will redirect out only the main VI's packet to enp2s0f4d1. It will not
> > perform redirect on the replicated packet coming from mirror VI.
> > If no VI index is programmed, then hardware will redirect out
> > both the main and mirror VI packets to enp2s0f4d1 and none of
> > the replicated packets will reach the driver, which is unexpected.
> >
> > I hope I'm making sense... :)
>
> What's the main use case for this feature? It appears that you're
> allocating queues in patch 2. At the same time I don't see SWITCHDEV
> mode / representors in this driver. So the use case is to redirect a
> packet out to another port while still receiving a copy?
The main use case is to sniff packets that are being switched out
by hardware. The requirement is that there would be higher priority
flower rules that will accept specific traffic and all the other
traffic will be switched out on one of the underlying ports.
Occasionally, we want to sniff the packets that are being switched
out by replacing the redirect action with mirror action.
The reason for allocating queues is that the VIs are isolated from
each other and can't access each other's queues. So, separate queues
must be allocated for mirror VI.
Thanks,
Rahul
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/3] cxgb4: add mirror action support for TC-MATCHALL
2020-06-27 6:44 ` Rahul Lakkireddy
@ 2020-06-29 22:53 ` Jakub Kicinski
0 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2020-06-29 22:53 UTC (permalink / raw)
To: Rahul Lakkireddy; +Cc: netdev, davem, nirranjan, vishal, dt
On Sat, 27 Jun 2020 12:14:07 +0530 Rahul Lakkireddy wrote:
> > What's the main use case for this feature? It appears that you're
> > allocating queues in patch 2. At the same time I don't see SWITCHDEV
> > mode / representors in this driver. So the use case is to redirect a
> > packet out to another port while still receiving a copy?
>
> The main use case is to sniff packets that are being switched out
> by hardware. The requirement is that there would be higher priority
> flower rules that will accept specific traffic and all the other
> traffic will be switched out on one of the underlying ports.
> Occasionally, we want to sniff the packets that are being switched
> out by replacing the redirect action with mirror action.
>
> The reason for allocating queues is that the VIs are isolated from
> each other and can't access each other's queues. So, separate queues
> must be allocated for mirror VI.
Sounds reasonable, please fix the use of refcount_t (perhaps just use
a normal integer since structures are protected), and repost.
^ permalink raw reply [flat|nested] 13+ messages in thread