On 12/5/18 12:46 PM, Pavel Machek wrote: > Hi! > >>>>> But I'm running into problems with tagging code, and I guess I'd like >>>>> some help understanding. >>>>> >>>>> tag_trailer: allocates new skb, then copies data around. >>>>> >>>>> tag_qca: does dev->stats.tx_packets++, and reuses existing skb. >>>>> >>>>> tag_brcm: reuses existing skb. >>> >>> Any idea why tag trailer allocates new skb, >> >> I wrote this code over 10 years ago, so I don't remember all that >> well, but I think that it is because you have to do manual checksumming >> of the packet, as there's no way to pass down the stack that you don't >> want to checksum all the way down to the end of the data area (and you >> don't want the tag to be included in the checksum), and so you want to >> do that before you add the trailer tag, and you'll probably have to >> reallocate the data area to be able to add the tag, and you probably >> won't get an exclusive skb here anyway, so you might as well allocate >> a new one. > > Ok, thanks. Whether tag is checksummed or not is indeed an interesting > question. > >>> and what is going on with dev->stats.tx_packets++? >> >> trailer_xmit would be the hard_start_xmit function for the virtual >> (slave) network interface, so this would be the right thing to do? > > Some tag_*.c do this and some do not, so I'm still confused.
This is likely just historical, or we don't have an easy way to determine whether the SKB transmitted succeeded or not because it was reallocated, and therefore the tagging code must make sure statistics are maintained. I suspect it is just historical and that tag module just has not been updated to leverage the statistics accounting done a layer above it within DSA. > >>> 6065 indeed has some kind of "egress tagging mode" (with four >>> options), but I have trouble understanding what it really does. >> >> What are the options? > > "transmit unmodified", "transmit untagged", "transmit tagged" and > "transmit all frames double tagged". It seems to me that this applies to VLAN headers, not DSA/trailer headers though not having the data sheet I should be flat out wrong. -- Florian
