Page MenuHomeFreeBSD
Differential D23963 Diff 71153 sys/netgraph/ng_bridge.c

Changeset View

Standalone View

View Options

sys/netgraph/ng_bridge.c

Show First 20 Lines • Show All 85 Lines • ▼ Show 20 Lines
#else #else
#define M_NETGRAPH_BRIDGE M_NETGRAPH #define M_NETGRAPH_BRIDGE M_NETGRAPH
#endif #endif
/* Per-link private data */ /* Per-link private data */
struct ng_bridge_link { struct ng_bridge_link {
hook_p hook; /* netgraph hook */ hook_p hook; /* netgraph hook */
u_int16_t loopCount; /* loop ignore timer */ u_int16_t loopCount; /* loop ignore timer */
int learnMac : 1, /* autolearn macs */
afedorovUnsubmitted
Done
Inline Actions

Maybe better to use unsigned values.

afedorov: Maybe better to use unsigned values.
sendUnknown : 1;/* send unknown macs out */
struct ng_bridge_link_stats stats; /* link stats */ struct ng_bridge_link_stats stats; /* link stats */
}; };
/* Per-node private data */ /* Per-node private data */
struct ng_bridge_private { struct ng_bridge_private {
struct ng_bridge_bucket *tab; /* hash table bucket array */ struct ng_bridge_bucket *tab; /* hash table bucket array */
struct ng_bridge_config conf; /* node configuration */ struct ng_bridge_config conf; /* node configuration */
node_p node; /* netgraph node */ node_p node; /* netgraph node */
▲ Show 20 Lines • Show All 227 Lines • ▼ Show 20 Lines
/* /*
* Method for attaching a new hook * Method for attaching a new hook
*/ */
static int static int
ng_bridge_newhook(node_p node, hook_p hook, const char *name) ng_bridge_newhook(node_p node, hook_p hook, const char *name)
{ {
const priv_p priv = NG_NODE_PRIVATE(node); const priv_p priv = NG_NODE_PRIVATE(node);
/* Check for a link hook */
if (strlen(name) > strlen(NG_BRIDGE_HOOK_LINK_PREFIX)) {
char linkName[NG_HOOKSIZ]; char linkName[NG_HOOKSIZ];
u_int32_t linkNum; u_int32_t linkNum;
link_p link; link_p link;
const char * prefix = NG_BRIDGE_HOOK_LINK_PREFIX;
afedorovUnsubmitted
Done
Inline Actions

To match the style: *prefix

afedorov: To match the style: *prefix
bool isUplink;
/* Check for a link hook */
if (strlen(name) <= strlen(prefix))
return (EINVAL); /* Unknown hook name */
isUplink = (name[0] == 'u');
if (isUplink)
prefix = NG_BRIDGE_HOOK_UPLINK_PREFIX;
/* primitive parsing */ /* primitive parsing */
linkNum = strtoul(name + strlen(NG_BRIDGE_HOOK_LINK_PREFIX), linkNum = strtoul(name + strlen(prefix), NULL, 10);
NULL, 10);
/* validation by comparing against the reconstucted name */ /* validation by comparing against the reconstucted name */
snprintf(linkName, sizeof(linkName), snprintf(linkName, sizeof(linkName), "%s%u", prefix, linkNum);
"%s%u", NG_BRIDGE_HOOK_LINK_PREFIX,
linkNum);
if (strcmp(linkName, name) != 0) if (strcmp(linkName, name) != 0)
kpUnsubmitted
Done
Inline Actions

This check is redundant. malloc(M_WAITOK) never fails. It may wait forever, but will never return NULL.

This was already redundant before this patch, so we can address this later.

kp: This check is redundant. malloc(M_WAITOK) never fails. It may wait forever, but will never…
donnerAuthorUnsubmitted
Done
Inline Actions

Adding a link to a busy bridge should not be stall the bridge function, so I'd prefer to fail.

donner: Adding a link to a busy bridge should not be stall the bridge function, so I'd prefer to fail.
return (EINVAL); return (EINVAL);
if (linkNum == 0 && isUplink)
return (EINVAL);
if(NG_PEER_NODE(hook) == node) if(NG_PEER_NODE(hook) == node)
return (ELOOP); return (ELOOP);
link = malloc(sizeof(*link), M_NETGRAPH_BRIDGE, link = malloc(sizeof(*link), M_NETGRAPH_BRIDGE, M_WAITOK|M_ZERO);
M_WAITOK|M_ZERO);
if (link == NULL) if (link == NULL)
return (ENOMEM); return (ENOMEM);
link->hook = hook; link->hook = hook;
if (isUplink) {
link->learnMac = 0;
link->sendUnknown = 1;
} else {
link->learnMac = 1;
link->sendUnknown = 1;
}
NG_HOOK_SET_PRIVATE(hook, link); NG_HOOK_SET_PRIVATE(hook, link);
priv->numLinks++; priv->numLinks++;
return (0); return (0);
} }
/* Unknown hook name */
return (EINVAL);
}
/* /*
* Receive a control message * Receive a control message
*/ */
static int static int
ng_bridge_reset_link(hook_p hook, void *arg __unused) ng_bridge_reset_link(hook_p hook, void *arg __unused)
{ {
link_p priv = NG_HOOK_PRIVATE(hook); link_p priv = NG_HOOK_PRIVATE(hook);
▲ Show 20 Lines • Show All 55 Lines • ▼ Show 20 Lines case NGM_BRIDGE_GET_TABLE:
error = ENOMEM; error = ENOMEM;
break; break;
} }
ary = (struct ng_bridge_host_tbl_ary *)resp->data; ary = (struct ng_bridge_host_tbl_ary *)resp->data;
ary->numHosts = priv->numHosts; ary->numHosts = priv->numHosts;
i = 0; i = 0;
for (bucket = 0; bucket < priv->numBuckets; bucket++) { for (bucket = 0; bucket < priv->numBuckets; bucket++) {
SLIST_FOREACH(hent, &priv->tab[bucket], next) { SLIST_FOREACH(hent, &priv->tab[bucket], next) {
char * name = NG_HOOK_NAME(hent->host.link->hook);
char * prefix = name[0] == 'u' ?
yuripvUnsubmitted
Done
Inline Actions

no space between * and variable names.

yuripv: no space between `*` and variable names.
yuripvUnsubmitted
Done
Inline Actions

const?

yuripv: const?
donnerAuthorUnsubmitted
Done
Inline Actions

good point

donner: good point
NG_BRIDGE_HOOK_UPLINK_PREFIX :
NG_BRIDGE_HOOK_LINK_PREFIX;
memcpy(ary->hosts[i].addr, memcpy(ary->hosts[i].addr,
hent->host.addr, hent->host.addr,
sizeof(ary->hosts[i].addr)); sizeof(ary->hosts[i].addr));
ary->hosts[i].age = hent->host.age; ary->hosts[i].age = hent->host.age;
ary->hosts[i].staleness = ary->hosts[i].staleness =
hent->host.staleness; hent->host.staleness;
ary->hosts[i].linkNum = strtol( ary->hosts[i].linkNum = strtol(
NG_HOOK_NAME(hent->host.link->hook) + name + strlen(prefix), NULL, 10);
strlen(NG_BRIDGE_HOOK_LINK_PREFIX),
NULL, 10);
i++; i++;
} }
} }
break; break;
} }
} }
/* If already handled break, otherwise use new ABI. */ /* If already handled break, otherwise use new ABI. */
if (resp != NULL || error != 0) if (resp != NULL || error != 0)
Show All 37 Lines case NGM_BRIDGE_RESET:
/* Reset all loop detection counters and stats */ /* Reset all loop detection counters and stats */
NG_NODE_FOREACH_HOOK(node, ng_bridge_reset_link, NULL, NG_NODE_FOREACH_HOOK(node, ng_bridge_reset_link, NULL,
rethook); rethook);
break; break;
} }
case NGM_BRIDGE_GET_STATS: case NGM_BRIDGE_GET_STATS:
case NGM_BRIDGE_CLR_STATS: case NGM_BRIDGE_CLR_STATS:
case NGM_BRIDGE_GETCLR_STATS: case NGM_BRIDGE_GETCLR_STATS: {
{
hook_p hook; hook_p hook;
link_p link; link_p link;
char linkName[NG_HOOKSIZ]; char linkName[NG_HOOKSIZ];
int linkNum;
/* Get link number */ /* Get link number */
if (msg->header.arglen != sizeof(u_int32_t)) { if (msg->header.arglen != sizeof(u_int32_t)) {
error = EINVAL; error = EINVAL;
break; break;
} }
linkNum = *((int32_t *)msg->data);
if (linkNum < 0)
yuripvUnsubmitted
Done
Inline Actions

might be more readable with braces around multiline statements

yuripv: might be more readable with braces around multiline statements
donnerAuthorUnsubmitted
Done
Inline Actions

This would violate style(9) ... at least a bit.

donner: This would violate style(9) ... at least a bit.
snprintf(linkName, sizeof(linkName), snprintf(linkName, sizeof(linkName),
"%s%u", NG_BRIDGE_HOOK_LINK_PREFIX, "%s%u", NG_BRIDGE_HOOK_UPLINK_PREFIX, -linkNum);
*((u_int32_t *)msg->data)); else
snprintf(linkName, sizeof(linkName),
"%s%u", NG_BRIDGE_HOOK_LINK_PREFIX, linkNum);
if ((hook = ng_findhook(node, linkName)) == NULL) { if ((hook = ng_findhook(node, linkName)) == NULL) {
printf("Not found %s for %d\n", linkName, linkNum);
afedorovUnsubmitted
Done
Inline Actions

Is this debug output really needed?

afedorov: Is this debug output really needed?
error = ENOTCONN; error = ENOTCONN;
break; break;
} }
link = NG_HOOK_PRIVATE(hook); link = NG_HOOK_PRIVATE(hook);
/* Get/clear stats */ /* Get/clear stats */
if (msg->header.cmd != NGM_BRIDGE_CLR_STATS) { if (msg->header.cmd != NGM_BRIDGE_CLR_STATS) {
NG_MKRESPONSE(resp, msg, NG_MKRESPONSE(resp, msg,
▲ Show 20 Lines • Show All 76 Lines • ▼ Show 20 Linesng_bridge_send_ctx(hook_p dst, void *arg)
struct mbuf *m2 = NULL; struct mbuf *m2 = NULL;
int error = 0; int error = 0;
/* Skip incoming link */ /* Skip incoming link */
if (destLink == ctx->incoming) { if (destLink == ctx->incoming) {
return (1); return (1);
} }
/* Skip sending unknowns to undesired links */
if (!ctx->manycast && !destLink->sendUnknown)
return (1);
if (ctx->foundFirst == NULL) { if (ctx->foundFirst == NULL) {
/* /*
* This is the first usable link we have found. * This is the first usable link we have found.
* Reserve it for the originals. * Reserve it for the originals.
* If we never find another we save a copy. * If we never find another we save a copy.
*/ */
ctx->foundFirst = destLink; ctx->foundFirst = destLink;
return (1); return (1);
▲ Show 20 Lines • Show All 129 Lines • ▼ Show 20 Lines if (host->link != ctx.incoming) {
NG_FREE_M(ctx.m); NG_FREE_M(ctx.m);
return (ELOOP); /* XXX appropriate? */ return (ELOOP); /* XXX appropriate? */
} }
/* Move host over to new link */ /* Move host over to new link */
host->link = ctx.incoming; host->link = ctx.incoming;
host->age = 0; host->age = 0;
} }
} else { } else if (ctx.incoming->learnMac) {
if (!ng_bridge_put(priv, eh->ether_shost, ctx.incoming)) { if (!ng_bridge_put(priv, eh->ether_shost, ctx.incoming)) {
ctx.incoming->stats.memoryFailures++; ctx.incoming->stats.memoryFailures++;
NG_FREE_ITEM(item); NG_FREE_ITEM(item);
NG_FREE_M(ctx.m); NG_FREE_M(ctx.m);
return (ENOMEM); return (ENOMEM);
} }
} }
▲ Show 20 Lines • Show All 342 Lines • Show Last 20 Lines