sfxge: improve error handling in ef10_ev_rx()
ClosedPublic
Actions

Authored by arybchik on Jan 18 2016, 7:04 AM.

Details

Reviewers

philip
np
gnn
bz

Commits

rS294310: sfxge: improve error handling in ef10_ev_rx()

Summary

Ensure that checksum flags and L3/L4 fields are ignored
if lower level errors are reported in the event.

Remove checks for CRC0_ERR (bad iSCSI header CRC) and
CRC1_ERR (bad iSCSI payload or FCoE/FCoIP CRC) as they
are not used by any existing code.

Submitted by: Andy Moreton <amoreton at solarflare.com>
Sponsored by: Solarflare Communications, Inc.
MFC after: 2 days

Test Plan

Build tested. Driver loaded/unloaded.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 2166
Build 2175: arc lint + arc unit

Event Timeline

arybchik updated this revision to Diff 12407.Jan 18 2016, 7:04 AM

arybchik retitled this revision from to sfxge: improve error handling in ef10_ev_rx().

arybchik updated this object.

arybchik edited the test plan for this revision. (Show Details)

arybchik added reviewers: gnn, philip, np, bz.

Herald added a subscriber: imp. · View Herald TranscriptJan 18 2016, 7:04 AM

Closed by commit rS294310: sfxge: improve error handling in ef10_ev_rx() (authored by arybchik). · Explain WhyJan 19 2016, 6:08 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

head/

sys/

dev/

sfxge/

common/

hunt_ev.c

176 lines

Diff 12407

View Options

head/sys/dev/sfxge/common/hunt_ev.c

Show First 20 Lines • Show All 530 Lines • ▼ Show 20 Lines	if (vs2 != NULL)
TAILQ_INSERT_BEFORE(vs2, vs, link);		TAILQ_INSERT_BEFORE(vs2, vs, link);
else		else
TAILQ_INSERT_TAIL(&vnet_constructors, vs, link);		TAILQ_INSERT_TAIL(&vnet_constructors, vs, link);

/*		/*
* Invoke the constructor on all the existing vnets when it is		* Invoke the constructor on all the existing vnets when it is
* registered.		* registered.
*/		*/
		VNET_LIST_RLOCK();
VNET_FOREACH(vnet) {		VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet);		CURVNET_SET_QUIET(vnet);
vs->func(vs->arg);		vs->func(vs->arg);
CURVNET_RESTORE();		CURVNET_RESTORE();
}		}
		VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK();		VNET_SYSINIT_WUNLOCK();
}		}

void		void
vnet_deregister_sysinit(void *arg)		vnet_deregister_sysinit(void *arg)
{		{
struct vnet_sysinit *vs;		struct vnet_sysinit *vs;

Show All 34 Lines	vnet_deregister_sysuninit(void *arg)
struct vnet *vnet;		struct vnet *vnet;

vs = arg;		vs = arg;

/*		/*
* Invoke the destructor on all the existing vnets when it is		* Invoke the destructor on all the existing vnets when it is
* deregistered.		* deregistered.
*/		*/
		VNET_LIST_RLOCK();
		zleiUnsubmitted Not Done Inline Actions In `vnet_register_sysinit()` we have lock order VNET_SYSINIT_WLOCK(); VNET_LIST_RLOCK; I think it is best to make lock order consistent with that in `vnet_register_sysinit()`, although dead lock is not possible. zlei: In `vnet_register_sysinit()` we have lock order ``` VNET_SYSINIT_WLOCK(); VNET_LIST_RLOCK; ```…
VNET_SYSINIT_WLOCK();		VNET_SYSINIT_WLOCK();
VNET_FOREACH(vnet) {		VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet);		CURVNET_SET_QUIET(vnet);
vs->func(vs->arg);		vs->func(vs->arg);
CURVNET_RESTORE();		CURVNET_RESTORE();
}		}

/* Remove the destructor from the global list of vnet destructors. */		/* Remove the destructor from the global list of vnet destructors. */
TAILQ_REMOVE(&vnet_destructors, vs, link);		TAILQ_REMOVE(&vnet_destructors, vs, link);
		VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK();		VNET_SYSINIT_WUNLOCK();
}		}
		mjgUnsubmitted Not Done Inline Actions you should unlock in the same order for consistency the VNET_FOREACH macro should presumably assert on the lock? mjg: you should unlock in the same order for consistency the VNET_FOREACH macro should presumably…
		kpAuthorUnsubmitted Not Done Inline Actions It should, but the locking for the vnet list uses two different locks for reasons I do not currently understand. So we'd have to `MPASS(sx_locked(&vnet_sxlock) \|\| rw_rowned(&vnet_rwlock));`. Which is doubly annoying because `rw_rowned()` doesn't exist. kp: It should, but the locking for the vnet list uses two different locks for reasons I do not…
		mjgUnsubmitted Not Done Inline Actions that is a gap i'm going to plug mjg: that is a gap i'm going to plug
		zleiUnsubmitted Not Done Inline Actions you should unlock in the same order for consistency I'd prefer to unlock in the reverse order. That is more natural for me. Anyway unlock order does not matter much. the VNET_FOREACH macro should presumably assert on the lock? `ipreass_drain()` in `sys/netinet/ip_reass.c` also miss read lock. I think it is good to have read lock assert in macro VNET_FOREACH . zlei: > you should unlock in the same order for consistency I'd prefer to unlock in the reverse…

/*		/*
* Invoke all registered vnet constructors on the current vnet. Used during		* Invoke all registered vnet constructors on the current vnet. Used during
* vnet construction. The caller is responsible for ensuring the new vnet is		* vnet construction. The caller is responsible for ensuring the new vnet is
* the current vnet and that the vnet_sysinit_sxlock lock is locked.		* the current vnet and that the vnet_sysinit_sxlock lock is locked.
*/		*/
static void		static void
vnet_sysinit(void)		vnet_sysinit(void)
▲ Show 20 Lines • Show All 235 Lines • Show Last 20 Lines

sfxge: improve error handling in ef10_ev_rx()ClosedPublicActions