Messages in this thread | | | Subject | Re: [PATCH net-next 2/2] net: dsa: lan9303: Learn addresses on CPU port when bridged | From | Egil Hjelmeland <> | Date | Wed, 25 Oct 2017 12:54:18 +0200 |
| |
Hi Woojung!
On 24. okt. 2017 19:18, Woojung.Huh@microchip.com wrote: > Hi Egil, > >> +static inline int lan9303_tx_use_arl(struct dsa_port *dp, u8 *dest_addr) >> +{ >> + struct lan9303 *chip = dp->ds->priv; >> + >> + return chip->is_bridged && !ether_addr_equal(dest_addr, >> eth_stp_addr); >> +} >> >> static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device >> *dev) >> { >> @@ -62,7 +80,10 @@ static struct sk_buff *lan9303_xmit(struct sk_buff *skb, >> struct net_device *dev) >> >> lan9303_tag = (u16 *)(skb->data + 2 * ETH_ALEN); >> lan9303_tag[0] = htons(ETH_P_8021Q); >> - lan9303_tag[1] = htons(dp->index | BIT(4)); >> + lan9303_tag[1] = lan9303_tx_use_arl(dp, skb->data) ? > > How about using skb_mac_header(skb) than skb->data? > >> + LAN9303_TAG_TX_USE_ALR : >> + dp->index | >
I am not the expert here.
I see that skb_mac_header() is (skb->head + skb->mac_header). So it will cost a few nano seconds per packet. Not the end of the world though. But I see that other net/dsa/tag_*.c use skb->data, assuming that skb->data point to mac header.
Anyway, it may be an idea to decrement skb->mac_header, in case the master interface driver uses it? What about skb->mac_len?
If to use skb_mac_header() at all, I would replace all use of skb->data, like this:
--- /* provide 'LAN9303_TAG_LEN' bytes additional space */ skb_push(skb, LAN9303_TAG_LEN);
/* make room between MACs and Ether-Type */ memmove(skb_mac_header(skb) - LAN9303_TAG_LEN, skb_mac_header(skb), 2 * ETH_ALEN); skb->mac_header -= LAN9303_TAG_LEN;
lan9303_tag = (u16 *)(skb_mac_header(skb) + 2 * ETH_ALEN); lan9303_tag[0] = htons(ETH_P_8021Q); lan9303_tag[1] = lan9303_tx_use_arl(dp, skb_mac_header(skb)) ? LAN9303_TAG_TX_USE_ALR : dp->index | LAN9303_TAG_TX_STP_OVERRIDE; lan9303_tag[1] = htons(lan9303_tag[1]); ---
But I will really like to hear the opinion from more people on this before going down that road. Anyway, I think it would belong to a separate patch.
> Thanks. > Woojung > >
Regards Egil
| |