Messages in this thread | | | Date | Wed, 15 Dec 2021 11:54:48 +0200 | From | Vladimir Oltean <> | 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@gmail.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 >
| |