On Tue, Jan 24, 2023 at 09:32:53PM +1000, David Gwynne wrote:
> On Mon, Jan 23, 2023 at 09:25:34AM +0100, Jan Klemkow wrote:
> > On Wed, Jan 18, 2023 at 03:49:25PM -0700, Theo de Raadt wrote:
> > > Jan Klemkow <j.klem...@wemelug.de> wrote:
> > > > On Wed, Jan 18, 2023 at 10:50:25AM +0300, Vitaliy Makkoveev wrote:
> > > > > On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote:
> > > > > > we have several drivers which have to parse the content of mbufs.
> > > > > > This
> > > > > > diff suggest a central parsing function for this. Thus, we can
> > > > > > reduce
> > > > > > redundant code.
> > > > > >
> > > > > > I just start with ix(4) and ixl(4) because it was easy to test for
> > > > > > me.
> > > > > > But, this could also improve em(4), igc(4), ale(4) and oce(4).
> > > > > >
> > > > > > I'm not sure about the name, the api nor the place of this code.
> > > > > > So, if
> > > > > > someone has a better idea: i'm open to anything.
> > > > >
> > > > > I like code this deduplication.
> > > > >
> > > > > This newly introduced function doesn't touch ifnet but only extracts
> > > > > protocol headers from mbuf(9). I guess mbuf_extract_headers() or
> > > > > something like is much better for name with the ern/uipc_mbuf2.c as
> > > > > place.
> > > >
> > > > Good Point. Updates diff below.
> > >
> > > I agree, "extract" is a better name. dlg, do you have a comment?
> >
> > Whats you opinion about this diff?
>
> it makes ix and ixl prettier, so that's a good enough reason to do
> it. it should go in net/if_ethersubr.c as ether_extract_headers()
> though.
>
> could you try using a struct to carry the header pointers around and see
> what that looks like?
>
> struct ether_extracted {
> struct ether_header *eh;
> struct ip *ip4;
> struct ip6_hdr *ip6;
> struct tcphdr *tcp;
> struct udphdr *udp;
> };
>
> void ether_extract_headers(struct mbuf *, struct ether_extracted *);
>
> you can add a depth or flags argument if you want to be able to
> tell it to return before looking for the tcp/udp headers if you
> want.
OK?
Thanks,
Jan
Index: dev/pci/if_ix.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
retrieving revision 1.189
diff -u -p -r1.189 if_ix.c
--- dev/pci/if_ix.c 2 Sep 2022 14:08:09 -0000 1.189
+++ dev/pci/if_ix.c 24 Jan 2023 13:34:17 -0000
@@ -2477,25 +2477,16 @@ static inline int
ixgbe_csum_offload(struct mbuf *mp, uint32_t *vlan_macip_lens,
uint32_t *type_tucmd_mlhl, uint32_t *olinfo_status)
{
- struct ether_header *eh = mtod(mp, struct ether_header *);
- struct mbuf *m;
- int hoff;
+ struct ether_extracted ext;
int offload = 0;
uint32_t iphlen;
- uint8_t ipproto;
- *vlan_macip_lens |= (sizeof(*eh) << IXGBE_ADVTXD_MACLEN_SHIFT);
+ ether_extract_headers(mp, &ext);
- switch (ntohs(eh->ether_type)) {
- case ETHERTYPE_IP: {
- struct ip *ip;
+ *vlan_macip_lens |= (sizeof(*ext.eh) << IXGBE_ADVTXD_MACLEN_SHIFT);
- m = m_getptr(mp, sizeof(*eh), &hoff);
- KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip));
- ip = (struct ip *)(mtod(m, caddr_t) + hoff);
-
- iphlen = ip->ip_hl << 2;
- ipproto = ip->ip_p;
+ if (ext.ip4) {
+ iphlen = ext.ip4->ip_hl << 2;
if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) {
*olinfo_status |= IXGBE_TXD_POPTS_IXSM << 8;
@@ -2503,46 +2494,30 @@ ixgbe_csum_offload(struct mbuf *mp, uint
}
*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV4;
- break;
- }
-
#ifdef INET6
- case ETHERTYPE_IPV6: {
- struct ip6_hdr *ip6;
-
- m = m_getptr(mp, sizeof(*eh), &hoff);
- KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip6));
- ip6 = (struct ip6_hdr *)(mtod(m, caddr_t) + hoff);
-
- iphlen = sizeof(*ip6);
- ipproto = ip6->ip6_nxt;
+ } else if (ext.ip6) {
+ iphlen = sizeof(*ext.ip6);
*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV6;
- break;
- }
#endif
-
- default:
+ } else {
return offload;
}
*vlan_macip_lens |= iphlen;
- switch (ipproto) {
- case IPPROTO_TCP:
+ if (ext.tcp) {
*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_L4T_TCP;
if (ISSET(mp->m_pkthdr.csum_flags, M_TCP_CSUM_OUT)) {
*olinfo_status |= IXGBE_TXD_POPTS_TXSM << 8;
offload = 1;
}
- break;
- case IPPROTO_UDP:
+ } else if (ext.udp) {
*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_L4T_UDP;
if (ISSET(mp->m_pkthdr.csum_flags, M_UDP_CSUM_OUT)) {
*olinfo_status |= IXGBE_TXD_POPTS_TXSM << 8;
offload = 1;
}
- break;
}
return offload;
Index: dev/pci/if_ixl.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_ixl.c,v
retrieving revision 1.84
diff -u -p -r1.84 if_ixl.c
--- dev/pci/if_ixl.c 5 Aug 2022 13:57:16 -0000 1.84
+++ dev/pci/if_ixl.c 24 Jan 2023 13:31:02 -0000
@@ -2784,10 +2784,8 @@ ixl_load_mbuf(bus_dma_tag_t dmat, bus_dm
static uint64_t
ixl_tx_setup_offload(struct mbuf *m0)
{
- struct mbuf *m;
- int hoff;
+ struct ether_extracted ext;
uint64_t hlen;
- uint8_t ipproto;
uint64_t offload = 0;
if (ISSET(m0->m_flags, M_VLANTAG)) {
@@ -2800,39 +2798,21 @@ ixl_tx_setup_offload(struct mbuf *m0)
M_IPV4_CSUM_OUT|M_TCP_CSUM_OUT|M_UDP_CSUM_OUT))
return (offload);
- switch (ntohs(mtod(m0, struct ether_header *)->ether_type)) {
- case ETHERTYPE_IP: {
- struct ip *ip;
-
- m = m_getptr(m0, ETHER_HDR_LEN, &hoff);
- KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip));
- ip = (struct ip *)(mtod(m, caddr_t) + hoff);
+ ether_extract_headers(m0, &ext);
+ if (ext.ip4) {
offload |= ISSET(m0->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT) ?
IXL_TX_DESC_CMD_IIPT_IPV4_CSUM :
IXL_TX_DESC_CMD_IIPT_IPV4;
- hlen = ip->ip_hl << 2;
- ipproto = ip->ip_p;
- break;
- }
-
+ hlen = ext.ip4->ip_hl << 2;
#ifdef INET6
- case ETHERTYPE_IPV6: {
- struct ip6_hdr *ip6;
-
- m = m_getptr(m0, ETHER_HDR_LEN, &hoff);
- KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip6));
- ip6 = (struct ip6_hdr *)(mtod(m, caddr_t) + hoff);
-
+ } else if (ext.ip6) {
offload |= IXL_TX_DESC_CMD_IIPT_IPV6;
- hlen = sizeof(*ip6);
- ipproto = ip6->ip6_nxt;
- break;
- }
+ hlen = sizeof(*ext.ip6);
#endif
- default:
+ } else {
panic("CSUM_OUT set for non-IP packet");
/* NOTREACHED */
}
@@ -2840,30 +2820,12 @@ ixl_tx_setup_offload(struct mbuf *m0)
offload |= (ETHER_HDR_LEN >> 1) << IXL_TX_DESC_MACLEN_SHIFT;
offload |= (hlen >> 2) << IXL_TX_DESC_IPLEN_SHIFT;
- switch (ipproto) {
- case IPPROTO_TCP: {
- struct tcphdr *th;
-
- if (!ISSET(m0->m_pkthdr.csum_flags, M_TCP_CSUM_OUT))
- break;
-
- m = m_getptr(m, hoff + hlen, &hoff);
- KASSERT(m != NULL && m->m_len - hoff >= sizeof(*th));
- th = (struct tcphdr *)(mtod(m, caddr_t) + hoff);
-
+ if (ext.tcp && ISSET(m0->m_pkthdr.csum_flags, M_TCP_CSUM_OUT)) {
offload |= IXL_TX_DESC_CMD_L4T_EOFT_TCP;
- offload |= (uint64_t)th->th_off << IXL_TX_DESC_L4LEN_SHIFT;
- break;
- }
-
- case IPPROTO_UDP:
- if (!ISSET(m0->m_pkthdr.csum_flags, M_UDP_CSUM_OUT))
- break;
-
+ offload |= (uint64_t)ext.tcp->th_off << IXL_TX_DESC_L4LEN_SHIFT;
+ } else if (ext.udp && ISSET(m0->m_pkthdr.csum_flags, M_UDP_CSUM_OUT)) {
offload |= IXL_TX_DESC_CMD_L4T_EOFT_UDP;
- offload |= (sizeof(struct udphdr) >> 2) <<
- IXL_TX_DESC_L4LEN_SHIFT;
- break;
+ offload |= (sizeof(*ext.udp) >> 2) << IXL_TX_DESC_L4LEN_SHIFT;
}
return (offload);
Index: net/if_ethersubr.c
===================================================================
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.284
diff -u -p -r1.284 if_ethersubr.c
--- net/if_ethersubr.c 29 Jun 2022 09:08:07 -0000 1.284
+++ net/if_ethersubr.c 24 Jan 2023 13:49:52 -0000
@@ -98,6 +98,10 @@ didn't get a copy, you may request one f
#include <netinet/in.h>
#include <netinet/if_ether.h>
#include <netinet/ip_ipsp.h>
+#include <netinet/ip.h>
+#include <netinet/ip6.h>
+#include <netinet/tcp.h>
+#include <netinet/udp.h>
#if NBPFILTER > 0
#include <net/bpf.h>
@@ -1033,4 +1037,61 @@ ether_e64_to_addr(struct ether_addr *ea,
ea->ether_addr_octet[--i] = e64;
e64 >>= 8;
} while (i > 0);
+}
+
+/* Parse different TCP/IP protocol headers for a quick view inside an mbuf. */
+void
+ether_extract_headers(struct mbuf *mp, struct ether_extracted *ext)
+{
+ struct mbuf *m;
+ uint64_t hlen;
+ int hoff;
+ uint8_t ipproto;
+
+ /* Return NULL if header was not recognized. */
+ ext->eh = NULL;
+ ext->ip4 = NULL;
+ ext->ip6 = NULL;
+ ext->tcp = NULL;
+ ext->udp = NULL;
+
+ ext->eh = mtod(mp, struct ether_header *);
+ switch (ntohs(ext->eh->ether_type)) {
+ case ETHERTYPE_IP:
+ m = m_getptr(mp, sizeof(*ext->eh), &hoff);
+ KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ext->ip4));
+ ext->ip4 = (struct ip *)(mtod(m, caddr_t) + hoff);
+
+ hlen = ext->ip4->ip_hl << 2;
+ ipproto = ext->ip4->ip_p;
+
+ break;
+#ifdef INET6
+ case ETHERTYPE_IPV6:
+ m = m_getptr(mp, sizeof(*ext->eh), &hoff);
+ KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ext->ip6));
+ ext->ip6 = (struct ip6_hdr *)(mtod(m, caddr_t) + hoff);
+
+ hlen = sizeof(*ext->ip6);
+ ipproto = ext->ip6->ip6_nxt;
+
+ break;
+#endif
+ default:
+ return;
+ }
+
+ switch (ipproto) {
+ case IPPROTO_TCP:
+ m = m_getptr(m, hoff + hlen, &hoff);
+ KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ext->tcp));
+ ext->tcp = (struct tcphdr *)(mtod(m, caddr_t) + hoff);
+ break;
+
+ case IPPROTO_UDP:
+ m = m_getptr(m, hoff + hlen, &hoff);
+ KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ext->udp));
+ ext->udp = (struct udphdr *)(mtod(m, caddr_t) + hoff);
+ break;
+ }
}
Index: netinet/if_ether.h
===================================================================
RCS file: /cvs/src/sys/netinet/if_ether.h,v
retrieving revision 1.84
diff -u -p -r1.84 if_ether.h
--- netinet/if_ether.h 27 Dec 2022 20:13:03 -0000 1.84
+++ netinet/if_ether.h 24 Jan 2023 13:57:19 -0000
@@ -297,6 +297,16 @@ const struct ether_brport *
uint64_t ether_addr_to_e64(const struct ether_addr *);
void ether_e64_to_addr(struct ether_addr *, uint64_t);
+struct ether_extracted {
+ struct ether_header *eh;
+ struct ip *ip4;
+ struct ip6_hdr *ip6;
+ struct tcphdr *tcp;
+ struct udphdr *udp;
+};
+
+void ether_extract_headers(struct mbuf *, struct ether_extracted *);
+
/*
* Ethernet multicast address structure. There is one of these for each
* multicast address or range of multicast addresses that we are supposed
