From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 01AE31B171 for ; Wed, 26 Sep 2018 15:59:27 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Sep 2018 06:59:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,306,1534834800"; d="scan'208";a="260406366" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.39]) ([10.237.221.39]) by orsmga005.jf.intel.com with ESMTP; 26 Sep 2018 06:59:25 -0700 To: Dan Gora , dev@dpdk.org Cc: Igor Ryzhov , Stephen Hemminger References: <20180911232906.18352-1-dg@adax.com> <20180919195549.5585-1-dg@adax.com> <20180919195549.5585-2-dg@adax.com> From: Ferruh Yigit Openpgp: preference=signencrypt Message-ID: <671135e5-a666-4254-c5c6-672c3863146b@intel.com> Date: Wed, 26 Sep 2018 14:59:23 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180919195549.5585-2-dg@adax.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 1/5] kni: add API to set link status on kernel interface X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Sep 2018 13:59:28 -0000 On 9/19/2018 8:55 PM, Dan Gora wrote: > Add a new API function to KNI, rte_kni_update_link() to allow DPDK > applications to update the link status for KNI network interfaces in > the linux kernel. > > Signed-off-by: Dan Gora <...> > +int __rte_experimental > +rte_kni_update_link(struct rte_kni *kni, struct rte_eth_link *link) > +{ > + char path[64]; > + char carrier[2]; > + const char *new_carrier; > + int fd, ret; > + > + if (kni == NULL || link == NULL) > + return -1; > + > + snprintf(path, sizeof(path), "/sys/devices/virtual/net/%s/carrier", > + kni->name); > + > + fd = open(path, O_RDWR); > + if (fd == -1) { > + RTE_LOG(ERR, KNI, "Failed to open file: %s.\n", path); > + return -1; > + } > + > + ret = read(fd, carrier, 2); > + if (ret < 1) { > + /* Cannot read carrier until interface is marked > + * 'up', so don't log an error. > + */ > + close(fd); > + return -1; > + } > + > + new_carrier = (link->link_status == ETH_LINK_UP) ? "1" : "0"; > + ret = write(fd, new_carrier, 1); > + if (ret < 1) { > + RTE_LOG(ERR, KNI, "Failed to write file: %s.\n", path); > + close(fd); > + return -1; > + } > + > + if (strncmp(carrier, new_carrier, 1)) { > + if (link->link_status == ETH_LINK_UP) { > + RTE_LOG(INFO, KNI, "%s NIC Link is Up %d Mbps %s %s.\n", > + kni->name, > + link->link_speed, > + link->link_autoneg ? "(AutoNeg)" : "(Fixed)", > + link->link_duplex ? > + "Full Duplex" : "Half Duplex"); These link values are coming from user and not reflected to the kni interface, printing them here can cause a misunderstanding that they have been applied. I think only link status should be printed here. <...> > @@ -148,9 +239,16 @@ test_kni_loop(__rte_unused void *arg) > ret = -1; > if (system(IFCONFIG TEST_KNI_PORT" mtu 1400") == -1) > ret = -1; > + > + ret = kni_change_link(); > + if (ret != 0) { > + test_kni_processing_flag = 1; > + return ret; > + } I thinks this is wrong place to call kni_change_link(), this is test_kni_loop() created by test_kni_processing() that does packet processing tests. I think better to call directly from test_kni(), perhaps before test_kni_processing() call?