Re: [PATCH net-next 1/2] net: dsa: tag_rtl8_4: add rtl8_4t tailing variant

[Vladimir Oltean <olteanv@gmail.com>]

Re: title. Tail or trailing?

On Wed, Feb 09, 2022 at 06:13:11PM -0300, Luiz Angelo Daros de Luca wrote:
> +static inline void rtl8_4_write_tag(struct sk_buff *skb, struct net_device *dev,
> +				    char *tag)
>  {
>  	struct dsa_port *dp = dsa_slave_to_port(dev);
> -	__be16 *tag;
> -
> -	skb_push(skb, RTL8_4_TAG_LEN);
> -
> -	dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN);
> -	tag = dsa_etype_header_pos_tx(skb);
> +	__be16 *tag16 = (__be16 *)tag;

Can the tail tag be aligned to an odd offset? In that case, should you
access byte by byte, maybe? I'm not sure how arches handle this.

>  
>  	/* Set Realtek EtherType */
> -	tag[0] = htons(ETH_P_REALTEK);
> +	tag16[0] = htons(ETH_P_REALTEK);
>  
>  	/* Set Protocol; zero REASON */
> -	tag[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB));
> +	tag16[1] = htons(FIELD_PREP(RTL8_4_PROTOCOL, RTL8_4_PROTOCOL_RTL8365MB));
>  
>  	/* Zero FID_EN, FID, PRI_EN, PRI, KEEP; set LEARN_DIS */
> -	tag[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1));
> +	tag16[2] = htons(FIELD_PREP(RTL8_4_LEARN_DIS, 1));
>  
>  	/* Zero ALLOW; set RX (CPU->switch) forwarding port mask */
> -	tag[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index)));
> +	tag16[3] = htons(FIELD_PREP(RTL8_4_RX, BIT(dp->index)));
> +}
> +
> +static struct sk_buff *rtl8_4_tag_xmit(struct sk_buff *skb,
> +				       struct net_device *dev)
> +{
> +	skb_push(skb, RTL8_4_TAG_LEN);
> +
> +	dsa_alloc_etype_header(skb, RTL8_4_TAG_LEN);
> +
> +	rtl8_4_write_tag(skb, dev, dsa_etype_header_pos_tx(skb));
>  
>  	return skb;
>  }
>  
> -static struct sk_buff *rtl8_4_tag_rcv(struct sk_buff *skb,
> -				      struct net_device *dev)
> +static struct sk_buff *rtl8_4t_tag_xmit(struct sk_buff *skb,
> +					struct net_device *dev)
> +{

Why don't you want to add:

	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
		return NULL;

and then you'll make this tagging protocol useful in production too.

> +	rtl8_4_write_tag(skb, dev, skb_put(skb, RTL8_4_TAG_LEN));
> +
> +	return skb;
> +}