in_mcast.c: need if_addr_lock around inm_release_deferred
ClosedPublic
Actions

Authored by rlibby on Jun 24 2019, 4:28 PM.

Details

Reviewers

glebius
• hselasky

Commits

rS355103: in_mcast.c: need if_addr_lock around inm_release_deferred

Summary

Apply a similar fix as for in6_mcast.c.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25154
Build 23848: arc lint + arc unit

Event Timeline

rlibby created this revision.Jun 24 2019, 4:28 PM

Harbormaster completed remote builds in B25003: Diff 58952.Jun 24 2019, 4:28 PM

Fix up for conflict with r349369.

Harbormaster completed remote builds in B25154: Diff 59265.Jul 1 2019, 4:04 PM

rlibby added a subscriber: • hselasky.Jul 1 2019, 4:08 PM

Patch looks good to me. Did you run with this patch?

rlibby mentioned this in D20739: in6_joingroup_locked: need if_addr_lock around in6m_disconnect_locked.Jul 2 2019, 4:20 PM

In D20740#451134, @hselasky wrote:

Patch looks good to me. Did you run with this patch?

Finally got around to testing this. I have been running with this patch, but wanted to demonstrate that the error paths were actually covered.

I was able to hit the first path (in in_joingroup_locked()) with an alloc M_NOWAIT fail point. I think the second one (in in_join_group()) is not possible to hit with the current code flow (is_new && imf → imf->imf_inm == NULL). But I inserted a fail point between in_joingroup_locked() and inm_acquire() and was able to light it up that way. No panics or witness warnings.

• hselasky accepted this revision.Nov 25 2019, 9:49 PM

This revision is now accepted and ready to land.Nov 25 2019, 9:49 PM

Closed by commit rS355103: in_mcast.c: need if_addr_lock around inm_release_deferred (authored by rlibby). · Explain WhyNov 25 2019, 10:25 PM

This revision was automatically updated to reflect the committed changes.

rlibby added a commit: rS355103: in_mcast.c: need if_addr_lock around inm_release_deferred.

Herald added a subscriber: imp. · View Herald TranscriptNov 25 2019, 10:25 PM

Revision Contents
Changeset List

Path

Size

sys/

netinet/

in_mcast.c

4 lines

Diff 59265

View Options

sys/netinet/in_mcast.c

Show All 37 Lines
#include <sys/limits.h>		#include <sys/limits.h>
#include <net/ethernet.h>		#include <net/ethernet.h>
#include <net/if_dl.h>		#include <net/if_dl.h>

#include "common/efx.h"		#include "common/efx.h"

#include "sfxge.h"		#include "sfxge.h"

		#define SFXGE_PARAM_STATS_UPDATE_PERIOD_MS \
		SFXGE_PARAM(stats_update_period_ms)
		static int sfxge_stats_update_period_ms = SFXGE_STATS_UPDATE_PERIOD_MS;
		TUNABLE_INT(SFXGE_PARAM_STATS_UPDATE_PERIOD_MS,
		&sfxge_stats_update_period_ms);
		SYSCTL_INT(_hw_sfxge, OID_AUTO, stats_update_period_ms, CTLFLAG_RDTUN,
		&sfxge_stats_update_period_ms, 0,
		"netstat interface statistics update period in milliseconds");

static int sfxge_phy_cap_mask(struct sfxge_softc , int, uint32_t );		static int sfxge_phy_cap_mask(struct sfxge_softc , int, uint32_t );

