From 927cc9c4082a84a50e38700709e1d7cd15984a14 Mon Sep 17 00:00:00 2001 From: Joerg Goltermann Date: Fri, 27 Feb 2009 11:09:37 +0000 Subject: fix mbuf problems and simplify code, well spotted and input by Alexander Sabourenkov. mbuf logic is based on claudio's recommendation Tested by Alexander Sabourenkov OK: henning@, claudio@ Theo: "In please..." --- sys/net/if_pflow.c | 185 +++++++++++++++++++++-------------------------------- sys/net/if_pflow.h | 7 +- 2 files changed, 74 insertions(+), 118 deletions(-) (limited to 'sys/net') diff --git a/sys/net/if_pflow.c b/sys/net/if_pflow.c index 15c84a899b5..b26f5daced4 100644 --- a/sys/net/if_pflow.c +++ b/sys/net/if_pflow.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_pflow.c,v 1.9 2009/01/03 21:47:32 gollo Exp $ */ +/* $OpenBSD: if_pflow.c,v 1.10 2009/02/27 11:09:36 gollo Exp $ */ /* * Copyright (c) 2008 Henning Brauer @@ -73,7 +73,7 @@ int pflowoutput(struct ifnet *, struct mbuf *, struct sockaddr *, int pflowioctl(struct ifnet *, u_long, caddr_t); void pflowstart(struct ifnet *); -struct mbuf *pflow_get_mbuf(struct pflow_softc *, void **); +struct mbuf *pflow_get_mbuf(struct pflow_softc *); int pflow_sendout(struct pflow_softc *); int pflow_sendout_mbuf(struct pflow_softc *, struct mbuf *); void pflow_timeout(void *); @@ -82,6 +82,7 @@ void copy_flow_data(struct pflow_flow *, struct pflow_flow *, int pflow_pack_flow(struct pf_state *, struct pflow_softc *); int pflow_get_dynport(void); int export_pflow_if(struct pf_state*, struct pflow_softc *); +int copy_flow_to_m(struct pflow_flow *flow, struct pflow_softc *sc); struct if_clone pflow_cloner = IF_CLONE_INITIALIZER("pflow", pflow_clone_create, @@ -286,75 +287,50 @@ pflow_setmtu(struct pflow_softc *sc, int mtu_req) else mtu = mtu_req; - sc->sc_maxcount = (mtu - sizeof(struct pflow_header)) / - sizeof(struct pflow_flow); + sc->sc_maxcount = (mtu - sizeof(struct pflow_header) - + sizeof (struct udpiphdr)) / sizeof(struct pflow_flow); if (sc->sc_maxcount > PFLOW_MAXFLOWS) sc->sc_maxcount = PFLOW_MAXFLOWS; sc->sc_if.if_mtu = sizeof(struct pflow_header) + + sizeof (struct udpiphdr) + sc->sc_maxcount * sizeof(struct pflow_flow); } struct mbuf * -pflow_get_mbuf(struct pflow_softc *sc, void **sp) +pflow_get_mbuf(struct pflow_softc *sc) { - struct pflow_header *h; - struct mbuf *m, *top = NULL, **mp = ⊤ - int len, totlen; + struct pflow_header h; + struct mbuf *m; MGETHDR(m, M_DONTWAIT, MT_DATA); if (m == NULL) { - sc->sc_if.if_oerrors++; + pflowstats.pflow_onomem++; return (NULL); } - len = MHLEN; - totlen = (sc->sc_maxcount * sizeof(struct pflow_flow)) + - sizeof(struct pflow_header); - - while (totlen > 0) { - if (top) { - MGET(m, M_DONTWAIT, MT_DATA); - if (m == 0) { - m_freem(top); - sc->sc_if.if_oerrors++; - return (NULL); - } - len = MLEN; - } - if (totlen >= MINCLSIZE) { - MCLGET(m, M_DONTWAIT); - if (m->m_flags & M_EXT) - len = MCLBYTES; - else { - m_free(m); - m_freem(top); - sc->sc_if.if_oerrors++; - return (NULL); - } - } - m->m_len = len = min(totlen, len); - totlen -= len; - *mp = m; - mp = &m->m_next; + MCLGET(m, M_DONTWAIT); + if ((m->m_flags & M_EXT) == 0) { + m_free(m); + pflowstats.pflow_onomem++; + return (NULL); } - top->m_pkthdr.rcvif = NULL; - top->m_len = m->m_pkthdr.len = sizeof(struct pflow_header); + m->m_len = m->m_pkthdr.len = 0; + m->m_pkthdr.rcvif = NULL; /* populate pflow_header */ - h = mtod(top, struct pflow_header *); - h->reserved1 = 0; - h->reserved2 = 0; - h->count = 0; - h->version = htons(PFLOW_VERSION); - h->flow_sequence = htonl(sc->sc_gcounter); - h->engine_type = PFLOW_ENGINE_TYPE; - h->engine_id = PFLOW_ENGINE_ID; + h.reserved1 = 0; + h.reserved2 = 0; + h.count = 0; + h.version = htons(PFLOW_VERSION); + h.flow_sequence = htonl(sc->sc_gcounter); + h.engine_type = PFLOW_ENGINE_TYPE; + h.engine_id = PFLOW_ENGINE_ID; + m_copyback(m, 0, PFLOW_HDRLEN, &h); sc->sc_count = 0; - *sp = (void *)((char *)h + PFLOW_HDRLEN); timeout_add_sec(&sc->sc_tmo, PFLOW_TIMEOUT); - return (top); + return (m); } void @@ -363,12 +339,10 @@ copy_flow_data(struct pflow_flow *flow1, struct pflow_flow *flow2, { struct pf_state_key *sk = st->key[PF_SK_WIRE]; - flow1->src_ip = flow2->dest_ip = - st->key[PF_SK_WIRE]->addr[src].v4.s_addr; - flow1->src_port = flow2->dest_port = st->key[PF_SK_WIRE]->port[src]; - flow1->dest_ip = flow2->src_ip = - st->key[PF_SK_WIRE]->addr[dst].v4.s_addr; - flow1->dest_port = flow2->src_port = st->key[PF_SK_WIRE]->port[dst]; + flow1->src_ip = flow2->dest_ip = sk->addr[src].v4.s_addr; + flow1->src_port = flow2->dest_port = sk->port[src]; + flow1->dest_ip = flow2->src_ip = sk->addr[dst].v4.s_addr; + flow1->dest_port = flow2->src_port = sk->port[dst]; flow1->dest_as = flow2->src_as = flow1->src_as = flow2->dest_as = 0; @@ -393,6 +367,10 @@ int export_pflow(struct pf_state *st) { struct pflow_softc *sc = NULL; + struct pf_state_key *sk = st->key[PF_SK_WIRE]; + + if (sk->af != AF_INET) + return (0); SLIST_FOREACH(sc, &pflowif_list, sc_next) { export_pflow_if(st, sc); @@ -405,11 +383,10 @@ int export_pflow_if(struct pf_state *st, struct pflow_softc *sc) { struct pf_state pfs_copy; - struct ifnet *ifp = NULL; + struct ifnet *ifp = &sc->sc_if; u_int64_t bytes[2]; int ret = 0; - ifp = &sc->sc_if; if (!(ifp->if_flags & IFF_RUNNING)) return (0); @@ -449,75 +426,54 @@ export_pflow_if(struct pf_state *st, struct pflow_softc *sc) } int -pflow_pack_flow(struct pf_state *st, struct pflow_softc *sc) +copy_flow_to_m(struct pflow_flow *flow, struct pflow_softc *sc) { - struct pflow_flow *flow1 = NULL; - struct pflow_flow flow2; - struct pf_state_key *sk = st->key[PF_SK_WIRE]; - int s, ret = 0; - - if (sk->af != AF_INET) - return (0); + int s, ret = 0; s = splnet(); - if (sc->sc_mbuf == NULL) { - if ((sc->sc_mbuf = pflow_get_mbuf(sc, - (void **)&sc->sc_flowp.s)) == NULL) { + if ((sc->sc_mbuf = pflow_get_mbuf(sc)) == NULL) { splx(s); - return (ENOMEM); + return (ENOBUFS); } } + m_copyback(sc->sc_mbuf, PFLOW_HDRLEN + + (sc->sc_count * sizeof (struct pflow_flow)), + sizeof (struct pflow_flow), flow); if (pflowstats.pflow_flows == sc->sc_gcounter) pflowstats.pflow_flows++; sc->sc_gcounter++; sc->sc_count++; - flow1 = sc->sc_flowp.s++; - sc->sc_mbuf->m_pkthdr.len = - sc->sc_mbuf->m_len += sizeof(struct pflow_flow); - bzero(flow1, sizeof(*flow1)); + if (sc->sc_count >= sc->sc_maxcount) + ret = pflow_sendout(sc); + + splx(s); + return(ret); +} + +int +pflow_pack_flow(struct pf_state *st, struct pflow_softc *sc) +{ + struct pflow_flow flow1; + struct pflow_flow flow2; + int ret = 0; + + bzero(&flow1, sizeof(flow1)); bzero(&flow2, sizeof(flow2)); if (st->direction == PF_OUT) - copy_flow_data(flow1, &flow2, st, 1, 0); + copy_flow_data(&flow1, &flow2, st, 1, 0); else - copy_flow_data(flow1, &flow2, st, 0, 1); - - if (st->bytes[0] != 0) { /* first flow from state */ - if (sc->sc_count >= sc->sc_maxcount) - ret = pflow_sendout(sc); - - if (st->bytes[1] != 0) { - /* one more flow, second part from state */ - if (sc->sc_mbuf == NULL) { - if ((sc->sc_mbuf = pflow_get_mbuf(sc, - (void **)&sc->sc_flowp.s)) == NULL) { - splx(s); - return (ENOMEM); - } - } - - if (pflowstats.pflow_flows == sc->sc_gcounter) - pflowstats.pflow_flows++; - sc->sc_gcounter++; - sc->sc_count++; - - flow1 = sc->sc_flowp.s++; - sc->sc_mbuf->m_pkthdr.len = - sc->sc_mbuf->m_len += sizeof(struct pflow_flow); - bzero(flow1, sizeof(*flow1)); - } - } + copy_flow_data(&flow1, &flow2, st, 0, 1); - if (st->bytes[1] != 0) { /* second flow from state */ - bcopy(&flow2, flow1, sizeof(*flow1)); - if (sc->sc_count >= sc->sc_maxcount) - ret = pflow_sendout(sc); - } + if (st->bytes[0] != 0) /* first flow from state */ + ret = copy_flow_to_m(&flow1, sc); + + if (st->bytes[1] != 0) /* second flow from state */ + ret = copy_flow_to_m(&flow2, sc); - splx(s); return (ret); } @@ -546,7 +502,6 @@ pflow_sendout(struct pflow_softc *sc) return (0); sc->sc_mbuf = NULL; - sc->sc_flowp.s = NULL; if (!(ifp->if_flags & IFF_RUNNING)) { m_freem(m); return (0); @@ -571,12 +526,13 @@ pflow_sendout_mbuf(struct pflow_softc *sc, struct mbuf *m) u_int16_t len = m->m_pkthdr.len; struct ifnet *ifp = &sc->sc_if; struct ip *ip; + int err; /* UDP Header*/ M_PREPEND(m, sizeof(struct udpiphdr), M_DONTWAIT); if (m == NULL) { pflowstats.pflow_onomem++; - return (0); + return (ENOBUFS); } ui = mtod(m, struct udpiphdr *); @@ -612,9 +568,14 @@ pflow_sendout_mbuf(struct pflow_softc *sc, struct mbuf *m) } #endif - if (ip_output(m, NULL, NULL, IP_RAWOUTPUT, &sc->sc_imo, NULL)) + sc->sc_if.if_opackets++; + sc->sc_if.if_obytes += m->m_pkthdr.len; + + if ((err = ip_output(m, NULL, NULL, IP_RAWOUTPUT, &sc->sc_imo, NULL))) { pflowstats.pflow_oerrors++; - return (0); + sc->sc_if.if_oerrors++; + } + return (err); } int diff --git a/sys/net/if_pflow.h b/sys/net/if_pflow.h index cceaf096b1a..51dc017ae5a 100644 --- a/sys/net/if_pflow.h +++ b/sys/net/if_pflow.h @@ -1,4 +1,4 @@ -/* $OpenBSD: if_pflow.h,v 1.4 2009/01/03 21:47:32 gollo Exp $ */ +/* $OpenBSD: if_pflow.h,v 1.5 2009/02/27 11:09:36 gollo Exp $ */ /* * Copyright (c) 2008 Henning Brauer @@ -56,10 +56,6 @@ struct pflow_flow { extern int pflow_ok; -union sc_flowp { - struct pflow_flow *s; -}; - struct pflow_softc { struct ifnet sc_if; struct ifnet *sc_pflow_ifp; @@ -73,7 +69,6 @@ struct pflow_softc { u_int16_t sc_sender_port; struct in_addr sc_receiver_ip; u_int16_t sc_receiver_port; - union sc_flowp sc_flowp; struct mbuf *sc_mbuf; /* current cumulative mbuf */ SLIST_ENTRY(pflow_softc) sc_next; }; -- cgit v1.2.3