From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: [PATCH 3/5] net: dsa: ksz: Factor out common tag code Date: Sat, 8 Dec 2018 12:34:48 +0100 Message-ID: <20181208113448.GD16502@lunn.ch> References: <20181207181845.21702-1-marex@denx.de> <20181207181845.21702-3-marex@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, f.fainelli@gmail.com, vivien.didelot@savoirfairelinux.com, Woojung.Huh@microchip.com, UNGLinuxDriver@microchip.com, Tristram Ha , "David S . Miller" To: Marek Vasut Return-path: Received: from vps0.lunn.ch ([185.16.172.187]:38880 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726111AbeLHLev (ORCPT ); Sat, 8 Dec 2018 06:34:51 -0500 Content-Disposition: inline In-Reply-To: <20181207181845.21702-3-marex@denx.de> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, Dec 07, 2018 at 07:18:43PM +0100, Marek Vasut wrote: > From: Tristram Ha > > Factor out common code from the tag_ksz , so that the code can be used > with other KSZ family switches which use differenly sized tags. I prefer this implementation over what Tristram recently submitted. It is also what we suggested a while back. However, since then we have had Spectra/meltdown, and we now know a function call through a pointer is expensive. This is the hot path, every frame comes through here, so it is worth taking the time to optimize this. Could you try to remove the ksz_tag_ops structure. xmit looks simple, since it is a tail call, so you can do that in ksz9477_xmit. Receive looks a bit more complex. I also think for the moment we need it ignore PTP until we have the big picture sorted out. If need be, the code can be refactored yet again. But i don't want PTP holding up getting more switches supported. > Signed-off-by: Tristram Ha > Signed-off-by: Marek Vasut > Cc: Vivien Didelot > Cc: Woojung Huh > Cc: David S. Miller > --- > net/dsa/tag_ksz.c | 125 +++++++++++++++++++++++++++++++++------------- > 1 file changed, 90 insertions(+), 35 deletions(-) > > diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c > index 036bc62198f28..d94bad1ab7e53 100644 > --- a/net/dsa/tag_ksz.c > +++ b/net/dsa/tag_ksz.c > @@ -14,34 +14,30 @@ > #include > #include "dsa_priv.h" > > -/* For Ingress (Host -> KSZ), 2 bytes are added before FCS. > - * --------------------------------------------------------------------------- > - * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes) > - * --------------------------------------------------------------------------- > - * tag0 : Prioritization (not used now) > - * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5) > - * > - * For Egress (KSZ -> Host), 1 byte is added before FCS. > - * --------------------------------------------------------------------------- > - * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes) > - * --------------------------------------------------------------------------- > - * tag0 : zero-based value represents port > - * (eg, 0x00=port1, 0x02=port3, 0x06=port7) > - */ > +struct ksz_tag_ops { > + void (*get_tag)(u8 *tag, unsigned int *port, unsigned int *len); > + void (*set_tag)(struct sk_buff *skb, struct net_device *dev); > +}; > > -#define KSZ_INGRESS_TAG_LEN 2 > -#define KSZ_EGRESS_TAG_LEN 1 > +/* Typically only one byte is used for tail tag. */ > +#define KSZ_EGRESS_TAG_LEN 1 > > -static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev) > +/* Frames with following addresse may need to be sent even when the port is > + * closed. > + */ > +static const u8 special_mult_addr[] = { > + 0x01, 0x80, 0xC2, 0x00, 0x00, 0x00 > +}; > + This is new functionality, not part of the refactoring the code. Please add that as a new patch. Also, you can probably make use of is_link_local_ether_addr(). Andrew