Changeset View
Standalone View
sys/kern/uipc_mbuf.c
Show First 20 Lines • Show All 1,233 Lines • ▼ Show 20 Lines | while (remainder > 0) { | ||||
m->m_next = n; | m->m_next = n; | ||||
m = n; | m = n; | ||||
} | } | ||||
if (m0->m_flags & M_PKTHDR) | if (m0->m_flags & M_PKTHDR) | ||||
m0->m_pkthdr.len += len - remainder; | m0->m_pkthdr.len += len - remainder; | ||||
return (remainder == 0); | return (remainder == 0); | ||||
} | } | ||||
/* Apply function f to the data in a single mbuf. */ | |||||
static int | |||||
m_apply_one(struct mbuf *m, int off, int len, | |||||
int (*f)(void *, void *, u_int), void *arg) | |||||
{ | |||||
void *p; | |||||
u_int i, count, pgoff; | |||||
int rval; | |||||
if ((m->m_flags & M_EXTPG) != 0) { | |||||
markj: Just a suggestion, but I'd find it nicer to handle the M_EXTPG case in a separate function. | |||||
jhbAuthorUnsubmitted Done Inline ActionsHmm, most of this is a single function? Would you rather inline the M_EXTPG check below and rename this to 'm_apply_extpg()' or would you want m_apply_one() to still do the check and have a separate 'm_apply_extpg_one'? jhb: Hmm, most of this is a single function? Would you rather inline the M_EXTPG check below and… | |||||
markjUnsubmitted Done Inline ActionsEither suggestion seems fine, I think the latter is a bit better. I'm ok keeping it as-is. I just don't like the fact that the extpg code has extra indentation even though almost all the function is extpg-specific. I'm ok keeping it as-is too though. markj: Either suggestion seems fine, I think the latter is a bit better. I'm ok keeping it as-is. I… | |||||
KASSERT(PMAP_HAS_DMAP, | |||||
("m_apply does not support unmapped mbufs")); | |||||
off += mtod(m, vm_offset_t); | |||||
markjUnsubmitted Not Done Inline ActionsI thought m_data == 0 for EXTPG mbufs. When is that not true? markj: I thought m_data == 0 for EXTPG mbufs. When is that not true? | |||||
jhbAuthorUnsubmitted Done Inline Actionsm_data is the offset into the mbuf, so when transmitting a portion of a TLS record via TCP m_data can be non-zero. For the purposes of KTLS SW crypto, m_data should always be zero, but all the other places that iterate over an unmapped mbuf honor the m_data and m_len subset range of the backing store so it is consistent (and safest for any potential future uses) to honor it here. jhb: `m_data` is the offset into the mbuf, so when transmitting a portion of a TLS record via TCP… | |||||
if (off < m->m_epg_hdrlen) { | |||||
count = min(m->m_epg_hdrlen - off, len); | |||||
rval = f(arg, m->m_epg_hdr + off, count); | |||||
if (rval) | |||||
return (rval); | |||||
len -= count; | |||||
off = 0; | |||||
} else | |||||
off -= m->m_epg_hdrlen; | |||||
pgoff = m->m_epg_1st_off; | |||||
markjUnsubmitted Done Inline ActionsDoesn't pgoff need to be reset to 0 after the first iteration of the loop? markj: Doesn't `pgoff` need to be reset to 0 after the first iteration of the loop? | |||||
jhbAuthorUnsubmitted Done Inline ActionsOof, so it does. jhb: Oof, so it does. | |||||
for (i = 0; i < m->m_epg_npgs && len > 0; i++) { | |||||
if (off < m_epg_pagelen(m, i, pgoff)) { | |||||
count = min(m_epg_pagelen(m, i, pgoff) - off, | |||||
len); | |||||
p = (void *)PHYS_TO_DMAP(m->m_epg_pa[i] + | |||||
pgoff); | |||||
rval = f(arg, p, count); | |||||
if (rval) | |||||
return (rval); | |||||
len -= count; | |||||
off = 0; | |||||
} else | |||||
off -= m_epg_pagelen(m, i, pgoff); | |||||
markjUnsubmitted Done Inline ActionsIt would be nice to call m_epg_pagelen() and save the result in a local var, there are three calls in this loop. markj: It would be nice to call m_epg_pagelen() and save the result in a local var, there are three… | |||||
} | |||||
if (off < m->m_epg_trllen && len > 0) { | |||||
jhbAuthorUnsubmitted Done Inline ActionsI've also simplified the logic a bit here (and added some assertions). Since off and len are constrained to the bounds of the mbuf (m_len), the offset and length should never be above the bounds of the mbuf, so this first test here should always be true, and similarly, the 'min' below should always return 'len'. jhb: I've also simplified the logic a bit here (and added some assertions). Since off and len are… | |||||
count = min(m->m_epg_trllen - off, len); | |||||
return (f(arg, m->m_epg_trail + off, count)); | |||||
} | |||||
return (0); | |||||
} | |||||
return (f(arg, mtod(m, caddr_t) + off, len)); | |||||
} | |||||
/* | /* | ||||
* Apply function f to the data in an mbuf chain starting "off" bytes from | * Apply function f to the data in an mbuf chain starting "off" bytes from | ||||
* the beginning, continuing for "len" bytes. | * the beginning, continuing for "len" bytes. | ||||
*/ | */ | ||||
int | int | ||||
m_apply(struct mbuf *m, int off, int len, | m_apply(struct mbuf *m, int off, int len, | ||||
int (*f)(void *, void *, u_int), void *arg) | int (*f)(void *, void *, u_int), void *arg) | ||||
{ | { | ||||
u_int count; | u_int count; | ||||
int rval; | int rval; | ||||
KASSERT(off >= 0, ("m_apply, negative off %d", off)); | KASSERT(off >= 0, ("m_apply, negative off %d", off)); | ||||
KASSERT(len >= 0, ("m_apply, negative len %d", len)); | KASSERT(len >= 0, ("m_apply, negative len %d", len)); | ||||
while (off > 0) { | while (off > 0) { | ||||
KASSERT(m != NULL, ("m_apply, offset > size of mbuf chain")); | KASSERT(m != NULL, ("m_apply, offset > size of mbuf chain")); | ||||
if (off < m->m_len) | if (off < m->m_len) | ||||
break; | break; | ||||
off -= m->m_len; | off -= m->m_len; | ||||
m = m->m_next; | m = m->m_next; | ||||
} | } | ||||
while (len > 0) { | while (len > 0) { | ||||
KASSERT(m != NULL, ("m_apply, offset > size of mbuf chain")); | KASSERT(m != NULL, ("m_apply, offset > size of mbuf chain")); | ||||
count = min(m->m_len - off, len); | count = min(m->m_len - off, len); | ||||
rval = (*f)(arg, mtod(m, caddr_t) + off, count); | rval = m_apply_one(m, off, count, f, arg); | ||||
if (rval) | if (rval) | ||||
return (rval); | return (rval); | ||||
len -= count; | len -= count; | ||||
off = 0; | off = 0; | ||||
m = m->m_next; | m = m->m_next; | ||||
} | } | ||||
return (0); | return (0); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 907 Lines • Show Last 20 Lines |
Just a suggestion, but I'd find it nicer to handle the M_EXTPG case in a separate function.