On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote:
> Hi,
>
> 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.
>
> bye,
> Jan
>
Hi,
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.
> 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 17 Jan 2023 16:31:19 -0000
> @@ -2477,23 +2477,18 @@ 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_header *eh = NULL;
> + struct ip *ip = NULL;
> + struct ip6_hdr *ip6 = NULL;
> int offload = 0;
> uint32_t iphlen;
> uint8_t ipproto;
>
> - *vlan_macip_lens |= (sizeof(*eh) << IXGBE_ADVTXD_MACLEN_SHIFT);
> + if_parse(mp, &eh, &ip, &ip6, NULL, NULL);
>
> - switch (ntohs(eh->ether_type)) {
> - case ETHERTYPE_IP: {
> - struct ip *ip;
> -
> - m = m_getptr(mp, sizeof(*eh), &hoff);
> - KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip));
> - ip = (struct ip *)(mtod(m, caddr_t) + hoff);
> + *vlan_macip_lens |= (sizeof(*eh) << IXGBE_ADVTXD_MACLEN_SHIFT);
>
> + if (ip) {
> iphlen = ip->ip_hl << 2;
> ipproto = ip->ip_p;
>
> @@ -2503,26 +2498,14 @@ 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);
> -
> + } else if (ip6) {
> iphlen = sizeof(*ip6);
> ipproto = ip6->ip6_nxt;
>
> *type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV6;
> - break;
> - }
> #endif
> -
> - default:
> + } else {
> 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 16 Jan 2023 23:58:05 -0000
> @@ -2784,12 +2784,15 @@ 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_header *eh = NULL;
> + struct ip *ip = NULL;
> + struct ip6_hdr *ip6 = NULL;
> + struct tcphdr *th = NULL;
> uint64_t hlen;
> uint8_t ipproto;
> uint64_t offload = 0;
>
> +
> if (ISSET(m0->m_flags, M_VLANTAG)) {
> uint64_t vtag = m0->m_pkthdr.ether_vtag;
> offload |= IXL_TX_DESC_CMD_IL2TAG1;
> @@ -2800,39 +2803,23 @@ 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);
> + if_parse(m0, &eh, &ip, &ip6, &th, NULL);
>
> + if (ip) {
> 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;
> - }
> -
> #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 (ip6) {
> offload |= IXL_TX_DESC_CMD_IIPT_IPV6;
>
> hlen = sizeof(*ip6);
> ipproto = ip6->ip6_nxt;
> - break;
> - }
> #endif
> - default:
> + } else {
> panic("CSUM_OUT set for non-IP packet");
> /* NOTREACHED */
> }
> @@ -2842,15 +2829,12 @@ ixl_tx_setup_offload(struct mbuf *m0)
>
> 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 (th == NULL)
> + break;
> +
> offload |= IXL_TX_DESC_CMD_L4T_EOFT_TCP;
> offload |= (uint64_t)th->th_off << IXL_TX_DESC_L4LEN_SHIFT;
> break;
> Index: net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.683
> diff -u -p -r1.683 if.c
> --- net/if.c 23 Nov 2022 16:57:37 -0000 1.683
> +++ net/if.c 16 Jan 2023 23:20:21 -0000
> @@ -96,6 +96,10 @@
> #include <net/netisr.h>
>
> #include <netinet/in.h>
> +#include <netinet/ip.h>
> +#include <netinet/ip6.h>
> +#include <netinet/tcp.h>
> +#include <netinet/udp.h>
> #include <netinet/if_ether.h>
> #include <netinet/igmp.h>
> #ifdef MROUTING
> @@ -3352,6 +3356,75 @@ if_rxr_ioctl(struct if_rxrinfo *ifri, co
> ifr.ifr_info = *rxr;
>
> return (if_rxr_info_ioctl(ifri, 1, &ifr));
> +}
> +
> +void
> +if_parse(struct mbuf *mp, struct ether_header **eh, struct ip **ip4,
> + struct ip6_hdr **ip6, struct tcphdr **tcp, struct udphdr **udp)
> +{
> + struct mbuf *m;
> + int hoff;
> + uint8_t ipproto;
> + uint64_t hlen;
> +
> + if (eh == NULL)
> + return;
> +
> + *eh = mtod(mp, struct ether_header *);
> + switch (ntohs((*eh)->ether_type)) {
> + case ETHERTYPE_IP:
> + if (ip4 == NULL)
> + return;
> +
> + m = m_getptr(mp, sizeof(**eh), &hoff);
> + KASSERT(m != NULL && m->m_len - hoff >= sizeof(**ip4));
> + *ip4 = (struct ip *)(mtod(m, caddr_t) + hoff);
> +
> + hlen = (*ip4)->ip_hl << 2;
> + ipproto = (*ip4)->ip_p;
> +
> + break;
> +
> +#ifdef INET6
> + case ETHERTYPE_IPV6:
> + if (ip6 == NULL)
> + return;
> +
> + 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);
> +
> + hlen = sizeof(**ip6);
> + ipproto = (*ip6)->ip6_nxt;
> +
> + break;
> +#endif
> +
> + default:
> + return;
> + }
> +
> + switch (ipproto) {
> + case IPPROTO_TCP:
> + if (tcp == NULL)
> + return;
> +
> + m = m_getptr(m, hoff + hlen, &hoff);
> + KASSERT(m != NULL && m->m_len - hoff >= sizeof(**tcp));
> + *tcp = (struct tcphdr *)(mtod(m, caddr_t) + hoff);
> + break;
> +
> + case IPPROTO_UDP:
> + if (udp == NULL)
> + return;
> +
> + m = m_getptr(m, hoff + hlen, &hoff);
> + KASSERT(m != NULL && m->m_len - hoff >= sizeof(**udp));
> + *udp = (struct udphdr *)(mtod(m, caddr_t) + hoff);
> + break;
> + }
> +
> + return;
> }
>
> /*
> Index: net/if_var.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if_var.h,v
> retrieving revision 1.122
> diff -u -p -r1.122 if_var.h
> --- net/if_var.h 23 Nov 2022 14:50:59 -0000 1.122
> +++ net/if_var.h 16 Jan 2023 21:50:06 -0000
> @@ -317,6 +317,13 @@ int niq_enlist(struct niqueue *, struct
>
> extern struct ifnet_head ifnetlist;
>
> +#include <netinet/in.h>
> +#include <netinet/ip.h>
> +#include <netinet/ip6.h>
> +#include <netinet/tcp.h>
> +#include <netinet/udp.h>
> +#include <netinet/if_ether.h>
> +
> void if_start(struct ifnet *);
> int if_enqueue(struct ifnet *, struct mbuf *);
> int if_enqueue_ifq(struct ifnet *, struct mbuf *);
> @@ -371,6 +378,8 @@ u_int if_rxr_get(struct if_rxring *, u_i
> int if_rxr_info_ioctl(struct if_rxrinfo *, u_int, struct if_rxring_info *);
> int if_rxr_ioctl(struct if_rxrinfo *, const char *, u_int,
> struct if_rxring *);
> +void if_parse(struct mbuf *, struct ether_header **, struct ip **,
> + struct ip6_hdr **, struct tcphdr **, struct udphdr **);
>
> void if_counters_alloc(struct ifnet *);
> void if_counters_free(struct ifnet *);
>
