Hi, > -----Original Message----- > From: dev <dev-boun...@dpdk.org> On Behalf Of Wenzhuo Lu > Sent: Wednesday, December 12, 2018 12:30 PM > To: dev@dpdk.org > Cc: Lu, Wenzhuo <wenzhuo...@intel.com> > Subject: [dpdk-dev] [PATCH v3 00/34] A new net PMD - ice > > This patch set adds the support of a new net PMD, Intel® Ethernet Network > Adapters E810, also called ice. > > Besides enabling this new NIC, also some other features supported on this NIC.
Can you mention the other features > Like below, > > Basic features: > 1, Basic device operations: probe, initialization, start/stop, configure, > info get. > 2, RX/TX queue operations: setup/release, start/stop, info get. > 3, RX/TX. > > HW Offload features: > 1, CRC Stripping/insertion. > 2, L2/L3 checksum strip/insertion. > 3, PVID set. > 4, TPID change. > 5, TSO (LRO/RSC not supported). > > Stats: > 1, statics & xstatics. > > Switch functions: > 1, MAC Filter Add/Delete. > 2, VLAN Filter Add/Delete. > > Power saving: > 1, RX interrupt mode. > > Misc: > 1, Interrupt For Link Status. > 2, firmware info query. > 3, Jumbo Frame Support. > 4, ptype check. > 5, EEPROM check and set. > Can you add section to highlight the changes with "---". This is part of 'http://doc.dpdk.org/guides/contributing/patches.html' for 'This can be added to the cover letter or the annotations' > v2: > - Fix shared lib compile issue. > - Add meson build support. > - Update documents. > - Fix more checkpatch issues. > > v3: > - Removed the support of secondary process. > - Splitted the base code to more patches. > - Pass NULL to rte_zmalloc. > - Changed some magic numbers to macros. > - Fixed the wrong implementation of a specific bitmap. Not all comments are addressed or closed from V2. So I have to assume you will be doing the same for v4. Some of the items [PATCH v2 03/20] net/ice: support device and queue ops > > > > + > > > > +static int > > > > +ice_dev_start(struct rte_eth_dev *dev) { > > > > + struct rte_eth_dev_data *data = dev->data; > > > > + struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data- > > > > >dev_private); > > > > + struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data- > > > >dev_private); > > > > + uint16_t nb_rxq = 0; > > > > + uint16_t nb_txq, i; > > > > + int ret; > > > > + > > > > + ICE_PROC_SECONDARY_CHECK; > > > > > > Device start is not supported, but how is this differentiated from > > > primary configured device vs secondary configured device. > > > > > > Ie: primary uses black list '-b BB:DD:F' while secondary uses '-w > > > BB:DD:F'. In this case since we are checking process type this > > > will return without > > start? > Two updates with respect to your comment, 1. tool and application like > dpdk-procinfo will no longer be able pull data since you are asking to black > list. > 2. If there are functions which needs to shared, like primary using rx-0 and > tx-0 then secondary rx-1 and tx-1 how to make this work? [PATCH v2 01/20] net/ice: add base code > Note: In version 1 I enquired about unit or DTS validation for PMD. Is > this still holding good? Yes, it's planned and on going. [PATCH v2 02/20] net/ice: support device initialization > +# Compile burst-oriented ICE PMD driver # CONFIG_RTE_LIBRTE_ICE_PMD=y Based on ' https://patches.dpdk.org/patch/48488/' it is suggested to configure. But here is it already set to 'y'. Is this correct? If yes can you update ' https://patches.dpdk.org/patch/48488/' [PATCH v2 20/20] net/ice: support meson build > > > > Should not meson build option be add start. That is in patch > > > > 1/20 so compile options does not fail? > > > It will not fail. Enabling the compile earlier only means the code > > > can be > > compiled. > > > But, to use this device we do need the whole patch set. From this > > > point of view, compiling it at the end maybe better. > > Thanks for update, so will 'meson-build' success if apply 3 patches? > Sure, meson build will not be broken by any one of these patches. Only > until this patch, what built by meson can support ice. Thanks for confirmation that you have tried './devtools/test-meson-builds.sh' and the intermediate build for ICE_DSI PMD does not fail. [PATCH v2 16/20] net/ice: support basic RX/TX [PATCH v2 14/20] net/ice: support statistics > > > + ns->eth.tx_bytes -= (ns->eth.tx_unicast + ns->eth.tx_multicast + > > > + ns->eth.tx_broadcast) * ETHER_CRC_LEN; > > > > In earlier patch for 'mtu set check' we added VSI SWITCH VLAN. > > Should we add VSI VLAN here? > Don't need. They're different functions. We add crc length here > because of HW counts the packet length before crc is added. So you are not fetching stats from HW registers from switch is this correct? How will you get stats for actually transmitted in xstats? As I understand xstats is for switch HW stats right? [PATCH v2 04/20] net/ice: support getting device information > > Does this mean per queue offload capability is not supported? If > > yes, can you mention this in release notes under 'support or limitation' > No, it's not supported. We have a document, ice.ini, to list all the > features supported. All the others are not supported. > BTW, I don't think anything not supported is limitation. If I understand correctly, ICE_DSI_PMD is advertising it has not offload for RX and TX. But you are stating in ice.ini you are listing offload supports. So let me rephrase the question 'if you support port level offload capability, it will reflect for all queues rx and tx. But if you reflect queue level offload as 0 for rx and tx, then APIs rte_eth_rx_queue_setup and rte_eth_tx_queue_setup if queue offload enabled should fail. Is this correct understanding?' > > If device switch is not configured (default value from NVM) should > > we highlight the switch can support speed 10, 100, 1000, 1000 and son on? > No, this's the capability getting from HW. If HW is supported or configured for 10, 100, 25G then those should be returned correctly this I agree. But when the device is queried for capability it should highlight all supported speeds of switch. Am I right? > > If speed is not true as stated above, can you please add this to > > release notes and documentation. > Here listed all the case we can get from HW. Please add to ice_dsi documentation also. snipped
