Re: [Qemu-devel] [PATCH v2 2/4] ftgmac100: add IEEE 802.1Q VLAN support

[Cédric Le Goater]

On 06/07/2018 02:41 PM, Peter Maydell wrote:
> On 30 May 2018 at 07:17, Cédric Le Goater <address@hidden> wrote:
>> The ftgmac100 NIC supports VLAN tag insertion and the MAC engine also
>> has a control to remove VLAN tags from received packets.
>>
>> The VLAN control bits and VLAN tag information are contained in the
>> second word of the transmit and receive descriptors. The Insert VLAN
>> bit and the VLAN Tag available bit are only valid in the first segment
>> of the packet.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> 
>> +        /* Check for VLAN */
>> +        if (bd.des0 & FTGMAC100_TXDES0_FTS &&
>> +            bd.des1 & FTGMAC100_TXDES1_INS_VLANTAG &&
>> +            be16_to_cpu(PKT_GET_ETH_HDR(ptr)->h_proto) != ETH_P_VLAN) {
> 
> This is kind of bogus because there's no guarantee that h_proto here
> is sufficiently aligned to do a halfword load. However this is
> a problem common to all the PKT_GET_* macros so we should probably
> attack it separately to this series.
> 
> Getting the ethernet h_proto field seems common enough that
> something like
> 
> static inline uint16_t eth_get_proto(uint8_t *pkt)
> {
>     return lduw_be_p(&PKT_GET_ETH_HDR(pkt)->h_proto);
> }
> 
> would be handy in eth.h.

yes. There are a few candidates :

hw/net/virtio-net.c:        int vid = lduw_be_p(ptr + 14) & 0xfff;
hw/net/e1000e_core.c:        uint16_t vid = lduw_be_p(buf + 14);
hw/net/rtl8139.c:            lduw_be_p(&buf[ETH_ALEN * 2]) == ETH_P_VLAN) {
hw/net/e1000x_common.c:    uint16_t eth_proto = lduw_be_p(buf + 12);
hw/net/e1000.c:        uint16_t vid = lduw_be_p(buf + 14);
hw/net/e1000.c:        vlan_special = cpu_to_le16(lduw_be_p(filter_buf + 14));

Thanks,

C. 

> Anyway, I've applied this series to target-arm.next.
> 
> thanks
> -- PMM
>