* [PATCH net 7/8] net: microchip: vcap api: Add a storage state to a VCAP rule
From: Steen Hegelund @ 2022-12-21 13:25 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Steen Hegelund, UNGLinuxDriver, Randy Dunlap, Casper Andersson,
Russell King, Wan Jiabing, Nathan Huckleberry, linux-kernel,
netdev, linux-arm-kernel, Steen Hegelund, Daniel Machon,
Horatiu Vultur, Lars Povlsen, Dan Carpenter
In-Reply-To: <20221221132517.2699698-1-steen.hegelund@microchip.com>
This allows a VCAP rule to be in one of 3 states:
- permanently stored in the VCAP HW (for rules that must always be present)
- enabled (stored in HW) when the corresponding lookup has been enabled
- disabled (stored in SW) when the lookup is disabled
This way important VCAP rules can be added even before the user enables the
VCAP lookups using a TC matchall filter.
Fixes: 4426b78c626d ("net: lan966x: Add port keyset config and callback interface")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
.../net/ethernet/microchip/vcap/vcap_api.c | 126 ++++++++++++++++--
.../microchip/vcap/vcap_api_debugfs.c | 52 +++++---
.../microchip/vcap/vcap_api_debugfs_kunit.c | 1 +
.../microchip/vcap/vcap_api_private.h | 9 +-
4 files changed, 161 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
index d438554355e4..94df0e7b58ea 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
@@ -950,9 +950,12 @@ int vcap_lookup_rule_by_cookie(struct vcap_control *vctrl, u64 cookie)
}
EXPORT_SYMBOL_GPL(vcap_lookup_rule_by_cookie);
-/* Make a shallow copy of the rule without the fields */
-struct vcap_rule_internal *vcap_dup_rule(struct vcap_rule_internal *ri)
+/* Make a copy of the rule, shallow or full */
+static struct vcap_rule_internal *vcap_dup_rule(struct vcap_rule_internal *ri,
+ bool full)
{
+ struct vcap_client_actionfield *caf, *newcaf;
+ struct vcap_client_keyfield *ckf, *newckf;
struct vcap_rule_internal *duprule;
/* Allocate the client part */
@@ -965,6 +968,27 @@ struct vcap_rule_internal *vcap_dup_rule(struct vcap_rule_internal *ri)
/* No elements in these lists */
INIT_LIST_HEAD(&duprule->data.keyfields);
INIT_LIST_HEAD(&duprule->data.actionfields);
+
+ /* A full rule copy includes keys and actions */
+ if (!full)
+ return duprule;
+
+ list_for_each_entry(ckf, &ri->data.keyfields, ctrl.list) {
+ newckf = kzalloc(sizeof(*newckf), GFP_KERNEL);
+ if (!newckf)
+ return ERR_PTR(-ENOMEM);
+ memcpy(newckf, ckf, sizeof(*newckf));
+ list_add_tail(&newckf->ctrl.list, &duprule->data.keyfields);
+ }
+
+ list_for_each_entry(caf, &ri->data.actionfields, ctrl.list) {
+ newcaf = kzalloc(sizeof(*newcaf), GFP_KERNEL);
+ if (!newcaf)
+ return ERR_PTR(-ENOMEM);
+ memcpy(newcaf, caf, sizeof(*newcaf));
+ list_add_tail(&newcaf->ctrl.list, &duprule->data.actionfields);
+ }
+
return duprule;
}
@@ -1877,8 +1901,8 @@ static int vcap_insert_rule(struct vcap_rule_internal *ri,
ri->addr = vcap_next_rule_addr(admin->last_used_addr, ri);
admin->last_used_addr = ri->addr;
- /* Add a shallow copy of the rule to the VCAP list */
- duprule = vcap_dup_rule(ri);
+ /* Add a copy of the rule to the VCAP list */
+ duprule = vcap_dup_rule(ri, ri->state == VCAP_RS_DISABLED);
if (IS_ERR(duprule))
return PTR_ERR(duprule);
@@ -1891,8 +1915,8 @@ static int vcap_insert_rule(struct vcap_rule_internal *ri,
ri->addr = vcap_next_rule_addr(addr, ri);
addr = ri->addr;
- /* Add a shallow copy of the rule to the VCAP list */
- duprule = vcap_dup_rule(ri);
+ /* Add a copy of the rule to the VCAP list */
+ duprule = vcap_dup_rule(ri, ri->state == VCAP_RS_DISABLED);
if (IS_ERR(duprule))
return PTR_ERR(duprule);
@@ -1939,6 +1963,72 @@ static bool vcap_is_chain_used(struct vcap_control *vctrl,
return false;
}
+/* Fetch the next chain in the enabled list for the port */
+static int vcap_get_next_chain(struct vcap_control *vctrl,
+ struct net_device *ndev,
+ int dst_cid)
+{
+ struct vcap_enabled_port *eport;
+ struct vcap_admin *admin;
+
+ list_for_each_entry(admin, &vctrl->list, list) {
+ list_for_each_entry(eport, &admin->enabled, list) {
+ if (eport->ndev != ndev)
+ continue;
+ if (eport->src_cid == dst_cid)
+ return eport->dst_cid;
+ }
+ }
+
+ return 0;
+}
+
+static bool vcap_path_exist(struct vcap_control *vctrl, struct net_device *ndev,
+ int dst_cid)
+{
+ struct vcap_enabled_port *eport, *elem;
+ struct vcap_admin *admin;
+ int tmp;
+
+ /* Find first entry that starts from chain 0*/
+ list_for_each_entry(admin, &vctrl->list, list) {
+ list_for_each_entry(elem, &admin->enabled, list) {
+ if (elem->src_cid == 0 && elem->ndev == ndev) {
+ eport = elem;
+ break;
+ }
+ }
+ if (eport)
+ break;
+ }
+
+ if (!eport)
+ return false;
+
+ tmp = eport->dst_cid;
+ while (tmp != dst_cid && tmp != 0)
+ tmp = vcap_get_next_chain(vctrl, ndev, tmp);
+
+ return !!tmp;
+}
+
+/* Internal clients can always store their rules in HW
+ * External clients can store their rules if the chain is enabled all
+ * the way from chain 0, otherwise the rule will be cached until
+ * the chain is enabled.
+ */
+static void vcap_rule_set_state(struct vcap_rule_internal *ri)
+{
+ if (ri->data.user <= VCAP_USER_QOS)
+ ri->state = VCAP_RS_PERMANENT;
+ else if (vcap_path_exist(ri->vctrl, ri->ndev, ri->data.vcap_chain_id))
+ ri->state = VCAP_RS_ENABLED;
+ else
+ ri->state = VCAP_RS_DISABLED;
+ /* For now always store directly in HW */
+ ri->state = VCAP_RS_PERMANENT;
+}
+
/* Encode and write a validated rule to the VCAP */
int vcap_add_rule(struct vcap_rule *rule)
{
@@ -1952,6 +2042,8 @@ int vcap_add_rule(struct vcap_rule *rule)
return ret;
/* Insert the new rule in the list of vcap rules */
mutex_lock(&ri->admin->lock);
+
+ vcap_rule_set_state(ri);
ret = vcap_insert_rule(ri, &move);
if (ret < 0) {
pr_err("%s:%d: could not insert rule in vcap list: %d\n",
@@ -1960,6 +2052,13 @@ int vcap_add_rule(struct vcap_rule *rule)
}
if (move.count > 0)
vcap_move_rules(ri, &move);
+
+ if (ri->state == VCAP_RS_DISABLED) {
+ /* Erase the rule area */
+ ri->vctrl->ops->init(ri->ndev, ri->admin, ri->addr, ri->size);
+ goto out;
+ }
+
vcap_erase_cache(ri);
ret = vcap_encode_rule(ri);
if (ret) {
@@ -2069,9 +2168,13 @@ struct vcap_rule *vcap_get_rule(struct vcap_control *vctrl, u32 id)
if (!elem)
return NULL;
mutex_lock(&elem->admin->lock);
- ri = vcap_dup_rule(elem);
+ ri = vcap_dup_rule(elem, elem->state == VCAP_RS_DISABLED);
if (IS_ERR(ri))
goto unlock;
+
+ if (ri->state == VCAP_RS_DISABLED)
+ goto unlock;
+
err = vcap_read_rule(ri);
if (err) {
ri = ERR_PTR(err);
@@ -2109,6 +2212,11 @@ int vcap_mod_rule(struct vcap_rule *rule)
return -ENOENT;
mutex_lock(&ri->admin->lock);
+
+ vcap_rule_set_state(ri);
+ if (ri->state == VCAP_RS_DISABLED)
+ goto out;
+
/* Encode the bitstreams to the VCAP cache */
vcap_erase_cache(ri);
err = vcap_encode_rule(ri);
@@ -2201,7 +2309,7 @@ int vcap_del_rule(struct vcap_control *vctrl, struct net_device *ndev, u32 id)
mutex_lock(&admin->lock);
list_del(&ri->list);
vctrl->ops->init(ndev, admin, admin->last_used_addr, ri->size + gap);
- kfree(ri);
+ vcap_free_rule(&ri->data);
mutex_unlock(&admin->lock);
/* Update the last used address, set to default when no rules */
@@ -2230,7 +2338,7 @@ int vcap_del_rules(struct vcap_control *vctrl, struct vcap_admin *admin)
list_for_each_entry_safe(ri, next_ri, &admin->rules, list) {
vctrl->ops->init(ri->ndev, admin, ri->addr, ri->size);
list_del(&ri->list);
- kfree(ri);
+ vcap_free_rule(&ri->data);
}
admin->last_used_addr = admin->last_valid_addr;
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
index e0b206247f2e..d6a09ce75e4f 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
@@ -152,37 +152,45 @@ vcap_debugfs_show_rule_actionfield(struct vcap_control *vctrl,
out->prf(out->dst, "\n");
}
-static int vcap_debugfs_show_rule_keyset(struct vcap_rule_internal *ri,
- struct vcap_output_print *out)
+static int vcap_debugfs_show_keysets(struct vcap_rule_internal *ri,
+ struct vcap_output_print *out)
{
- struct vcap_control *vctrl = ri->vctrl;
struct vcap_admin *admin = ri->admin;
enum vcap_keyfield_set keysets[10];
- const struct vcap_field *keyfield;
- enum vcap_type vt = admin->vtype;
- struct vcap_client_keyfield *ckf;
struct vcap_keyset_list matches;
- u32 *maskstream;
- u32 *keystream;
- int res;
+ int err;
- keystream = admin->cache.keystream;
- maskstream = admin->cache.maskstream;
matches.keysets = keysets;
matches.cnt = 0;
matches.max = ARRAY_SIZE(keysets);
- res = vcap_find_keystream_keysets(vctrl, vt, keystream, maskstream,
+
+ err = vcap_find_keystream_keysets(ri->vctrl, admin->vtype,
+ admin->cache.keystream,
+ admin->cache.maskstream,
false, 0, &matches);
- if (res < 0) {
+ if (err) {
pr_err("%s:%d: could not find valid keysets: %d\n",
- __func__, __LINE__, res);
- return -EINVAL;
+ __func__, __LINE__, err);
+ return err;
}
+
out->prf(out->dst, " keysets:");
for (int idx = 0; idx < matches.cnt; ++idx)
out->prf(out->dst, " %s",
- vcap_keyset_name(vctrl, matches.keysets[idx]));
+ vcap_keyset_name(ri->vctrl, matches.keysets[idx]));
out->prf(out->dst, "\n");
+ return 0;
+}
+
+static int vcap_debugfs_show_rule_keyset(struct vcap_rule_internal *ri,
+ struct vcap_output_print *out)
+{
+ struct vcap_control *vctrl = ri->vctrl;
+ struct vcap_admin *admin = ri->admin;
+ const struct vcap_field *keyfield;
+ struct vcap_client_keyfield *ckf;
+
+ vcap_debugfs_show_keysets(ri, out);
out->prf(out->dst, " keyset_sw: %d\n", ri->keyset_sw);
out->prf(out->dst, " keyset_sw_regs: %d\n", ri->keyset_sw_regs);
@@ -233,6 +241,18 @@ static void vcap_show_admin_rule(struct vcap_control *vctrl,
out->prf(out->dst, " chain_id: %d\n", ri->data.vcap_chain_id);
out->prf(out->dst, " user: %d\n", ri->data.user);
out->prf(out->dst, " priority: %d\n", ri->data.priority);
+ out->prf(out->dst, " state: ");
+ switch (ri->state) {
+ case VCAP_RS_PERMANENT:
+ out->prf(out->dst, "permanent\n");
+ break;
+ case VCAP_RS_DISABLED:
+ out->prf(out->dst, "disabled\n");
+ break;
+ case VCAP_RS_ENABLED:
+ out->prf(out->dst, "enabled\n");
+ break;
+ }
vcap_debugfs_show_rule_keyset(ri, out);
vcap_debugfs_show_rule_actionset(ri, out);
}
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c
index bef0b28a4a50..9211cb453a3d 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs_kunit.c
@@ -445,6 +445,7 @@ static const char * const test_admin_expect[] = {
" chain_id: 0\n",
" user: 0\n",
" priority: 0\n",
+ " state: permanent\n",
" keysets: VCAP_KFS_MAC_ETYPE\n",
" keyset_sw: 6\n",
" keyset_sw_regs: 2\n",
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_private.h b/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
index 4fd21da97679..ce35af9a853d 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
@@ -13,6 +13,12 @@
#define to_intrule(rule) container_of((rule), struct vcap_rule_internal, data)
+enum vcap_rule_state {
+ VCAP_RS_PERMANENT, /* the rule is always stored in HW */
+ VCAP_RS_ENABLED, /* enabled in HW but can be disabled */
+ VCAP_RS_DISABLED, /* disabled (stored in SW) and can be enabled */
+};
+
/* Private VCAP API rule data */
struct vcap_rule_internal {
struct vcap_rule data; /* provided by the client */
@@ -29,6 +35,7 @@ struct vcap_rule_internal {
u32 addr; /* address in the VCAP at insertion */
u32 counter_id; /* counter id (if a dedicated counter is available) */
struct vcap_counter counter; /* last read counter value */
+ enum vcap_rule_state state; /* rule storage state */
};
/* Bit iterator for the VCAP cache streams */
@@ -43,8 +50,6 @@ struct vcap_stream_iter {
/* Check that the control has a valid set of callbacks */
int vcap_api_check(struct vcap_control *ctrl);
-/* Make a shallow copy of the rule without the fields */
-struct vcap_rule_internal *vcap_dup_rule(struct vcap_rule_internal *ri);
/* Erase the VCAP cache area used or encoding and decoding */
void vcap_erase_cache(struct vcap_rule_internal *ri);
--
2.39.0
^ permalink raw reply related
* [PATCH net 8/8] net: microchip: vcap api: Enable/Disable rules via chains in VCAP HW
From: Steen Hegelund @ 2022-12-21 13:25 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Steen Hegelund, UNGLinuxDriver, Randy Dunlap, Casper Andersson,
Russell King, Wan Jiabing, Nathan Huckleberry, linux-kernel,
netdev, linux-arm-kernel, Steen Hegelund, Daniel Machon,
Horatiu Vultur, Lars Povlsen, Dan Carpenter
In-Reply-To: <20221221132517.2699698-1-steen.hegelund@microchip.com>
This supports that individual rules are enabled and disabled via chain
information.
This is done by keeping disabled rules in the VCAP list (cached) until they
are enabled, and only at this time are the rules written to the VCAP HW.
Fixes: 4426b78c626d ("net: lan966x: Add port keyset config and callback interface")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
.../net/ethernet/microchip/vcap/vcap_api.c | 195 +++++++++++++++++-
.../ethernet/microchip/vcap/vcap_api_client.h | 1 -
.../microchip/vcap/vcap_api_debugfs.c | 11 +-
.../ethernet/microchip/vcap/vcap_api_kunit.c | 11 +-
.../microchip/vcap/vcap_api_private.h | 3 +
5 files changed, 211 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
index 94df0e7b58ea..7cb7086248c3 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
@@ -2025,8 +2025,6 @@ static void vcap_rule_set_state(struct vcap_rule_internal *ri)
ri->state = VCAP_RS_ENABLED;
else
ri->state = VCAP_RS_DISABLED;
- /* For now always store directly in HW */
- ri->state = VCAP_RS_PERMANENT;
}
/* Encode and write a validated rule to the VCAP */
@@ -2709,6 +2707,119 @@ void vcap_set_tc_exterr(struct flow_cls_offload *fco, struct vcap_rule *vrule)
}
EXPORT_SYMBOL_GPL(vcap_set_tc_exterr);
+/* Write a rule to VCAP HW to enable it */
+static int vcap_enable_rule(struct vcap_rule_internal *ri)
+{
+ struct vcap_client_actionfield *af, *naf;
+ struct vcap_client_keyfield *kf, *nkf;
+
+ vcap_erase_cache(ri);
+ vcap_encode_rule(ri);
+ vcap_write_rule(ri);
+
+ /* Deallocate the list of keys and actions */
+ list_for_each_entry_safe(kf, nkf, &ri->data.keyfields, ctrl.list) {
+ list_del(&kf->ctrl.list);
+ kfree(kf);
+ }
+ list_for_each_entry_safe(af, naf, &ri->data.actionfields, ctrl.list) {
+ list_del(&af->ctrl.list);
+ kfree(af);
+ }
+ ri->state = VCAP_RS_ENABLED;
+ return 0;
+}
+
+/* Enable all disabled rules for a specific chain/port in the VCAP HW */
+static int vcap_enable_rules(struct vcap_control *vctrl,
+ struct net_device *ndev, int chain)
+{
+ struct vcap_rule_internal *ri;
+ struct vcap_admin *admin;
+ int err = 0;
+
+ list_for_each_entry(admin, &vctrl->list, list) {
+ if (!(chain >= admin->first_cid && chain <= admin->last_cid))
+ continue;
+
+ /* Found the admin, now find the offloadable rules */
+ mutex_lock(&admin->lock);
+ list_for_each_entry(ri, &admin->rules, list) {
+ if (ri->data.vcap_chain_id != chain)
+ continue;
+
+ if (ri->ndev != ndev)
+ continue;
+
+ if (ri->state != VCAP_RS_DISABLED)
+ continue;
+
+ err = vcap_enable_rule(ri);
+ if (err)
+ break;
+ }
+ mutex_unlock(&admin->lock);
+ if (err)
+ break;
+ }
+ return err;
+}
+
+/* Read and erase a rule from VCAP HW to disable it */
+static int vcap_disable_rule(struct vcap_rule_internal *ri)
+{
+ int err;
+
+ err = vcap_read_rule(ri);
+ if (err)
+ return err;
+ err = vcap_decode_keyset(ri);
+ if (err)
+ return err;
+ err = vcap_decode_actionset(ri);
+ if (err)
+ return err;
+
+ ri->state = VCAP_RS_DISABLED;
+ ri->vctrl->ops->init(ri->ndev, ri->admin, ri->addr, ri->size);
+ return 0;
+}
+
+/* Disable all enabled rules for a specific chain/port in the VCAP HW */
+static int vcap_disable_rules(struct vcap_control *vctrl,
+ struct net_device *ndev, int chain)
+{
+ struct vcap_rule_internal *ri;
+ struct vcap_admin *admin;
+ int err = 0;
+
+ list_for_each_entry(admin, &vctrl->list, list) {
+ if (!(chain >= admin->first_cid && chain <= admin->last_cid))
+ continue;
+
+ /* Found the admin, now find the rules on the chain */
+ mutex_lock(&admin->lock);
+ list_for_each_entry(ri, &admin->rules, list) {
+ if (ri->data.vcap_chain_id != chain)
+ continue;
+
+ if (ri->ndev != ndev)
+ continue;
+
+ if (ri->state != VCAP_RS_ENABLED)
+ continue;
+
+ err = vcap_disable_rule(ri);
+ if (err)
+ break;
+ }
+ mutex_unlock(&admin->lock);
+ if (err)
+ break;
+ }
+ return err;
+}
+
/* Check if this port is already enabled for this VCAP instance */
static bool vcap_is_enabled(struct vcap_control *vctrl, struct net_device *ndev,
int dst_cid)
@@ -2750,6 +2861,15 @@ static int vcap_enable(struct vcap_control *vctrl, struct net_device *ndev,
list_add_tail(&eport->list, &admin->enabled);
mutex_unlock(&admin->lock);
+ /* Enable chained lookups */
+ while (dst_cid) {
+ admin = vcap_find_admin(vctrl, dst_cid);
+ if (!admin)
+ return -ENOENT;
+
+ vcap_enable_rules(vctrl, ndev, dst_cid);
+ dst_cid = vcap_get_next_chain(vctrl, ndev, dst_cid);
+ }
return 0;
}
@@ -2759,6 +2879,7 @@ static int vcap_disable(struct vcap_control *vctrl, struct net_device *ndev,
{
struct vcap_enabled_port *elem, *eport = NULL;
struct vcap_admin *found = NULL, *admin;
+ int dst_cid;
list_for_each_entry(admin, &vctrl->list, list) {
list_for_each_entry(elem, &admin->enabled, list) {
@@ -2775,6 +2896,17 @@ static int vcap_disable(struct vcap_control *vctrl, struct net_device *ndev,
if (!eport)
return -ENOENT;
+ /* Disable chained lookups */
+ dst_cid = eport->dst_cid;
+ while (dst_cid) {
+ admin = vcap_find_admin(vctrl, dst_cid);
+ if (!admin)
+ return -ENOENT;
+
+ vcap_disable_rules(vctrl, ndev, dst_cid);
+ dst_cid = vcap_get_next_chain(vctrl, ndev, dst_cid);
+ }
+
mutex_lock(&found->lock);
list_del(&eport->list);
mutex_unlock(&found->lock);
@@ -2901,6 +3033,65 @@ int vcap_rule_get_counter(struct vcap_rule *rule, struct vcap_counter *ctr)
}
EXPORT_SYMBOL_GPL(vcap_rule_get_counter);
+/* Get a copy of a client key field */
+static int vcap_rule_get_key(struct vcap_rule *rule,
+ enum vcap_key_field key,
+ struct vcap_client_keyfield *ckf)
+{
+ struct vcap_client_keyfield *field;
+
+ field = vcap_find_keyfield(rule, key);
+ if (!field)
+ return -EINVAL;
+ memcpy(ckf, field, sizeof(*ckf));
+ INIT_LIST_HEAD(&ckf->ctrl.list);
+ return 0;
+}
+
+/* Get the keysets that matches the rule key type/mask */
+int vcap_rule_get_keysets(struct vcap_rule_internal *ri,
+ struct vcap_keyset_list *matches)
+{
+ struct vcap_control *vctrl = ri->vctrl;
+ enum vcap_type vt = ri->admin->vtype;
+ const struct vcap_set *keyfield_set;
+ struct vcap_client_keyfield kf = {};
+ u32 value, mask;
+ int err, idx;
+
+ err = vcap_rule_get_key(&ri->data, VCAP_KF_TYPE, &kf);
+ if (err)
+ return err;
+
+ if (kf.ctrl.type == VCAP_FIELD_BIT) {
+ value = kf.data.u1.value;
+ mask = kf.data.u1.mask;
+ } else if (kf.ctrl.type == VCAP_FIELD_U32) {
+ value = kf.data.u32.value;
+ mask = kf.data.u32.mask;
+ } else {
+ return -EINVAL;
+ }
+
+ keyfield_set = vctrl->vcaps[vt].keyfield_set;
+ for (idx = 0; idx < vctrl->vcaps[vt].keyfield_set_size; ++idx) {
+ if (keyfield_set[idx].sw_per_item != ri->keyset_sw)
+ continue;
+
+ if (keyfield_set[idx].type_id == (u8)-1) {
+ vcap_keyset_list_add(matches, idx);
+ continue;
+ }
+
+ if ((keyfield_set[idx].type_id & mask) == value)
+ vcap_keyset_list_add(matches, idx);
+ }
+ if (matches->cnt > 0)
+ return 0;
+
+ return -EINVAL;
+}
+
static int vcap_rule_mod_key(struct vcap_rule *rule,
enum vcap_key_field key,
enum vcap_field_type ftype,
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_client.h b/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
index f44228436051..b8980b22352f 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
@@ -264,5 +264,4 @@ int vcap_rule_mod_action_u32(struct vcap_rule *rule,
/* Get a 32 bit key field value and mask from the rule */
int vcap_rule_get_key_u32(struct vcap_rule *rule, enum vcap_key_field key,
u32 *value, u32 *mask);
-
#endif /* __VCAP_API_CLIENT__ */
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
index d6a09ce75e4f..dc06f6d4f513 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_debugfs.c
@@ -164,10 +164,13 @@ static int vcap_debugfs_show_keysets(struct vcap_rule_internal *ri,
matches.cnt = 0;
matches.max = ARRAY_SIZE(keysets);
- err = vcap_find_keystream_keysets(ri->vctrl, admin->vtype,
- admin->cache.keystream,
- admin->cache.maskstream,
- false, 0, &matches);
+ if (ri->state == VCAP_RS_DISABLED)
+ err = vcap_rule_get_keysets(ri, &matches);
+ else
+ err = vcap_find_keystream_keysets(ri->vctrl, admin->vtype,
+ admin->cache.keystream,
+ admin->cache.maskstream,
+ false, 0, &matches);
if (err) {
pr_err("%s:%d: could not find valid keysets: %d\n",
__func__, __LINE__, err);
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
index fdef9102a9b3..22690c669028 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
@@ -1305,8 +1305,8 @@ static void vcap_api_encode_rule_test(struct kunit *test)
struct vcap_admin is2_admin = {
.vtype = VCAP_TYPE_IS2,
- .first_cid = 10000,
- .last_cid = 19999,
+ .first_cid = 8000000,
+ .last_cid = 8099999,
.lookups = 4,
.last_valid_addr = 3071,
.first_valid_addr = 0,
@@ -1319,7 +1319,7 @@ static void vcap_api_encode_rule_test(struct kunit *test)
};
struct vcap_rule *rule;
struct vcap_rule_internal *ri;
- int vcap_chain_id = 10005;
+ int vcap_chain_id = 8000000;
enum vcap_user user = VCAP_USER_VCAP_UTIL;
u16 priority = 10;
int id = 100;
@@ -1391,6 +1391,11 @@ static void vcap_api_encode_rule_test(struct kunit *test)
KUNIT_EXPECT_EQ(test, 2, ri->keyset_sw_regs);
KUNIT_EXPECT_EQ(test, 4, ri->actionset_sw_regs);
+ /* Enable lookup, so the rule will be written */
+ ret = vcap_enable_lookups(&test_vctrl, &test_netdev, 0,
+ rule->vcap_chain_id, rule->cookie, true);
+ KUNIT_EXPECT_EQ(test, 0, ret);
+
/* Add rule with write callback */
ret = vcap_add_rule(rule);
KUNIT_EXPECT_EQ(test, 0, ret);
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_private.h b/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
index ce35af9a853d..86542accffe6 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_private.h
@@ -115,4 +115,7 @@ int vcap_find_keystream_keysets(struct vcap_control *vctrl, enum vcap_type vt,
u32 *keystream, u32 *mskstream, bool mask,
int sw_max, struct vcap_keyset_list *kslist);
+/* Get the keysets that matches the rule key type/mask */
+int vcap_rule_get_keysets(struct vcap_rule_internal *ri,
+ struct vcap_keyset_list *matches);
#endif /* __VCAP_API_PRIVATE__ */
--
2.39.0
^ permalink raw reply related
* Re: [PATCH net v2 2/5] net: ngbe: Remove structure ngbe_hw
From: Andrew Lunn @ 2022-12-21 13:55 UTC (permalink / raw)
To: Jiawen Wu; +Cc: netdev, mengyuanlou
In-Reply-To: <20221214064133.2424570-3-jiawenwu@trustnetic.com>
On Wed, Dec 14, 2022 at 02:41:30PM +0800, Jiawen Wu wrote:
> Remove useless structure ngbe_hw to make the codes clear.
Thanks for splitting this up. It makes it a lot easier to review.
> -static int ngbe_reset_misc(struct ngbe_hw *hw)
> +static int ngbe_reset_misc(struct ngbe_adapter *adapter)
> {
> - struct wx_hw *wxhw = &hw->wxhw;
> + struct wx_hw *wxhw = &adapter->wxhw;
>
> wx_reset_misc(wxhw);
> - if (hw->mac_type == ngbe_mac_type_rgmii)
> + if (adapter->mac_type == ngbe_mac_type_rgmii)
> wr32(wxhw, NGBE_MDIO_CLAUSE_SELECT, 0xF);
> - if (hw->gpio_ctrl) {
> + if (adapter->gpio_ctrl) {
> /* gpio0 is used to power on/off control*/
> wr32(wxhw, NGBE_GPIO_DDR, 0x1);
> wr32(wxhw, NGBE_GPIO_DR, NGBE_GPIO_DR_0);
> -static void txgbe_reset_misc(struct txgbe_hw *hw)
> +static void txgbe_reset_misc(struct wx_hw *wxhw)
> {
> - struct wx_hw *wxhw = &hw->wxhw;
> -
> wx_reset_misc(wxhw);
> - txgbe_init_thermal_sensor_thresh(hw);
> + txgbe_init_thermal_sensor_thresh(wxhw);
> }
The names suggest these two functions do the same thing. So it would
be nice if the parameter naming and structure names where
similar. Maybe consider renaming struct wx_hw to txgbe_adaptor? And
wxhw to adaptor? Then the two drivers would look more like each
other. When they look like each other it then becomes easier to see
identical code and move it into the library.
You can do this as a follow up series. Please submit these patches
once net-dev reopens.
Andrew
^ permalink raw reply
* Re: [PATCH] staging: qlge: remove unnecessary spaces before function pointer args
From: Simon Horman @ 2022-12-21 14:28 UTC (permalink / raw)
To: Aldas Taraškevičius
Cc: manishc, GR-Linux-NIC-Dev, gregkh, netdev, linux-staging,
linux-kernel
In-Reply-To: <20221214205147.2172-1-aldas60@gmail.com>
On Wed, Dec 14, 2022 at 10:51:47PM +0200, Aldas Taraškevičius wrote:
> Remove unnecessary spaces before the function pointer arguments as
> warned by checkpatch.
>
> Signed-off-by: Aldas Taraškevičius <aldas60@gmail.com>
> ---
> drivers/staging/qlge/qlge.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/qlge/qlge.h b/drivers/staging/qlge/qlge.h
> index fc8c5ca89..05e4f4744 100644
> --- a/drivers/staging/qlge/qlge.h
> +++ b/drivers/staging/qlge/qlge.h
> @@ -2057,8 +2057,8 @@ enum {
> };
>
> struct nic_operations {
> - int (*get_flash) (struct ql_adapter *);
> - int (*port_initialize) (struct ql_adapter *);
> + int (*get_flash)(struct ql_adapter *);
> + int (*port_initialize)(struct ql_adapter *);
> };
I'm not clear if there is consensus on the style issue at play here.
And so I am neutral on this patch.
But perhaps if these lines are being updated then the following check patch
warnings could be addressed.
$ scripts/checkpatch.pl --strict this.patch
WARNING: function definition argument 'struct ql_adapter *' should also have an identifier name
#122: FILE: drivers/staging/qlge/qlge.h:2060:
+ int (*get_flash)(struct ql_adapter *);
WARNING: function definition argument 'struct ql_adapter *' should also have an identifier name
#123: FILE: drivers/staging/qlge/qlge.h:2061:
+ int (*port_initialize)(struct ql_adapter *);
total: 0 errors, 2 warnings, 0 checks, 10 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] staging: qlge: remove unnecessary spaces before function pointer args" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: net: Add rfkill-gpio binding
From: Rob Herring @ 2022-12-21 14:45 UTC (permalink / raw)
To: Philipp Zabel
Cc: Johannes Berg, netdev, devicetree, linux-kernel, linux-wireless,
kernel
In-Reply-To: <20221221104803.1693874-1-p.zabel@pengutronix.de>
On Wed, Dec 21, 2022 at 11:48:02AM +0100, Philipp Zabel wrote:
> Add a device tree binding document for GPIO controlled rfkill switches.
> The name, type, shutdown-gpios and reset-gpios properties are the same
> as defined for ACPI.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> .../devicetree/bindings/net/rfkill-gpio.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/rfkill-gpio.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/rfkill-gpio.yaml b/Documentation/devicetree/bindings/net/rfkill-gpio.yaml
> new file mode 100644
> index 000000000000..6e62e6c96456
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/rfkill-gpio.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/net/rfkill-gpio.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: GPIO controlled rfkill switch
> +
> +maintainers:
> + - Johannes Berg <johannes@sipsolutions.net>
> + - Philipp Zabel <p.zabel@pengutronix.de>
> +
> +properties:
> + compatible:
> + const: rfkill-gpio
> +
> + name:
Did you test this? Something should complain, but maybe not. The problem
is 'name' is already a property in the unflattened DT (and old FDT
formats).
'label' would be appropriate perhaps, but why do we care what the name
is?
> + $ref: /schemas/types.yaml#/definitions/string
> + description: rfkill switch name, defaults to node name
> +
> + type:
Too generic. Property names should ideally have 1 type globally. I think
'type' is already in use. 'radio-type' instead?
> + description: rfkill radio type
> + enum:
> + - wlan
> + - bluetooth
> + - ultrawideband
> + - wimax
> + - wwan
> + - gps
> + - fm
> + - nfc
> +
> + shutdown-gpios:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
I'm lost as to why there are 2 GPIOs.
> +
> +required:
> + - compatible
> + - type
> +
> +oneOf:
> + - required:
> + - shutdown-gpios
> + - required:
> + - reset-gpios
But only 1 can be present? So just define 1 GPIO name.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + rfkill-pcie-wlan {
Node names should be generic.
> + compatible = "rfkill-gpio";
> + name = "rfkill-pcie-wlan";
> + type = "wlan";
> + shutdown-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH>;
> + };
> --
> 2.30.2
>
>
^ permalink raw reply
* [PATCH RFC net 0/2] tcp: Fix bhash2 and TIME_WAIT regression.
From: Kuniyuki Iwashima @ 2022-12-21 15:12 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Jiri Slaby, Joanne Koong, Kuniyuki Iwashima, Kuniyuki Iwashima,
netdev
We forgot to add TIME_WAIT sockets to bhash2. Therefore twsk cannot
prevent bind() to the same local address and port.
The first patch fixes the issue, but the layout change in struct
sock could have a negative impact. So, this series is RFC.
Kuniyuki Iwashima (2):
tcp: Add TIME_WAIT sockets in bhash2.
tcp: Add selftest for bind() and TIME_WAIT.
include/net/inet_timewait_sock.h | 2 +
include/net/sock.h | 5 +-
net/ipv4/inet_hashtables.c | 5 +-
net/ipv4/inet_timewait_sock.c | 31 ++++++-
tools/testing/selftests/net/.gitignore | 1 +
tools/testing/selftests/net/bind_timewait.c | 93 +++++++++++++++++++++
6 files changed, 131 insertions(+), 6 deletions(-)
create mode 100644 tools/testing/selftests/net/bind_timewait.c
--
2.30.2
^ permalink raw reply
* [PATCH RFC net 1/2] tcp: Add TIME_WAIT sockets in bhash2.
From: Kuniyuki Iwashima @ 2022-12-21 15:12 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Jiri Slaby, Joanne Koong, Kuniyuki Iwashima, Kuniyuki Iwashima,
netdev
In-Reply-To: <20221221151258.25748-1-kuniyu@amazon.com>
Jiri Slaby reported regression of bind() with a simple repro. [0]
The repro creates a TIME_WAIT socket and tries to bind() a new socket
with the same local address and port. Before commit 28044fc1d495 ("net:
Add a bhash2 table hashed by port and address"), the bind() failed with
-EADDRINUSE, but now it succeeds.
The cited commit should have put TIME_WAIT sockets into bhash2; otherwise,
inet_bhash2_conflict() misses TIME_WAIT sockets when validating bind()
requests if the address is not a wildcard one.
[0]: https://lore.kernel.org/netdev/6b971a4e-c7d8-411e-1f92-fda29b5b2fb9@kernel.org/
Fixes: 28044fc1d495 ("net: Add a bhash2 table hashed by port and address")
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
include/net/inet_timewait_sock.h | 2 ++
include/net/sock.h | 5 +++--
net/ipv4/inet_hashtables.c | 5 +++--
net/ipv4/inet_timewait_sock.c | 31 +++++++++++++++++++++++++++++--
4 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
index 5b47545f22d3..c46ed239ad9a 100644
--- a/include/net/inet_timewait_sock.h
+++ b/include/net/inet_timewait_sock.h
@@ -44,6 +44,7 @@ struct inet_timewait_sock {
#define tw_bound_dev_if __tw_common.skc_bound_dev_if
#define tw_node __tw_common.skc_nulls_node
#define tw_bind_node __tw_common.skc_bind_node
+#define tw_bind2_node __tw_common.skc_bind2_node
#define tw_refcnt __tw_common.skc_refcnt
#define tw_hash __tw_common.skc_hash
#define tw_prot __tw_common.skc_prot
@@ -73,6 +74,7 @@ struct inet_timewait_sock {
u32 tw_priority;
struct timer_list tw_timer;
struct inet_bind_bucket *tw_tb;
+ struct inet_bind2_bucket *tw_tb2;
};
#define tw_tclass tw_tos
diff --git a/include/net/sock.h b/include/net/sock.h
index dcd72e6285b2..aaec985c1b5b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -156,6 +156,7 @@ typedef __u64 __bitwise __addrpair;
* @skc_tw_rcv_nxt: (aka tw_rcv_nxt) TCP window next expected seq number
* [union with @skc_incoming_cpu]
* @skc_refcnt: reference count
+ * @skc_bind2_node: bind node in the bhash2 table
*
* This is the minimal network layer representation of sockets, the header
* for struct sock and struct inet_timewait_sock.
@@ -241,6 +242,7 @@ struct sock_common {
u32 skc_window_clamp;
u32 skc_tw_snd_nxt; /* struct tcp_timewait_sock */
};
+ struct hlist_node skc_bind2_node;
/* public: */
};
@@ -351,7 +353,6 @@ struct sk_filter;
* @sk_txtime_report_errors: set report errors mode for SO_TXTIME
* @sk_txtime_unused: unused txtime flags
* @ns_tracker: tracker for netns reference
- * @sk_bind2_node: bind node in the bhash2 table
*/
struct sock {
/*
@@ -384,6 +385,7 @@ struct sock {
#define sk_net_refcnt __sk_common.skc_net_refcnt
#define sk_bound_dev_if __sk_common.skc_bound_dev_if
#define sk_bind_node __sk_common.skc_bind_node
+#define sk_bind2_node __sk_common.skc_bind2_node
#define sk_prot __sk_common.skc_prot
#define sk_net __sk_common.skc_net
#define sk_v6_daddr __sk_common.skc_v6_daddr
@@ -542,7 +544,6 @@ struct sock {
#endif
struct rcu_head sk_rcu;
netns_tracker ns_tracker;
- struct hlist_node sk_bind2_node;
};
enum sk_pacing {
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index d039b4e732a3..1e81dc7c6de4 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -1103,15 +1103,16 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
/* Head lock still held and bh's disabled */
inet_bind_hash(sk, tb, tb2, port);
- spin_unlock(&head2->lock);
-
if (sk_unhashed(sk)) {
inet_sk(sk)->inet_sport = htons(port);
inet_ehash_nolisten(sk, (struct sock *)tw, NULL);
}
if (tw)
inet_twsk_bind_unhash(tw, hinfo);
+
+ spin_unlock(&head2->lock);
spin_unlock(&head->lock);
+
if (tw)
inet_twsk_deschedule_put(tw);
local_bh_enable();
diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 66fc940f9521..bec037d9ab8e 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -29,6 +29,7 @@
void inet_twsk_bind_unhash(struct inet_timewait_sock *tw,
struct inet_hashinfo *hashinfo)
{
+ struct inet_bind2_bucket *tb2 = tw->tw_tb2;
struct inet_bind_bucket *tb = tw->tw_tb;
if (!tb)
@@ -37,6 +38,11 @@ void inet_twsk_bind_unhash(struct inet_timewait_sock *tw,
__hlist_del(&tw->tw_bind_node);
tw->tw_tb = NULL;
inet_bind_bucket_destroy(hashinfo->bind_bucket_cachep, tb);
+
+ __hlist_del(&tw->tw_bind2_node);
+ tw->tw_tb2 = NULL;
+ inet_bind2_bucket_destroy(hashinfo->bind2_bucket_cachep, tb2);
+
__sock_put((struct sock *)tw);
}
@@ -45,7 +51,7 @@ static void inet_twsk_kill(struct inet_timewait_sock *tw)
{
struct inet_hashinfo *hashinfo = tw->tw_dr->hashinfo;
spinlock_t *lock = inet_ehash_lockp(hashinfo, tw->tw_hash);
- struct inet_bind_hashbucket *bhead;
+ struct inet_bind_hashbucket *bhead, *bhead2;
spin_lock(lock);
sk_nulls_del_node_init_rcu((struct sock *)tw);
@@ -54,9 +60,13 @@ static void inet_twsk_kill(struct inet_timewait_sock *tw)
/* Disassociate with bind bucket. */
bhead = &hashinfo->bhash[inet_bhashfn(twsk_net(tw), tw->tw_num,
hashinfo->bhash_size)];
+ bhead2 = inet_bhashfn_portaddr(hashinfo, (struct sock *)tw,
+ twsk_net(tw), tw->tw_num);
spin_lock(&bhead->lock);
+ spin_lock(&bhead2->lock);
inet_twsk_bind_unhash(tw, hashinfo);
+ spin_unlock(&bhead2->lock);
spin_unlock(&bhead->lock);
refcount_dec(&tw->tw_dr->tw_refcount);
@@ -93,6 +103,12 @@ static void inet_twsk_add_bind_node(struct inet_timewait_sock *tw,
hlist_add_head(&tw->tw_bind_node, list);
}
+static void inet_twsk_add_bind2_node(struct inet_timewait_sock *tw,
+ struct hlist_head *list)
+{
+ hlist_add_head(&tw->tw_bind2_node, list);
+}
+
/*
* Enter the time wait state. This is called with locally disabled BH.
* Essentially we whip up a timewait bucket, copy the relevant info into it
@@ -105,17 +121,28 @@ void inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
const struct inet_connection_sock *icsk = inet_csk(sk);
struct inet_ehash_bucket *ehead = inet_ehash_bucket(hashinfo, sk->sk_hash);
spinlock_t *lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
- struct inet_bind_hashbucket *bhead;
+ struct inet_bind_hashbucket *bhead, *bhead2;
+
/* Step 1: Put TW into bind hash. Original socket stays there too.
Note, that any socket with inet->num != 0 MUST be bound in
binding cache, even if it is closed.
*/
bhead = &hashinfo->bhash[inet_bhashfn(twsk_net(tw), inet->inet_num,
hashinfo->bhash_size)];
+ bhead2 = inet_bhashfn_portaddr(hashinfo, sk, twsk_net(tw), inet->inet_num);
+
spin_lock(&bhead->lock);
+ spin_lock(&bhead2->lock);
+
tw->tw_tb = icsk->icsk_bind_hash;
WARN_ON(!icsk->icsk_bind_hash);
inet_twsk_add_bind_node(tw, &tw->tw_tb->owners);
+
+ tw->tw_tb2 = icsk->icsk_bind2_hash;
+ WARN_ON(!icsk->icsk_bind2_hash);
+ inet_twsk_add_bind2_node(tw, &tw->tw_tb2->owners);
+
+ spin_unlock(&bhead2->lock);
spin_unlock(&bhead->lock);
spin_lock(lock);
--
2.30.2
^ permalink raw reply related
* [PATCH RFC net 2/2] tcp: Add selftest for bind() and TIME_WAIT.
From: Kuniyuki Iwashima @ 2022-12-21 15:12 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Jiri Slaby, Joanne Koong, Kuniyuki Iwashima, Kuniyuki Iwashima,
netdev
In-Reply-To: <20221221151258.25748-1-kuniyu@amazon.com>
bhash2 split the bind() validation logic into wildcard and non-wildcard
cases. Let's add a test to catch the same regression.
Before the previous patch:
# ./bind_timewait
TAP version 13
1..2
# Starting 2 tests from 3 test cases.
# RUN bind_timewait.localhost.1 ...
# bind_timewait.c:87:1:Expected ret (0) == -1 (-1)
# 1: Test terminated by assertion
# FAIL bind_timewait.localhost.1
not ok 1 bind_timewait.localhost.1
# RUN bind_timewait.addrany.1 ...
# OK bind_timewait.addrany.1
ok 2 bind_timewait.addrany.1
# FAILED: 1 / 2 tests passed.
# Totals: pass:1 fail:1 xfail:0 xpass:0 skip:0 error:0
After:
# ./bind_timewait
TAP version 13
1..2
# Starting 2 tests from 3 test cases.
# RUN bind_timewait.localhost.1 ...
# OK bind_timewait.localhost.1
ok 1 bind_timewait.localhost.1
# RUN bind_timewait.addrany.1 ...
# OK bind_timewait.addrany.1
ok 2 bind_timewait.addrany.1
# PASSED: 2 / 2 tests passed.
# Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
tools/testing/selftests/net/.gitignore | 1 +
tools/testing/selftests/net/bind_timewait.c | 93 +++++++++++++++++++++
2 files changed, 94 insertions(+)
create mode 100644 tools/testing/selftests/net/bind_timewait.c
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 9cc84114741d..a6911cae368c 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
bind_bhash
+bind_timewait
csum
cmsg_sender
diag_uid
diff --git a/tools/testing/selftests/net/bind_timewait.c b/tools/testing/selftests/net/bind_timewait.c
new file mode 100644
index 000000000000..2d40403128ff
--- /dev/null
+++ b/tools/testing/selftests/net/bind_timewait.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright Amazon.com Inc. or its affiliates. */
+
+#include "../kselftest_harness.h"
+
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <netinet/tcp.h>
+
+FIXTURE(bind_timewait)
+{
+ struct sockaddr_in addr;
+ socklen_t addrlen;
+};
+
+FIXTURE_VARIANT(bind_timewait)
+{
+ __u32 addr_const;
+};
+
+FIXTURE_VARIANT_ADD(bind_timewait, localhost)
+{
+ .addr_const = INADDR_LOOPBACK
+};
+
+FIXTURE_VARIANT_ADD(bind_timewait, addrany)
+{
+ .addr_const = INADDR_ANY
+};
+
+FIXTURE_SETUP(bind_timewait)
+{
+ self->addr.sin_family = AF_INET;
+ self->addr.sin_port = 0;
+ self->addr.sin_addr.s_addr = htonl(variant->addr_const);
+ self->addrlen = sizeof(self->addr);
+}
+
+FIXTURE_TEARDOWN(bind_timewait)
+{
+}
+
+void create_timewait_socket(struct __test_metadata *_metadata,
+ FIXTURE_DATA(bind_timewait) *self)
+{
+ int server_fd, client_fd, child_fd, ret;
+ struct sockaddr_in addr;
+ socklen_t addrlen;
+
+ server_fd = socket(AF_INET, SOCK_STREAM, 0);
+ ASSERT_GT(server_fd, 0);
+
+ ret = bind(server_fd, (struct sockaddr *)&self->addr, self->addrlen);
+ ASSERT_EQ(ret, 0);
+
+ ret = listen(server_fd, 1);
+ ASSERT_EQ(ret, 0);
+
+ ret = getsockname(server_fd, (struct sockaddr *)&self->addr, &self->addrlen);
+ ASSERT_EQ(ret, 0);
+
+ client_fd = socket(AF_INET, SOCK_STREAM, 0);
+ ASSERT_GT(client_fd, 0);
+
+ ret = connect(client_fd, (struct sockaddr *)&self->addr, self->addrlen);
+ ASSERT_EQ(ret, 0);
+
+ addrlen = sizeof(addr);
+ child_fd = accept(server_fd, (struct sockaddr *)&addr, &addrlen);
+ ASSERT_GT(child_fd, 0);
+
+ close(child_fd);
+ close(client_fd);
+ close(server_fd);
+}
+
+TEST_F(bind_timewait, 1)
+{
+ int fd, ret;
+
+ create_timewait_socket(_metadata, self);
+
+ fd = socket(AF_INET, SOCK_STREAM, 0);
+ ASSERT_GT(fd, 0);
+
+ ret = bind(fd, (struct sockaddr *)&self->addr, self->addrlen);
+ ASSERT_EQ(ret, -1);
+ ASSERT_EQ(errno, EADDRINUSE);
+
+ close(fd);
+}
+
+TEST_HARNESS_MAIN
--
2.30.2
^ permalink raw reply related
* [PATCH 00/18] can: m_can: Optimizations for m_can/tcan part 2
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
Hi Marc and everyone,
this is the second part now. I know it is the merge window right now but
I am quite sure this won't be merged immediately anyways, so if you have
some time for some comments I would appreciate it. So it is still based
on v6.1-rc8 + the patches that got applied.
I tried to do as small patches as possible so it is easier to
understand. The series changed a lot compared to v1 I sent so I didn't
call it v2. There are a lot of new patches as well.
The series contains a few small fixes and optimizations at the
beginning, then adding coalescing support and at the end removing the
restrictions on the number of parallel transmits in flight.
Note that the last patch 'Implement transmit submission coalescing' does
not perform well for me in a loopback testing setup. However I think it
may work well in normal testcases. I attached this mechanism to the
tx-frames coalescing option, let me know if this is the correct option.
Best,
Markus
part 1:
v1 - https://lore.kernel.org/lkml/20221116205308.2996556-1-msp@baylibre.com
v2 - https://lore.kernel.org/lkml/20221206115728.1056014-1-msp@baylibre.com
Markus Schneider-Pargmann (18):
can: tcan4x5x: Remove reserved register 0x814 from writable table
can: tcan4x5x: Check size of mram configuration
can: m_can: Remove repeated check for is_peripheral
can: m_can: Always acknowledge all interrupts
can: m_can: Remove double interrupt enable
can: m_can: Disable unused interrupts
can: m_can: Keep interrupts enabled during peripheral read
can: m_can: Write transmit header and data in one transaction
can: m_can: Implement receive coalescing
can: m_can: Implement transmit coalescing
can: m_can: Add rx coalescing ethtool support
can: m_can: Add tx coalescing ethtool support
can: m_can: Cache tx putidx
can: m_can: Use the workqueue as queue
can: m_can: Introduce a tx_fifo_in_flight counter
can: m_can: Use tx_fifo_in_flight for netif_queue control
can: m_can: Implement BQL
can: m_can: Implement transmit submission coalescing
drivers/net/can/m_can/m_can.c | 498 ++++++++++++++++++------
drivers/net/can/m_can/m_can.h | 36 +-
drivers/net/can/m_can/tcan4x5x-core.c | 5 +
drivers/net/can/m_can/tcan4x5x-regmap.c | 1 -
4 files changed, 418 insertions(+), 122 deletions(-)
--
2.38.1
^ permalink raw reply
* [PATCH 03/18] can: m_can: Remove repeated check for is_peripheral
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
Merge both if-blocks to fix this.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index f3ee21ce6109..a43abc667757 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1568,10 +1568,8 @@ static int m_can_close(struct net_device *dev)
cdev->tx_skb = NULL;
destroy_workqueue(cdev->tx_wq);
cdev->tx_wq = NULL;
- }
-
- if (cdev->is_peripheral)
can_rx_offload_disable(&cdev->offload);
+ }
close_candev(dev);
--
2.38.1
^ permalink raw reply related
* [PATCH 02/18] can: tcan4x5x: Check size of mram configuration
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
To reduce debugging effort in case the mram is misconfigured, add this
size check of the DT configuration. Currently if the mram configuration
doesn't fit into the available MRAM it just overwrites other areas of
the MRAM.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 16 ++++++++++++++++
drivers/net/can/m_can/m_can.h | 1 +
drivers/net/can/m_can/tcan4x5x-core.c | 5 +++++
3 files changed, 22 insertions(+)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 56f07f2023dd..f3ee21ce6109 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1860,6 +1860,22 @@ static int register_m_can_dev(struct net_device *dev)
return register_candev(dev);
}
+int m_can_check_mram_cfg(struct m_can_classdev *cdev, u32 mram_max_size)
+{
+ u32 total_size;
+
+ total_size = cdev->mcfg[MRAM_TXB].off - cdev->mcfg[MRAM_SIDF].off +
+ cdev->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
+ if (total_size > mram_max_size) {
+ dev_err(cdev->dev, "Total size of mram config(%u) exceeds mram(%u)\n",
+ total_size, mram_max_size);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(m_can_check_mram_cfg);
+
static void m_can_of_parse_mram(struct m_can_classdev *cdev,
const u32 *mram_config_vals)
{
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 4c0267f9f297..d2c584232c1a 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -101,6 +101,7 @@ int m_can_class_register(struct m_can_classdev *cdev);
void m_can_class_unregister(struct m_can_classdev *cdev);
int m_can_class_get_clocks(struct m_can_classdev *cdev);
int m_can_init_ram(struct m_can_classdev *priv);
+int m_can_check_mram_cfg(struct m_can_classdev *cdev, u32 mram_max_size);
int m_can_class_suspend(struct device *dev);
int m_can_class_resume(struct device *dev);
diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index efa2381bf85b..4f5a3ade6de2 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -80,6 +80,7 @@
TCAN4X5X_MCAN_IR_RF1F)
#define TCAN4X5X_MRAM_START 0x8000
+#define TCAN4X5X_MRAM_SIZE 0x800
#define TCAN4X5X_MCAN_OFFSET 0x1000
#define TCAN4X5X_CLEAR_ALL_INT 0xffffffff
@@ -312,6 +313,10 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
if (!mcan_class)
return -ENOMEM;
+ ret = m_can_check_mram_cfg(mcan_class, TCAN4X5X_MRAM_SIZE);
+ if (ret)
+ goto out_m_can_class_free_dev;
+
priv = cdev_to_priv(mcan_class);
priv->power = devm_regulator_get_optional(&spi->dev, "vsup");
--
2.38.1
^ permalink raw reply related
* [PATCH 01/18] can: tcan4x5x: Remove reserved register 0x814 from writable table
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
The mentioned register is not writable. It is reserved and should not be
written.
Fixes: 39dbb21b6a29 ("can: tcan4x5x: Specify separate read/write ranges")
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/tcan4x5x-regmap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
index 2b218ce04e9f..fafa6daa67e6 100644
--- a/drivers/net/can/m_can/tcan4x5x-regmap.c
+++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
@@ -95,7 +95,6 @@ static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
regmap_reg_range(0x000c, 0x0010),
/* Device configuration registers and Interrupt Flags*/
regmap_reg_range(0x0800, 0x080c),
- regmap_reg_range(0x0814, 0x0814),
regmap_reg_range(0x0820, 0x0820),
regmap_reg_range(0x0830, 0x0830),
/* M_CAN */
--
2.38.1
^ permalink raw reply related
* [PATCH 04/18] can: m_can: Always acknowledge all interrupts
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
The code already exits the function on !ir before this condition. No
need to check again if anything is set as IR_ALL_INT is 0xffffffff.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a43abc667757..4b387403a7c7 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1073,8 +1073,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
return IRQ_NONE;
/* ACK all irqs */
- if (ir & IR_ALL_INT)
- m_can_write(cdev, M_CAN_IR, ir);
+ m_can_write(cdev, M_CAN_IR, ir);
if (cdev->ops->clear_interrupts)
cdev->ops->clear_interrupts(cdev);
--
2.38.1
^ permalink raw reply related
* [PATCH 05/18] can: m_can: Remove double interrupt enable
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
Interrupts are enabled a few lines further down as well. Remove this
second call to enable all interrupts.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 4b387403a7c7..a76465016e17 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1347,7 +1347,6 @@ static void m_can_chip_config(struct net_device *dev)
m_can_write(cdev, M_CAN_TEST, test);
/* Enable interrupts */
- m_can_write(cdev, M_CAN_IR, IR_ALL_INT);
if (!(cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
if (cdev->version == 30)
m_can_write(cdev, M_CAN_IE, IR_ALL_INT &
--
2.38.1
^ permalink raw reply related
* [PATCH 07/18] can: m_can: Keep interrupts enabled during peripheral read
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
Interrupts currently get disabled if the interrupt status shows new
received data. Non-peripheral chips handle receiving in a worker thread,
but peripheral chips are handling the receive process in the threaded
interrupt routine itself without scheduling it for a different worker.
So there is no need to disable interrupts for peripheral chips.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 9749a3248517..bcd3bcdc5123 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -962,8 +962,8 @@ static int m_can_rx_peripheral(struct net_device *dev, u32 irqstatus)
/* Don't re-enable interrupts if the driver had a fatal error
* (e.g., FIFO read failure).
*/
- if (work_done >= 0)
- m_can_enable_all_interrupts(cdev);
+ if (work_done < 0)
+ m_can_disable_all_interrupts(cdev);
return work_done;
}
@@ -1085,11 +1085,12 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
*/
if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) {
cdev->irqstatus = ir;
- m_can_disable_all_interrupts(cdev);
- if (!cdev->is_peripheral)
+ if (!cdev->is_peripheral) {
+ m_can_disable_all_interrupts(cdev);
napi_schedule(&cdev->napi);
- else if (m_can_rx_peripheral(dev, ir) < 0)
+ } else if (m_can_rx_peripheral(dev, ir) < 0) {
goto out_fail;
+ }
}
if (cdev->version == 30) {
--
2.38.1
^ permalink raw reply related
* [PATCH 06/18] can: m_can: Disable unused interrupts
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
There are a number of interrupts that are not used by the driver at the
moment. Disable all of these.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a76465016e17..9749a3248517 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1252,6 +1252,12 @@ static void m_can_chip_config(struct net_device *dev)
{
struct m_can_classdev *cdev = netdev_priv(dev);
u32 cccr, test;
+ u32 interrupts = IR_ALL_INT;
+
+ /* Disable unused interrupts */
+ interrupts &= ~(IR_ARA | IR_ELO | IR_DRX | IR_TEFF | IR_TEFW | IR_TFE |
+ IR_TCF | IR_HPM | IR_RF1F | IR_RF1W | IR_RF1N |
+ IR_RF0F | IR_RF0W);
m_can_config_endisable(cdev, true);
@@ -1347,15 +1353,13 @@ static void m_can_chip_config(struct net_device *dev)
m_can_write(cdev, M_CAN_TEST, test);
/* Enable interrupts */
- if (!(cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
+ if (!(cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)) {
if (cdev->version == 30)
- m_can_write(cdev, M_CAN_IE, IR_ALL_INT &
- ~(IR_ERR_LEC_30X));
+ interrupts &= ~(IR_ERR_LEC_30X);
else
- m_can_write(cdev, M_CAN_IE, IR_ALL_INT &
- ~(IR_ERR_LEC_31X));
- else
- m_can_write(cdev, M_CAN_IE, IR_ALL_INT);
+ interrupts &= ~(IR_ERR_LEC_31X);
+ }
+ m_can_write(cdev, M_CAN_IE, interrupts);
/* route all interrupts to INT0 */
m_can_write(cdev, M_CAN_ILS, ILS_ALL_INT0);
--
2.38.1
^ permalink raw reply related
* [PATCH 08/18] can: m_can: Write transmit header and data in one transaction
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
Combine header and data before writing to the transmit fifo to reduce
the overhead for peripheral chips.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index bcd3bcdc5123..9b5ad222aef7 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1657,6 +1657,7 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
m_can_write(cdev, M_CAN_TXBAR, 0x1);
/* End of xmit function for version 3.0.x */
} else {
+ char buf[TXB_ELEMENT_SIZE];
/* Transmit routine for version >= v3.1.x */
txfqs = m_can_read(cdev, M_CAN_TXFQS);
@@ -1696,12 +1697,11 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
fifo_header.dlc = FIELD_PREP(TX_BUF_MM_MASK, putidx) |
FIELD_PREP(TX_BUF_DLC_MASK, can_fd_len2dlc(cf->len)) |
fdflags | TX_BUF_EFC;
- err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, &fifo_header, 2);
- if (err)
- goto out_fail;
+ memcpy(buf, &fifo_header, 8);
+ memcpy(&buf[8], &cf->data, cf->len);
- err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_DATA,
- cf->data, DIV_ROUND_UP(cf->len, 4));
+ err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID,
+ buf, 8 + DIV_ROUND_UP(cf->len, 4));
if (err)
goto out_fail;
--
2.38.1
^ permalink raw reply related
* [PATCH 09/18] can: m_can: Implement receive coalescing
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
m_can offers the possibility to set an interrupt on reaching a watermark
level in the receive FIFO. This can be used to implement coalescing.
Unfortunately there is no hardware timeout available to trigger an
interrupt if only a few messages were received within a given time. To
solve this I am using a hrtimer to wake up the irq thread after x
microseconds.
The timer is always started if receive coalescing is enabled and new
received frames were available during an interrupt. The timer is stopped
if during a interrupt handling no new data was available.
If the timer is started the new item interrupt is disabled and the
watermark interrupt takes over. If the timer is not started again, the
new item interrupt is enabled again, notifying the handler about every
new item received.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 69 ++++++++++++++++++++++++++++++++---
drivers/net/can/m_can/m_can.h | 7 ++++
2 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 9b5ad222aef7..2e664313101b 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1060,15 +1060,55 @@ static int m_can_echo_tx_event(struct net_device *dev)
return err;
}
+static void m_can_interrupt_enable(struct m_can_classdev *cdev, u32 interrupts)
+{
+ if (cdev->active_interrupts == interrupts)
+ return;
+ cdev->ops->write_reg(cdev, M_CAN_IE, interrupts);
+ cdev->active_interrupts = interrupts;
+}
+
+static void m_can_coalescing_disable(struct m_can_classdev *cdev)
+{
+ u32 new_interrupts = cdev->active_interrupts | IR_RF0N;
+
+ hrtimer_cancel(&cdev->irq_timer);
+ m_can_interrupt_enable(cdev, new_interrupts);
+}
+
+static void m_can_coalescing_update(struct m_can_classdev *cdev, u32 ir)
+{
+ u32 new_interrupts = cdev->active_interrupts;
+ bool enable_timer = false;
+
+ if (cdev->rx_coalesce_usecs_irq > 0 && (ir & (IR_RF0N | IR_RF0W))) {
+ enable_timer = true;
+ new_interrupts &= ~IR_RF0N;
+ } else if (!hrtimer_active(&cdev->irq_timer)) {
+ new_interrupts |= IR_RF0N;
+ }
+
+ m_can_interrupt_enable(cdev, new_interrupts);
+ if (enable_timer) {
+ hrtimer_start(&cdev->irq_timer,
+ ns_to_ktime(cdev->rx_coalesce_usecs_irq * NSEC_PER_USEC),
+ HRTIMER_MODE_REL);
+ }
+}
+
static irqreturn_t m_can_isr(int irq, void *dev_id)
{
struct net_device *dev = (struct net_device *)dev_id;
struct m_can_classdev *cdev = netdev_priv(dev);
u32 ir;
- if (pm_runtime_suspended(cdev->dev))
+ if (pm_runtime_suspended(cdev->dev)) {
+ m_can_coalescing_disable(cdev);
return IRQ_NONE;
+ }
+
ir = m_can_read(cdev, M_CAN_IR);
+ m_can_coalescing_update(cdev, ir);
if (!ir)
return IRQ_NONE;
@@ -1083,13 +1123,17 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
* - state change IRQ
* - bus error IRQ and bus error reporting
*/
- if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) {
+ if (ir & (IR_RF0N | IR_RF0W | IR_ERR_ALL_30X)) {
cdev->irqstatus = ir;
if (!cdev->is_peripheral) {
m_can_disable_all_interrupts(cdev);
napi_schedule(&cdev->napi);
- } else if (m_can_rx_peripheral(dev, ir) < 0) {
- goto out_fail;
+ } else {
+ int pkts;
+
+ pkts = m_can_rx_peripheral(dev, ir);
+ if (pkts < 0)
+ goto out_fail;
}
}
@@ -1125,6 +1169,15 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static enum hrtimer_restart m_can_irq_timer(struct hrtimer *timer)
+{
+ struct m_can_classdev *cdev = container_of(timer, struct m_can_classdev, irq_timer);
+
+ irq_wake_thread(cdev->net->irq, cdev->net);
+
+ return HRTIMER_NORESTART;
+}
+
static const struct can_bittiming_const m_can_bittiming_const_30X = {
.name = KBUILD_MODNAME,
.tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
@@ -1258,7 +1311,7 @@ static void m_can_chip_config(struct net_device *dev)
/* Disable unused interrupts */
interrupts &= ~(IR_ARA | IR_ELO | IR_DRX | IR_TEFF | IR_TEFW | IR_TFE |
IR_TCF | IR_HPM | IR_RF1F | IR_RF1W | IR_RF1N |
- IR_RF0F | IR_RF0W);
+ IR_RF0F);
m_can_config_endisable(cdev, true);
@@ -1302,6 +1355,7 @@ static void m_can_chip_config(struct net_device *dev)
/* rx fifo configuration, blocking mode, fifo size 1 */
m_can_write(cdev, M_CAN_RXF0C,
+ FIELD_PREP(RXFC_FWM_MASK, cdev->rx_max_coalesced_frames_irq) |
FIELD_PREP(RXFC_FS_MASK, cdev->mcfg[MRAM_RXF0].num) |
cdev->mcfg[MRAM_RXF0].off);
@@ -1360,7 +1414,7 @@ static void m_can_chip_config(struct net_device *dev)
else
interrupts &= ~(IR_ERR_LEC_31X);
}
- m_can_write(cdev, M_CAN_IE, interrupts);
+ m_can_interrupt_enable(cdev, interrupts);
/* route all interrupts to INT0 */
m_can_write(cdev, M_CAN_ILS, ILS_ALL_INT0);
@@ -2030,6 +2084,9 @@ int m_can_class_register(struct m_can_classdev *cdev)
of_can_transceiver(cdev->net);
+ hrtimer_init(&cdev->irq_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ cdev->irq_timer.function = m_can_irq_timer;
+
dev_info(cdev->dev, "%s device registered (irq=%d, version=%d)\n",
KBUILD_MODNAME, cdev->net->irq, cdev->version);
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index d2c584232c1a..4943e1e9aff0 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -84,6 +84,8 @@ struct m_can_classdev {
struct sk_buff *tx_skb;
struct phy *transceiver;
+ struct hrtimer irq_timer;
+
struct m_can_ops *ops;
int version;
@@ -92,6 +94,11 @@ struct m_can_classdev {
int pm_clock_support;
int is_peripheral;
+ // Cached M_CAN_IE register content
+ u32 active_interrupts;
+ u32 rx_max_coalesced_frames_irq;
+ u32 rx_coalesce_usecs_irq;
+
struct mram_cfg mcfg[MRAM_CFG_NUM];
};
--
2.38.1
^ permalink raw reply related
* [PATCH 10/18] can: m_can: Implement transmit coalescing
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
Extend the coalescing implementation for transmits.
In normal mode the chip raises an interrupt for every finished transmit.
This implementation switches to coalescing mode as soon as an interrupt
handled a transmit. For coalescing the watermark level interrupt is used
to interrupt exactly after x frames were sent. It switches back into
normal mode once there was an interrupt with no finished transmit and
the timer being inactive.
The timer is shared with receive coalescing. The time for receive and
transmit coalescing timers have to be the same for that to work. The
benefit is to have only a single running timer.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 33 ++++++++++++++++++++-------------
drivers/net/can/m_can/m_can.h | 3 +++
2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 2e664313101b..a84a17386c02 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -254,6 +254,7 @@ enum m_can_reg {
#define TXESC_TBDS_64B 0x7
/* Tx Event FIFO Configuration (TXEFC) */
+#define TXEFC_EFWM_MASK GENMASK(29, 24)
#define TXEFC_EFS_MASK GENMASK(21, 16)
/* Tx Event FIFO Status (TXEFS) */
@@ -1070,7 +1071,7 @@ static void m_can_interrupt_enable(struct m_can_classdev *cdev, u32 interrupts)
static void m_can_coalescing_disable(struct m_can_classdev *cdev)
{
- u32 new_interrupts = cdev->active_interrupts | IR_RF0N;
+ u32 new_interrupts = cdev->active_interrupts | IR_RF0N | IR_TEFN;
hrtimer_cancel(&cdev->irq_timer);
m_can_interrupt_enable(cdev, new_interrupts);
@@ -1079,21 +1080,26 @@ static void m_can_coalescing_disable(struct m_can_classdev *cdev)
static void m_can_coalescing_update(struct m_can_classdev *cdev, u32 ir)
{
u32 new_interrupts = cdev->active_interrupts;
- bool enable_timer = false;
+ bool enable_rx_timer = false;
+ bool enable_tx_timer = false;
if (cdev->rx_coalesce_usecs_irq > 0 && (ir & (IR_RF0N | IR_RF0W))) {
- enable_timer = true;
+ enable_rx_timer = true;
new_interrupts &= ~IR_RF0N;
- } else if (!hrtimer_active(&cdev->irq_timer)) {
- new_interrupts |= IR_RF0N;
}
+ if (cdev->tx_coalesce_usecs_irq > 0 && (ir & (IR_TEFN | IR_TEFW))) {
+ enable_tx_timer = true;
+ new_interrupts &= ~IR_TEFN;
+ }
+ if (!enable_rx_timer && !hrtimer_active(&cdev->irq_timer))
+ new_interrupts |= IR_RF0N;
+ if (!enable_tx_timer && !hrtimer_active(&cdev->irq_timer))
+ new_interrupts |= IR_TEFN;
m_can_interrupt_enable(cdev, new_interrupts);
- if (enable_timer) {
- hrtimer_start(&cdev->irq_timer,
- ns_to_ktime(cdev->rx_coalesce_usecs_irq * NSEC_PER_USEC),
+ if (enable_rx_timer | enable_tx_timer)
+ hrtimer_start(&cdev->irq_timer, cdev->irq_timer_wait,
HRTIMER_MODE_REL);
- }
}
static irqreturn_t m_can_isr(int irq, void *dev_id)
@@ -1148,7 +1154,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
netif_wake_queue(dev);
}
} else {
- if (ir & IR_TEFN) {
+ if (ir & (IR_TEFN | IR_TEFW)) {
/* New TX FIFO Element arrived */
if (m_can_echo_tx_event(dev) != 0)
goto out_fail;
@@ -1309,9 +1315,8 @@ static void m_can_chip_config(struct net_device *dev)
u32 interrupts = IR_ALL_INT;
/* Disable unused interrupts */
- interrupts &= ~(IR_ARA | IR_ELO | IR_DRX | IR_TEFF | IR_TEFW | IR_TFE |
- IR_TCF | IR_HPM | IR_RF1F | IR_RF1W | IR_RF1N |
- IR_RF0F);
+ interrupts &= ~(IR_ARA | IR_ELO | IR_DRX | IR_TEFF | IR_TFE | IR_TCF |
+ IR_HPM | IR_RF1F | IR_RF1W | IR_RF1N | IR_RF0F);
m_can_config_endisable(cdev, true);
@@ -1348,6 +1353,8 @@ static void m_can_chip_config(struct net_device *dev)
} else {
/* Full TX Event FIFO is used */
m_can_write(cdev, M_CAN_TXEFC,
+ FIELD_PREP(TXEFC_EFWM_MASK,
+ cdev->tx_max_coalesced_frames_irq) |
FIELD_PREP(TXEFC_EFS_MASK,
cdev->mcfg[MRAM_TXE].num) |
cdev->mcfg[MRAM_TXE].off);
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 4943e1e9aff0..62631a613076 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -85,6 +85,7 @@ struct m_can_classdev {
struct phy *transceiver;
struct hrtimer irq_timer;
+ ktime_t irq_timer_wait;
struct m_can_ops *ops;
@@ -98,6 +99,8 @@ struct m_can_classdev {
u32 active_interrupts;
u32 rx_max_coalesced_frames_irq;
u32 rx_coalesce_usecs_irq;
+ u32 tx_max_coalesced_frames_irq;
+ u32 tx_coalesce_usecs_irq;
struct mram_cfg mcfg[MRAM_CFG_NUM];
};
--
2.38.1
^ permalink raw reply related
* [PATCH 12/18] can: m_can: Add tx coalescing ethtool support
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
Add get/set functions for ethtool coalescing. tx-frames-irq and
tx-usecs-irq can only be set/unset together. tx-frames-irq needs to be
less than TXE and TXB.
As rx and tx share the same timer, rx-usecs-irq and tx-usecs-irq can be
enabled/disabled individually but they need to have the same value if
enabled.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 38 ++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 4d6fc8ade4d6..fc5a269f4930 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1918,6 +1918,8 @@ static int m_can_get_coalesce(struct net_device *dev,
ec->rx_max_coalesced_frames_irq = cdev->rx_max_coalesced_frames_irq;
ec->rx_coalesce_usecs_irq = cdev->rx_coalesce_usecs_irq;
+ ec->tx_max_coalesced_frames_irq = cdev->tx_max_coalesced_frames_irq;
+ ec->tx_coalesce_usecs_irq = cdev->tx_coalesce_usecs_irq;
return 0;
}
@@ -1944,16 +1946,50 @@ static int m_can_set_coalesce(struct net_device *dev,
netdev_err(dev, "rx-frames-irq and rx-usecs-irq can only be set together\n");
return -EINVAL;
}
+ if (ec->tx_max_coalesced_frames_irq > cdev->mcfg[MRAM_TXE].num) {
+ netdev_err(dev, "tx-frames-irq (%u) greater than the TX event FIFO (%u)\n",
+ ec->tx_max_coalesced_frames_irq,
+ cdev->mcfg[MRAM_TXE].num);
+ return -EINVAL;
+ }
+ if (ec->tx_max_coalesced_frames_irq > cdev->mcfg[MRAM_TXB].num) {
+ netdev_err(dev, "tx-frames-irq (%u) greater than the TX FIFO (%u)\n",
+ ec->tx_max_coalesced_frames_irq,
+ cdev->mcfg[MRAM_TXB].num);
+ return -EINVAL;
+ }
+ if ((ec->tx_max_coalesced_frames_irq == 0) != (ec->tx_coalesce_usecs_irq == 0)) {
+ netdev_err(dev, "tx-frames-irq and tx-usecs-irq can only be set together\n");
+ return -EINVAL;
+ }
+ if (ec->rx_coalesce_usecs_irq != 0 && ec->tx_coalesce_usecs_irq != 0 &&
+ ec->rx_coalesce_usecs_irq != ec->tx_coalesce_usecs_irq) {
+ netdev_err(dev, "rx-usecs-irq (%u) needs to be equal to tx-usecs-irq (%u) if both are enabled\n",
+ ec->rx_coalesce_usecs_irq,
+ ec->tx_coalesce_usecs_irq);
+ return -EINVAL;
+ }
cdev->rx_max_coalesced_frames_irq = ec->rx_max_coalesced_frames_irq;
cdev->rx_coalesce_usecs_irq = ec->rx_coalesce_usecs_irq;
+ cdev->tx_max_coalesced_frames_irq = ec->tx_max_coalesced_frames_irq;
+ cdev->tx_coalesce_usecs_irq = ec->tx_coalesce_usecs_irq;
+
+ if (cdev->rx_coalesce_usecs_irq)
+ cdev->irq_timer_wait =
+ ns_to_ktime(cdev->rx_coalesce_usecs_irq * NSEC_PER_USEC);
+ else
+ cdev->irq_timer_wait =
+ ns_to_ktime(cdev->tx_coalesce_usecs_irq * NSEC_PER_USEC);
return 0;
}
static const struct ethtool_ops m_can_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS_IRQ |
- ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ,
+ ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ |
+ ETHTOOL_COALESCE_TX_USECS_IRQ |
+ ETHTOOL_COALESCE_TX_MAX_FRAMES_IRQ,
.get_ts_info = ethtool_op_get_ts_info,
.get_coalesce = m_can_get_coalesce,
.set_coalesce = m_can_set_coalesce,
--
2.38.1
^ permalink raw reply related
* [PATCH 11/18] can: m_can: Add rx coalescing ethtool support
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
Add the possibility to set coalescing parameters with ethtool.
rx-frames-irq and rx-usecs-irq can only be set and unset together as the
implemented mechanism would not work otherwise. rx-frames-irq can't be
greater than the RX FIFO size.
Also all values can only be changed if the chip is not active.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 46 +++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a84a17386c02..4d6fc8ade4d6 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1909,8 +1909,54 @@ static const struct net_device_ops m_can_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};
+static int m_can_get_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *ec,
+ struct kernel_ethtool_coalesce *kec,
+ struct netlink_ext_ack *ext_ack)
+{
+ struct m_can_classdev *cdev = netdev_priv(dev);
+
+ ec->rx_max_coalesced_frames_irq = cdev->rx_max_coalesced_frames_irq;
+ ec->rx_coalesce_usecs_irq = cdev->rx_coalesce_usecs_irq;
+
+ return 0;
+}
+
+static int m_can_set_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *ec,
+ struct kernel_ethtool_coalesce *kec,
+ struct netlink_ext_ack *ext_ack)
+{
+ struct m_can_classdev *cdev = netdev_priv(dev);
+
+ if (cdev->can.state != CAN_STATE_STOPPED) {
+ netdev_err(dev, "Device is in use, please shut it down first\n");
+ return -EBUSY;
+ }
+
+ if (ec->rx_max_coalesced_frames_irq > cdev->mcfg[MRAM_RXF0].num) {
+ netdev_err(dev, "rx-frames-irq (%u) greater than the RX FIFO (%u)\n",
+ ec->rx_max_coalesced_frames_irq,
+ cdev->mcfg[MRAM_RXF0].num);
+ return -EINVAL;
+ }
+ if ((ec->rx_max_coalesced_frames_irq == 0) != (ec->rx_coalesce_usecs_irq == 0)) {
+ netdev_err(dev, "rx-frames-irq and rx-usecs-irq can only be set together\n");
+ return -EINVAL;
+ }
+
+ cdev->rx_max_coalesced_frames_irq = ec->rx_max_coalesced_frames_irq;
+ cdev->rx_coalesce_usecs_irq = ec->rx_coalesce_usecs_irq;
+
+ return 0;
+}
+
static const struct ethtool_ops m_can_ethtool_ops = {
+ .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS_IRQ |
+ ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ,
.get_ts_info = ethtool_op_get_ts_info,
+ .get_coalesce = m_can_get_coalesce,
+ .set_coalesce = m_can_set_coalesce,
};
static int register_m_can_dev(struct net_device *dev)
--
2.38.1
^ permalink raw reply related
* [PATCH 13/18] can: m_can: Cache tx putidx
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
m_can_tx_handler is the only place where data is written to the tx fifo.
We can calculate the putidx in the driver code here to avoid the
dependency on the txfqs register.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 8 +++++++-
drivers/net/can/m_can/m_can.h | 3 +++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index fc5a269f4930..5b882c2fec52 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1452,6 +1452,10 @@ static void m_can_start(struct net_device *dev)
cdev->can.state = CAN_STATE_ERROR_ACTIVE;
m_can_enable_all_interrupts(cdev);
+
+ if (cdev->version > 30)
+ cdev->tx_fifo_putidx = FIELD_GET(TXFQS_TFQPI_MASK,
+ m_can_read(cdev, M_CAN_TXFQS));
}
static int m_can_set_mode(struct net_device *dev, enum can_mode mode)
@@ -1740,7 +1744,7 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
}
/* get put index for frame */
- putidx = FIELD_GET(TXFQS_TFQPI_MASK, txfqs);
+ putidx = cdev->tx_fifo_putidx;
/* Construct DLC Field, with CAN-FD configuration.
* Use the put index of the fifo as the message marker,
@@ -1773,6 +1777,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
/* Enable TX FIFO element to start transfer */
m_can_write(cdev, M_CAN_TXBAR, (1 << putidx));
+ cdev->tx_fifo_putidx = (++cdev->tx_fifo_putidx >= cdev->can.echo_skb_max ?
+ 0 : cdev->tx_fifo_putidx);
/* stop network queue if fifo full */
if (m_can_tx_fifo_full(cdev) ||
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 62631a613076..185289a3719c 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -102,6 +102,9 @@ struct m_can_classdev {
u32 tx_max_coalesced_frames_irq;
u32 tx_coalesce_usecs_irq;
+ // Store this internally to avoid fetch delays on peripheral chips
+ int tx_fifo_putidx;
+
struct mram_cfg mcfg[MRAM_CFG_NUM];
};
--
2.38.1
^ permalink raw reply related
* [PATCH 15/18] can: m_can: Introduce a tx_fifo_in_flight counter
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
Keep track of the number of transmits in flight.
This patch prepares the driver to control the network interface queue
based on this counter. By itself this counter be
implemented with an atomic, but as we need to do other things in the
critical sections later I am using a spinlock instead.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 27 ++++++++++++++++++++++++++-
drivers/net/can/m_can/m_can.h | 4 ++++
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 42cde31fc0a8..90c9ff474149 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -452,6 +452,10 @@ static void m_can_clean(struct net_device *net)
for (int i = 0; i != cdev->can.echo_skb_max; ++i)
can_free_echo_skb(cdev->net, i, NULL);
+
+ spin_lock(&cdev->tx_handling_spinlock);
+ cdev->tx_fifo_in_flight = 0;
+ spin_unlock(&cdev->tx_handling_spinlock);
}
/* For peripherals, pass skb to rx-offload, which will push skb from
@@ -1022,6 +1026,7 @@ static int m_can_echo_tx_event(struct net_device *dev)
int i = 0;
int err = 0;
unsigned int msg_mark;
+ int processed = 0;
struct m_can_classdev *cdev = netdev_priv(dev);
@@ -1051,12 +1056,17 @@ static int m_can_echo_tx_event(struct net_device *dev)
/* update stats */
m_can_tx_update_stats(cdev, msg_mark, timestamp);
+ ++processed;
}
if (ack_fgi != -1)
m_can_write(cdev, M_CAN_TXEFA, FIELD_PREP(TXEFA_EFAI_MASK,
ack_fgi));
+ spin_lock(&cdev->tx_handling_spinlock);
+ cdev->tx_fifo_in_flight -= processed;
+ spin_unlock(&cdev->tx_handling_spinlock);
+
return err;
}
@@ -1821,11 +1831,26 @@ static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
}
netif_stop_queue(cdev->net);
+
+ spin_lock(&cdev->tx_handling_spinlock);
+ ++cdev->tx_fifo_in_flight;
+ spin_unlock(&cdev->tx_handling_spinlock);
+
m_can_tx_queue_skb(cdev, skb);
return NETDEV_TX_OK;
}
+static netdev_tx_t m_can_start_fast_xmit(struct m_can_classdev *cdev,
+ struct sk_buff *skb)
+{
+ spin_lock(&cdev->tx_handling_spinlock);
+ ++cdev->tx_fifo_in_flight;
+ spin_unlock(&cdev->tx_handling_spinlock);
+
+ return m_can_tx_handler(cdev, skb);
+}
+
static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
@@ -1837,7 +1862,7 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
if (cdev->is_peripheral)
return m_can_start_peripheral_xmit(cdev, skb);
else
- return m_can_tx_handler(cdev, skb);
+ return m_can_start_fast_xmit(cdev, skb);
}
static int m_can_open(struct net_device *dev)
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index bf2d710c982f..adbd4765accc 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -109,6 +109,10 @@ struct m_can_classdev {
// Store this internally to avoid fetch delays on peripheral chips
int tx_fifo_putidx;
+ /* Protects shared state between start_xmit and m_can_isr */
+ spinlock_t tx_handling_spinlock;
+ int tx_fifo_in_flight;
+
struct m_can_tx_op *tx_ops;
int nr_tx_ops;
int next_tx_op;
--
2.38.1
^ permalink raw reply related
* [PATCH 16/18] can: m_can: Use tx_fifo_in_flight for netif_queue control
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
The network queue is currently always stopped in start_xmit and
continued in the interrupt handler. This is not possible anymore if we
want to keep multiple transmits in flight in parallel.
Use the previously introduced tx_fifo_in_flight counter to control the
network queue instead. This has the benefit of not needing to ask the
hardware about fifo status.
This patch stops the network queue in start_xmit if the number of
transmits in flight reaches the size of the fifo and wakes up the queue
from the interrupt handler once the transmits in flight drops below the
fifo size. This means any skbs over the limit will be rejected
immediately in start_xmit (it shouldn't be possible at all to reach that
state anyways).
The maximum number of transmits in flight is the size of the fifo.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 58 +++++++++++------------------------
1 file changed, 18 insertions(+), 40 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 90c9ff474149..076fa60317c2 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -369,16 +369,6 @@ m_can_txe_fifo_read(struct m_can_classdev *cdev, u32 fgi, u32 offset, u32 *val)
return cdev->ops->read_fifo(cdev, addr_offset, val, 1);
}
-static inline bool _m_can_tx_fifo_full(u32 txfqs)
-{
- return !!(txfqs & TXFQS_TFQF);
-}
-
-static inline bool m_can_tx_fifo_full(struct m_can_classdev *cdev)
-{
- return _m_can_tx_fifo_full(m_can_read(cdev, M_CAN_TXFQS));
-}
-
static void m_can_config_endisable(struct m_can_classdev *cdev, bool enable)
{
u32 cccr = m_can_read(cdev, M_CAN_CCCR);
@@ -1064,6 +1054,8 @@ static int m_can_echo_tx_event(struct net_device *dev)
ack_fgi));
spin_lock(&cdev->tx_handling_spinlock);
+ if (cdev->tx_fifo_in_flight >= cdev->nr_tx_ops && processed > 0)
+ netif_wake_queue(cdev->net);
cdev->tx_fifo_in_flight -= processed;
spin_unlock(&cdev->tx_handling_spinlock);
@@ -1167,10 +1159,6 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
/* New TX FIFO Element arrived */
if (m_can_echo_tx_event(dev) != 0)
goto out_fail;
-
- if (netif_queue_stopped(dev) &&
- !m_can_tx_fifo_full(cdev))
- netif_wake_queue(dev);
}
}
@@ -1677,7 +1665,6 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
struct net_device *dev = cdev->net;
struct id_and_dlc fifo_header;
u32 cccr, fdflags;
- u32 txfqs;
int err;
int putidx;
@@ -1733,24 +1720,6 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
char buf[TXB_ELEMENT_SIZE];
/* Transmit routine for version >= v3.1.x */
- txfqs = m_can_read(cdev, M_CAN_TXFQS);
-
- /* Check if FIFO full */
- if (_m_can_tx_fifo_full(txfqs)) {
- /* This shouldn't happen */
- netif_stop_queue(dev);
- netdev_warn(dev,
- "TX queue active although FIFO is full.");
-
- if (cdev->is_peripheral) {
- kfree_skb(skb);
- dev->stats.tx_dropped++;
- return NETDEV_TX_OK;
- } else {
- return NETDEV_TX_BUSY;
- }
- }
-
/* get put index for frame */
putidx = cdev->tx_fifo_putidx;
@@ -1787,11 +1756,6 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
m_can_write(cdev, M_CAN_TXBAR, (1 << putidx));
cdev->tx_fifo_putidx = (++cdev->tx_fifo_putidx >= cdev->can.echo_skb_max ?
0 : cdev->tx_fifo_putidx);
-
- /* stop network queue if fifo full */
- if (m_can_tx_fifo_full(cdev) ||
- m_can_next_echo_skb_occupied(dev, putidx))
- netif_stop_queue(dev);
}
return NETDEV_TX_OK;
@@ -1830,10 +1794,16 @@ static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
return NETDEV_TX_OK;
}
- netif_stop_queue(cdev->net);
-
spin_lock(&cdev->tx_handling_spinlock);
++cdev->tx_fifo_in_flight;
+ if (cdev->tx_fifo_in_flight >= cdev->nr_tx_ops) {
+ netif_stop_queue(cdev->net);
+ if (cdev->tx_fifo_in_flight > cdev->nr_tx_ops) {
+ netdev_err(cdev->net, "hard_xmit called while TX FIFO full\n");
+ spin_unlock(&cdev->tx_handling_spinlock);
+ return NETDEV_TX_BUSY;
+ }
+ }
spin_unlock(&cdev->tx_handling_spinlock);
m_can_tx_queue_skb(cdev, skb);
@@ -1846,6 +1816,14 @@ static netdev_tx_t m_can_start_fast_xmit(struct m_can_classdev *cdev,
{
spin_lock(&cdev->tx_handling_spinlock);
++cdev->tx_fifo_in_flight;
+ if (cdev->tx_fifo_in_flight >= cdev->nr_tx_ops) {
+ netif_stop_queue(cdev->net);
+ if (cdev->tx_fifo_in_flight > cdev->nr_tx_ops) {
+ netdev_err(cdev->net, "hard_xmit called while TX FIFO full\n");
+ spin_unlock(&cdev->tx_handling_spinlock);
+ return NETDEV_TX_BUSY;
+ }
+ }
spin_unlock(&cdev->tx_handling_spinlock);
return m_can_tx_handler(cdev, skb);
--
2.38.1
^ permalink raw reply related
* [PATCH 14/18] can: m_can: Use the workqueue as queue
From: Markus Schneider-Pargmann @ 2022-12-21 15:25 UTC (permalink / raw)
To: Marc Kleine-Budde, Chandrasekar Ramakrishnan, Wolfgang Grandegger
Cc: Vincent MAILHOL, linux-can, netdev, linux-kernel,
Markus Schneider-Pargmann
In-Reply-To: <20221221152537.751564-1-msp@baylibre.com>
The current implementation uses the workqueue for peripheral chips to
submit work. Only a single work item is queued and used at any time.
To be able to keep more than one transmit in flight at a time, prepare
the workqueue to support multiple transmits at the same time.
Each work item now has a separate storage for a skb and a pointer to
cdev. This assures that each workitem can be processed individually.
The workqueue is replaced by an ordered workqueue which makes sure that
only a single worker processes the items queued on the workqueue. Also
items are ordered by the order they were enqueued. This removes most of
the concurrency the workqueue normally offers. It is not necessary for
this driver.
The cleanup functions have to be adopted a bit to handle this new
mechanism.
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
drivers/net/can/m_can/m_can.c | 109 ++++++++++++++++++++--------------
drivers/net/can/m_can/m_can.h | 12 +++-
2 files changed, 74 insertions(+), 47 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 5b882c2fec52..42cde31fc0a8 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -442,17 +442,16 @@ static void m_can_clean(struct net_device *net)
{
struct m_can_classdev *cdev = netdev_priv(net);
- if (cdev->tx_skb) {
- int putidx = 0;
+ for (int i = 0; i != cdev->nr_tx_ops; ++i) {
+ if (!cdev->tx_ops[i].skb)
+ continue;
net->stats.tx_errors++;
- if (cdev->version > 30)
- putidx = FIELD_GET(TXFQS_TFQPI_MASK,
- m_can_read(cdev, M_CAN_TXFQS));
-
- can_free_echo_skb(cdev->net, putidx, NULL);
- cdev->tx_skb = NULL;
+ cdev->tx_ops[i].skb = NULL;
}
+
+ for (int i = 0; i != cdev->can.echo_skb_max; ++i)
+ can_free_echo_skb(cdev->net, i, NULL);
}
/* For peripherals, pass skb to rx-offload, which will push skb from
@@ -1632,8 +1631,9 @@ static int m_can_close(struct net_device *dev)
m_can_clk_stop(cdev);
free_irq(dev->irq, dev);
+ m_can_clean(dev);
+
if (cdev->is_peripheral) {
- cdev->tx_skb = NULL;
destroy_workqueue(cdev->tx_wq);
cdev->tx_wq = NULL;
can_rx_offload_disable(&cdev->offload);
@@ -1660,19 +1660,17 @@ static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)
return !!cdev->can.echo_skb[next_idx];
}
-static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
+static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
+ struct sk_buff *skb)
{
- struct canfd_frame *cf = (struct canfd_frame *)cdev->tx_skb->data;
+ struct canfd_frame *cf = (struct canfd_frame *)skb->data;
struct net_device *dev = cdev->net;
- struct sk_buff *skb = cdev->tx_skb;
struct id_and_dlc fifo_header;
u32 cccr, fdflags;
u32 txfqs;
int err;
int putidx;
- cdev->tx_skb = NULL;
-
/* Generate ID field for TX buffer Element */
/* Common to all supported M_CAN versions */
if (cf->can_id & CAN_EFF_FLAG) {
@@ -1796,10 +1794,36 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
static void m_can_tx_work_queue(struct work_struct *ws)
{
- struct m_can_classdev *cdev = container_of(ws, struct m_can_classdev,
- tx_work);
+ struct m_can_tx_op *op = container_of(ws, struct m_can_tx_op, work);
+ struct m_can_classdev *cdev = op->cdev;
+ struct sk_buff *skb = op->skb;
- m_can_tx_handler(cdev);
+ op->skb = NULL;
+ m_can_tx_handler(cdev, skb);
+}
+
+static void m_can_tx_queue_skb(struct m_can_classdev *cdev, struct sk_buff *skb)
+{
+ cdev->tx_ops[cdev->next_tx_op].skb = skb;
+ queue_work(cdev->tx_wq, &cdev->tx_ops[cdev->next_tx_op].work);
+
+ ++cdev->next_tx_op;
+ if (cdev->next_tx_op >= cdev->nr_tx_ops)
+ cdev->next_tx_op = 0;
+}
+
+static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
+ struct sk_buff *skb)
+{
+ if (cdev->can.state == CAN_STATE_BUS_OFF) {
+ m_can_clean(cdev->net);
+ return NETDEV_TX_OK;
+ }
+
+ netif_stop_queue(cdev->net);
+ m_can_tx_queue_skb(cdev, skb);
+
+ return NETDEV_TX_OK;
}
static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
@@ -1810,30 +1834,10 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
if (can_dev_dropped_skb(dev, skb))
return NETDEV_TX_OK;
- if (cdev->is_peripheral) {
- if (cdev->tx_skb) {
- netdev_err(dev, "hard_xmit called while tx busy\n");
- return NETDEV_TX_BUSY;
- }
-
- if (cdev->can.state == CAN_STATE_BUS_OFF) {
- m_can_clean(dev);
- } else {
- /* Need to stop the queue to avoid numerous requests
- * from being sent. Suggested improvement is to create
- * a queueing mechanism that will queue the skbs and
- * process them in order.
- */
- cdev->tx_skb = skb;
- netif_stop_queue(cdev->net);
- queue_work(cdev->tx_wq, &cdev->tx_work);
- }
- } else {
- cdev->tx_skb = skb;
- return m_can_tx_handler(cdev);
- }
-
- return NETDEV_TX_OK;
+ if (cdev->is_peripheral)
+ return m_can_start_peripheral_xmit(cdev, skb);
+ else
+ return m_can_tx_handler(cdev, skb);
}
static int m_can_open(struct net_device *dev)
@@ -1861,15 +1865,17 @@ static int m_can_open(struct net_device *dev)
/* register interrupt handler */
if (cdev->is_peripheral) {
- cdev->tx_skb = NULL;
- cdev->tx_wq = alloc_workqueue("mcan_wq",
- WQ_FREEZABLE | WQ_MEM_RECLAIM, 0);
+ cdev->tx_wq = alloc_ordered_workqueue("mcan_wq",
+ WQ_FREEZABLE | WQ_MEM_RECLAIM);
if (!cdev->tx_wq) {
err = -ENOMEM;
goto out_wq_fail;
}
- INIT_WORK(&cdev->tx_work, m_can_tx_work_queue);
+ for (int i = 0; i != cdev->nr_tx_ops; ++i) {
+ cdev->tx_ops[i].cdev = cdev;
+ INIT_WORK(&cdev->tx_ops[i].work, m_can_tx_work_queue);
+ }
err = request_threaded_irq(dev->irq, NULL, m_can_isr,
IRQF_ONESHOT,
@@ -2153,6 +2159,19 @@ int m_can_class_register(struct m_can_classdev *cdev)
{
int ret;
+ if (cdev->is_peripheral) {
+ cdev->nr_tx_ops = min(cdev->mcfg[MRAM_TXB].num,
+ cdev->mcfg[MRAM_TXE].num);
+ cdev->tx_ops =
+ devm_kzalloc(cdev->dev,
+ cdev->nr_tx_ops * sizeof(*cdev->tx_ops),
+ GFP_KERNEL);
+ if (!cdev->tx_ops) {
+ dev_err(cdev->dev, "Failed to allocate tx_ops for workqueue\n");
+ return -ENOMEM;
+ }
+ }
+
if (cdev->pm_clock_support) {
ret = m_can_clk_start(cdev);
if (ret)
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 185289a3719c..bf2d710c982f 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -70,6 +70,12 @@ struct m_can_ops {
int (*init)(struct m_can_classdev *cdev);
};
+struct m_can_tx_op {
+ struct m_can_classdev *cdev;
+ struct work_struct work;
+ struct sk_buff *skb;
+};
+
struct m_can_classdev {
struct can_priv can;
struct can_rx_offload offload;
@@ -80,8 +86,6 @@ struct m_can_classdev {
struct clk *cclk;
struct workqueue_struct *tx_wq;
- struct work_struct tx_work;
- struct sk_buff *tx_skb;
struct phy *transceiver;
struct hrtimer irq_timer;
@@ -105,6 +109,10 @@ struct m_can_classdev {
// Store this internally to avoid fetch delays on peripheral chips
int tx_fifo_putidx;
+ struct m_can_tx_op *tx_ops;
+ int nr_tx_ops;
+ int next_tx_op;
+
struct mram_cfg mcfg[MRAM_CFG_NUM];
};
--
2.38.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox