Re: [RFC PATCH v7 08/16] net: dsa: tag_qca: add support for handling mgmt and MIB Ethernet packet

Florian Fainelli
2022-01-26T03:34:59Z

On 1/22/2022 5:33 PM, Ansuel Smith wrote:
> Add connect/disconnect helper to assign private struct to the dsa switch.
> Add support for Ethernet mgm and MIB if the dsa driver provide an handler
> to correctly parse and elaborate the data.

s/mgm/mgmt/
s/dsa/DSA/

>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
> include/linux/dsa/tag_qca.h | 7 +++++++
> net/dsa/tag_qca.c | 39 ++++++++++++++++++++++++++++++++++---
> 2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/dsa/tag_qca.h b/include/linux/dsa/tag_qca.h
> index 87dd84e31304..de5a45f5b398 100644
> --- a/include/linux/dsa/tag_qca.h
> +++ b/include/linux/dsa/tag_qca.h
> @@ -72,4 +72,11 @@ struct mib_ethhdr {
> __be16 hdr; /* qca hdr */
> } __packed;
>
> +struct qca_tagger_data {
> + void (*rw_reg_ack_handler)(struct dsa_switch *ds,
> + struct sk_buff *skb);
> + void (*mib_autocast_handler)(struct dsa_switch *ds,
> + struct sk_buff *skb);
> +};
> +
> #endif /* __TAG_QCA_H */
> diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
> index fdaa1b322d25..dc81c72133eb 100644
> --- a/net/dsa/tag_qca.c
> +++ b/net/dsa/tag_qca.c
> @@ -5,6 +5,7 @@
>
> #include <linux/etherdevice.h>
> #include <linux/bitfield.h>
> +#include <net/dsa.h>
> #include <linux/dsa/tag_qca.h>
>
> #include "dsa_priv.h"
> @@ -32,11 +33,16 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
>
> static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
> {
> + struct qca_tagger_data *tagger_data;
> + struct dsa_port *dp = dev->dsa_ptr;
> + struct dsa_switch *ds = dp->ds;
> u16 hdr, pk_type;
> __be16 *phdr;
> int port;
> u8 ver;
>
> + tagger_data = ds->tagger_data;
> +
> if (unlikely(!pskb_may_pull(skb, QCA_HDR_LEN)))
> return NULL;
>
> @@ -51,13 +57,19 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
> /* Get pk type */
> pk_type = FIELD_GET(QCA_HDR_RECV_TYPE, hdr);
>
> - /* Ethernet MDIO read/write packet */
> - if (pk_type == QCA_HDR_RECV_TYPE_RW_REG_ACK)
> + /* Ethernet mgmt read/write packet */
> + if (pk_type == QCA_HDR_RECV_TYPE_RW_REG_ACK) {
> + if (tagger_data->rw_reg_ack_handler)

if (likely()) in case that happens to make a difference?

> + tagger_data->rw_reg_ack_handler(ds, skb);
> return NULL;
> + }
>
> /* Ethernet MIB counter packet */
> - if (pk_type == QCA_HDR_RECV_TYPE_MIB)
> + if (pk_type == QCA_HDR_RECV_TYPE_MIB) {
> + if (tagger_data->mib_autocast_handler)

Likewise

In any case, this looks good to me:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian