Page MenuHomeFreeBSD
Differential D36182 Diff 109475 usr.sbin/bhyve/pci_e82545.c

Changeset View

Standalone View

View Options

usr.sbin/bhyve/pci_e82545.c

Show First 20 Lines • Show All 227 Lines • ▼ Show 20 Linesstruct ck_info {
uint8_t ck_off; /* offset of cksum insertion */ uint8_t ck_off; /* offset of cksum insertion */
uint16_t ck_len; /* length of cksum calc: 0 is to packet-end */ uint16_t ck_len; /* length of cksum calc: 0 is to packet-end */
}; };
/* /*
* Debug printf * Debug printf
*/ */
static int e82545_debug = 0; static int e82545_debug = 0;
#define WPRINTF(msg,params...) PRINTLN("e82545: " msg, params) #define WPRINTF(msg,params...) PRINTLN("e82545: " msg, ##params)
emasteUnsubmitted
Not Done
Inline Actions

this is because we had no WPRINTFs w/o params before?

emaste: this is because we had no `WPRINTF`s w/o params before?
jhbAuthorUnsubmitted
Done
Inline Actions

Yes.

jhb: Yes.
#define DPRINTF(msg,params...) if (e82545_debug) WPRINTF(msg, params) #define DPRINTF(msg,params...) if (e82545_debug) WPRINTF(msg, params)
#define MIN(a,b) (((a)<(b))?(a):(b)) #define MIN(a,b) (((a)<(b))?(a):(b))
#define MAX(a,b) (((a)>(b))?(a):(b)) #define MAX(a,b) (((a)>(b))?(a):(b))
/* s/w representation of the RAL/RAH regs */ /* s/w representation of the RAL/RAH regs */
struct eth_uni { struct eth_uni {
int eu_valid; int eu_valid;
▲ Show 20 Lines • Show All 836 Lines • ▼ Show 20 Linese82545_transmit(struct e82545_softc *sc, uint16_t head, uint16_t tail,
struct iovec iovb[I82545_MAX_TXSEGS + 2]; struct iovec iovb[I82545_MAX_TXSEGS + 2];
struct iovec tiov[I82545_MAX_TXSEGS + 2]; struct iovec tiov[I82545_MAX_TXSEGS + 2];
struct e1000_context_desc *cd; struct e1000_context_desc *cd;
struct ck_info ckinfo[2]; struct ck_info ckinfo[2];
struct iovec *iov; struct iovec *iov;
union e1000_tx_udesc *dsc; union e1000_tx_udesc *dsc;
int desc, dtype, len, ntype, iovcnt, tcp, tso; int desc, dtype, len, ntype, iovcnt, tcp, tso;
int mss, paylen, seg, tiovcnt, left, now, nleft, nnow, pv, pvoff; int mss, paylen, seg, tiovcnt, left, now, nleft, nnow, pv, pvoff;
unsigned hdrlen, vlen; unsigned hdrlen, vlen, pktlen;
uint32_t tcpsum, tcpseq; uint32_t tcpsum, tcpseq;
uint16_t ipcs, tcpcs, ipid, ohead; uint16_t ipcs, tcpcs, ipid, ohead;
bool invalid;
ckinfo[0].ck_valid = ckinfo[1].ck_valid = 0; ckinfo[0].ck_valid = ckinfo[1].ck_valid = 0;
iovcnt = 0; iovcnt = 0;
ntype = 0; ntype = 0;
tso = 0; tso = 0;
pktlen = 0;
ohead = head; ohead = head;
invalid = false;
/* iovb[0/1] may be used for writable copy of headers. */ /* iovb[0/1] may be used for writable copy of headers. */
iov = &iovb[2]; iov = &iovb[2];
for (desc = 0; ; desc++, head = (head + 1) % dsize) { for (desc = 0; ; desc++, head = (head + 1) % dsize) {
if (head == tail) { if (head == tail) {
*rhead = head; *rhead = head;
return (0); return (0);
Show All 33 Lines if (desc == 0) {
assert(dtype == ntype); assert(dtype == ntype);
DPRINTF("tx next desc idx %d: %08x%08x", DPRINTF("tx next desc idx %d: %08x%08x",
head, dsc->td.upper.data, dsc->td.lower.data); head, dsc->td.upper.data, dsc->td.lower.data);
} }
len = (dtype == E1000_TXD_TYP_L) ? dsc->td.lower.flags.length : len = (dtype == E1000_TXD_TYP_L) ? dsc->td.lower.flags.length :
dsc->dd.lower.data & 0xFFFFF; dsc->dd.lower.data & 0xFFFFF;
if (len > 0) {
/* Strip checksum supplied by guest. */ /* Strip checksum supplied by guest. */
if ((dsc->td.lower.data & E1000_TXD_CMD_EOP) != 0 && if ((dsc->td.lower.data & E1000_TXD_CMD_EOP) != 0 &&
(dsc->td.lower.data & E1000_TXD_CMD_IFCS) == 0) (dsc->td.lower.data & E1000_TXD_CMD_IFCS) == 0) {
if (len <= 2) {
WPRINTF("final descriptor too short (%d) -- dropped",
len);
invalid = true;
} else
len -= 2; len -= 2;
if (iovcnt < I82545_MAX_TXSEGS) {
iov[iovcnt].iov_base = paddr_guest2host(
sc->esc_ctx, dsc->td.buffer_addr, len);
iov[iovcnt].iov_len = len;
} }
if (len > 0 && iovcnt < I82545_MAX_TXSEGS) {
emasteUnsubmitted
Not Done
Inline Actions

should iovcnt >= I82545_MAX_TXSEGS also set invalid?

emaste: should iovcnt >= I82545_MAX_TXSEGS also set `invalid`?
jhbAuthorUnsubmitted
Done
Inline Actions

Right now invalid assumes you've already output an error message, but the existing error message for the iovcnt case is after the loop.

Part of the reason this is a bit more complex is we have to finish the loop to get to the EOP descriptor instead of just jumping directly to 'done' on an error.

I could perhaps move the check for iovcnt overflow and error message up here and then set invalid, but that's a bit larger of a change.

jhb: Right now `invalid` assumes you've already output an error message, but the existing error…
emasteUnsubmitted
Not Done
Inline Actions

Ok, i see

emaste: Ok, i see
iov[iovcnt].iov_base = paddr_guest2host(sc->esc_ctx,
dsc->td.buffer_addr, len);
iov[iovcnt].iov_len = len;
iovcnt++; iovcnt++;
pktlen += len;
} }
/* /*
* Pull out info that is valid in the final descriptor * Pull out info that is valid in the final descriptor
* and exit descriptor loop. * and exit descriptor loop.
*/ */
if (dsc->td.lower.data & E1000_TXD_CMD_EOP) { if (dsc->td.lower.data & E1000_TXD_CMD_EOP) {
if (dtype == E1000_TXD_TYP_L) { if (dtype == E1000_TXD_TYP_L) {
Show All 31 Lines if (dsc->td.lower.data & E1000_TXD_CMD_EOP) {
ckinfo[1].ck_len = ckinfo[1].ck_len =
cd->upper_setup.tcp_fields.tucse; cd->upper_setup.tcp_fields.tucse;
} }
} }
break; break;
} }
} }
if (invalid)
goto done;
if (iovcnt > I82545_MAX_TXSEGS) { if (iovcnt > I82545_MAX_TXSEGS) {
WPRINTF("tx too many descriptors (%d > %d) -- dropped", WPRINTF("tx too many descriptors (%d > %d) -- dropped",
iovcnt, I82545_MAX_TXSEGS); iovcnt, I82545_MAX_TXSEGS);
goto done; goto done;
} }
hdrlen = vlen = 0; hdrlen = vlen = 0;
/* Estimate writable space for VLAN header insertion. */ /* Estimate writable space for VLAN header insertion. */
▲ Show 20 Lines • Show All 77 Lines • ▼ Show 20 Lines if (!tso) {
} }
if (ckinfo[1].ck_valid && hdrlen < ckinfo[1].ck_off + 2) { if (ckinfo[1].ck_valid && hdrlen < ckinfo[1].ck_off + 2) {
WPRINTF("TSO hdrlen too small for TCP/UDP fields " WPRINTF("TSO hdrlen too small for TCP/UDP fields "
"(%d) -- dropped", hdrlen); "(%d) -- dropped", hdrlen);
goto done; goto done;
} }
} }
if (pktlen < hdrlen + vlen) {
WPRINTF("packet too small for writable header");
goto done;
}
/* Allocate, fill and prepend writable header vector. */ /* Allocate, fill and prepend writable header vector. */
if (hdrlen != 0) { if (hdrlen + vlen != 0) {
markjUnsubmitted
Not Done
Inline Actions

Is this a functional change? I can't see how it's possible to have vlen != 0 and hdrlen == 0. Not to say that the change is incorrect, though.

markj: Is this a functional change? I can't see how it's possible to have vlen != 0 and hdrlen == 0.
emasteUnsubmitted
Not Done
Inline Actions

I think that case is not possible -- I believe vlen != 0 implies hdrlen is at least ETHER_ADDR_LEN*2. AFAICT this change is correct, but not strictly necessary.

emaste: I think that case is not possible -- I believe `vlen != 0` implies `hdrlen` is at least…
jhbAuthorUnsubmitted
Done
Inline Actions

I think I mostly made it for readability since the builtin_alloca() takes the combined size, and the new check I've added above takes the combined size. Possibly the VLAN insertion should also be moved inside this if as well, but that would add a lot of noise (maybe it can be done as a followup)

jhb: I think I mostly made it for readability since the builtin_alloca() takes the combined size…
hdr = __builtin_alloca(hdrlen + vlen); hdr = __builtin_alloca(hdrlen + vlen);
hdr += vlen; hdr += vlen;
for (left = hdrlen, hdrp = hdr; left > 0; for (left = hdrlen, hdrp = hdr; left > 0;
left -= now, hdrp += now) { left -= now, hdrp += now) {
now = MIN(left, iov->iov_len); now = MIN(left, iov->iov_len);
memcpy(hdrp, iov->iov_base, now); memcpy(hdrp, iov->iov_base, now);
iov->iov_base += now; iov->iov_base += now;
iov->iov_len -= now; iov->iov_len -= now;
▲ Show 20 Lines • Show All 1,206 Lines • Show Last 20 Lines