static int		static int
sfxge_mac_stat_update(struct sfxge_softc *sc)		sfxge_mac_stat_update(struct sfxge_softc *sc)
{		{
struct sfxge_port *port = &sc->port;		struct sfxge_port *port = &sc->port;
efsys_mem_t *esmp = &(port->mac_stats.dma_buf);		efsys_mem_t *esmp = &(port->mac_stats.dma_buf);
clock_t now;		clock_t now;
unsigned int min_ticks;		unsigned int min_ticks;
unsigned int count;		unsigned int count;
int rc;		int rc;

SFXGE_PORT_LOCK_ASSERT_OWNED(port);		SFXGE_PORT_LOCK_ASSERT_OWNED(port);

if (__predict_false(port->init_state != SFXGE_PORT_STARTED)) {		if (__predict_false(port->init_state != SFXGE_PORT_STARTED)) {
rc = 0;		rc = 0;
goto out;		goto out;
}		}

min_ticks = (unsigned int)hz * SFXGE_STATS_UPDATE_PERIOD_MS / 1000;		min_ticks = (unsigned int)hz * port->stats_update_period_ms / 1000;

now = ticks;		now = ticks;
if ((unsigned int)(now - port->mac_stats.update_time) < min_ticks) {		if ((unsigned int)(now - port->mac_stats.update_time) < min_ticks) {
rc = 0;		rc = 0;
goto out;		goto out;
}		}

port->mac_stats.update_time = now;		port->mac_stats.update_time = now;
▲ Show 20 Lines • Show All 436 Lines • ▼ Show 20 Lines	sfxge_port_start(struct sfxge_softc *sc)
if_addr_runlock(ifp);		if_addr_runlock(ifp);
if ((rc = efx_mac_addr_set(enp, mac_addr)) != 0)		if ((rc = efx_mac_addr_set(enp, mac_addr)) != 0)
goto fail4;		goto fail4;

sfxge_mac_filter_set_locked(sc);		sfxge_mac_filter_set_locked(sc);

/* Update MAC stats by DMA every period */		/* Update MAC stats by DMA every period */
if ((rc = efx_mac_stats_periodic(enp, &port->mac_stats.dma_buf,		if ((rc = efx_mac_stats_periodic(enp, &port->mac_stats.dma_buf,
SFXGE_STATS_UPDATE_PERIOD_MS,		port->stats_update_period_ms,
B_FALSE)) != 0)		B_FALSE)) != 0)
goto fail6;		goto fail6;

if ((rc = efx_mac_drain(enp, B_FALSE)) != 0)		if ((rc = efx_mac_drain(enp, B_FALSE)) != 0)
goto fail8;		goto fail8;

if ((rc = sfxge_phy_cap_mask(sc, sc->media.ifm_cur->ifm_media,		if ((rc = sfxge_phy_cap_mask(sc, sc->media.ifm_cur->ifm_media,
&phy_cap_mask)) != 0)		&phy_cap_mask)) != 0)
▲ Show 20 Lines • Show All 141 Lines • ▼ Show 20 Lines	sfxge_port_fini(struct sfxge_softc *sc)
sfxge_dma_free(esmp);		sfxge_dma_free(esmp);
free(port->mac_stats.decode_buf, M_SFXGE);		free(port->mac_stats.decode_buf, M_SFXGE);

SFXGE_PORT_LOCK_DESTROY(port);		SFXGE_PORT_LOCK_DESTROY(port);

port->sc = NULL;		port->sc = NULL;
}		}

		static uint16_t
		sfxge_port_stats_update_period_ms(struct sfxge_softc *sc)
		{
		int period_ms = sfxge_stats_update_period_ms;

		if (period_ms < 0) {
		device_printf(sc->dev,
		"treat negative stats update period %d as 0 (disable)\n",
		period_ms);
		period_ms = 0;
		} else if (period_ms > UINT16_MAX) {
		device_printf(sc->dev,
		"treat too big stats update period %d as %u\n",
		period_ms, UINT16_MAX);
		period_ms = UINT16_MAX;
		}

		return period_ms;
		}

int		int
sfxge_port_init(struct sfxge_softc *sc)		sfxge_port_init(struct sfxge_softc *sc)
{		{
struct sfxge_port *port;		struct sfxge_port *port;
struct sysctl_ctx_list *sysctl_ctx;		struct sysctl_ctx_list *sysctl_ctx;
struct sysctl_oid *sysctl_tree;		struct sysctl_oid *sysctl_tree;
efsys_mem_t mac_stats_buf, phy_stats_buf;		efsys_mem_t mac_stats_buf, phy_stats_buf;
int rc;		int rc;
Show All 32 Lines	SYSCTL_ADD_PROC(sysctl_ctx, SYSCTL_CHILDREN(sysctl_tree), OID_AUTO,
sfxge_port_link_fc_handler, "IU", "link flow control mode");		sfxge_port_link_fc_handler, "IU", "link flow control mode");
#endif		#endif

DBGPRINT(sc->dev, "alloc MAC stats");		DBGPRINT(sc->dev, "alloc MAC stats");
port->mac_stats.decode_buf = malloc(EFX_MAC_NSTATS * sizeof(uint64_t),		port->mac_stats.decode_buf = malloc(EFX_MAC_NSTATS * sizeof(uint64_t),
M_SFXGE, M_WAITOK \| M_ZERO);		M_SFXGE, M_WAITOK \| M_ZERO);
if ((rc = sfxge_dma_alloc(sc, EFX_MAC_STATS_SIZE, mac_stats_buf)) != 0)		if ((rc = sfxge_dma_alloc(sc, EFX_MAC_STATS_SIZE, mac_stats_buf)) != 0)
goto fail2;		goto fail2;
		port->stats_update_period_ms = sfxge_port_stats_update_period_ms(sc);
sfxge_mac_stat_init(sc);		sfxge_mac_stat_init(sc);

port->init_state = SFXGE_PORT_INITIALIZED;		port->init_state = SFXGE_PORT_INITIALIZED;

DBGPRINT(sc->dev, "success");		DBGPRINT(sc->dev, "success");
return (0);		return (0);

fail2:		fail2:
▲ Show 20 Lines • Show All 274 Lines • Show Last 20 Lines

in_mcast.c: need if_addr_lock around inm_release_deferredClosedPublicActions