From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id 2AA1F1B3B8 for ; Wed, 26 Sep 2018 16:56:15 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id f10-v6so6332001wrs.0 for ; Wed, 26 Sep 2018 07:56:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=JRjZD4ucwuDwWV/GSGFLrT0HmwhEdrA0iBTvJrunaJw=; b=BZKH6dGePI1iREQkv4WIkATS8FCtLCbRw3D/FVE62VeFBWD1Pb+icvRPN/5vtWTt3/ C4UmqliLPc4rcekhIauHlA6eZ3cn/Kcz/tX05G6L3n4Boa2v+o6HcRaMpJTEQY9d7+pI cOSc7bJCUgaznVpvtR63fWvg1gNSTrWseuW8LPZvCnrDGxyORZagJa91im6GBljKotoH HwW1OzY2w6SQ0PN9VPV4EWW/V6Vwsc3eyPgabWlxDl2UKPaPCispw+AhjPG4+lv5Vgd0 /Oi1E6f6cCS2e6k5plWT/v7E7hxtDH/f2kTauBdGI2qZYrB6dUd4YJQi84TJ0VZqpGuG S/9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=JRjZD4ucwuDwWV/GSGFLrT0HmwhEdrA0iBTvJrunaJw=; b=PQsGcQj3QcPrgJkL5LxZdef2He9Iakant1mo7O3LbTMOM2u0kdvg0EftWH2WSEgCxe D1IKTvqNvbvTAbIC8KHhXogtxS8gHE+mAKGsFcwcTO2vrg/Xk1mk9a4I4hTpfa1jKqSO HW4ou/VXFiyrSG46HYEccwT0inBm4SDUVjwvrd1+R9uhmcbyICe0vny3UOIEF8APoxin MaDM+pkX/D1vg3gUE1XEDRtu/h+DSCAPZGXRBzPQJXoPFYiW9J6pgUVjTEgP+0foWIFp uzTHw7K5NZlph0dwiZZbj5MHU2PjqaBCDFJTRd0hM8U/+151DjeAIeYjpPL/8kMhZUQ/ 6x4g== X-Gm-Message-State: ABuFfojUISKhqbD5Lxl3dVnP/KTP3fwzv8qxPqLmBIbOFKEnw8Uulcdg LtsJNLpQNBvVyV+QGL7aeTckj6lVNQZGxbzIeTE= X-Google-Smtp-Source: ACcGV616dSFPSDGSv0DjT2bVFA6Uecxi7SCRRJyfOrcdDd4tm6HZ3vRSXOR32MFgskuFmi70BrgWUeT/NERquhTfJ3E= X-Received: by 2002:adf:bd10:: with SMTP id j16-v6mr5039984wrh.267.1537973774482; Wed, 26 Sep 2018 07:56:14 -0700 (PDT) MIME-Version: 1.0 Sender: dan.gora@gmail.com Received: by 2002:adf:fbc1:0:0:0:0:0 with HTTP; Wed, 26 Sep 2018 07:55:33 -0700 (PDT) In-Reply-To: <671135e5-a666-4254-c5c6-672c3863146b@intel.com> References: <20180911232906.18352-1-dg@adax.com> <20180919195549.5585-1-dg@adax.com> <20180919195549.5585-2-dg@adax.com> <671135e5-a666-4254-c5c6-672c3863146b@intel.com> From: Dan Gora Date: Wed, 26 Sep 2018 11:55:33 -0300 X-Google-Sender-Auth: xdJcZ8ced66WFua-8kSOa-DNIMw Message-ID: To: Ferruh Yigit Cc: dev@dpdk.org, Igor Ryzhov , Stephen Hemminger Content-Type: text/plain; charset="UTF-8" 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 14:56:15 -0000 On Wed, Sep 26, 2018 at 10:59 AM, Ferruh Yigit wrote: > 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. > There is nothing to "reflect" to the kernel interface, nor to apply to the kernel interface. This is exactly how every other kernel driver works on link status changes. There is no "netif_set_speed()' function. When a link status change occurs the kernel driver calls netif_carrier_on/off() and prints a message like this one. > <...> > >> @@ -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. > Well, no it's not "wrong". The interface has to be up to change the link state, so this is a convenient place to do it. > I think better to call directly from test_kni(), perhaps before > test_kni_processing() call? Because then we'd have to add code to set the interface up and down again, and there really is no point. This is as good a place as any. It does not affect the data transfer tests at all. -d