Re: [PATCH 08/40] igb: Always copy ethernet header

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2023/04/14 23:46, Philippe Mathieu-Daudé wrote:
> On 14/4/23 13:37, Akihiko Odaki wrote:
>> igb_receive_internal() used to check the iov length to determine
>> copy the iovs to a contiguous buffer, but the check is flawed in two
>> ways:
>> - It does not ensure that iovcnt > 0.
>> - It does not take virtio-net header into consideration.
>>
>> The size of this copy is just 22 octets, which can be even less than
>> the code size required for checks. This (wrong) optimization is probably
>> not worth so just remove it. Removing this also allows igb to assume
>> aligned accesses for the ethernet header.
>>
>> Fixes: 3a977deebe ("Intrdocue igb device emulation")
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   hw/net/igb_core.c | 39 +++++++++++++++++++++------------------
>>   1 file changed, 21 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
>> index 53f60fc3d3..1d188b526c 100644
>> --- a/hw/net/igb_core.c
>> +++ b/hw/net/igb_core.c
> 
> 
>> -static uint16_t igb_receive_assign(IGBCore *core, const struct 
>> eth_header *ehdr,
>> +static uint16_t igb_receive_assign(IGBCore *core, const L2Header 
>> *l2_header,
>>                                      size_t size, E1000E_RSSInfo 
>> *rss_info,
>>                                      bool *external_tx)
>>   {
>>       static const int ta_shift[] = { 4, 3, 2, 0 };
>> +    const struct eth_header *ehdr = &l2_header->eth;
>>       uint32_t f, ra[2], *macp, rctl = core->mac[RCTL];
>>       uint16_t queues = 0;
>>       uint16_t oversized = 0;
>> -    uint16_t vid = lduw_be_p(&PKT_GET_VLAN_HDR(ehdr)->h_tci) & 
>> VLAN_VID_MASK;
>> +    uint16_t vid = be16_to_cpu(l2_header->vlan[0].h_tci) & 
>> VLAN_VID_MASK;
> 
> Why this API change? Are we certain tci is aligned in host memory?

This change makes the VLAN tag always copied to the host memory, which 
ensures that tci is aligned.