From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0072.outbound.protection.outlook.com [104.47.32.72]) by dpdk.org (Postfix) with ESMTP id 9C7A01B61A for ; Tue, 26 Dec 2017 11:08:18 +0100 (CET) Received: from BLUPR0301CA0038.namprd03.prod.outlook.com (10.162.113.176) by CY4PR03MB2695.namprd03.prod.outlook.com (10.173.43.138) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.323.15; Tue, 26 Dec 2017 10:08:17 +0000 Received: from BN1AFFO11FD041.protection.gbl (2a01:111:f400:7c10::122) by BLUPR0301CA0038.outlook.office365.com (2a01:111:e400:5259::48) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.345.14 via Frontend Transport; Tue, 26 Dec 2017 10:08:16 +0000 Received-SPF: Fail (protection.outlook.com: domain of nxp.com does not designate 192.88.168.50 as permitted sender) receiver=protection.outlook.com; client-ip=192.88.168.50; helo=tx30smr01.am.freescale.net; Received: from tx30smr01.am.freescale.net (192.88.168.50) by BN1AFFO11FD041.mail.protection.outlook.com (10.58.52.252) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.20.345.12 via Frontend Transport; Tue, 26 Dec 2017 10:07:57 +0000 Received: from [10.232.133.65] ([10.232.133.65]) by tx30smr01.am.freescale.net (8.14.3/8.14.0) with ESMTP id vBQA8DW2008497; Tue, 26 Dec 2017 03:08:14 -0700 To: Ferruh Yigit , References: <1512042367-6361-1-git-send-email-hemant.agrawal@nxp.com> <1146be98-43a9-91f5-8ad2-53ae31b48b4c@intel.com> From: Hemant Agrawal Message-ID: <8b76ea77-73cf-25a6-2532-c380762f4c6a@nxp.com> Date: Tue, 26 Dec 2017 15:38:13 +0530 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1146be98-43a9-91f5-8ad2-53ae31b48b4c@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-EOPAttributedMessage: 0 X-Matching-Connectors: 131587564779941358; (91ab9b29-cfa4-454e-5278-08d120cd25b8); () X-Forefront-Antispam-Report: CIP:192.88.168.50; IPV:NLI; CTRY:US; EFV:NLI; SFV:NSPM; SFS:(10009020)(336005)(396003)(376002)(346002)(39860400002)(39380400002)(2980300002)(1109001)(1110001)(339900001)(3190300001)(199004)(189003)(24454002)(76104003)(65956001)(498600001)(97736004)(83506002)(77096006)(229853002)(81166006)(8936002)(106466001)(6246003)(65806001)(47776003)(76176011)(53546011)(230700001)(59450400001)(86362001)(36756003)(2950100002)(53936002)(31696002)(23676004)(110136005)(58126008)(2486003)(356003)(316002)(81156014)(68736007)(5660300001)(65826007)(104016004)(105606002)(305945005)(67846002)(50466002)(8676002)(64126003)(2906002)(31686004); DIR:OUT; SFP:1101; SCL:1; SRVR:CY4PR03MB2695; H:tx30smr01.am.freescale.net; FPR:; SPF:Fail; PTR:InfoDomainNonexistent; A:1; MX:1; LANG:en; X-Microsoft-Exchange-Diagnostics: 1; BN1AFFO11FD041; 1:kdjRx1tmXV20cMSyflEPkb+S9uklZexBbgjsEo86MWUs3TM7lWpIDWTYgGJVN1bJCoqfHSBCjOSOCAIA9hQ0dnCpSLu2kjUvPDx2FpA4W3z3u3dK6ulkxMhP9948/UK8 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 50c1f23a-512f-4307-2dee-08d54c488a08 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(4534020)(4628075)(201703131517081)(5600026)(4604075)(2017052603307); SRVR:CY4PR03MB2695; X-Microsoft-Exchange-Diagnostics: 1; CY4PR03MB2695; 3:o0FtG0kmKUE7SBg20jW6mLxnSmlTn4S/UF8IWIna4vykPdQXrv3jDOjU1coBDvvXOLkdJUVYm2cIwgrCxQ6Z5++Bdw2tvjFrz2PX43EyxPM20nivak6ydFERPgV4P2YVRgKysz9Yq3rrncZjPaS1bTiC01MdxUSGxQG3wXq1LRNlYjX3AlyeRrVNgiUPoi7SUXAcnYrF2qrnoJ7JlvgizB4BXOoxUVpP1QQBoFu/7+g8DKsXdHmwxtOr4gIaltEpyJuXZwBgy5VzDFsVsMTBnL6qS7C8A3k5IrsZIaiWlU9Taj5KeJ7Qgusz8RDrzCiqlCsHm3EXIzDRrQXR+e+3tXr0AIZBRQI26E4ZDqgaJ4Y=; 25:Ofr74Tsi4ab2jAefynwvy7Xgl9FuPINKmPaSs8F4yxyaWbrh6Y5xrFeZe5pTmMIClcLiscb45LEpPViOLxNVdgQcMYFeQ2K0zax7KycD0Q4ZEKk5EiC8eoVyLc72onBctUZJSppOC25CtOXtVlZnd0td90INDc7so03fMC0cwBGRC8dssLOMPJoW29doPzIAs7Uf2TQuQBJy6nyVn6bgVaa84kn8OY7IyVXpw93sxb/8mhK3jDvfmL7qWaYfywIdxS/AdtKKfqVzedhNV4ix1GomwVS9e0PCzPdOVfODmnYCelN4V7EBRL0OgOrtvZcFfpRVzPmbJmCYDf/e70i1e1im3B/41BGX1ry/O6gj8bE= X-MS-TrafficTypeDiagnostic: CY4PR03MB2695: X-Microsoft-Exchange-Diagnostics: 1; CY4PR03MB2695; 31:B54MdvLjbCTQFmtFY6RIvcFaVAyPz33VpzdXAdNg0S8XZinTfze9OnQqICegFUZ4QUIifOEXmFMRcpmveSgGaJxCPL5CzBiwufsNgG0SdPSal4HpIi+ycdMbr8mx61SqUJIVP4Z2t8Ea+kYRzu3z6UkTAnBB+pjLWLnkuSSwwek7LNlCwgMPeLBGI0O9EwyEvZp4t/9xjGwumfQj5AcR8rGeNy2gOBB53/alfFBsX7g=; 4:XdTCZyfA4edxfwF1rq1OIz/kBcGBCvtszdK7+ZsesNvKdyVqK/JhPH+l46wSAhw6CfEHpbGhLf6h2z8nxmX4sNorKpn5Fg56KDSm5mDqHV1zTxGZTef2r7J2qu27/hoy3RgB5Llxy9usYNBJ/oepVyVcFk5w7EvTtoUXDJs2tM7lAHxLoRUlDFH+ShIHT7Liqfha2WX3QXG+NDPcf2LSXSJDdAbH2rrjyq19H/zOC8onqwp2hYqRWJ3hF9cffCZ3x94+64W9U+vYqKRUW45hTcEsclYZTXTjnvDBNvwrhcjTEBVWUjvzi+kS54MNsb+w X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(185117386973197); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6095135)(2401047)(8121501046)(5005006)(3231023)(944510075)(944921075)(946801075)(946901075)(93006095)(93001095)(10201501046)(3002001)(6055026)(6096035)(201703131430075)(201703131433075)(201703131448075)(201703161259150)(201703151042153)(20161123556025)(20161123561025)(20161123565025)(20161123559100)(20161123563025)(201708071742011); SRVR:CY4PR03MB2695; BCL:0; PCL:0; RULEID:(100000803101)(100110400095)(400006); SRVR:CY4PR03MB2695; X-Forefront-PRVS: 053315510E X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtDWTRQUjAzTUIyNjk1OzIzOjNvM0xQNWtoeTZFcExFMGVXV3AzTTloMGtZ?= =?utf-8?B?NGJnYjFmYk9DS0h0d0JOdlBVN1lQWXpNWGlDQUcwVjREWmplYVc5MWFWVnk0?= =?utf-8?B?aDRYRklHbmNGR3JxRmNBTzFrb0RWTVY0dGtiakpZMlM3VERLYmJpY1MzenEz?= =?utf-8?B?SjJXWURIc0xHbk5pcWcrVjJMRGZ2RkxaVzVpZWdUT0ExMkM4VUd3MDRMemRz?= =?utf-8?B?SjhOQStWdVVXbEJYY1VyeGVOUmNkZ2ZBZGVwdHRQQjRzRHd6ZnhTazBja04y?= =?utf-8?B?WFZYeWpEcGd3b2lxR3pNUFZhcFU0VzVUbVBGOGlXTTkzcXNwZkhpWDV5Uk4v?= =?utf-8?B?UzdnbE9NRU1UOVdrWnBZNnFFNlF3OVc1TE1XSGl5TXhIajU3RG5IYlJPWjRO?= =?utf-8?B?YzFuVG80WXJrQ1dYSUVyODh1U3JleE1ONFltb1VBMHlXZGRjOFlheFVsZHgv?= =?utf-8?B?R09uT2Voc01HZzUyTytuRDliM0lxQ2hZTWRqMGtXYTlvaWdrTHJMNFRTUXRN?= =?utf-8?B?WW9GOWJia1ZXSXRzK2c2NEF0V1ZEOFVjeDJ2eXR0bEc2Q3pGdWlDT1hnN1Jp?= =?utf-8?B?T2xFa0wvaVlWcXNYNzRvcEY1MlRucmN2ZEZXYkVyaVE5eWV6aTlPa214Tjd1?= =?utf-8?B?TVhXUnl5SGlrak82aWdjcXUwNHV5NFZxNFR1Sm9Ta0o0Y20yMFBqZnprSEpZ?= =?utf-8?B?RVFBeG1nQlBLYmRkRmVvcWdhTlBBS1JhbDJTeUJSZk5WOWo3YXFkOVJFWEZ5?= =?utf-8?B?ZDlYNkgwSG5KaGQvNFR4NW1Cd3U0SndjOHBEMThSU01QVm4wU0hnNTFtTTlP?= =?utf-8?B?dmVhTGVxazVOckYwMHFQRTd3Rm1CMnZ6WVNLVmM5VDZGRWNWeUJyQVhxcHVp?= =?utf-8?B?cE9kbTRmUEpud0NuWmQvUkZkR1VQR2lTZStSS2hKU2YxRHV2TFovbHRET29Y?= =?utf-8?B?S1BacW5jTEZEQVhzdGdwT1hGY1pmTEJzY1BsdUllUGNEc080ZlJtdHY2cEk0?= =?utf-8?B?UHFwZ1hYMXgrTzk5Rmg3MHVxVU4rUGJ4cm5ocDVRUk9mT2dWbEkzOVk2TFhP?= =?utf-8?B?WXZaMStGQjU2NlNHaHZ4T2JDd0ZsRDE0TjJJazhzcVRNL3NabnJiYmJmajFS?= =?utf-8?B?L3pkamlzYVNUNFlYWVI5b01VM2VwUWQ3YTc0NjFQSGNmRnpMNXJ1NEd4Mms2?= =?utf-8?B?RktCc2VrMjVGRUpXa2tkSGx5T3p4dXFJV2tWeVdiZTVpTDJCRlI2OW00RlNQ?= =?utf-8?B?NVpPNTRmZUU0aE9qKzV2cUFZVXFWNERsS0NpN2J2MzJnTzdkNzRvbHRpZWwz?= =?utf-8?B?SWM5SGhmdVU4NHBNSWxLNlFVcU1YR3QzVmtCbkRtR3YyZjFtM2V6ZFgvMEFG?= =?utf-8?B?cFhZRE1zaUg2RStSOXduSHE5Znp0dm1Pbm53US9odnZDSXJja3JXM21ldlM2?= =?utf-8?B?R3ducllGRHlQNGZhM2Z6UXh2QTlmeS85eVZuRFpwTU92WVdROU43enhKWUFZ?= =?utf-8?B?Y3BmT01jU0lpYXdFYnJma2EyMEVEMmxBelBCMnFPQS9FOS94QUV4Q1Z0bHhS?= =?utf-8?B?eHhXQ1lCS0o0MFRDSE14N29KV2hPaThxMGV0YWFqeXc0NlRkblJkVVErVXpJ?= =?utf-8?B?T1VtbFFOWnlzcDFUdzJaejd6WU5FSVpKdDRzS1NISmxBcVl3dDVyM3doRlFn?= =?utf-8?B?OWxPbVlHamM5eFo2dXJQcjJKREM3UjdKNmZBREViellmeTBLTGtWSnBCOHNr?= =?utf-8?B?NHpCNEFyVnF1bVdkSkxudz09?= X-Microsoft-Exchange-Diagnostics: 1; CY4PR03MB2695; 6:l3nVWh2Gwc9E3bZuXWSZQ4F6F/5l9ZQjhzjCOszI1ursBgrCKkLhhqRXoNKp44o3MMKqt5m5g5QVDvCof7ACP4EKtGSGXkNO5wN6tPMdhI81PjtiSGmtnvYA5bn3hgBIw2J3WzIv52U1GjnCoBxGPLtDpWo9jlbTR3XEeDHgmiXcg2yykuU5jivqnPVa0tBs6HSZJnB1QNr+W/KiLXXW1tZ4c9r03RyouCslr0DIZs/nBhcX1d4FPyh5noXQ8oSBENXEh47DA/Ffw3M05JH38IMl09P/2TQmrg18Z2Yny1lJlLWBZK/+TMYtm20k6LxZM66+jtaxud1zbdJ99PWg1FDeTIEgr07hw3WVKSF5Qjw=; 5:ZNhOdjd5URuxIqsEuqDSXjeIkLXTAG2iGUxpT/Ef0AvTotmh4TEXlYKY+e8AAP+EXiXHLKAJ/95AXPCzg2Nvo7d8GoXIPW9HAny6WqkzF73iSnfp0vcBinIsOPjbo4fIwieH/8Blm+skxO1iOs663QvB/5dyffJWvww7Ocy7YNY=; 24:cSyuG+FI/ygS+ibmEsJwDorr0Vy1hkcaDZMXNpicEgMG31WAU50yC5iC0tpltA+9tkttwhSfFlUCmuQqL3EHdhvs2gGMM3ogzXfo7DmbKKs=; 7:CANmpnXqzEgMxYYFChsNvI9jj+QedAI3xHobdeRDLwfsd1WaSa3LrizRqSUDHXPQXAC2sQ/fU8zWOvAdQ1zkYamjIs4QNJ+9McFC+C45OaYZ2K2MdGwpnMaMJ8KPyZ9mMZe9oDocSI5RlaHoLUDkJYX/Sup32HUu5qCsZpKKBsiBfWnhu1/gbubROQk15ouldPTLrRUDqGYag568WZwqsr2ef4YHHc3cmMJcf9wCcBRKali8AjCrjxV00+vuE4Xi SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Dec 2017 10:07:57.8225 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 50c1f23a-512f-4307-2dee-08d54c488a08 X-MS-Exchange-CrossTenant-Id: 5afe0b00-7697-4969-b663-5eab37d5f47e X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=5afe0b00-7697-4969-b663-5eab37d5f47e; Ip=[192.88.168.50]; Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR03MB2695 Subject: Re: [dpdk-dev] [PATCH 1/3] kni: support for MAC addr change 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: Tue, 26 Dec 2017 10:08:19 -0000 Hi Ferruh, On 12/23/2017 3:25 AM, Ferruh Yigit wrote: > On 11/30/2017 3:46 AM, Hemant Agrawal wrote: >> This patch adds following: >> 1. Option to configure the mac address during create. Generate random >> address only if the user has not provided any valid address. >> 2. Inform usespace, if mac address is being changed in linux. >> 3. Implement default handling of mac address change in the corresponding >> ethernet device.> >> Signed-off-by: Hemant Agrawal > > Overall lgtm, there are a few issues commented below. > > Thanks, > ferruh > > <...> >> @@ -587,3 +603,26 @@ Currently, setting a new MTU and configuring the network interface (up/ down) ar >> RTE_LOG(ERR, APP, "Failed to start port %d\n", port_id); >> return ret; >> } >> + >> + /* Callback for request of configuring device mac address */ >> + >> + static int >> + kni_config_mac_address(uint16_t port_id, uint8_t mac_addr[]) >> + { >> + int ret = 0; >> + >> + if (port_id >= rte_eth_dev_count() || port_id >= RTE_MAX_ETHPORTS) { >> + RTE_LOG(ERR, KNI, "Invalid port id %d\n", port_id); >> + return -EINVAL; >> + } >> + >> + RTE_LOG(INFO, KNI, "Configure mac address of %d", port_id); >> + /* Configure network interface mac address */ >> + ret = rte_eth_dev_default_mac_addr_set(port_id, >> + (struct ether_addr *)mac_addr); >> + if (ret < 0) >> + RTE_LOG(ERR, KNI, "Failed to config mac_addr for port %d\n", >> + port_id); >> + >> + return ret; >> + } > > > It is hard to maintain code in doc, I am aware other related code is already in > document but what do you think keeping this minimal, like: > > static int > kni_config_mac_address(uint16_t port_id, uint8_t mac_addr[]) > { > .... > } agree. > > > <...> > >> @@ -559,6 +583,14 @@ rte_kni_handle_request(struct rte_kni *kni) >> req->result = kni->ops.config_network_if(\ >> kni->ops.port_id, req->if_up); >> break; >> + case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */ >> + if (kni->ops.config_mac_address) >> + req->result = kni->ops.config_mac_address( >> + kni->ops.port_id, req->mac_addr); >> + else >> + req->result = kni_config_mac_address( >> + kni->ops.port_id, req->mac_addr); > > ops.port_id can be unset if there is no physically backing device the kni > interface. And I guess for that case port_id will be 0 and it will corrupt other > interface's data. There needs to find a way to handle the port_id not set case. got it. I will set it as UINT16_MAX and check for it. > > Since kni sample always creates a KNI interface backed by pyhsical device, this > is not an issue for kni sample app but please think about kni pmd case. > > <...> > >> @@ -87,6 +91,7 @@ struct rte_kni_conf { >> unsigned mbuf_size; /* mbuf size */ >> struct rte_pci_addr addr; >> struct rte_pci_id id; >> + char mac_addr[ETHER_ADDR_LEN]; /* MAC address assigned to KNI */ >> >> __extension__ >> uint8_t force_bind : 1; /* Flag to bind kernel thread */ > > "struct rte_kni_conf" is a public struct. Adding a variable into the middle of > the struct will break the ABI. > But I think it is OK to add to the end, unless struct is not used as array. > > <...> > yes. adding it into middle breaks the ABIs. However adding into end is ok.