Hi Ivan, Thanks very much for your detailed response for this issue, I think your recommendation makes sense, and I will update the naming and re-send a patch for link-up and link-down.
Best regards, Changchun -----Original Message----- From: Ivan Boule [mailto:ivan.bo...@6wind.com] Sent: Friday, May 23, 2014 5:25 PM To: Ouyang, Changchun; dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH 0/3] Support administrative link up and link down On 05/23/2014 04:08 AM, Ouyang, Changchun wrote: > Hi Ivan > > To some extent, I also agree with you. > But customer hope DPDK can provide an interface like "ifconfig up" and > "ifconfig down" in linux, They can invoke such an interface in user > application to repeated stop and start dev frequently, and Make sure > RX and TX work fine after each start, I think it is not necessary to > do really device start and stop at Each time, just need start and stop RX and > TX function, so the straightforward method is to enable and disable tx lazer > in ixgbe. > But in the ether level we need a more generic api name, here is > rte_eth_dev_admin_link_up/down, while enable_tx_laser is not suitable, Enable > and disable tx laser is a way in ixgbe to fulfill the administrative link up > and link down. > maybe Fortville and future generation NIC will use other ways to fulfill the > admin_link_up/down. > Hi Changchun, I do not understand what your customer effectively needs. First of all, if I understand well, your customer's application does not really need to invoke the DPDK functions "eth_dev_stop" and "eth_dev_start" for addressing its problem, for instance to reconfigure RX/TX queues of the port. When considering the implementation in the ixgbe PMD of the function "rte_eth_dev_admin_link_down", its only visible effects from the DPDK application perspective is that no input packet can be received anymore, and output packets cannot be transmitted (once having filled the TX queues). Conversely, the only visible effect of the "rte_eth_dev_admin_link_up" function is that input packets are received again, and that output packets can be successfully transmitted. In fact, by disabling the TX laser on a ixgbe port, the only interesting effect of the function "rte_eth_dev_admin_link_down" is that it notifies the peer system of a hardware link DOWN event (with no physical link unplug on the peer side). Conversely, by enabling the TX laser on a ixgbe port, the only interesting effect of the function "rte_eth_dev_admin_link_up" is that it notifies the peer system of a hardware link UP event. Is that the actions that your customer's application actually needs to perform? If so, then this certainly deserves a real operational use case that it is worth describing in the patch log. This would help DPDK PMD implementors to understand what such functions can be used for, and to decide whether they actually need to be supported by the PMD. Assuming that these 2 functions need to be provided to address the issue described above, I do not think that the word "admin" brings anything for understanding their role. In fact, the word "admin" rather suggests a pure "software" down/up setting, instead of a physical one. Naming these 2 functions "rte_eth_dev_set_link_down" and "rte_eth_dev_set_link_up" better describes their expected effect. Regards, Ivan > > On 05/22/2014 04:44 PM, Ouyang, Changchun wrote: >> Hi Ivan >> For this one, it seems long story for that... >> In short, >> Some customer have such kind of requirement, they want to repeatedly >> start(rte_dev_start) and stop(rte_dev_stop) the port for RX and TX, >> but they find after several times start and stop, the RX and TX can't work >> well even the port starts, and the packets error number increase. >> >> To resolve this error number increase issue, and let port work fine >> even after repeatedly start and stop, We need a new API to do it, after >> discussing, we have these 2 API, admin link up and admin link down. > > If I understand well, this "feature" is not needed by itself, but only as a > work-around to address issues when repeatedly invoking the functions > ixgbe_dev_stop and ixgbe_dev_start. > Do such issues appear when performing the same operations with the Linux > kernel driver? > > Anyway, I suppose that such functions have to be automatically invoked > by the same code of the network application that invokes the functions > ixgbe_dev_stop and ixgbe_dev_start (said differently, there is no need > for a manual assistance !) > > In that case, would not it be possible - and highly preferable - to directly > invoke the functions ixgbe_disable_tx_laser and, then, ixgbe_enable_tx_laser > from the appropriate step during the execution of the function > ixgbe_dev_start(), waiting for some appropriate delays between the two > operations, if so needed? > > Regards, > Ivan > > >> >> Any difference if use " dev_link_start/stop" or " dev_link_up/down"? >> to me, admin_link_up/down is better than dev_link_start/stop, >> >> If most people think we need change the name, it is ok to rename it. >> >> I don't think we need it in non-physical PMDs. So no implementation in >> virtio PMD. >> >> Thanks >> Changchun >> >> >> -----Original Message----- >> From: Ivan Boule [mailto:ivan.boule at 6wind.com] >> Sent: Thursday, May 22, 2014 9:17 PM >> To: Ouyang, Changchun; dev at dpdk.org >> Subject: Re: [dpdk-dev] [PATCH 0/3] Support administrative link up >> and link down >> >> On 05/22/2014 08:11 AM, Ouyang Changchun wrote: >>> This patch series contain the following 3 items: >>> 1. Add API to support administrative link up and down. >>> 2. Implement the functionality of administrative link up and down in IXGBE >>> PMD. >>> 3. Add command in testpmd to test the functionality of administrative link >>> up and down of PMD. >>> > ... > >> Hi Changchun, >> >> The 2 functions "rte_eth_dev_admin_link_up" and "rte_eth_dev_admin_link_down" >> don't have an equivalent in the Linux kernel, thus I am wondering what is >> their effective usage from a network application perspective. >> Could you briefly explain in which use case these functions can be used for? >> >> By the way, it's not completely evident to infer the exact semantics of >> these 2 functions from their name. >> In particular, I do not see what the term "admin" brings to the >> understanding of their role. If it is to suggest that these functions are >> intended to force the link to a different state of its initial >> [self-detected] state, then the term "force" would be more appropriate. >> >> Otherwise, if eventually these functions appear to be mandatory, I suggest >> to rename them "rte_eth_dev_link_start" and "rte_eth_dev_link_stop" >> respectively, and to apply the same naming conventions in the 2 other >> patches. >> >> It might also be worth documenting in the comment section of the prototype >> of these 2 functions whether it makes sense or not to support a notion of >> link that can be dynamically started or stopped in non-physical PMDs >> (vmxnet3, virtio, etc). > > > -- > Ivan Boule > 6WIND Development Engineer > -- Ivan Boule 6WIND Development Engineer
