M_PREPEND in vxlan encap on not strict-alignment archs

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

M_PREPEND in vxlan encap on not strict-alignment archs

David Gwynne-5
if the arch can cope with prepending on an unaligned address in
vxlan, then let it do it.

this means less work if we can get away with it.

ok?

Index: if_vxlan.c
===================================================================
RCS file: /cvs/src/sys/net/if_vxlan.c,v
retrieving revision 1.49
diff -u -p -r1.49 if_vxlan.c
--- if_vxlan.c 7 Oct 2016 06:16:03 -0000 1.49
+++ if_vxlan.c 10 Oct 2016 03:42:36 -0000
@@ -766,20 +765,29 @@ vxlan_output(struct ifnet *ifp, struct m
 #endif
  int error, af;
  uint32_t tag;
- struct mbuf *m0;
 
  /* VXLAN header */
- MGETHDR(m0, M_DONTWAIT, m->m_type);
- if (m0 == NULL) {
- ifp->if_oerrors++;
- return (ENOBUFS);
+ if (!ALIGNED_POINTER(mtod(m, caddr_t), uint32_t)) {
+ struct mbuf *m0;
+
+ MGETHDR(m0, M_DONTWAIT, m->m_type);
+ if (m0 == NULL) {
+ ifp->if_oerrors++;
+ return (ENOBUFS);
+ }
+ M_MOVE_PKTHDR(m0, m);
+ m0->m_next = m;
+ m = m0;
+ MH_ALIGN(m, sizeof(*vu));
+ m->m_len = sizeof(*vu);
+ m->m_pkthdr.len += sizeof(*vu);
+ } else {
+ M_PREPEND(m, sizeof(*vu), M_DONTWAIT);
+ if (m == NULL) {
+ ifp->if_oerrors++;
+ return (ENOBUFS);
+ }
  }
- M_MOVE_PKTHDR(m0, m);
- m0->m_next = m;
- m = m0;
- MH_ALIGN(m, sizeof(*vu));
- m->m_len = sizeof(*vu);
- m->m_pkthdr.len += sizeof(*vu);
 
  src = (struct sockaddr *)&sc->sc_src;
  dst = (struct sockaddr *)&sc->sc_dst;

Reply | Threaded
Open this post in threaded view
|

Re: M_PREPEND in vxlan encap on not strict-alignment archs

YASUOKA Masahiko-4
ok yasuoka

thanks,

On Mon, 10 Oct 2016 13:48:18 +1000
David Gwynne <[hidden email]> wrote:

> if the arch can cope with prepending on an unaligned address in
> vxlan, then let it do it.
>
> this means less work if we can get away with it.
>
> ok?
>
> Index: if_vxlan.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_vxlan.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 if_vxlan.c
> --- if_vxlan.c 7 Oct 2016 06:16:03 -0000 1.49
> +++ if_vxlan.c 10 Oct 2016 03:42:36 -0000
> @@ -766,20 +765,29 @@ vxlan_output(struct ifnet *ifp, struct m
>  #endif
>   int error, af;
>   uint32_t tag;
> - struct mbuf *m0;
>  
>   /* VXLAN header */
> - MGETHDR(m0, M_DONTWAIT, m->m_type);
> - if (m0 == NULL) {
> - ifp->if_oerrors++;
> - return (ENOBUFS);
> + if (!ALIGNED_POINTER(mtod(m, caddr_t), uint32_t)) {
> + struct mbuf *m0;
> +
> + MGETHDR(m0, M_DONTWAIT, m->m_type);
> + if (m0 == NULL) {
> + ifp->if_oerrors++;
> + return (ENOBUFS);
> + }
> + M_MOVE_PKTHDR(m0, m);
> + m0->m_next = m;
> + m = m0;
> + MH_ALIGN(m, sizeof(*vu));
> + m->m_len = sizeof(*vu);
> + m->m_pkthdr.len += sizeof(*vu);
> + } else {
> + M_PREPEND(m, sizeof(*vu), M_DONTWAIT);
> + if (m == NULL) {
> + ifp->if_oerrors++;
> + return (ENOBUFS);
> + }
>   }
> - M_MOVE_PKTHDR(m0, m);
> - m0->m_next = m;
> - m = m0;
> - MH_ALIGN(m, sizeof(*vu));
> - m->m_len = sizeof(*vu);
> - m->m_pkthdr.len += sizeof(*vu);
>  
>   src = (struct sockaddr *)&sc->sc_src;
>   dst = (struct sockaddr *)&sc->sc_dst;
>

Reply | Threaded
Open this post in threaded view
|

Re: M_PREPEND in vxlan encap on not strict-alignment archs

Mark Kettenis
In reply to this post by David Gwynne-5
> Date: Mon, 10 Oct 2016 13:48:18 +1000
> From: David Gwynne <[hidden email]>
>
> if the arch can cope with prepending on an unaligned address in
> vxlan, then let it do it.
>
> this means less work if we can get away with it.
>
> ok?

Let's face it.  The vxlan protocol is badly designed.  Should we
really create multiple code paths for strict-align and
non-strict-align architectures?  Is vxlan really used in
performance-critical setups?

> Index: if_vxlan.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_vxlan.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 if_vxlan.c
> --- if_vxlan.c 7 Oct 2016 06:16:03 -0000 1.49
> +++ if_vxlan.c 10 Oct 2016 03:42:36 -0000
> @@ -766,20 +765,29 @@ vxlan_output(struct ifnet *ifp, struct m
>  #endif
>   int error, af;
>   uint32_t tag;
> - struct mbuf *m0;
>  
>   /* VXLAN header */
> - MGETHDR(m0, M_DONTWAIT, m->m_type);
> - if (m0 == NULL) {
> - ifp->if_oerrors++;
> - return (ENOBUFS);
> + if (!ALIGNED_POINTER(mtod(m, caddr_t), uint32_t)) {
> + struct mbuf *m0;
> +
> + MGETHDR(m0, M_DONTWAIT, m->m_type);
> + if (m0 == NULL) {
> + ifp->if_oerrors++;
> + return (ENOBUFS);
> + }
> + M_MOVE_PKTHDR(m0, m);
> + m0->m_next = m;
> + m = m0;
> + MH_ALIGN(m, sizeof(*vu));
> + m->m_len = sizeof(*vu);
> + m->m_pkthdr.len += sizeof(*vu);
> + } else {
> + M_PREPEND(m, sizeof(*vu), M_DONTWAIT);
> + if (m == NULL) {
> + ifp->if_oerrors++;
> + return (ENOBUFS);
> + }
>   }
> - M_MOVE_PKTHDR(m0, m);
> - m0->m_next = m;
> - m = m0;
> - MH_ALIGN(m, sizeof(*vu));
> - m->m_len = sizeof(*vu);
> - m->m_pkthdr.len += sizeof(*vu);
>  
>   src = (struct sockaddr *)&sc->sc_src;
>   dst = (struct sockaddr *)&sc->sc_dst;
>
>

Reply | Threaded
Open this post in threaded view
|

Re: M_PREPEND in vxlan encap on not strict-alignment archs

Mike Belopuhov-5
On 10 October 2016 at 10:56, Mark Kettenis <[hidden email]> wrote:

>> Date: Mon, 10 Oct 2016 13:48:18 +1000
>> From: David Gwynne <[hidden email]>
>>
>> if the arch can cope with prepending on an unaligned address in
>> vxlan, then let it do it.
>>
>> this means less work if we can get away with it.
>>
>> ok?
>
> Let's face it.  The vxlan protocol is badly designed.  Should we
> really create multiple code paths for strict-align and
> non-strict-align architectures?  Is vxlan really used in
> performance-critical setups?
>

Definitely.
<lighthearted-sarcasm>Unlike strict alignment archs (-;</lighthearted-sarcasm>

Reply | Threaded
Open this post in threaded view
|

Re: M_PREPEND in vxlan encap on not strict-alignment archs

Mike Belopuhov-5
In reply to this post by David Gwynne-5
On 10 October 2016 at 05:48, David Gwynne <[hidden email]> wrote:
> if the arch can cope with prepending on an unaligned address in
> vxlan, then let it do it.
>
> this means less work if we can get away with it.
>
> ok?
>

I think it's safe to use mtod here.  Getting an IP packet with an empty mbuf
shouldn't be possible in this codepath.  OK mikeb