move gif_encap to simplify gif_start

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

move gif_encap to simplify gif_start

David Gwynne-5
this avoids having to parse the gif packet before sending it to
bpf. instead, we stash the address family in the mbuf and add it
after bpf.

however, a gif_encap error will not be propagated back to the sender.
i dont think this is a huge loss as the ip packet is usually
encapsulated itself after ip_send/ip_output (eg, by ethernet), which
could fail without propagating an error back either.

ok?

Index: if_gif.c
===================================================================
RCS file: /cvs/src/sys/net/if_gif.c,v
retrieving revision 1.107
diff -u -p -r1.107 if_gif.c
--- if_gif.c 9 Jan 2018 15:24:24 -0000 1.107
+++ if_gif.c 11 Jan 2018 03:22:09 -0000
@@ -175,55 +175,16 @@ gif_start(struct ifnet *ifp)
 
 #if NBPFILTER > 0
  if (ifp->if_bpf) {
- int offset;
- sa_family_t family;
- u_int8_t proto;
-
- /* must decapsulate outer header for bpf */
- switch (sc->gif_psrc->sa_family) {
- case AF_INET:
- offset = sizeof(struct ip);
- proto = mtod(m, struct ip *)->ip_p;
- break;
-#ifdef INET6
- case AF_INET6:
- offset = sizeof(struct ip6_hdr);
- proto = mtod(m, struct ip6_hdr *)->ip6_nxt;
- break;
-#endif
- default:
- proto = 0;
- break;
- }
- switch (proto) {
- case IPPROTO_IPV4:
- family = AF_INET;
- break;
- case IPPROTO_IPV6:
- family = AF_INET6;
- break;
- case IPPROTO_ETHERIP:
- family = AF_LINK;
- offset += sizeof(struct etherip_header);
- break;
- case IPPROTO_MPLS:
- family = AF_MPLS;
- break;
- default:
- offset = 0;
- family = sc->gif_psrc->sa_family;
- break;
- }
- m->m_data += offset;
- m->m_len -= offset;
- m->m_pkthdr.len -= offset;
- bpf_mtap_af(ifp->if_bpf, family, m, BPF_DIRECTION_OUT);
- m->m_data -= offset;
- m->m_len += offset;
- m->m_pkthdr.len += offset;
+ bpf_mtap_af(ifp->if_bpf, m->m_pkthdr.ph_family, m,
+    BPF_DIRECTION_OUT);
  }
 #endif
 
+ if (gif_encap(ifp, &m, m->m_pkthdr.ph_family) != 0) {
+ m_freem(m);
+ continue;
+ }
+
  /* XXX we should cache the outgoing route */
 
  switch (sc->gif_psrc->sa_family) {
@@ -294,9 +255,7 @@ gif_output(struct ifnet *ifp, struct mbu
  goto end;
  }
 
- error = gif_encap(ifp, &m, dst->sa_family);
- if (error)
- goto end;
+ m->m_pkthdr.ph_family = dst->sa_family;
 
  error = if_enqueue(ifp, m);
 

Reply | Threaded
Open this post in threaded view
|

Re: move gif_encap to simplify gif_start

David Gwynne-5
On Thu, Jan 11, 2018 at 01:27:56PM +1000, David Gwynne wrote:

> this avoids having to parse the gif packet before sending it to
> bpf. instead, we stash the address family in the mbuf and add it
> after bpf.
>
> however, a gif_encap error will not be propagated back to the sender.
> i dont think this is a huge loss as the ip packet is usually
> encapsulated itself after ip_send/ip_output (eg, by ethernet), which
> could fail without propagating an error back either.
>
> ok?

visa@ noted that gif_encap frees the mbuf on failure so gif_start
doesn't have to.

Index: if_gif.c
===================================================================
RCS file: /cvs/src/sys/net/if_gif.c,v
retrieving revision 1.107
diff -u -p -r1.107 if_gif.c
--- if_gif.c 9 Jan 2018 15:24:24 -0000 1.107
+++ if_gif.c 11 Jan 2018 04:36:37 -0000
@@ -175,55 +175,14 @@ gif_start(struct ifnet *ifp)
 
 #if NBPFILTER > 0
  if (ifp->if_bpf) {
- int offset;
- sa_family_t family;
- u_int8_t proto;
-
- /* must decapsulate outer header for bpf */
- switch (sc->gif_psrc->sa_family) {
- case AF_INET:
- offset = sizeof(struct ip);
- proto = mtod(m, struct ip *)->ip_p;
- break;
-#ifdef INET6
- case AF_INET6:
- offset = sizeof(struct ip6_hdr);
- proto = mtod(m, struct ip6_hdr *)->ip6_nxt;
- break;
-#endif
- default:
- proto = 0;
- break;
- }
- switch (proto) {
- case IPPROTO_IPV4:
- family = AF_INET;
- break;
- case IPPROTO_IPV6:
- family = AF_INET6;
- break;
- case IPPROTO_ETHERIP:
- family = AF_LINK;
- offset += sizeof(struct etherip_header);
- break;
- case IPPROTO_MPLS:
- family = AF_MPLS;
- break;
- default:
- offset = 0;
- family = sc->gif_psrc->sa_family;
- break;
- }
- m->m_data += offset;
- m->m_len -= offset;
- m->m_pkthdr.len -= offset;
- bpf_mtap_af(ifp->if_bpf, family, m, BPF_DIRECTION_OUT);
- m->m_data -= offset;
- m->m_len += offset;
- m->m_pkthdr.len += offset;
+ bpf_mtap_af(ifp->if_bpf, m->m_pkthdr.ph_family, m,
+    BPF_DIRECTION_OUT);
  }
 #endif
 
+ if (gif_encap(ifp, &m, m->m_pkthdr.ph_family) != 0)
+ continue;
+
  /* XXX we should cache the outgoing route */
 
  switch (sc->gif_psrc->sa_family) {
@@ -294,9 +253,7 @@ gif_output(struct ifnet *ifp, struct mbu
  goto end;
  }
 
- error = gif_encap(ifp, &m, dst->sa_family);
- if (error)
- goto end;
+ m->m_pkthdr.ph_family = dst->sa_family;
 
  error = if_enqueue(ifp, m);