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

[Florian Fainelli]

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@xxxxxxxxx>
> ---
>   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@xxxxxxxxx>
--
Florian