Hi,

> Also, I believe I was wrong insisting on requiring explicit
> ETHER_ALIGN alignment.  It's not really necessary if we're going to
> pull-up anyways.

Without +-ETHER_ALIGN here is easier to understand for me.

> And finally I'm using "sizeof(struct ether_header)" instead
> of the define for consistency with the rest of the code.
> 
> OK?

ok yasuoka

Thanks,

On Wed, 12 Oct 2016 12:59:06 +0200
Mike Belopuhov <m...@belopuhov.com> wrote:
> After some additional consideration I think I can improve
> upon my previous diff.  Pull-up of the Ethernet header can
> and should be made conditional: since an Ethernet header
> requires 2 byte alignment, any position within a real life
> mbuf is a valid one so unless there's not enough data in
> the first mbuf, we shouldn't do anything.  This also puts
> this m_pullup in line with the other ones in the tree in
> terms of the preceeding length check.
> 
> Also, I believe I was wrong insisting on requiring explicit
> ETHER_ALIGN alignment.  It's not really necessary if we're
> going to pull-up anyways.
> 
> And finally I'm using "sizeof(struct ether_header)" instead
> of the define for consistency with the rest of the code.
> 
> OK?
> 
> 
> diff --git sys/net/if_vxlan.c sys/net/if_vxlan.c
> index 034e651..f010e3e 100644
> --- sys/net/if_vxlan.c
> +++ sys/net/if_vxlan.c
> @@ -577,11 +577,12 @@ vxlan_lookup(struct mbuf *m, struct udphdr *uh, int 
> iphlen,
>       struct ifnet            *ifp;
>       int                      skip;
>  #if NBRIDGE > 0
>       struct bridge_tunneltag *brtag;
>  #endif
> -     struct mbuf             *m0;
> +     struct mbuf             *n;
> +     int                      off;
>  
>       /* XXX Should verify the UDP port first before copying the packet */
>       skip = iphlen + sizeof(*uh);
>       if (m->m_pkthdr.len - skip < sizeof(v))
>               return (0);
> @@ -634,12 +635,14 @@ vxlan_lookup(struct mbuf *m, struct udphdr *uh, int 
> iphlen,
>  
>       /* not found */
>       return (0);
>  
>   found:
> -     if (m->m_pkthdr.len < skip + sizeof(struct ether_header))
> +     if (m->m_pkthdr.len < skip + sizeof(struct ether_header)) {
> +             m_freem(m);
>               return (EINVAL);
> +     }
>  
>       m_adj(m, skip);
>       ifp = &sc->sc_ac.ac_if;
>  
>  #if NBRIDGE > 0
> @@ -656,19 +659,26 @@ vxlan_lookup(struct mbuf *m, struct udphdr *uh, int 
> iphlen,
>       m->m_flags &= ~(M_MCAST|M_BCAST);
>  
>  #if NPF > 0
>       pf_pkt_addr_changed(m);
>  #endif
> -     if ((m = m_pullup(m, sizeof(struct ether_header))) == NULL)
> +     if ((m->m_len < sizeof(struct ether_header)) &&
> +         (m = m_pullup(m, sizeof(struct ether_header))) == NULL)
>               return (ENOBUFS);
>  
> -     if (!ALIGNED_POINTER(mtod(m, caddr_t) + ETHER_HDR_LEN, uint32_t)) {
> -             m0 = m;
> -             m = m_dup_pkt(m0, ETHER_ALIGN, M_NOWAIT);
> -             m_freem(m0);
> -             if (m == NULL)
> +     n = m_getptr(m, sizeof(struct ether_header), &off);
> +     if (n == NULL) {
> +             m_freem(m);
> +             return (EINVAL);
> +     }
> +     if (!ALIGNED_POINTER(mtod(n, caddr_t) + off, uint32_t)) {
> +             n = m_dup_pkt(m, ETHER_ALIGN, M_NOWAIT);
> +             /* Dispose of the original mbuf chain */
> +             m_freem(m);
> +             if (n == NULL)
>                       return (ENOBUFS);
> +             m = n;
>       }
>  
>       ml_enqueue(&ml, m);
>       if_input(ifp, &ml);
>  

Reply via email to