svn commit: r286700 - in head: sbin/ifconfig sys/net
Ravi Pokala
rpokala at mac.com
Sat Jan 21 02:37:29 UTC 2017
-----Original Message-----
> From: <owner-src-committers at freebsd.org> on behalf of Hiren Panchasara <hiren at freebsd.org>
> Date: 2017-01-18, Wednesday at 14:38
> To: Alan Somers <asomers at freebsd.org>, <lakshmi.n at msystechnologies.com>, <rpokala at FreeBSD.org>, <smh at FreeBSD.org>
> Cc: "src-committers at freebsd.org" <src-committers at freebsd.org>, "svn-src-all at freebsd.org" <svn-src-all at freebsd.org>, "svn-src-head at freebsd.org" <svn-src-head at freebsd.org>
> Subject: Re: svn commit: r286700 - in head: sbin/ifconfig sys/net
>
> Adding the submitter and other reviewers for their comments.
>
> On 01/18/17 at 03:03P, Alan Somers wrote:
>> Is the change to lacp_port_create correct? The comment indicates that
>> fast is configurable, but it's actually constant. Later on, there's
>> some dead code that depends on the value of fast (it was dead before
>> this commit, too).
>>
>> CID: 1305734
>> CID: 1305692
You're right that in lacp_port_create(), "fast" (and "active") are both constant. I think the comment intended to indicate that "fast" could be changed via ioctl *after create*.
It seems to me that the right thing to do would be to remove both "fast" and "active" from lacp_port_create(), and have it unconditionally set "lp->lp_state" to LACP_STATE_ACTIVITY. That would eliminate the unclear comments and fix both Coverity issues, without changing any behavior.
-Ravi (rpokala@)
>> -Alan
>>
>> On Wed, Aug 12, 2015 at 2:21 PM, Hiren Panchasara <hiren at freebsd.org> wrote:
>>> Author: hiren
>>> Date: Wed Aug 12 20:21:04 2015
>>> New Revision: 286700
>>> URL: https://svnweb.freebsd.org/changeset/base/286700
>>>
>>> Log:
>>> Make LAG LACP fast timeout tunable through IOCTL.
>>>
>>> Differential Revision: D3300
>>> Submitted by: LN Sundararajan <lakshmi.n at msystechnologies>
>>> Reviewed by: wblock, smh, gnn, hiren, rpokala at panasas
>>> MFC after: 2 weeks
>>> Sponsored by: Panasas
>>>
>>> Modified:
>>> head/sbin/ifconfig/ifconfig.8
>>> head/sbin/ifconfig/iflagg.c
>>> head/sys/net/ieee8023ad_lacp.c
>>> head/sys/net/ieee8023ad_lacp.h
>>> head/sys/net/if_lagg.c
>>> head/sys/net/if_lagg.h
>>>
>>> Modified: head/sbin/ifconfig/ifconfig.8
>>> ==============================================================================
>>> --- head/sbin/ifconfig/ifconfig.8 Wed Aug 12 20:16:13 2015 (r286699)
>>> +++ head/sbin/ifconfig/ifconfig.8 Wed Aug 12 20:21:04 2015 (r286700)
>>> @@ -28,7 +28,7 @@
>>> .\" From: @(#)ifconfig.8 8.3 (Berkeley) 1/5/94
>>> .\" $FreeBSD$
>>> .\"
>>> -.Dd May 15, 2015
>>> +.Dd Aug 12, 2015
>>> .Dt IFCONFIG 8
>>> .Os
>>> .Sh NAME
>>> @@ -2396,6 +2396,10 @@ Disable local hash computation for RSS h
>>> Set a shift parameter for RSS local hash computation.
>>> Hash is calculated by using flowid bits in a packet header mbuf
>>> which are shifted by the number of this parameter.
>>> +.It Cm lacp_fast_timeout
>>> +Enable lacp fast-timeout on the interface.
>>> +.It Cm -lacp_fast_timeout
>>> +Disable lacp fast-timeout on the interface.
>>> .El
>>> .Pp
>>> The following parameters are specific to IP tunnel interfaces,
>>>
>>> Modified: head/sbin/ifconfig/iflagg.c
>>> ==============================================================================
>>> --- head/sbin/ifconfig/iflagg.c Wed Aug 12 20:16:13 2015 (r286699)
>>> +++ head/sbin/ifconfig/iflagg.c Wed Aug 12 20:21:04 2015 (r286700)
>>> @@ -115,6 +115,8 @@ setlaggsetopt(const char *val, int d, in
>>> case -LAGG_OPT_LACP_TXTEST:
>>> case LAGG_OPT_LACP_RXTEST:
>>> case -LAGG_OPT_LACP_RXTEST:
>>> + case LAGG_OPT_LACP_TIMEOUT:
>>> + case -LAGG_OPT_LACP_TIMEOUT:
>>> break;
>>> default:
>>> err(1, "Invalid lagg option");
>>> @@ -293,6 +295,8 @@ static struct cmd lagg_cmds[] = {
>>> DEF_CMD("-lacp_txtest", -LAGG_OPT_LACP_TXTEST, setlaggsetopt),
>>> DEF_CMD("lacp_rxtest", LAGG_OPT_LACP_RXTEST, setlaggsetopt),
>>> DEF_CMD("-lacp_rxtest", -LAGG_OPT_LACP_RXTEST, setlaggsetopt),
>>> + DEF_CMD("lacp_fast_timeout", LAGG_OPT_LACP_TIMEOUT, setlaggsetopt),
>>> + DEF_CMD("-lacp_fast_timeout", -LAGG_OPT_LACP_TIMEOUT, setlaggsetopt),
>>> DEF_CMD_ARG("flowid_shift", setlaggflowidshift),
>>> };
>>> static struct afswtch af_lagg = {
>>>
>>> Modified: head/sys/net/ieee8023ad_lacp.c
>>> ==============================================================================
>>> --- head/sys/net/ieee8023ad_lacp.c Wed Aug 12 20:16:13 2015 (r286699)
>>> +++ head/sys/net/ieee8023ad_lacp.c Wed Aug 12 20:21:04 2015 (r286700)
>>> @@ -522,7 +522,7 @@ lacp_port_create(struct lagg_port *lgp)
>>> int error;
>>>
>>> boolean_t active = TRUE; /* XXX should be configurable */
>>> - boolean_t fast = FALSE; /* XXX should be configurable */
>>> + boolean_t fast = FALSE; /* Configurable via ioctl */
>>>
>>> link_init_sdl(ifp, (struct sockaddr *)&sdl, IFT_ETHER);
>>> sdl.sdl_alen = ETHER_ADDR_LEN;
>>>
>>> Modified: head/sys/net/ieee8023ad_lacp.h
>>> ==============================================================================
>>> --- head/sys/net/ieee8023ad_lacp.h Wed Aug 12 20:16:13 2015 (r286699)
>>> +++ head/sys/net/ieee8023ad_lacp.h Wed Aug 12 20:21:04 2015 (r286700)
>>> @@ -251,6 +251,7 @@ struct lacp_softc {
>>> u_int32_t lsc_tx_test;
>>> } lsc_debug;
>>> u_int32_t lsc_strict_mode;
>>> + boolean_t lsc_fast_timeout; /* if set, fast timeout */
>>> };
>>>
>>> #define LACP_TYPE_ACTORINFO 1
>>>
>>> Modified: head/sys/net/if_lagg.c
>>> ==============================================================================
>>> --- head/sys/net/if_lagg.c Wed Aug 12 20:16:13 2015 (r286699)
>>> +++ head/sys/net/if_lagg.c Wed Aug 12 20:21:04 2015 (r286700)
>>> @@ -1257,6 +1257,8 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd
>>> ro->ro_opts |= LAGG_OPT_LACP_RXTEST;
>>> if (lsc->lsc_strict_mode != 0)
>>> ro->ro_opts |= LAGG_OPT_LACP_STRICT;
>>> + if (lsc->lsc_fast_timeout != 0)
>>> + ro->ro_opts |= LAGG_OPT_LACP_TIMEOUT;
>>>
>>> ro->ro_active = sc->sc_active;
>>> } else {
>>> @@ -1292,6 +1294,8 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd
>>> case -LAGG_OPT_LACP_RXTEST:
>>> case LAGG_OPT_LACP_STRICT:
>>> case -LAGG_OPT_LACP_STRICT:
>>> + case LAGG_OPT_LACP_TIMEOUT:
>>> + case -LAGG_OPT_LACP_TIMEOUT:
>>> valid = lacp = 1;
>>> break;
>>> default:
>>> @@ -1320,6 +1324,7 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd
>>> sc->sc_opts &= ~ro->ro_opts;
>>> } else {
>>> struct lacp_softc *lsc;
>>> + struct lacp_port *lp;
>>>
>>> lsc = (struct lacp_softc *)sc->sc_psc;
>>>
>>> @@ -1342,6 +1347,20 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd
>>> case -LAGG_OPT_LACP_STRICT:
>>> lsc->lsc_strict_mode = 0;
>>> break;
>>> + case LAGG_OPT_LACP_TIMEOUT:
>>> + LACP_LOCK(lsc);
>>> + LIST_FOREACH(lp, &lsc->lsc_ports, lp_next)
>>> + lp->lp_state |= LACP_STATE_TIMEOUT;
>>> + LACP_UNLOCK(lsc);
>>> + lsc->lsc_fast_timeout = 1;
>>> + break;
>>> + case -LAGG_OPT_LACP_TIMEOUT:
>>> + LACP_LOCK(lsc);
>>> + LIST_FOREACH(lp, &lsc->lsc_ports, lp_next)
>>> + lp->lp_state &= ~LACP_STATE_TIMEOUT;
>>> + LACP_UNLOCK(lsc);
>>> + lsc->lsc_fast_timeout = 0;
>>> + break;
>>> }
>>> }
>>> LAGG_WUNLOCK(sc);
>>>
>>> Modified: head/sys/net/if_lagg.h
>>> ==============================================================================
>>> --- head/sys/net/if_lagg.h Wed Aug 12 20:16:13 2015 (r286699)
>>> +++ head/sys/net/if_lagg.h Wed Aug 12 20:21:04 2015 (r286700)
>>> @@ -150,6 +150,7 @@ struct lagg_reqopts {
>>> #define LAGG_OPT_LACP_STRICT 0x10 /* LACP strict mode */
>>> #define LAGG_OPT_LACP_TXTEST 0x20 /* LACP debug: txtest */
>>> #define LAGG_OPT_LACP_RXTEST 0x40 /* LACP debug: rxtest */
>>> +#define LAGG_OPT_LACP_TIMEOUT 0x80 /* LACP timeout */
>>> u_int ro_count; /* number of ports */
>>> u_int ro_active; /* active port count */
>>> u_int ro_flapping; /* number of flapping */
>>>
More information about the svn-src-head
mailing list