On Wed, 5 Oct 2016 14:30:27 +0200
Mike Belopuhov <[email protected]> wrote:
> On Wed, Oct 05, 2016 at 10:58 +0900, YASUOKA Masahiko wrote:
>> On Tue, 4 Oct 2016 17:27:12 +0200
>> Mike Belopuhov <[email protected]> wrote:
>> > On Tue, Oct 04, 2016 at 01:07 +0200, Vincent Gross wrote:
>> >> On Sat, 24 Sep 2016 10:58:10 +0200
>> >> Vincent Gross <[email protected]> wrote:
>> >> 
>> >> > Hi,
>> >> > 
>> >> [snip]
>> >> > 
>> >> > Aside from the mbuf issue, is this Ok ?
>> >> 
>> >> I will go back on the mbuff stuff later.
>> >> 
>> >> Diff rebased, ok anyone ?
>> >> 
>> >> Index: net/if_vxlan.c
>> >> ===================================================================
>> >> RCS file: /cvs/src/sys/net/if_vxlan.c,v
>> >> retrieving revision 1.48
>> >> diff -u -p -r1.48 if_vxlan.c
>> >> --- net/if_vxlan.c        30 Sep 2016 10:22:05 -0000      1.48
>> >> +++ net/if_vxlan.c        3 Oct 2016 23:12:42 -0000
>> >> @@ -638,7 +749,9 @@ vxlan_lookup(struct mbuf *m, struct udph
>> >>   if (m->m_pkthdr.len < skip + sizeof(struct ether_header))
>> >>           return (EINVAL);
>> >>  
>> >> - m_adj(m, skip);
>> >> + m_adj(m, skip - ETHER_ALIGN);
>> >> + m = m_pullup(m, ETHER_HDR_LEN + ETHER_ALIGN);
>> >> + m_adj(m, ETHER_ALIGN);
>> >>   ifp = &sc->sc_ac.ac_if;
>> >>  
>> >>  #if NBRIDGE > 0
>> > 
>> > I think this chunk is correct.  First you ensure that m->m_data
>> > points to a contiguous and well-aligned ethernet header and then
>> > you trim the alignment so that mtod() points directly at it.
>> 
>> Isn't it possible that m_pullup() may return non aligned pointer?
>> 
> 
> Do you mean if m->m_data pointer can be not aligned properly after
> an m_pullup?

Sorry.  Yes, I meant that.

> Of course, if the header layout is such that m_pullup doesn't need
> to move data around and at the same time the header that we're going
> to m_adj[ust] to is not aligned properly, this is going to be a
> problem.

The diff I sent to hackers@ is including my lastest proposal

@@ -657,6 +658,16 @@ vxlan_lookup(struct mbuf *m, struct udph
 #if NPF > 0
        pf_pkt_addr_changed(m);
 #endif
+       if ((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)
+                       return (ENOBUFS);
+       }
 
        ml_enqueue(&ml, m);
        if_input(ifp, &ml);


We might have a better way, but I think above diff does what we need
to do here simply.

  1. ensure entire ethernet header is on first mbuf for ether_input()
  2. align 32bit at the ethernet payload (eg. IP)

--yasuoka

Reply via email to