Re: [PATCH 1/1] staging: wilc1000: Use common structs to parse ip packets

[Dan Carpenter]

On Tue, May 29, 2018 at 09:08:39PM +0200, Thibaut Robert wrote:
>  static inline void tcp_process(struct net_device *dev, struct txq_entry_t 
> *tqe)
>  {
> -     u8 *eth_hdr_ptr;
> +     const struct ethhdr *eth_hdr_ptr = (const struct ethhdr *)tqe->buffer;
> +
>       u8 *buffer = tqe->buffer;

No blank line, please.

> -     unsigned short h_proto;
>       int i;
>       unsigned long flags;
>       struct wilc_vif *vif;
> @@ -197,37 +199,25 @@ static inline void tcp_process(struct net_device *dev, 
> struct txq_entry_t *tqe)
>  
>       spin_lock_irqsave(&wilc->txq_spinlock, flags);
>  
> -     eth_hdr_ptr = &buffer[0];
> -     h_proto = ntohs(*((unsigned short *)&eth_hdr_ptr[12]));
> -     if (h_proto == ETH_P_IP) {
> -             u8 *ip_hdr_ptr;
> -             u8 protocol;
> -
> -             ip_hdr_ptr = &buffer[ETHERNET_HDR_LEN];
> -             protocol = ip_hdr_ptr[9];
> +     if (eth_hdr_ptr->h_proto == htons(ETH_P_IP)) {
> +             const struct iphdr *ip_hdr_ptr = (const struct iphdr *)
> +               (buffer + ETH_HLEN);

If you declared buffer as a void pointer you could remove this cast.

                const struct iphdr *ip_hdr_ptr = buf + ETH_HLEN;


>  
> -             if (protocol == 0x06) {
> -                     u8 *tcp_hdr_ptr;
> +             if (ip_hdr_ptr->protocol == IPPROTO_TCP) {
> +                     const struct tcphdr *tcp_hdr_ptr;
>                       u32 IHL, total_length, data_offset;
>  
> -                     tcp_hdr_ptr = &ip_hdr_ptr[IP_HDR_LEN];
> -                     IHL = (ip_hdr_ptr[0] & 0xf) << 2;
> -                     total_length = ((u32)ip_hdr_ptr[2] << 8) +
> -                                     (u32)ip_hdr_ptr[3];
> -                     data_offset = ((u32)tcp_hdr_ptr[12] & 0xf0) >> 2;
> +                     IHL = ip_hdr_ptr->ihl << 2;
> +                     tcp_hdr_ptr = (const struct tcphdr *)
> +                       ((u8 *)ip_hdr_ptr + IHL);

This alignment is a bit unfortunate...  Perhaps?

                        tcp_hdr_ptr = buf + ETH_HLEN + IHL;

regards,
dan carpenter

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel