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

[Jason Wang <jasowang@redhat.com>]

On 2016年05月31日 15:20, Dmitry Fleytman wrote:
> From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
>
> 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 <dmitry.fleytman@ravellosystems.com>
> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
> ---
>   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().

> -    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)
>       }
>   }
>   

[...]