[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20211215095448.r362nnr25yq22ixm@skbuf>
Date: Wed, 15 Dec 2021 11:54:48 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Ansuel Smith <ansuelsmth@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [net-next PATCH RFC v6 10/16] net: dsa: tag_qca: add support for
handling mdio Ethernet and MIB packet
On Tue, Dec 14, 2021 at 11:44:03PM +0100, Ansuel Smith wrote:
> Add connect/disconnect helper to assign private struct to the cpu port
> dsa priv.
> Add support for Ethernet mdio packet and MIB packet if the dsa driver
> provide an handler to correctly parse and elaborate the data.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@...il.com>
> ---
> include/linux/dsa/tag_qca.h | 7 +++++++
> net/dsa/tag_qca.c | 35 +++++++++++++++++++++++++++++++++--
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/dsa/tag_qca.h b/include/linux/dsa/tag_qca.h
> index cd6275bac103..203e72dad9bb 100644
> --- a/include/linux/dsa/tag_qca.h
> +++ b/include/linux/dsa/tag_qca.h
> @@ -69,4 +69,11 @@ struct mib_ethhdr {
> __be16 hdr; /* qca hdr */
> } __packed;
>
> +struct tag_qca_priv {
Could you keep the "priv" name for later, in case you need to hide some
tagger storage from the switch driver? This could be renamed to
"qca_tagger_data".
> + void (*rw_reg_ack_handler)(struct dsa_port *dp,
> + struct sk_buff *skb);
> + void (*mib_autocast_handler)(struct dsa_port *dp,
> + struct sk_buff *skb);
> +};
> +
> #endif /* __TAG_QCA_H */
> diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
> index f5547d357647..59e04157f53b 100644
> --- a/net/dsa/tag_qca.c
> +++ b/net/dsa/tag_qca.c
> @@ -32,11 +32,15 @@ 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 dsa_port *dp = dev->dsa_ptr;
cpu_dp, for clarity
> + struct tag_qca_priv *priv;
> u16 hdr, pk_type;
> __be16 *phdr;
> int port;
> u8 ver;
>
> + priv = dp->ds->tagger_data;
> +
> if (unlikely(!pskb_may_pull(skb, QCA_HDR_LEN)))
> return NULL;
>
> @@ -52,12 +56,18 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
> pk_type = FIELD_GET(QCA_HDR_RECV_TYPE, hdr);
>
> /* Ethernet MDIO read/write packet */
> - if (pk_type == QCA_HDR_RECV_TYPE_RW_REG_ACK)
> + if (pk_type == QCA_HDR_RECV_TYPE_RW_REG_ACK) {
> + if (priv->rw_reg_ack_handler)
> + priv->rw_reg_ack_handler(dp, skb);
Minor nitpick, but why not pass the "ds" as argument?
> return NULL;
> + }
>
> /* Ethernet MIB counter packet */
> - if (pk_type == QCA_HDR_RECV_TYPE_MIB)
> + if (pk_type == QCA_HDR_RECV_TYPE_MIB) {
> + if (priv->mib_autocast_handler)
> + priv->mib_autocast_handler(dp, skb);
> return NULL;
> + }
>
> /* Remove QCA tag and recalculate checksum */
> skb_pull_rcsum(skb, QCA_HDR_LEN);
> @@ -73,9 +83,30 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev)
> return skb;
> }
>
> +static int qca_tag_connect(struct dsa_switch *ds)
> +{
> + struct tag_qca_priv *priv;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ds->tagger_data = priv;
> +
> + return 0;
> +}
> +
> +static void qca_tag_disconnect(struct dsa_switch *ds)
> +{
> + kfree(ds->tagger_data);
> + ds->tagger_data = NULL;
> +}
> +
> static const struct dsa_device_ops qca_netdev_ops = {
> .name = "qca",
> .proto = DSA_TAG_PROTO_QCA,
> + .connect = qca_tag_connect,
> + .disconnect = qca_tag_disconnect,
> .xmit = qca_tag_xmit,
> .rcv = qca_tag_rcv,
> .needed_headroom = QCA_HDR_LEN,
> --
> 2.33.1
>
Powered by blists - more mailing lists