[dpdk-dev] [PATCH] net/mlx5: fix packet type and offload flags in Rx
In mlx5 rx function, the packet_type and ol_flags mbuf fields are not
properly initialized when no rx offload feature is enabled (checksum, l2
tun checksum, vlan_strip, crc). Thus, these fields can have a value
different of 0 depending on their value when the mbuf was freed.
This can result in an incorrect application behavior if invalid
ol_flags/ptype are set, or memory corruptions if IND_ATTACHED_MBUF is
set in ol_flags.
Fixes: 081f7eae242e ("mlx5: process offload flags only when requested")
Signed-off-by: Maxime Leroy
---
drivers/net/mlx5/mlx5_rxtx.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 0c352f3f..4132fd74 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1599,6 +1599,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
uint16_t pkts_n)
pkt = seg;
assert(len >= (rxq->crc_present << 2));
/* Update packet information. */
+ pkt->packet_type = 0;
+ pkt->ol_flags = 0;
if (rxq->csum | rxq->csum_l2tun | rxq->vlan_strip |
rxq->crc_present) {
if (rxq->csum) {
--
2.1.4
[dpdk-dev] [PATCH v13 12/13] eal/pci: Add rte_eal_dev_attach/detach() functions
Hi Tetsuya,
On Tue, Feb 24, 2015 at 5:49 AM, Tetsuya Mukawa wrote:
> These functions are used for attaching or detaching a port.
[...]
> +static int
> +rte_eal_vdev_init(const char *name, const char *args)
> +{
> + struct rte_driver *driver;
> +
> + if (name == NULL)
> + return -EINVAL;
> +
> + TAILQ_FOREACH(driver, _driver_list, next) {
> + if (driver->type != PMD_VDEV)
> + continue;
> +
> + /*
> +* search a driver prefix in virtual device name.
> +* For example, if the driver is pcap PMD, driver->name
> +* will be "eth_pcap", but "name" will be "eth_pcapN".
> +* So use strncmp to compare.
> +*/
> + if (!strncmp(driver->name, name, strlen(driver->name)))
> + return driver->init(name, args);
> + }
> +
> + if (driver == NULL) {
This test is not needed anymore. You should remove it.
> + RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
> + return -EINVAL;
> + }
> + return 0;
> +}
[...]
> }
> +
> +/* So far, DPDK hotplug function only supports linux */
> +#ifdef RTE_LIBRTE_EAL_HOTPLUG
> +static int
> +rte_eal_vdev_uninit(const char *name)
> +{
> + struct rte_driver *driver;
> +
> + if (name == NULL)
> + return -EINVAL;
> +
> + TAILQ_FOREACH(driver, _driver_list, next) {
> + if (driver->type != PMD_VDEV)
> + continue;
> +
> + /*
> +* search a driver prefix in virtual device name.
> +* For example, if the driver is pcap PMD, driver->name
> +* will be "eth_pcap", but "name" will be "eth_pcapN".
> +* So use strncmp to compare.
> +*/
> + if (!strncmp(driver->name, name, strlen(driver->name)))
> + return driver->uninit(name);
> + }
> +
> + if (driver == NULL) {
This test is not needed anymore . You should remove it.
> + RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
[...]
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile
> b/lib/librte_eal/linuxapp/eal/Makefile
> index e117cec..b59b201 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -45,6 +45,7 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
> CFLAGS += -I$(RTE_SDK)/lib/librte_ring
> CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
> CFLAGS += -I$(RTE_SDK)/lib/librte_malloc
> +CFLAGS += -I$(RTE_SDK)/lib/librte_mbuf
Why do you need to add librte_mbuf into EAL Makefile ?
> CFLAGS += -I$(RTE_SDK)/lib/librte_ether
> CFLAGS += -I$(RTE_SDK)/lib/librte_ivshmem
> CFLAGS += -I$(RTE_SDK)/lib/librte_pmd_ring
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c
> b/lib/librte_eal/linuxapp/eal/eal_pci.c
[...]
> --
> 1.9.1
>
Except these 3 points, ack for this patch.
Regards,
Maxime
[dpdk-dev] [PATCH v11 12/13] eal/pci: Add rte_eal_dev_attach/detach() functions
Hi Tetsuya,
On Mon, Feb 23, 2015 at 6:09 AM, Tetsuya Mukawa wrote:
> These functions are used for attaching or detaching a port.
[...]
>
> +static int
> +rte_eal_vdev_init(const char *name, const char *args)
> +{
> + struct rte_driver *driver;
> +
> + if (name == NULL)
> + return -EINVAL;
> +
> + TAILQ_FOREACH(driver, _driver_list, next) {
> + if (driver->type != PMD_VDEV)
> + continue;
> +
> + /*
> +* search a driver prefix in virtual device name.
> +* For example, if the driver is pcap PMD, driver->name
> +* will be "eth_pcap", but "name" will be "eth_pcapN".
> +* So use strncmp to compare.
> +*/
> + if (!strncmp(driver->name, name, strlen(driver->name))) {
> + driver->init(name, args);
> + break;
Please return the value given by init: return driver->init(name, args); .
> + }
> + }
> +
> + if (driver == NULL) {
> + RTE_LOG(WARNING, EAL, "no driver found for %s\n", name);
--> should be : RTE_LOG(ERR .
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> int
> rte_eal_dev_init(void)
> {
> @@ -79,23 +113,10 @@ rte_eal_dev_init(void)
> if (devargs->type != RTE_DEVTYPE_VIRTUAL)
> continue;
>
> - TAILQ_FOREACH(driver, _driver_list, next) {
> - if (driver->type != PMD_VDEV)
> - continue;
> -
> - /* search a driver prefix in virtual device name */
> - if (!strncmp(driver->name, devargs->virtual.drv_name,
> - strlen(driver->name))) {
> - driver->init(devargs->virtual.drv_name,
> - devargs->args);
> - break;
> - }
> - }
> -
> - if (driver == NULL) {
> + if (rte_eal_vdev_init(devargs->virtual.drv_name,
> + devargs->args))
> rte_panic("no driver found for %s\n",
> devargs->virtual.drv_name);
instead of that:
if (rte_eal_vdev_init(devargs->virtual.drv_name, devargs->args)) {
RTE_LOG(ERR, "failed to initialize %s device\n",
devargs->virtual.drv_name);
return -1;
}
> - }
> }
>
> /* Once the vdevs are initalized, start calling all the pdev drivers
> */
> @@ -107,3 +128,237 @@ rte_eal_dev_init(void)
> }
> return 0;
> }
> +
> +/* So far, DPDK hotplug function only supports linux */
> +#ifdef RTE_LIBRTE_EAL_HOTPLUG
> +static int
> +rte_eal_vdev_uninit(const char *name)
> +{
> + struct rte_driver *driver;
> +
> + if (name == NULL)
> + return -EINVAL;
> +
> + TAILQ_FOREACH(driver, _driver_list, next) {
> + if (driver->type != PMD_VDEV)
> + continue;
> +
> + /*
> +* search a driver prefix in virtual device name.
> +* For example, if the driver is pcap PMD, driver->name
> +* will be "eth_pcap", but "name" will be "eth_pcapN".
> +* So use strncmp to compare.
> +*/
> + if (!strncmp(driver->name, name, strlen(driver->name))) {
> + driver->uninit(name);
Please return the value given by uninit: return driver->uninit(name, args);
> + break;
> + }
> + }
> +
> + if (driver == NULL) {
> + RTE_LOG(WARNING, EAL, "no driver found for %s\n", name);
> + return 1;
As it's an error, the function should return a negative value ( i.e. -EINVAL).
Please set the log level to ERR.
> + }
> + return 0;
> +}
> +
[...]
> +}
> +
> +/* attach the new virtual device, then store port_id of the device */
> +static int
> +rte_eal_dev_attach_vdev(const char *vdevargs, uint8_t *port_id)
> +{
> + char *name = NULL, *args = NULL;
> + uint8_t new_port_id;
> + struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
> + int ret = -1;
> +
> + if ((vdevargs == NULL) || (port_id == NULL))
> + goto end;
> +
> + /* parse vdevargs, then retrieve device name and args */
> + if (rte_eal_parse_devargs_str(vdevargs, , ))
> + goto end;
> +
> + /* save current port status */
> + if (rte_eth_dev_save(devs, sizeof(devs)))
> + goto end;
> + /* walk around dev_driver_list to find the driver of the device,
> +* then invoke probe function o the driver.
> +* TODO:
> +* rte_eal_vdev_init() should return port_id,
> +* And rte_eth_dev_save() and rte_eth_dev_get_changed_port()
> +*
[dpdk-dev] [PATCH v10 13/14] eal/pci: Add rte_eal_dev_attach/detach() functions
Hi Tetsuya,
On Sat, Feb 21, 2015 at 4:49 AM, Tetsuya Mukawa wrote:
> On 2015/02/21 0:20, Maxime Leroy wrote:
[...]
>> Why you want to add devargs in the devargs_list, if there are no needs
>> to store this information ?
>
> In eal initialization code, virtual device names stored in devargs are
> checked not to register a same device name twice.
> And each init function of PMD just trust a device name received by eal.
> So there is no code in PMD to check whether device name is unique.
>
I disagree with you. This check is not present in the master branch.
You have added this check in your hotplug patchset, in this patch:
[PATCH v10 10/14] eal/pci: Add a function to remove the entry of
devargs list
See: http://dpdk.org/ml/archives/dev/2015-February/013712.html
Thus the problem should be already exist without your patches in the
master branch.
For example according to you, this testpmd command should create 2
devices with the same name:
testpmd -c 0xc --vdev eth_pcap0,iface=eth0 --vdev eth_pcap0,iface=eth1
-n 2 -- -i
But it's not the case:
PMD: Initializing pmd_pcap for eth_pcap0
PMD: Creating pcap-backed ethdev on numa socket 0
PMD: Initializing pmd_pcap for eth_pcap0
PMD: Creating pcap-backed ethdev on numa socket 0
PMD: rte_eth_dev_allocate: Ethernet Device with name eth_pcap0 already
allocated!
In fact, it's not possible for any PMD_VDEV in the dpdk repo to create
2 devices with the same name.
All the virtual device initialization functions use the
rte_eth_dev_allocate function. This function prevents to create two
ethernet devices with the same name:
if (rte_eth_dev_allocated(name) != NULL) {
PMD_DEBUG_TRACE("Ethernet Device with name %s already
allocated!\n", name);
return NULL;
}
> For example, according to your suggestion, how to prevent below case?
> $ ./testpmd -c f -n 1 -- -i
> testpmd> port attach eth_pcap0,iface=eth0
> testpmd> port attach eth_pcap0,iface=eth1
>
> Also, type below, after doing above.
> testpmd> port detach 0
>
> Probably port 0 will be "eth_pcap0,iface=eth0".
> But uninit code of PMD only receives a device name like 'eth_pcap0'.
> (We have 2 'eth_pcap0' devices in PMD.)
>
> To prevent above case, probably we have 2 options at least.
> One is changing init code of all virtual PMDs not to register same
> device name.
There are no need to change init code of all virtual PMDs to not
register the same device name 2 times.
Because it's already not possible to create 2 virtual device with the
same name. (see my point above)
> The other is to use devargs_list in EAL, and call init code of PMD with
> a unique device name.
Thus there are no needs to use the devargs_list for that.
>
[..]
>>
>> But you don't call rte_eal_devargs_add with RTE_DEVTYPE_WHILISTED_PCI
>> in rte_eal_dev_attach_pdev ?
>
> Yes, I don't.
> Hotplug functions should not change BLACKLIST and WHITELIST.
> So not to touch the list is correct behavior.
Yes the correct behaviour for Hotplug functions is to not use the
devargs_list for physical and virtual devices !
Regards,
Maxime
[dpdk-dev] [PATCH v10 13/14] eal/pci: Add rte_eal_dev_attach/detach() functions
Hi Tetsuya, Thanks for your comment. On Fri, Feb 20, 2015 at 11:32 AM, Tetsuya Mukawa wrote: > On 2015/02/20 19:14, Maxime Leroy wrote: >> Hi Tetsuya, [...] >> > > Hi Maxime, > > I appreciate for your comment. > > When rte_eal_init() is called, if we have "--vdev" options, these will > be stored in vdevargs as you describe above. > I agree this is the current behavior of DPDK. > > When we call hotplug functions, I guess doing same thing will be more > consistent design. The rte_eal_devargs_add is here to store a white list parameters for later creating the devices. It means that the devargs_list is only needed at the init to store the devargs parsed . I think we should not use the devargs_list after eal initialization. Why you want to add devargs in the devargs_list, if there are no needs to store this information ? At the end, it adds extra codes for nothing. > > For example, we can do like below. > 1. $ ./testpmd --vdev 'eth_pcap' -- -i > 2. testpmd>port detach It's exactly the same for physical device: 1. $./testpmd -w :08:00:1 -- -i 2. testpmd> port detach But you don't call rte_eal_devargs_add with RTE_DEVTYPE_WHILISTED_PCI in rte_eal_dev_attach_pdev ? Thus it makes the hotplug implementation not coherent for me. What do you think ? Regards, Maxime
[dpdk-dev] [PATCH v10 13/14] eal/pci: Add rte_eal_dev_attach/detach() functions
Hi Tetsuya,
On Fri, Feb 20, 2015 at 7:39 AM, Tetsuya Mukawa wrote:
> These functions are used for attaching or detaching a port.
[..]
> +
> +static void
> +get_vdev_name(char *vdevargs)
> +{
> + char *sep;
> +
> + if (vdevargs == NULL)
> + return;
> +
> + /* set the first ',' to '\0' to split name and arguments */
> + sep = strchr(vdevargs, ',');
> + if (sep != NULL)
> + sep[0] = '\0';
> +}
> +
> +/* attach the new virtual device, then store port_id of the device */
> +static int
> +rte_eal_dev_attach_vdev(const char *vdevargs, uint8_t *port_id)
> +{
> + char *args;
> + uint8_t new_port_id;
> + struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
> +
> + if ((vdevargs == NULL) || (port_id == NULL))
> + goto err0;
> +
> + args = strdup(vdevargs);
> + if (args == NULL)
> + goto err0;
> +
> + /* save current port status */
> + if (rte_eth_dev_save(devs, sizeof(devs)))
> + goto err1;
> + /* add the vdevargs to devargs_list */
> + if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, args))
> + goto err1;
You don't need to add devargs to the devargs_list.
The devargs_list is only needed at the init to store the arguments
when we parse the command line. Then, at initialization,
rte_eal_dev_init creates the devices from this list .
Instead of adding the devargs in the list, you could have the following code:
static int
rte_eal_dev_attach_vdev(const char *vdevargs, uint8_t *port_id)
{
...
/* save current port status */
if (rte_eth_dev_save(devs, sizeof(devs)))
goto err;
devargs = rte_eal_parse_devargs_str(RTE_
DEVTYPE_VIRTUAL, vdevargs);
if (devargs == NULL)
goto err;
if (rte_eal_vdev_devinit(devargs) < 0)
goto err;
if (rte_eth_dev_get_changed_port(devs, _port_id))
goto err;
...
}
What do you think ?
> + /* parse vdevargs, then retrieve device name */
> + get_vdev_name(args);
> + /* walk around dev_driver_list to find the driver of the device,
> +* then invoke probe function o the driver.
> +* TODO:
> +* rte_eal_vdev_find_and_init() should return port_id,
> +* And rte_eth_dev_save() and rte_eth_dev_get_changed_port()
> +* should be removed. */
> + if (rte_eal_vdev_find_and_init(args))
> + goto err2;
> + /* get port_id enabled by above procedures */
> + if (rte_eth_dev_get_changed_port(devs, _port_id))
> + goto err2;
> +
> + free(args);
> + *port_id = new_port_id;
> + return 0;
> +err2:
> + rte_eal_devargs_remove(RTE_DEVTYPE_VIRTUAL, args);
> +err1:
> + free(args);
> +err0:
> + RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n");
> + return -1;
> +}
> +
> +/* detach the new virtual device, then store the name of the device */
> +static int
> +rte_eal_dev_detach_vdev(uint8_t port_id, char *vdevname)
> +{
> + char name[RTE_ETH_NAME_MAX_LEN];
> +
> + if (vdevname == NULL)
> + goto err;
> +
> + /* check whether the driver supports detach feature, or not */
> + if (rte_eth_dev_is_detachable(port_id))
> + goto err;
> +
> + /* get device name by port id */
> + if (rte_eth_dev_get_name_by_port(port_id, name))
> + goto err;
> + /* walk around dev_driver_list to find the driver of the device,
> +* then invoke close function o the driver */
> + if (rte_eal_vdev_find_and_uninit(name))
> + goto err;
> + /* remove the vdevname from devargs_list */
> + if (rte_eal_devargs_remove(RTE_DEVTYPE_VIRTUAL, name))
> + goto err;
The same here, you don't need to remove devargs from the devargs_list.
Instead of removing the devargs in the list, you could have the following code::
static int
rte_eal_dev_detach_vdev(uint8_t port_id, char *vdevname)
{
...
/* check whether the driver supports detach feature, or not */
if (rte_eth_dev_is_detachable(port_id))
goto err;
/* get device name by port id */
if (rte_eth_dev_get_name_by_port(port_id, name))
goto err;
if (rte_eal_vdev_uninit(name))
goto err;
...
}
What do you think ?
Regards,
Maxime
[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions
Hi Tetsuya, On Tue, Feb 17, 2015 at 9:51 AM, Tetsuya Mukawa wrote: > > > >> +/* get port_id enabled by above procedures */ > >> +if (rte_eth_dev_get_changed_port(devs, _port_id)) > >> +goto err2; > > [...] > > > >> /** > >> + * Uninitilization function called for each device driver once. > >> + */ > >> +typedef int (rte_dev_uninit_t)(const char *name, const char *args); > > Why do you need args for uninit? > > > > I just added for the case that finalization code of PMD needs it. > But, probably "args" parameter can be removed. > > I think there are no needs to have any args in the uninit function: 1) You librte_pmd_null doesn't use it 2) You give exactly the same argument that was used by the init function. A driver should have already stored these parameters in an internal private structure at initialization. So it's not needed to give me back for uninit method. >From my understanding devargs_list is only needed at the init to store the arguments when we parse the command line. Then, at initialization, rte_eal_dev_init creates the devices from this list . By removing args from uninit function, you doesn't need to add and remove anymore devargs in devargs_list to (de)attach a new device. What do you think ? Maxime
[dpdk-dev] [PATCH] ixgbe: support flow director for X540
Thanks On Mon, Mar 24, 2014 at 10:35 PM, Thomas Monjalon wrote: > From: Mauro Annarumma > > Flow director in X540 uses the same registers as in 82599. > So it just has to be enabled in the 82599 implementation. > > Signed-off-by: Mauro Annarumma > > Acked-by: Maxime Leroy
[dpdk-dev] Is Flow Director supported on the x540 chipset?
Hi Fulvio, I have been checked few information about the datasheet of the X540 card. The X540 card is a derivative of the 82599. It's very similar to the 82599 card. The both cards use the same hardware registers for the flow director. Thus, the current code of the dpdk for the flow director should work with X540 card too. I think you only need to change few lines in lib/librte_pmd_ixgbe/ixgbe_fdir.c: - if (hw->mac.type != ixgbe_mac_82599EB) + if (hw->mac.type != ixgbe_mac_82599EB || hw->mac.type != ixgbe_mac_X540) Let me know if you can test it and please provide a patch if it works. Thanks. Regards, Maxime On Thu, Jan 16, 2014 at 7:47 AM, Fulvio Risso wrote: > Dear all, > > by digging into the DPDK code it seems to me that FDIR is not supported on > the x540 chipset, while it is supported on 82599. > > Another message seems to mention the same problem here: > > http://dpdk.org/ml/archives/dev/2013-November/000806.html > > Is there any plan to support FDIR to the x540 chipset? > > fulvio
[dpdk-dev] [PATCH v3] app/testpmd: fix RSS rx by setting mq_mode
> The mq_mode was not set when rxq is > 0; it's defaulted to ETH_MQ_RX_NONE. > As a result, RSS remains inactive. The fix is to set mq_mode to ETH_MQ_RX_RSS > when hf is non-zero. > > This bug was introduced by commit 243db2ddee3094a2cb39fdd4b17e26df4e7735e1 > igb/ixgbe: ETH_MQ_RX_NONE should disable RSS > > Signed-off-by: Daniel Kan Acked-by: Maxime Leroy Thanks.
[dpdk-dev] [PATCH v2] app/testpmd: fix RSS by setting mq_mode
Hello, Thanks for your patch fixing the regression introduced by my commit (igb/ixgbe: ETH_MQ_RX_NONE should disable RSS). I have one comment about your fix. I don't think there are any reasons to not enable RSS with only one RX queue in testpmd. RSS is mainly used in testpmd to spread traffic on the different rx queues. But you can use RSS with one rx queue for debbugging purpose. For example, you can use the rxonly forward engine of the testpmd to display the RSS hash. (see pkt_burst_receive in app/test-pmd/rxonly.c) An other issue about not enabling RSS with 1 queue, when you use the command show_rss_key in the testpmd, this one will display that the RSS is enabled. (because rss_hf != 0; see port_rss_hash_conf_show function in app/test-pmd/config.c) Do you agree with my analyze ? Thanks. -- Maxime Leroy
[dpdk-dev] Comments regarding Flow Director support in PMD IXGBE
Hi Robert, On Fri, Jan 3, 2014 at 8:52 PM, Robert Sanford wrote: > Issue #1: > Our reading of the 82599 data sheet leads us to believe that > Flow Director can simultaneously handle *both* IPv4 and IPv6 filters, > with separate filter rules, of course. > > Thus, at the bottom of ixgbe_fdir.c:fdir_set_input_mask_82599( ), > we could remove the "if (!input_mask->set_ipv6_mask)" / "else" > around the setting of FDIRSIP4M, FDIRDIP4M, and FDIRIP6M. > (This would also eliminate the need for the set_ipv6_masks flag itself.) > > We performed limited testing on this change. We have successfully > added both IPv4 and IPv6 signature filters, but so far have only > exercised them with IPv4 traffic. > > One would think that the designers of this chip feature envisioned > users filtering mixed traffic (both IPv4 and IPv6). By reading the 82599 datasheet, I have the same analyze than you, the flow director masks seems to be independent for ipv4 and ipv6. But it will be nice to have a small test with ipv6 traffic to be sure about this point. Would you like to provide a patch to remove this useless "if" please ? (Note: the set_ipv6_mask field of the input_mask structure need to be removed too) > Issue #2: > Apparently, API rte_eth_dev_fdir_set_masks( ) expects IPv4 address > and port masks in host-byte-order (little-endian), while > rte_eth_dev_fdir_add_signature_filter( ) expects IPv4 addresses and > ports in network-byte-order (big-endian). > > (Contrast the writing into IXGBE_FDIRSIP4M in ixgbe_fdir.c: > fdir_set_input_mask_82599( ), versus ixgbe/ixgbe_82599.c: > ixgbe_fdir_set_input_mask_82599( ). The former includes an extra > IXGBE_NTOHL( ) on the mask's complement.) > > Not knowing this made it a bit tricky to get signature filters working > properly. Perhaps it is too late to change the byte-ordering in the > (set masks) API? Whether we change it or not, we probably should > at least document these details, to avoid confusion. First, you probably know this point, a good way to test flow director in dpdk is to use the testpmd application. And it's also a good example to understand how to use rte_eth_dev_fdir_* api. So by reading the app/test-pmd/cmdline.c file, I can understand that the mask is parsed in little-endian for rte_eth_dev_fdir_set_masks. And the src/dst ip addresses are parsed in big-endian for rte_eth_dev_fdir_add_signature_filter. Thus I agree with your analyze, the fdir api is not coherent. I think all the parameters of the fdir api should be in network order. + About a patch to fix the api: As you said, IXGBE_NTOHL need to be removed and IXGBE_WRITE_REG need to be used instead of IXGBE_WRITE_REG_BE32 (in lib/librte_pmd_ixgbe/ixgbe_fdir.c): /* Store source and destination IPv4 masks (big-endian) */ - IXGBE_WRITE_REG_BE32(hw, IXGBE_FDIRSIP4M, -IXGBE_NTOHL(~input_mask->src_ipv4_mask)); + IXGBE_WRITE_REG(hw, IXGBE_FDIRSIP4M, +~input_mask->src_ipv4_mask); The testpmd application need to be updated in consequence to provide ip mask in network order (in lib/librte_cmdline/cmdline.c): - fdir_masks.dst_ipv4_mask = res->ip_dst_mask; - fdir_masks.src_ipv4_mask = res->ip_src_mask; + fdir_masks.dst_ipv4_mask = rte_cpu_to_be_32(res->ip_dst_mask); + fdir_masks.dst_ipv4_mask = rte_cpu_to_be_32(res->ip_dst_mask); Would you like to provide and test a patch to fix this issue, please ? Thanks. Best Regards, --- Maxime Leroy maxime.leroy at 6wind.com
[dpdk-dev] [PATCH 2/2] igb/ixgbe: allow RSS with only one Rx queue
We should be able to enable RSS with one Rx queue.
RSS hash can be useful independently of the number of queues.
Applications can use RSS hash to identify different flows.
Signed-off-by: Maxime Leroy
---
lib/librte_pmd_e1000/igb_rxtx.c |7 ++-
lib/librte_pmd_ixgbe/ixgbe_rxtx.c |7 ++-
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index 641ceea..8c1e2cc 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -1743,8 +1743,7 @@ igb_dev_mq_rx_configure(struct rte_eth_dev *dev)
/*
* SRIOV inactive scheme
*/
- if (dev->data->nb_rx_queues > 1)
- switch (dev->data->dev_conf.rxmode.mq_mode) {
+ switch (dev->data->dev_conf.rxmode.mq_mode) {
case ETH_MQ_RX_RSS:
igb_rss_configure(dev);
break;
@@ -1757,9 +1756,7 @@ igb_dev_mq_rx_configure(struct rte_eth_dev *dev)
default:
igb_rss_disable(dev);
break;
- }
- else
- igb_rss_disable(dev);
+ }
}
return 0;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index e1b90f9..ae9eda8 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -3215,8 +3215,7 @@ ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)
* SRIOV inactive scheme
* any DCB/RSS w/o VMDq multi-queue setting
*/
- if (dev->data->nb_rx_queues > 1)
- switch (dev->data->dev_conf.rxmode.mq_mode) {
+ switch (dev->data->dev_conf.rxmode.mq_mode) {
case ETH_MQ_RX_RSS:
ixgbe_rss_configure(dev);
break;
@@ -3232,9 +3231,7 @@ ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)
case ETH_MQ_RX_NONE:
/* if mq_mode is none, disable rss mode.*/
default: ixgbe_rss_disable(dev);
- }
- else
- ixgbe_rss_disable(dev);
+ }
} else {
switch (RTE_ETH_DEV_SRIOV(dev).active) {
/*
--
1.7.10.4
