* [PATCH net-next 00/10] Updates to MCTP core
@ 2021-09-29 7:26 Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 01/10] mctp: Allow MCTP on tun devices Matt Johnston
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
Hi,
This series adds timeouts for MCTP tags (a limited resource), and a few
other improvements to the MCTP core.
Cheers,
Matt
Jeremy Kerr (7):
mctp: Allow local delivery to the null EID
mctp: locking, lifetime and validity changes for sk_keys
mctp: Add refcounts to mctp_dev
mctp: Implement a timeout for tags
mctp: Add tracepoints for tag/key handling
mctp: Do inits as a subsys_initcall
doc/mctp: Add a little detail about kernel internals
Matt Johnston (3):
mctp: Allow MCTP on tun devices
mctp: Set route MTU via netlink
mctp: Warn if pointer is set for a wrong dev type
Documentation/networking/mctp.rst | 59 ++++++++++
include/net/mctp.h | 56 ++++++---
include/net/mctpdevice.h | 5 +
include/trace/events/mctp.h | 75 ++++++++++++
net/mctp/af_mctp.c | 66 +++++++++--
net/mctp/device.c | 53 +++++++--
net/mctp/neigh.c | 4 +-
net/mctp/route.c | 190 ++++++++++++++++++++++++------
8 files changed, 431 insertions(+), 77 deletions(-)
create mode 100644 include/trace/events/mctp.h
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 01/10] mctp: Allow MCTP on tun devices
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 02/10] mctp: Allow local delivery to the null EID Matt Johnston
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
Allowing TUN is useful for testing, to route packets to userspace or to
tunnel between machines.
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
---
net/mctp/device.c | 7 +++++--
net/mctp/route.c | 13 ++++++++-----
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/net/mctp/device.c b/net/mctp/device.c
index b9f38e765f61..c34963974cc1 100644
--- a/net/mctp/device.c
+++ b/net/mctp/device.c
@@ -356,9 +356,12 @@ static int mctp_register(struct net_device *dev)
if (rtnl_dereference(dev->mctp_ptr))
return 0;
- /* only register specific types; MCTP-specific and loopback for now */
- if (dev->type != ARPHRD_MCTP && dev->type != ARPHRD_LOOPBACK)
+ /* only register specific types (inc. NONE for TUN devices) */
+ if (!(dev->type == ARPHRD_MCTP ||
+ dev->type == ARPHRD_LOOPBACK ||
+ dev->type == ARPHRD_NONE)) {
return 0;
+ }
mdev = mctp_add_dev(dev);
if (IS_ERR(mdev))
diff --git a/net/mctp/route.c b/net/mctp/route.c
index 5ca186d53cb0..a953f83ed02b 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -821,13 +821,18 @@ static int mctp_pkttype_receive(struct sk_buff *skb, struct net_device *dev,
struct net_device *orig_dev)
{
struct net *net = dev_net(dev);
+ struct mctp_dev *mdev;
struct mctp_skb_cb *cb;
struct mctp_route *rt;
struct mctp_hdr *mh;
- /* basic non-data sanity checks */
- if (dev->type != ARPHRD_MCTP)
+ rcu_read_lock();
+ mdev = __mctp_dev_get(dev);
+ rcu_read_unlock();
+ if (!mdev) {
+ /* basic non-data sanity checks */
goto err_drop;
+ }
if (!pskb_may_pull(skb, sizeof(struct mctp_hdr)))
goto err_drop;
@@ -841,9 +846,7 @@ static int mctp_pkttype_receive(struct sk_buff *skb, struct net_device *dev,
goto err_drop;
cb = __mctp_cb(skb);
- rcu_read_lock();
- cb->net = READ_ONCE(__mctp_dev_get(dev)->net);
- rcu_read_unlock();
+ cb->net = READ_ONCE(mdev->net);
rt = mctp_route_lookup(net, cb->net, mh->dest);
if (!rt)
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 02/10] mctp: Allow local delivery to the null EID
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 01/10] mctp: Allow MCTP on tun devices Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 03/10] mctp: locking, lifetime and validity changes for sk_keys Matt Johnston
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
From: Jeremy Kerr <jk@codeconstruct.com.au>
We may need to receive packets addressed to the null EID (==0), but
addressed to us at the physical layer.
This change adds a lookup for local routes when we see a packet
addressed to EID 0, and a local phys address.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
net/mctp/route.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/net/mctp/route.c b/net/mctp/route.c
index a953f83ed02b..224fd25b3678 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -474,6 +474,10 @@ static int mctp_alloc_local_tag(struct mctp_sock *msk,
int rc = -EAGAIN;
u8 tagbits;
+ /* for NULL destination EIDs, we may get a response from any peer */
+ if (daddr == MCTP_ADDR_NULL)
+ daddr = MCTP_ADDR_ANY;
+
/* be optimistic, alloc now */
key = mctp_key_alloc(msk, saddr, daddr, 0, GFP_KERNEL);
if (!key)
@@ -552,6 +556,20 @@ struct mctp_route *mctp_route_lookup(struct net *net, unsigned int dnet,
return rt;
}
+static struct mctp_route *mctp_route_lookup_null(struct net *net,
+ struct net_device *dev)
+{
+ struct mctp_route *rt;
+
+ list_for_each_entry_rcu(rt, &net->mctp.routes, list) {
+ if (rt->dev->dev == dev && rt->type == RTN_LOCAL &&
+ refcount_inc_not_zero(&rt->refs))
+ return rt;
+ }
+
+ return NULL;
+}
+
/* sends a skb to rt and releases the route. */
int mctp_do_route(struct mctp_route *rt, struct sk_buff *skb)
{
@@ -849,6 +867,11 @@ static int mctp_pkttype_receive(struct sk_buff *skb, struct net_device *dev,
cb->net = READ_ONCE(mdev->net);
rt = mctp_route_lookup(net, cb->net, mh->dest);
+
+ /* NULL EID, but addressed to our physical address */
+ if (!rt && mh->dest == MCTP_ADDR_NULL && skb->pkt_type == PACKET_HOST)
+ rt = mctp_route_lookup_null(net, dev);
+
if (!rt)
goto err_drop;
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 03/10] mctp: locking, lifetime and validity changes for sk_keys
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 01/10] mctp: Allow MCTP on tun devices Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 02/10] mctp: Allow local delivery to the null EID Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 04/10] mctp: Add refcounts to mctp_dev Matt Johnston
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
From: Jeremy Kerr <jk@codeconstruct.com.au>
We will want to invalidate sk_keys in a future change, which will
require a boolean flag to mark invalidated items in the socket & net
namespace lists. We'll also need to take a reference to keys, held over
non-atomic contexts, so we need a refcount on keys also.
This change adds a validity flag (currently always true) and refcount to
struct mctp_sk_key. With a refcount on the keys, using RCU no longer
makes much sense; we have exact indications on the lifetime of keys. So,
we also change the RCU list traversal to a locked implementation.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
include/net/mctp.h | 46 ++++++++++++------
net/mctp/af_mctp.c | 14 +++---
net/mctp/route.c | 118 +++++++++++++++++++++++++++++++++------------
3 files changed, 125 insertions(+), 53 deletions(-)
diff --git a/include/net/mctp.h b/include/net/mctp.h
index a824d47c3c6d..bf783dc3ea45 100644
--- a/include/net/mctp.h
+++ b/include/net/mctp.h
@@ -67,30 +67,36 @@ struct mctp_sock {
/* Key for matching incoming packets to sockets or reassembly contexts.
* Packets are matched on (src,dest,tag).
*
- * Lifetime requirements:
+ * Lifetime / locking requirements:
*
- * - keys are free()ed via RCU
+ * - individual key data (ie, the struct itself) is protected by key->lock;
+ * changes must be made with that lock held.
+ *
+ * - the lookup fields: peer_addr, local_addr and tag are set before the
+ * key is added to lookup lists, and never updated.
+ *
+ * - A ref to the key must be held (throuh key->refs) if a pointer to the
+ * key is to be accessed after key->lock is released.
*
* - a mctp_sk_key contains a reference to a struct sock; this is valid
* for the life of the key. On sock destruction (through unhash), the key is
- * removed from lists (see below), and will not be observable after a RCU
- * grace period.
- *
- * any RX occurring within that grace period may still queue to the socket,
- * but will hit the SOCK_DEAD case before the socket is freed.
+ * removed from lists (see below), and marked invalid.
*
* - these mctp_sk_keys appear on two lists:
* 1) the struct mctp_sock->keys list
* 2) the struct netns_mctp->keys list
*
- * updates to either list are performed under the netns_mctp->keys
- * lock.
+ * presences on these lists requires a (single) refcount to be held; both
+ * lists are updated as a single operation.
+ *
+ * Updates and lookups in either list are performed under the
+ * netns_mctp->keys lock. Lookup functions will need to lock the key and
+ * take a reference before unlocking the keys_lock. Consequently, the list's
+ * keys_lock *cannot* be acquired with the individual key->lock held.
*
* - a key may have a sk_buff attached as part of an in-progress message
- * reassembly (->reasm_head). The reassembly context is protected by
- * reasm_lock, which may be acquired with the keys lock (above) held, if
- * necessary. Consequently, keys lock *cannot* be acquired with the
- * reasm_lock held.
+ * reassembly (->reasm_head). The reasm data is protected by the individual
+ * key->lock.
*
* - there are two destruction paths for a mctp_sk_key:
*
@@ -116,14 +122,22 @@ struct mctp_sk_key {
/* per-socket list */
struct hlist_node sklist;
+ /* lock protects against concurrent updates to the reassembly and
+ * expiry data below.
+ */
+ spinlock_t lock;
+
+ /* Keys are referenced during the output path, which may sleep */
+ refcount_t refs;
+
/* incoming fragment reassembly context */
- spinlock_t reasm_lock;
struct sk_buff *reasm_head;
struct sk_buff **reasm_tailp;
bool reasm_dead;
u8 last_seq;
- struct rcu_head rcu;
+ /* key validity */
+ bool valid;
};
struct mctp_skb_cb {
@@ -191,6 +205,8 @@ int mctp_do_route(struct mctp_route *rt, struct sk_buff *skb);
int mctp_local_output(struct sock *sk, struct mctp_route *rt,
struct sk_buff *skb, mctp_eid_t daddr, u8 req_tag);
+void mctp_key_unref(struct mctp_sk_key *key);
+
/* routing <--> device interface */
unsigned int mctp_default_net(struct net *net);
int mctp_default_net_set(struct net *net, unsigned int index);
diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
index a9526ac29dff..2767d548736b 100644
--- a/net/mctp/af_mctp.c
+++ b/net/mctp/af_mctp.c
@@ -263,21 +263,21 @@ static void mctp_sk_unhash(struct sock *sk)
/* remove tag allocations */
spin_lock_irqsave(&net->mctp.keys_lock, flags);
hlist_for_each_entry_safe(key, tmp, &msk->keys, sklist) {
- hlist_del_rcu(&key->sklist);
- hlist_del_rcu(&key->hlist);
+ hlist_del(&key->sklist);
+ hlist_del(&key->hlist);
- spin_lock(&key->reasm_lock);
+ spin_lock(&key->lock);
if (key->reasm_head)
kfree_skb(key->reasm_head);
key->reasm_head = NULL;
key->reasm_dead = true;
- spin_unlock(&key->reasm_lock);
+ key->valid = false;
+ spin_unlock(&key->lock);
- kfree_rcu(key, rcu);
+ /* key is no longer on the lookup lists, unref */
+ mctp_key_unref(key);
}
spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
-
- synchronize_rcu();
}
static struct proto mctp_proto = {
diff --git a/net/mctp/route.c b/net/mctp/route.c
index 224fd25b3678..b2243b150e71 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -83,25 +83,43 @@ static bool mctp_key_match(struct mctp_sk_key *key, mctp_eid_t local,
return true;
}
+/* returns a key (with key->lock held, and refcounted), or NULL if no such
+ * key exists.
+ */
static struct mctp_sk_key *mctp_lookup_key(struct net *net, struct sk_buff *skb,
- mctp_eid_t peer)
+ mctp_eid_t peer,
+ unsigned long *irqflags)
+ __acquires(&key->lock)
{
struct mctp_sk_key *key, *ret;
+ unsigned long flags;
struct mctp_hdr *mh;
u8 tag;
- WARN_ON(!rcu_read_lock_held());
-
mh = mctp_hdr(skb);
tag = mh->flags_seq_tag & (MCTP_HDR_TAG_MASK | MCTP_HDR_FLAG_TO);
ret = NULL;
+ spin_lock_irqsave(&net->mctp.keys_lock, flags);
- hlist_for_each_entry_rcu(key, &net->mctp.keys, hlist) {
- if (mctp_key_match(key, mh->dest, peer, tag)) {
+ hlist_for_each_entry(key, &net->mctp.keys, hlist) {
+ if (!mctp_key_match(key, mh->dest, peer, tag))
+ continue;
+
+ spin_lock(&key->lock);
+ if (key->valid) {
+ refcount_inc(&key->refs);
ret = key;
break;
}
+ spin_unlock(&key->lock);
+ }
+
+ if (ret) {
+ spin_unlock(&net->mctp.keys_lock);
+ *irqflags = flags;
+ } else {
+ spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
}
return ret;
@@ -121,11 +139,19 @@ static struct mctp_sk_key *mctp_key_alloc(struct mctp_sock *msk,
key->local_addr = local;
key->tag = tag;
key->sk = &msk->sk;
- spin_lock_init(&key->reasm_lock);
+ key->valid = true;
+ spin_lock_init(&key->lock);
+ refcount_set(&key->refs, 1);
return key;
}
+void mctp_key_unref(struct mctp_sk_key *key)
+{
+ if (refcount_dec_and_test(&key->refs))
+ kfree(key);
+}
+
static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk)
{
struct net *net = sock_net(&msk->sk);
@@ -138,12 +164,17 @@ static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk)
hlist_for_each_entry(tmp, &net->mctp.keys, hlist) {
if (mctp_key_match(tmp, key->local_addr, key->peer_addr,
key->tag)) {
- rc = -EEXIST;
- break;
+ spin_lock(&tmp->lock);
+ if (tmp->valid)
+ rc = -EEXIST;
+ spin_unlock(&tmp->lock);
+ if (rc)
+ break;
}
}
if (!rc) {
+ refcount_inc(&key->refs);
hlist_add_head(&key->hlist, &net->mctp.keys);
hlist_add_head(&key->sklist, &msk->keys);
}
@@ -153,28 +184,35 @@ static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk)
return rc;
}
-/* Must be called with key->reasm_lock, which it will release. Will schedule
- * the key for an RCU free.
+/* We're done with the key; unset valid and remove from lists. There may still
+ * be outstanding refs on the key though...
*/
static void __mctp_key_unlock_drop(struct mctp_sk_key *key, struct net *net,
unsigned long flags)
- __releases(&key->reasm_lock)
+ __releases(&key->lock)
{
struct sk_buff *skb;
skb = key->reasm_head;
key->reasm_head = NULL;
key->reasm_dead = true;
- spin_unlock_irqrestore(&key->reasm_lock, flags);
+ key->valid = false;
+ spin_unlock_irqrestore(&key->lock, flags);
spin_lock_irqsave(&net->mctp.keys_lock, flags);
- hlist_del_rcu(&key->hlist);
- hlist_del_rcu(&key->sklist);
+ hlist_del(&key->hlist);
+ hlist_del(&key->sklist);
spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
- kfree_rcu(key, rcu);
+
+ /* one unref for the lists */
+ mctp_key_unref(key);
+
+ /* and one for the local reference */
+ mctp_key_unref(key);
if (skb)
kfree_skb(skb);
+
}
static int mctp_frag_queue(struct mctp_sk_key *key, struct sk_buff *skb)
@@ -248,8 +286,10 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
rcu_read_lock();
- /* lookup socket / reasm context, exactly matching (src,dest,tag) */
- key = mctp_lookup_key(net, skb, mh->src);
+ /* lookup socket / reasm context, exactly matching (src,dest,tag).
+ * we hold a ref on the key, and key->lock held.
+ */
+ key = mctp_lookup_key(net, skb, mh->src, &f);
if (flags & MCTP_HDR_FLAG_SOM) {
if (key) {
@@ -260,10 +300,12 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
* key for reassembly - we'll create a more specific
* one for future packets if required (ie, !EOM).
*/
- key = mctp_lookup_key(net, skb, MCTP_ADDR_ANY);
+ key = mctp_lookup_key(net, skb, MCTP_ADDR_ANY, &f);
if (key) {
msk = container_of(key->sk,
struct mctp_sock, sk);
+ spin_unlock_irqrestore(&key->lock, f);
+ mctp_key_unref(key);
key = NULL;
}
}
@@ -282,11 +324,11 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
if (flags & MCTP_HDR_FLAG_EOM) {
sock_queue_rcv_skb(&msk->sk, skb);
if (key) {
- spin_lock_irqsave(&key->reasm_lock, f);
/* we've hit a pending reassembly; not much we
* can do but drop it
*/
__mctp_key_unlock_drop(key, net, f);
+ key = NULL;
}
rc = 0;
goto out_unlock;
@@ -303,7 +345,7 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
goto out_unlock;
}
- /* we can queue without the reasm lock here, as the
+ /* we can queue without the key lock here, as the
* key isn't observable yet
*/
mctp_frag_queue(key, skb);
@@ -318,17 +360,17 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
if (rc)
kfree(key);
- } else {
- /* existing key: start reassembly */
- spin_lock_irqsave(&key->reasm_lock, f);
+ /* we don't need to release key->lock on exit */
+ key = NULL;
+ } else {
if (key->reasm_head || key->reasm_dead) {
/* duplicate start? drop everything */
__mctp_key_unlock_drop(key, net, f);
rc = -EEXIST;
+ key = NULL;
} else {
rc = mctp_frag_queue(key, skb);
- spin_unlock_irqrestore(&key->reasm_lock, f);
}
}
@@ -337,8 +379,6 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
* using the message-specific key
*/
- spin_lock_irqsave(&key->reasm_lock, f);
-
/* we need to be continuing an existing reassembly... */
if (!key->reasm_head)
rc = -EINVAL;
@@ -352,8 +392,7 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
sock_queue_rcv_skb(key->sk, key->reasm_head);
key->reasm_head = NULL;
__mctp_key_unlock_drop(key, net, f);
- } else {
- spin_unlock_irqrestore(&key->reasm_lock, f);
+ key = NULL;
}
} else {
@@ -363,6 +402,10 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
out_unlock:
rcu_read_unlock();
+ if (key) {
+ spin_unlock_irqrestore(&key->lock, f);
+ mctp_key_unref(key);
+ }
out:
if (rc)
kfree_skb(skb);
@@ -459,6 +502,7 @@ static void mctp_reserve_tag(struct net *net, struct mctp_sk_key *key,
*/
hlist_add_head_rcu(&key->hlist, &mns->keys);
hlist_add_head_rcu(&key->sklist, &msk->keys);
+ refcount_inc(&key->refs);
}
/* Allocate a locally-owned tag value for (saddr, daddr), and reserve
@@ -492,14 +536,26 @@ static int mctp_alloc_local_tag(struct mctp_sock *msk,
* tags. If we find a conflict, clear that bit from tagbits
*/
hlist_for_each_entry(tmp, &mns->keys, hlist) {
+ /* We can check the lookup fields (*_addr, tag) without the
+ * lock held, they don't change over the lifetime of the key.
+ */
+
/* if we don't own the tag, it can't conflict */
if (tmp->tag & MCTP_HDR_FLAG_TO)
continue;
- if ((tmp->peer_addr == daddr ||
- tmp->peer_addr == MCTP_ADDR_ANY) &&
- tmp->local_addr == saddr)
+ if (!((tmp->peer_addr == daddr ||
+ tmp->peer_addr == MCTP_ADDR_ANY) &&
+ tmp->local_addr == saddr))
+ continue;
+
+ spin_lock(&tmp->lock);
+ /* key must still be valid. If we find a match, clear the
+ * potential tag value
+ */
+ if (tmp->valid)
tagbits &= ~(1 << tmp->tag);
+ spin_unlock(&tmp->lock);
if (!tagbits)
break;
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 04/10] mctp: Add refcounts to mctp_dev
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
` (2 preceding siblings ...)
2021-09-29 7:26 ` [PATCH net-next 03/10] mctp: locking, lifetime and validity changes for sk_keys Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 05/10] mctp: Implement a timeout for tags Matt Johnston
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
From: Jeremy Kerr <jk@codeconstruct.com.au>
Currently, we tie the struct mctp_dev lifetime to the underlying struct
net_device, and hold/put that device as a proxy for a separate mctp_dev
refcount. This works because we're not holding any references to the
mctp_dev that are different from the netdev lifetime.
In a future change we'll break that assumption though, as we'll need to
hold mctp_dev references in a workqueue, which might live past the
netdev unregister notification.
In order to support that, this change introduces a refcount on the
mctp_dev, currently taken by the net_device->mctp_ptr reference, and
released on netdev unregister events. We can then use this for future
references that might outlast the net device.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
include/net/mctpdevice.h | 5 +++++
net/mctp/device.c | 25 ++++++++++++++++---------
net/mctp/neigh.c | 4 ++--
net/mctp/route.c | 4 ++--
4 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/include/net/mctpdevice.h b/include/net/mctpdevice.h
index 71a11012fac7..3a439463f055 100644
--- a/include/net/mctpdevice.h
+++ b/include/net/mctpdevice.h
@@ -17,6 +17,8 @@
struct mctp_dev {
struct net_device *dev;
+ refcount_t refs;
+
unsigned int net;
/* Only modified under RTNL. Reads have addrs_lock held */
@@ -32,4 +34,7 @@ struct mctp_dev {
struct mctp_dev *mctp_dev_get_rtnl(const struct net_device *dev);
struct mctp_dev *__mctp_dev_get(const struct net_device *dev);
+void mctp_dev_hold(struct mctp_dev *mdev);
+void mctp_dev_put(struct mctp_dev *mdev);
+
#endif /* __NET_MCTPDEVICE_H */
diff --git a/net/mctp/device.c b/net/mctp/device.c
index c34963974cc1..f556c6d01abc 100644
--- a/net/mctp/device.c
+++ b/net/mctp/device.c
@@ -35,14 +35,6 @@ struct mctp_dev *mctp_dev_get_rtnl(const struct net_device *dev)
return rtnl_dereference(dev->mctp_ptr);
}
-static void mctp_dev_destroy(struct mctp_dev *mdev)
-{
- struct net_device *dev = mdev->dev;
-
- dev_put(dev);
- kfree_rcu(mdev, rcu);
-}
-
static int mctp_fill_addrinfo(struct sk_buff *skb, struct netlink_callback *cb,
struct mctp_dev *mdev, mctp_eid_t eid)
{
@@ -255,6 +247,19 @@ static int mctp_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
return 0;
}
+void mctp_dev_hold(struct mctp_dev *mdev)
+{
+ refcount_inc(&mdev->refs);
+}
+
+void mctp_dev_put(struct mctp_dev *mdev)
+{
+ if (refcount_dec_and_test(&mdev->refs)) {
+ dev_put(mdev->dev);
+ kfree_rcu(mdev, rcu);
+ }
+}
+
static struct mctp_dev *mctp_add_dev(struct net_device *dev)
{
struct mctp_dev *mdev;
@@ -270,7 +275,9 @@ static struct mctp_dev *mctp_add_dev(struct net_device *dev)
mdev->net = mctp_default_net(dev_net(dev));
/* associate to net_device */
+ refcount_set(&mdev->refs, 1);
rcu_assign_pointer(dev->mctp_ptr, mdev);
+
dev_hold(dev);
mdev->dev = dev;
@@ -345,7 +352,7 @@ static void mctp_unregister(struct net_device *dev)
mctp_neigh_remove_dev(mdev);
kfree(mdev->addrs);
- mctp_dev_destroy(mdev);
+ mctp_dev_put(mdev);
}
static int mctp_register(struct net_device *dev)
diff --git a/net/mctp/neigh.c b/net/mctp/neigh.c
index 90ed2f02d1fb..5cc042121493 100644
--- a/net/mctp/neigh.c
+++ b/net/mctp/neigh.c
@@ -47,7 +47,7 @@ static int mctp_neigh_add(struct mctp_dev *mdev, mctp_eid_t eid,
}
INIT_LIST_HEAD(&neigh->list);
neigh->dev = mdev;
- dev_hold(neigh->dev->dev);
+ mctp_dev_hold(neigh->dev);
neigh->eid = eid;
neigh->source = source;
memcpy(neigh->ha, lladdr, lladdr_len);
@@ -63,7 +63,7 @@ static void __mctp_neigh_free(struct rcu_head *rcu)
{
struct mctp_neigh *neigh = container_of(rcu, struct mctp_neigh, rcu);
- dev_put(neigh->dev->dev);
+ mctp_dev_put(neigh->dev);
kfree(neigh);
}
diff --git a/net/mctp/route.c b/net/mctp/route.c
index b2243b150e71..37aa67847a5a 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -455,7 +455,7 @@ static int mctp_route_output(struct mctp_route *route, struct sk_buff *skb)
static void mctp_route_release(struct mctp_route *rt)
{
if (refcount_dec_and_test(&rt->refs)) {
- dev_put(rt->dev->dev);
+ mctp_dev_put(rt->dev);
kfree_rcu(rt, rcu);
}
}
@@ -815,7 +815,7 @@ static int mctp_route_add(struct mctp_dev *mdev, mctp_eid_t daddr_start,
rt->max = daddr_start + daddr_extent;
rt->mtu = mtu;
rt->dev = mdev;
- dev_hold(rt->dev->dev);
+ mctp_dev_hold(rt->dev);
rt->type = type;
rt->output = rtfn;
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 05/10] mctp: Implement a timeout for tags
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
` (3 preceding siblings ...)
2021-09-29 7:26 ` [PATCH net-next 04/10] mctp: Add refcounts to mctp_dev Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 06/10] mctp: Add tracepoints for tag/key handling Matt Johnston
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
From: Jeremy Kerr <jk@codeconstruct.com.au>
Currently, a MCTP (local-eid,remote-eid,tag) tuple is allocated to a
socket on send, and only expires when the socket is closed.
This change introduces a tag timeout, freeing the tuple after a fixed
expiry - currently six seconds. This is greater than (but close to) the
max response timeout in upper-layer bindings.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
include/net/mctp.h | 10 ++++++++++
net/mctp/af_mctp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
net/mctp/route.c | 8 ++++++++
3 files changed, 62 insertions(+)
diff --git a/include/net/mctp.h b/include/net/mctp.h
index bf783dc3ea45..b9ed62a63c24 100644
--- a/include/net/mctp.h
+++ b/include/net/mctp.h
@@ -62,6 +62,11 @@ struct mctp_sock {
* by sk->net->keys_lock
*/
struct hlist_head keys;
+
+ /* mechanism for expiring allocated keys; will release an allocated
+ * tag, and any netdev state for a request/response pairing
+ */
+ struct timer_list key_expiry;
};
/* Key for matching incoming packets to sockets or reassembly contexts.
@@ -107,6 +112,8 @@ struct mctp_sock {
* the (complete) reply, or during reassembly errors. Here, we clean up
* the reassembly context (marking reasm_dead, to prevent another from
* starting), and remove the socket from the netns & socket lists.
+ *
+ * - through an expiry timeout, on a per-socket timer
*/
struct mctp_sk_key {
mctp_eid_t peer_addr;
@@ -138,6 +145,9 @@ struct mctp_sk_key {
/* key validity */
bool valid;
+
+ /* expiry timeout; valid (above) cleared on expiry */
+ unsigned long expiry;
};
struct mctp_skb_cb {
diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
index 2767d548736b..46e5ede385cb 100644
--- a/net/mctp/af_mctp.c
+++ b/net/mctp/af_mctp.c
@@ -223,16 +223,60 @@ static const struct proto_ops mctp_dgram_ops = {
.sendpage = sock_no_sendpage,
};
+static void mctp_sk_expire_keys(struct timer_list *timer)
+{
+ struct mctp_sock *msk = container_of(timer, struct mctp_sock,
+ key_expiry);
+ struct net *net = sock_net(&msk->sk);
+ unsigned long next_expiry, flags;
+ struct mctp_sk_key *key;
+ struct hlist_node *tmp;
+ bool next_expiry_valid = false;
+
+ spin_lock_irqsave(&net->mctp.keys_lock, flags);
+
+ hlist_for_each_entry_safe(key, tmp, &msk->keys, sklist) {
+ spin_lock(&key->lock);
+
+ if (!time_after_eq(key->expiry, jiffies)) {
+ key->valid = false;
+ hlist_del_rcu(&key->hlist);
+ hlist_del_rcu(&key->sklist);
+ spin_unlock(&key->lock);
+ mctp_key_unref(key);
+ continue;
+ }
+
+ if (next_expiry_valid) {
+ if (time_before(key->expiry, next_expiry))
+ next_expiry = key->expiry;
+ } else {
+ next_expiry = key->expiry;
+ next_expiry_valid = true;
+ }
+ spin_unlock(&key->lock);
+ }
+
+ spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
+
+ if (next_expiry_valid)
+ mod_timer(timer, next_expiry);
+}
+
static int mctp_sk_init(struct sock *sk)
{
struct mctp_sock *msk = container_of(sk, struct mctp_sock, sk);
INIT_HLIST_HEAD(&msk->keys);
+ timer_setup(&msk->key_expiry, mctp_sk_expire_keys, 0);
return 0;
}
static void mctp_sk_close(struct sock *sk, long timeout)
{
+ struct mctp_sock *msk = container_of(sk, struct mctp_sock, sk);
+
+ del_timer_sync(&msk->key_expiry);
sk_common_release(sk);
}
diff --git a/net/mctp/route.c b/net/mctp/route.c
index 37aa67847a5a..c342adf4f97f 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -24,6 +24,8 @@
#include <net/sock.h>
static const unsigned int mctp_message_maxlen = 64 * 1024;
+static const unsigned long mctp_key_lifetime = 6 * CONFIG_HZ;
+
/* route output callbacks */
static int mctp_route_discard(struct mctp_route *route, struct sk_buff *skb)
@@ -175,6 +177,9 @@ static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk)
if (!rc) {
refcount_inc(&key->refs);
+ key->expiry = jiffies + mctp_key_lifetime;
+ timer_reduce(&msk->key_expiry, key->expiry);
+
hlist_add_head(&key->hlist, &net->mctp.keys);
hlist_add_head(&key->sklist, &msk->keys);
}
@@ -497,6 +502,9 @@ static void mctp_reserve_tag(struct net *net, struct mctp_sk_key *key,
lockdep_assert_held(&mns->keys_lock);
+ key->expiry = jiffies + mctp_key_lifetime;
+ timer_reduce(&msk->key_expiry, key->expiry);
+
/* we hold the net->key_lock here, allowing updates to both
* then net and sk
*/
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 06/10] mctp: Add tracepoints for tag/key handling
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
` (4 preceding siblings ...)
2021-09-29 7:26 ` [PATCH net-next 05/10] mctp: Implement a timeout for tags Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 07/10] mctp: Do inits as a subsys_initcall Matt Johnston
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
From: Jeremy Kerr <jk@codeconstruct.com.au>
The tag allocation, release and bind events are somewhat opaque outside
the kernel; this change adds a few tracepoints to assist in
instrumentation and debugging.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
include/trace/events/mctp.h | 75 +++++++++++++++++++++++++++++++++++++
net/mctp/af_mctp.c | 6 +++
net/mctp/route.c | 12 +++++-
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 include/trace/events/mctp.h
diff --git a/include/trace/events/mctp.h b/include/trace/events/mctp.h
new file mode 100644
index 000000000000..175b057c507f
--- /dev/null
+++ b/include/trace/events/mctp.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM mctp
+
+#if !defined(_TRACE_MCTP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_MCTP_H
+
+#include <linux/tracepoint.h>
+
+#ifndef __TRACE_MCTP_ENUMS
+#define __TRACE_MCTP_ENUMS
+enum {
+ MCTP_TRACE_KEY_TIMEOUT,
+ MCTP_TRACE_KEY_REPLIED,
+ MCTP_TRACE_KEY_INVALIDATED,
+ MCTP_TRACE_KEY_CLOSED,
+};
+#endif /* __TRACE_MCTP_ENUMS */
+
+TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_TIMEOUT);
+TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_REPLIED);
+TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_INVALIDATED);
+TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_CLOSED);
+
+TRACE_EVENT(mctp_key_acquire,
+ TP_PROTO(const struct mctp_sk_key *key),
+ TP_ARGS(key),
+ TP_STRUCT__entry(
+ __field(__u8, paddr)
+ __field(__u8, laddr)
+ __field(__u8, tag)
+ ),
+ TP_fast_assign(
+ __entry->paddr = key->peer_addr;
+ __entry->laddr = key->local_addr;
+ __entry->tag = key->tag;
+ ),
+ TP_printk("local %d, peer %d, tag %1x",
+ __entry->laddr,
+ __entry->paddr,
+ __entry->tag
+ )
+);
+
+TRACE_EVENT(mctp_key_release,
+ TP_PROTO(const struct mctp_sk_key *key, int reason),
+ TP_ARGS(key, reason),
+ TP_STRUCT__entry(
+ __field(__u8, paddr)
+ __field(__u8, laddr)
+ __field(__u8, tag)
+ __field(int, reason)
+ ),
+ TP_fast_assign(
+ __entry->paddr = key->peer_addr;
+ __entry->laddr = key->local_addr;
+ __entry->tag = key->tag;
+ __entry->reason = reason;
+ ),
+ TP_printk("local %d, peer %d, tag %1x %s",
+ __entry->laddr,
+ __entry->paddr,
+ __entry->tag,
+ __print_symbolic(__entry->reason,
+ { MCTP_TRACE_KEY_TIMEOUT, "timeout" },
+ { MCTP_TRACE_KEY_REPLIED, "replied" },
+ { MCTP_TRACE_KEY_INVALIDATED, "invalidated" },
+ { MCTP_TRACE_KEY_CLOSED, "closed" })
+ )
+);
+
+#endif
+
+#include <trace/define_trace.h>
diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
index 46e5ede385cb..28cb1633bed6 100644
--- a/net/mctp/af_mctp.c
+++ b/net/mctp/af_mctp.c
@@ -16,6 +16,9 @@
#include <net/mctpdevice.h>
#include <net/sock.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/mctp.h>
+
/* socket implementation */
static int mctp_release(struct socket *sock)
@@ -239,6 +242,7 @@ static void mctp_sk_expire_keys(struct timer_list *timer)
spin_lock(&key->lock);
if (!time_after_eq(key->expiry, jiffies)) {
+ trace_mctp_key_release(key, MCTP_TRACE_KEY_TIMEOUT);
key->valid = false;
hlist_del_rcu(&key->hlist);
hlist_del_rcu(&key->sklist);
@@ -310,6 +314,8 @@ static void mctp_sk_unhash(struct sock *sk)
hlist_del(&key->sklist);
hlist_del(&key->hlist);
+ trace_mctp_key_release(key, MCTP_TRACE_KEY_CLOSED);
+
spin_lock(&key->lock);
if (key->reasm_head)
kfree_skb(key->reasm_head);
diff --git a/net/mctp/route.c b/net/mctp/route.c
index c342adf4f97f..acc5bb39e16d 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -23,10 +23,11 @@
#include <net/netlink.h>
#include <net/sock.h>
+#include <trace/events/mctp.h>
+
static const unsigned int mctp_message_maxlen = 64 * 1024;
static const unsigned long mctp_key_lifetime = 6 * CONFIG_HZ;
-
/* route output callbacks */
static int mctp_route_discard(struct mctp_route *route, struct sk_buff *skb)
{
@@ -332,6 +333,8 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
/* we've hit a pending reassembly; not much we
* can do but drop it
*/
+ trace_mctp_key_release(key,
+ MCTP_TRACE_KEY_REPLIED);
__mctp_key_unlock_drop(key, net, f);
key = NULL;
}
@@ -365,12 +368,16 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
if (rc)
kfree(key);
+ trace_mctp_key_acquire(key);
+
/* we don't need to release key->lock on exit */
key = NULL;
} else {
if (key->reasm_head || key->reasm_dead) {
/* duplicate start? drop everything */
+ trace_mctp_key_release(key,
+ MCTP_TRACE_KEY_INVALIDATED);
__mctp_key_unlock_drop(key, net, f);
rc = -EEXIST;
key = NULL;
@@ -396,6 +403,7 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
if (!rc && flags & MCTP_HDR_FLAG_EOM) {
sock_queue_rcv_skb(key->sk, key->reasm_head);
key->reasm_head = NULL;
+ trace_mctp_key_release(key, MCTP_TRACE_KEY_REPLIED);
__mctp_key_unlock_drop(key, net, f);
key = NULL;
}
@@ -572,6 +580,8 @@ static int mctp_alloc_local_tag(struct mctp_sock *msk,
if (tagbits) {
key->tag = __ffs(tagbits);
mctp_reserve_tag(net, key, msk);
+ trace_mctp_key_acquire(key);
+
*tagp = key->tag;
rc = 0;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 07/10] mctp: Do inits as a subsys_initcall
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
` (5 preceding siblings ...)
2021-09-29 7:26 ` [PATCH net-next 06/10] mctp: Add tracepoints for tag/key handling Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 08/10] doc/mctp: Add a little detail about kernel internals Matt Johnston
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
From: Jeremy Kerr <jk@codeconstruct.com.au>
In a future change, we'll want to provide a registration call for
mctp-specific devices. This requires us to have the networks established
before device driver inits, so run the core init as a subsys_initcall.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
net/mctp/af_mctp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
index 28cb1633bed6..66a411d60b6c 100644
--- a/net/mctp/af_mctp.c
+++ b/net/mctp/af_mctp.c
@@ -435,7 +435,7 @@ static __exit void mctp_exit(void)
sock_unregister(PF_MCTP);
}
-module_init(mctp_init);
+subsys_initcall(mctp_init);
module_exit(mctp_exit);
MODULE_DESCRIPTION("MCTP core");
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 08/10] doc/mctp: Add a little detail about kernel internals
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
` (6 preceding siblings ...)
2021-09-29 7:26 ` [PATCH net-next 07/10] mctp: Do inits as a subsys_initcall Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 09/10] mctp: Set route MTU via netlink Matt Johnston
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
From: Jeremy Kerr <jk@codeconstruct.com.au>
Describe common flows and refcounting behaviour.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
Documentation/networking/mctp.rst | 59 +++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/Documentation/networking/mctp.rst b/Documentation/networking/mctp.rst
index 6100cdc220f6..2c54b029f990 100644
--- a/Documentation/networking/mctp.rst
+++ b/Documentation/networking/mctp.rst
@@ -211,3 +211,62 @@ remote address is already known, or the message does not require a reply.
Like the send calls, sockets will only receive responses to requests they have
sent (TO=1) and may only respond (TO=0) to requests they have received.
+
+Kernel internals
+================
+
+There are a few possible packet flows in the MCTP stack:
+
+1. local TX to remote endpoint, message <= MTU::
+
+ sendmsg()
+ -> mctp_local_output()
+ : route lookup
+ -> rt->output() (== mctp_route_output)
+ -> dev_queue_xmit()
+
+2. local TX to remote endpoint, message > MTU::
+
+ sendmsg()
+ -> mctp_local_output()
+ -> mctp_do_fragment_route()
+ : creates packet-sized skbs. For each new skb:
+ -> rt->output() (== mctp_route_output)
+ -> dev_queue_xmit()
+
+3. remote TX to local endpoint, single-packet message::
+
+ mctp_pkttype_receive()
+ : route lookup
+ -> rt->output() (== mctp_route_input)
+ : sk_key lookup
+ -> sock_queue_rcv_skb()
+
+4. remote TX to local endpoint, multiple-packet message::
+
+ mctp_pkttype_receive()
+ : route lookup
+ -> rt->output() (== mctp_route_input)
+ : sk_key lookup
+ : stores skb in struct sk_key->reasm_head
+
+ mctp_pkttype_receive()
+ : route lookup
+ -> rt->output() (== mctp_route_input)
+ : sk_key lookup
+ : finds existing reassembly in sk_key->reasm_head
+ : appends new fragment
+ -> sock_queue_rcv_skb()
+
+Key refcounts
+-------------
+
+ * keys are refed by:
+
+ - a skb: during route output, stored in ``skb->cb``.
+
+ - netns and sock lists.
+
+ * keys can be associated with a device, in which case they hold a
+ reference to the dev (set through ``key->dev``, counted through
+ ``dev->key_count``). Multiple keys can reference the device.
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 09/10] mctp: Set route MTU via netlink
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
` (7 preceding siblings ...)
2021-09-29 7:26 ` [PATCH net-next 08/10] doc/mctp: Add a little detail about kernel internals Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 10/10] mctp: Warn if pointer is set for a wrong dev type Matt Johnston
2021-09-29 10:10 ` [PATCH net-next 00/10] Updates to MCTP core patchwork-bot+netdevbpf
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
A route's RTAX_MTU can be set in nested RTAX_METRICS
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
---
net/mctp/route.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/net/mctp/route.c b/net/mctp/route.c
index acc5bb39e16d..e20f3096d067 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -1026,10 +1026,15 @@ static int mctp_route_nlparse(struct sk_buff *skb, struct nlmsghdr *nlh,
return 0;
}
+static const struct nla_policy rta_metrics_policy[RTAX_MAX + 1] = {
+ [RTAX_MTU] = { .type = NLA_U32 },
+};
+
static int mctp_newroute(struct sk_buff *skb, struct nlmsghdr *nlh,
struct netlink_ext_ack *extack)
{
struct nlattr *tb[RTA_MAX + 1];
+ struct nlattr *tbx[RTAX_MAX + 1];
mctp_eid_t daddr_start;
struct mctp_dev *mdev;
struct rtmsg *rtm;
@@ -1046,8 +1051,15 @@ static int mctp_newroute(struct sk_buff *skb, struct nlmsghdr *nlh,
return -EINVAL;
}
- /* TODO: parse mtu from nlparse */
mtu = 0;
+ if (tb[RTA_METRICS]) {
+ rc = nla_parse_nested(tbx, RTAX_MAX, tb[RTA_METRICS],
+ rta_metrics_policy, NULL);
+ if (rc < 0)
+ return rc;
+ if (tbx[RTAX_MTU])
+ mtu = nla_get_u32(tbx[RTAX_MTU]);
+ }
if (rtm->rtm_type != RTN_UNICAST)
return -EINVAL;
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next 10/10] mctp: Warn if pointer is set for a wrong dev type
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
` (8 preceding siblings ...)
2021-09-29 7:26 ` [PATCH net-next 09/10] mctp: Set route MTU via netlink Matt Johnston
@ 2021-09-29 7:26 ` Matt Johnston
2021-09-29 10:10 ` [PATCH net-next 00/10] Updates to MCTP core patchwork-bot+netdevbpf
10 siblings, 0 replies; 12+ messages in thread
From: Matt Johnston @ 2021-09-29 7:26 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev; +Cc: Jeremy Kerr
Should not occur but is a sanity check.
May help tracking down Trinity reported issue
https://lore.kernel.org/lkml/20210913030701.GA5926@xsang-OptiPlex-9020/
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
---
net/mctp/device.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/net/mctp/device.c b/net/mctp/device.c
index f556c6d01abc..3827d62f52c9 100644
--- a/net/mctp/device.c
+++ b/net/mctp/device.c
@@ -337,12 +337,26 @@ static int mctp_set_link_af(struct net_device *dev, const struct nlattr *attr,
return 0;
}
+/* Matches netdev types that should have MCTP handling */
+static bool mctp_known(struct net_device *dev)
+{
+ /* only register specific types (inc. NONE for TUN devices) */
+ return dev->type == ARPHRD_MCTP ||
+ dev->type == ARPHRD_LOOPBACK ||
+ dev->type == ARPHRD_NONE;
+}
+
static void mctp_unregister(struct net_device *dev)
{
struct mctp_dev *mdev;
mdev = mctp_dev_get_rtnl(dev);
-
+ if (mctp_known(dev) != (bool)mdev) {
+ // Sanity check, should match what was set in mctp_register
+ netdev_warn(dev, "%s: mdev pointer %d but type (%d) match is %d",
+ __func__, (bool)mdev, mctp_known(dev), dev->type);
+ return;
+ }
if (!mdev)
return;
@@ -360,16 +374,19 @@ static int mctp_register(struct net_device *dev)
struct mctp_dev *mdev;
/* Already registered? */
- if (rtnl_dereference(dev->mctp_ptr))
- return 0;
+ mdev = rtnl_dereference(dev->mctp_ptr);
- /* only register specific types (inc. NONE for TUN devices) */
- if (!(dev->type == ARPHRD_MCTP ||
- dev->type == ARPHRD_LOOPBACK ||
- dev->type == ARPHRD_NONE)) {
+ if (mdev) {
+ if (!mctp_known(dev))
+ netdev_warn(dev, "%s: mctp_dev set for unknown type %d",
+ __func__, dev->type);
return 0;
}
+ /* only register specific types */
+ if (!mctp_known(dev))
+ return 0;
+
mdev = mctp_add_dev(dev);
if (IS_ERR(mdev))
return PTR_ERR(mdev);
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 00/10] Updates to MCTP core
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
` (9 preceding siblings ...)
2021-09-29 7:26 ` [PATCH net-next 10/10] mctp: Warn if pointer is set for a wrong dev type Matt Johnston
@ 2021-09-29 10:10 ` patchwork-bot+netdevbpf
10 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-09-29 10:10 UTC (permalink / raw)
To: Matt Johnston; +Cc: davem, kuba, netdev, jk
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Wed, 29 Sep 2021 15:26:04 +0800 you wrote:
> Hi,
>
> This series adds timeouts for MCTP tags (a limited resource), and a few
> other improvements to the MCTP core.
>
> Cheers,
> Matt
>
> [...]
Here is the summary with links:
- [net-next,01/10] mctp: Allow MCTP on tun devices
https://git.kernel.org/netdev/net-next/c/f364dd71d92f
- [net-next,02/10] mctp: Allow local delivery to the null EID
https://git.kernel.org/netdev/net-next/c/1f6c77ac9e6e
- [net-next,03/10] mctp: locking, lifetime and validity changes for sk_keys
https://git.kernel.org/netdev/net-next/c/73c618456dc5
- [net-next,04/10] mctp: Add refcounts to mctp_dev
https://git.kernel.org/netdev/net-next/c/43f55f23f708
- [net-next,05/10] mctp: Implement a timeout for tags
https://git.kernel.org/netdev/net-next/c/7b14e15ae6f4
- [net-next,06/10] mctp: Add tracepoints for tag/key handling
https://git.kernel.org/netdev/net-next/c/4f9e1ba6de45
- [net-next,07/10] mctp: Do inits as a subsys_initcall
https://git.kernel.org/netdev/net-next/c/97f09abffcb9
- [net-next,08/10] doc/mctp: Add a little detail about kernel internals
https://git.kernel.org/netdev/net-next/c/f4d41c59135d
- [net-next,09/10] mctp: Set route MTU via netlink
https://git.kernel.org/netdev/net-next/c/6183569db80e
- [net-next,10/10] mctp: Warn if pointer is set for a wrong dev type
https://git.kernel.org/netdev/net-next/c/7b1871af75f3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-09-29 10:10 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29 7:26 [PATCH net-next 00/10] Updates to MCTP core Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 01/10] mctp: Allow MCTP on tun devices Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 02/10] mctp: Allow local delivery to the null EID Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 03/10] mctp: locking, lifetime and validity changes for sk_keys Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 04/10] mctp: Add refcounts to mctp_dev Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 05/10] mctp: Implement a timeout for tags Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 06/10] mctp: Add tracepoints for tag/key handling Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 07/10] mctp: Do inits as a subsys_initcall Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 08/10] doc/mctp: Add a little detail about kernel internals Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 09/10] mctp: Set route MTU via netlink Matt Johnston
2021-09-29 7:26 ` [PATCH net-next 10/10] mctp: Warn if pointer is set for a wrong dev type Matt Johnston
2021-09-29 10:10 ` [PATCH net-next 00/10] Updates to MCTP core patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.