Changeset View
Changeset View
Standalone View
Standalone View
sys/net/if_lagg.c
Show First 20 Lines • Show All 909 Lines • ▼ Show 20 Lines | case SIOCSIFCAP: | ||||
/* Update lagg interface capabilities */ | /* Update lagg interface capabilities */ | ||||
LAGG_XLOCK(sc); | LAGG_XLOCK(sc); | ||||
lagg_capabilities(sc); | lagg_capabilities(sc); | ||||
LAGG_XUNLOCK(sc); | LAGG_XUNLOCK(sc); | ||||
VLAN_CAPABILITIES(sc->sc_ifp); | VLAN_CAPABILITIES(sc->sc_ifp); | ||||
break; | break; | ||||
case SIOCSIFMTU: | |||||
/* Do not allow the MTU to be changed once joined */ | |||||
error = EINVAL; | |||||
break; | |||||
default: | default: | ||||
goto fallback; | goto fallback; | ||||
} | } | ||||
return (error); | return (error); | ||||
fallback: | fallback: | ||||
if (lp != NULL && lp->lp_ioctl != NULL) | if (lp != NULL && lp->lp_ioctl != NULL) | ||||
▲ Show 20 Lines • Show All 528 Lines • ▼ Show 20 Lines | case SIOCSIFCAP: | ||||
} | } | ||||
lagg_capabilities(sc); | lagg_capabilities(sc); | ||||
LAGG_XUNLOCK(sc); | LAGG_XUNLOCK(sc); | ||||
VLAN_CAPABILITIES(ifp); | VLAN_CAPABILITIES(ifp); | ||||
error = 0; | error = 0; | ||||
break; | break; | ||||
case SIOCSIFMTU: | case SIOCSIFMTU: | ||||
/* Do not allow the MTU to be directly changed */ | if (ifr->ifr_mtu < 576) { | ||||
error = EINVAL; | error = EINVAL; | ||||
break; | |||||
} | |||||
if (CK_SLIST_EMPTY(&sc->sc_ports)) { | |||||
sc->sc_ifp->if_mtu = ifr->ifr_mtu; | |||||
break; | |||||
} | |||||
LAGG_XLOCK(sc); | |||||
CK_SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) { | |||||
if (lp->lp_ifp->if_mtu != ifr->ifr_mtu) { | |||||
if_printf(sc->sc_ifp, | |||||
"invalid MTU: %u(%s) != %d\n", | |||||
0mp: I'm not sure if that's important but this is not compliant with [[ https://www.freebsd. | |||||
Done Inline ActionsWhoops, I've linked style(9) incorrectly in the previous comment. Sorry about that! 0mp: Whoops, I've linked [[ https://www.freebsd.org/cgi/man.cgi? | |||||
Done Inline ActionsThat is correct!!! I can fix that when I commit it. araujo: That is correct!!! I can fix that when I commit it. | |||||
lp->lp_ifp->if_mtu, | |||||
lp->lp_ifp->if_xname, ifr->ifr_mtu); | |||||
error = EINVAL; | |||||
break; | |||||
} | |||||
} | |||||
if (!error) | |||||
Done Inline ActionsShouldn't it be if (error != 0) as in line 1675? 0mp: Shouldn't it be ` if (error != 0)` as in line 1675? | |||||
Done Inline ActionsIt has the same result! araujo: It has the same result! | |||||
Done Inline ActionsTrue. It's just that style(9) says: Do not use ! for tests unless it is a boolean, e.g., use: if (*p == '\0') not: if (!*p) but I'm pretty new here so I'm happy with whatever you find reasonable :) 0mp: True. It's just that style(9) says:
```
Do not use ! for tests unless it is a boolean, e. | |||||
Done Inline ActionsIt should be if (error == 0) though. The MTU will only change if there is no error. freqlabs: It should be `if (error == 0)` though. The MTU will only change if there is no error. | |||||
sc->sc_ifp->if_mtu = ifr->ifr_mtu; | |||||
LAGG_XUNLOCK(sc); | |||||
Done Inline ActionsWhat is this 576? 0mp: What is this `576`? | |||||
Done Inline ActionsIt is the minimum accepted datagram, it cannot be less than 576 octets. araujo: It is the minimum accepted datagram, it cannot be less than 576 octets. | |||||
Done Inline ActionsOh, cool. I just thought that we should use a constant for values like this one but I guess that it's not really necessary in this case, right? 0mp: Oh, cool.
I just thought that we should use a constant for values like this one but I guess… | |||||
break; | break; | ||||
default: | default: | ||||
error = ether_ioctl(ifp, cmd, data); | error = ether_ioctl(ifp, cmd, data); | ||||
break; | break; | ||||
} | } | ||||
return (error); | return (error); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 724 Lines • Show Last 20 Lines |
I'm not sure if that's important but this is not compliant with style(9). It should be indented with 4 spaces, right?