Re: [Qemu-devel] [PATCH v7 12/17] net_pkt: Extend packet abstraction as required by e1000e functionality

[Dmitry Fleytman]

> On 1 Jun 2016, at 07:25 AM, Jason Wang <address@hidden> wrote:
> 
> 
> 
> On 2016年05月31日 15:20, Dmitry Fleytman wrote:
>> From: Dmitry Fleytman <address@hidden>
>> 
>> This patch extends the TX/RX packet abstractions with features that will
>> be used by the e1000e device implementation.
>> 
>> Changes are:
>> 
>>   1. Support iovec lists for RX buffers
>>   2. Deeper RX packets parsing
>>   3. Loopback option for TX packets
>>   4. Extended VLAN headers handling
>>   5. RSS processing for RX packets
>> 
>> Signed-off-by: Dmitry Fleytman <address@hidden>
>> Signed-off-by: Leonid Bloch <address@hidden>
>> ---
>>  hw/net/net_rx_pkt.c    | 473 
>> +++++++++++++++++++++++++++++++++++++++++++++----
>>  hw/net/net_rx_pkt.h    | 193 +++++++++++++++++++-
>>  hw/net/net_tx_pkt.c    | 204 +++++++++++++--------
>>  hw/net/net_tx_pkt.h    |  60 ++++++-
>>  include/net/checksum.h |   4 +-
>>  include/net/eth.h      | 153 +++++++++++-----
>>  net/checksum.c         |   7 +-
>>  net/eth.c              | 410 +++++++++++++++++++++++++++++++++++++-----
>>  trace-events           |  40 +++++
>>  9 files changed, 1336 insertions(+), 208 deletions(-)
> 
> [...]
> 
>>  struct udp_hdr {
>>    uint16_t uh_sport;           /* source port */
>>    uint16_t uh_dport;           /* destination port */
>> @@ -169,19 +194,22 @@ struct tcp_hdr {
>>  #define PKT_GET_IP_HDR(p)         \
>>      ((struct ip_header *)(((uint8_t *)(p)) + eth_get_l2_hdr_length(p)))
>>  #define IP_HDR_GET_LEN(p)         \
>> -    ((((struct ip_header *)p)->ip_ver_len & 0x0F) << 2)
>> +    ((((struct ip_header *)(p))->ip_ver_len & 0x0F) << 2)
>>  #define PKT_GET_IP_HDR_LEN(p)     \
>>      (IP_HDR_GET_LEN(PKT_GET_IP_HDR(p)))
>>  #define PKT_GET_IP6_HDR(p)        \
>>      ((struct ip6_header *) (((uint8_t *)(p)) + eth_get_l2_hdr_length(p)))
>>  #define IP_HEADER_VERSION(ip)     \
>> -    ((ip->ip_ver_len >> 4)&0xf)
>> +    (((ip)->ip_ver_len >> 4) & 0xf)
>> +#define IP4_IS_FRAGMENT(ip) \
>> +    ((be16_to_cpu((ip)->ip_off) & (IP_OFFMASK | IP_MF)) != 0)
>>    #define ETH_P_IP                  (0x0800)      /* Internet Protocol 
>> packet  */
>>  #define ETH_P_ARP                 (0x0806)      /* Address Resolution 
>> packet */
>>  #define ETH_P_IPV6                (0x86dd)
>>  #define ETH_P_VLAN                (0x8100)
>>  #define ETH_P_DVLAN               (0x88a8)
>> +#define ETH_P_UNKNOWN             (0xffff)
>>  #define VLAN_VID_MASK             0x0fff
>>  #define IP_HEADER_VERSION_4       (4)
>>  #define IP_HEADER_VERSION_6       (6)
>> @@ -258,15 +286,25 @@ get_eth_packet_type(const struct eth_header *ehdr)
>>  }
>>    static inline uint32_t
>> -eth_get_l2_hdr_length(const void *p)
>> +eth_get_l2_hdr_length(const struct iovec *iov, int iovcnt)
>>  {
> 
> Looks like this changes breaks the above PKT_GET_IP_HDR and PKT_GET_IP6_HDR. 
> This will be a problem e.g ENET series depends on this.
> 
> A solution is keeping current eth_get_l2_hdr_length() and call it in a new 
> helper e.g eth_get_l2_hdr_length_iov().

Right, sending fixed series.
Thanks!

> 
>> -    uint16_t proto = be16_to_cpu(PKT_GET_ETH_HDR(p)->h_proto);
>> -    struct vlan_header *hvlan = PKT_GET_VLAN_HDR(p);
>> +    uint8_t p[sizeof(struct eth_header) + sizeof(struct vlan_header)];
>> +    size_t copied = iov_to_buf(iov, iovcnt, 0, p, ARRAY_SIZE(p));
>> +    uint16_t proto;
>> +    struct vlan_header *hvlan;
>> +
>> +    if (copied < ARRAY_SIZE(p)) {
>> +        return copied;
>> +    }
>> +
>> +    proto = be16_to_cpu(PKT_GET_ETH_HDR(p)->h_proto);
>> +    hvlan = PKT_GET_VLAN_HDR(p);
>> +
>>      switch (proto) {
>>      case ETH_P_VLAN:
>>          return sizeof(struct eth_header) + sizeof(struct vlan_header);
>>      case ETH_P_DVLAN:
>> -        if (hvlan->h_proto == ETH_P_VLAN) {
>> +        if (be16_to_cpu(hvlan->h_proto) == ETH_P_VLAN) {
>>              return sizeof(struct eth_header) + 2 * sizeof(struct 
>> vlan_header);
>>          } else {
>>              return sizeof(struct eth_header) + sizeof(struct vlan_header);
>> @@ -290,51 +328,67 @@ eth_get_pkt_tci(const void *p)
>>      }
>>  }
>>  
> 
> [...]