From owner-svn-src-head@freebsd.org Sat Jan 21 02:37:29 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2B40ECBA3C3; Sat, 21 Jan 2017 02:37:29 +0000 (UTC) (envelope-from rpokala@mac.com) Received: from mr11p00im-asmtp002.me.com (mr11p00im-asmtp002.me.com [17.110.69.253]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0CF9D191B; Sat, 21 Jan 2017 02:37:29 +0000 (UTC) (envelope-from rpokala@mac.com) Received: from process-dkim-sign-daemon.mr11p00im-asmtp002.me.com by mr11p00im-asmtp002.me.com (Oracle Communications Messaging Server 7.0.5.38.0 64bit (built Feb 26 2016)) id <0OK300100Z8AC800@mr11p00im-asmtp002.me.com>; Sat, 21 Jan 2017 02:37:22 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mac.com; s=4d515a; t=1484966242; bh=PgUnH2YQsIJ1jTVH59xewH6rc7YBt+pY+E+ZHxF2LJ4=; h=Date:Subject:From:To:Message-id:MIME-version:Content-type; b=KPBN2OPT86M9oFBmXPdiBGK6numugkfRsjiUmv6OCsQYiGMDykiRh09hKcd8h9Qdu FFdYx7qGmWub5N4g3PhnRiwZxYadaa/vViaqrDwIzEv3VY/x1VFkEqEvZtAcOjKreW qNgrD0yeVVmCgc4DUT+GSIMvPw1Ahn00o4p8KfqbEvaFj0UYpUrhqq4UyM74IsGvHg alWJdfjGnyCTAWyoxLtmXoBS095k6U+o/2mzKUW5u1RcHfAfD5Z0kSh9BgTAK/q64G r1ODjbVU4sif0k5zpusAFXjZAvKBJ6kZ9qVLdqTb+ZEdtc/TYshnxX4yWMefVGagA8 u/f0DioRoNmNQ== Received: from [172.17.133.77] (unknown [12.202.168.51]) by mr11p00im-asmtp002.me.com (Oracle Communications Messaging Server 7.0.5.38.0 64bit (built Feb 26 2016)) with ESMTPSA id <0OK3005RJZA9KH20@mr11p00im-asmtp002.me.com>; Sat, 21 Jan 2017 02:37:22 +0000 (GMT) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-01-20_16:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 clxscore=1034 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1603290000 definitions=main-1701210037 User-Agent: Microsoft-MacOutlook/f.1e.0.170107 Date: Fri, 20 Jan 2017 18:37:20 -0800 Subject: Re: svn commit: r286700 - in head: sbin/ifconfig sys/net From: Ravi Pokala Sender: "Pokala, Ravi" To: Hiren Panchasara , Alan Somers , lakshmi.n@msystechnologies.com, smh@FreeBSD.org Cc: "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Message-id: Thread-topic: svn commit: r286700 - in head: sbin/ifconfig sys/net References: <201508122021.t7CKL5wk016750@repo.freebsd.org> <20170118223827.GJ86256@strugglingcoder.info> In-reply-to: <20170118223827.GJ86256@strugglingcoder.info> MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 21 Jan 2017 02:37:29 -0000 -----Original Message----- > From: on behalf of Hiren Panchasara > Date: 2017-01-18, Wednesday at 14:38 > To: Alan Somers , , , > Cc: "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@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 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 >>> 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 */ >>>