From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ve0-f171.google.com (mail-ve0-f171.google.com [209.85.128.171]) by dpdk.org (Postfix) with ESMTP id 4AF9EDE0 for ; Fri, 10 Jan 2014 14:35:08 +0100 (CET) Received: by mail-ve0-f171.google.com with SMTP id cz12so445150veb.16 for ; Fri, 10 Jan 2014 05:36:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=m7zkEHqPxfIe3mjkfkD5McOzCIh5VuIU+VE185OsMjM=; b=d54z4WYiveHdjNjC+rA6Ov1tqXO40aC/Ol2oSTPWEJL2W2NtcPCRUz8tYy2fkqUc7P hgVRbqGeVnXU4PU6cTEiJ/nVdgBWFD764qzvyLna/miWdGouiyOBMRKYyxZ4nEB8FBos qPM73+Xc+ERuxWF0aNGkszAi9Ux30z7Iakd/ViXbPiXqr+FUbnTSZTrf1kl1MXmdWRDJ kyfaT8fZb0ccutUsAvv7JM/kR0hm4yUtpnuCAmLcrJye/Rkhc0GnYBx3TZ29TtvbPj1N jVC48Pb6KFO72wNd1ArI0IX+T55bFw1aJG9WfnrSBlpHaFuUnv/W1im/yPcHVfKFM+5C nkwg== X-Gm-Message-State: ALoCoQkmddrVFdw5ORJwuDM6dntLw0lOvr77QneiNMA/RIw4j/1HgzoKTZ5PNXNAMt5vLW06L54a MIME-Version: 1.0 X-Received: by 10.58.39.66 with SMTP id n2mr8848vek.81.1389360980577; Fri, 10 Jan 2014 05:36:20 -0800 (PST) Received: by 10.58.171.133 with HTTP; Fri, 10 Jan 2014 05:36:20 -0800 (PST) In-Reply-To: References: Date: Fri, 10 Jan 2014 14:36:20 +0100 Message-ID: From: Maxime Leroy To: Robert Sanford Content-Type: text/plain; charset=ISO-8859-1 Cc: dev@dpdk.org Subject: Re: [dpdk-dev] Comments regarding Flow Director support in PMD IXGBE X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Jan 2014 13:35:08 -0000 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@6wind.com