[{"id":1765510,"web_url":"http://patchwork.ozlabs.org/comment/1765510/","msgid":"<20170908164754.GD7356@vergenet.net>","list_archive_url":null,"date":"2017-09-08T16:47:55","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Tue, Sep 05, 2017 at 05:22:57PM +0800, Yuanhan Liu wrote:\n> From: Finn Christensen \n> \n> The basic yet the major part of this patch is to translate the \"match\"\n> to rte flow patterns. And then, we create a rte flow with a MARK action.\n> Afterwards, all pkts matches the flow will have the mark id in the mbuf.\n> \n> For any unsupported flows, such as MPLS, -1 is returned, meaning the\n> flow offload is failed and then skipped.\n\n...\n\n> +static int\n> +netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,\n> + const struct match *match,\n> + struct nlattr *nl_actions OVS_UNUSED,\n> + size_t actions_len,\n> + const ovs_u128 *ufid,\n> + struct offload_info *info)\n> +{\n> + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);\n> + const struct rte_flow_attr flow_attr = {\n> + .group = 0,\n> + .priority = 0,\n> + .ingress = 1,\n> + .egress = 0\n> + };\n> + struct flow_patterns patterns = { .items = NULL, .cnt = 0 };\n> + struct flow_actions actions = { .actions = NULL, .cnt = 0 };\n\nI believe the following will give the same result as the above,\nless verbosely.\n\n+ const struct rte_flow_attr flow_attr = { .ingress = 1 };\n+ struct flow_patterns patterns = { .cnt = 0 };\n+ struct flow_actions actions = { .cnt = 0 };\n\n> + struct rte_flow *flow;\n> + struct rte_flow_error error;\n> + uint8_t *ipv4_next_proto_mask = NULL;\n> + int ret = 0;\n> +\n> + /* Eth */\n> + struct rte_flow_item_eth eth_spec;\n> + struct rte_flow_item_eth eth_mask;\n\nEmpty line here please.\n\n> + memset(ð_mask, 0, sizeof(eth_mask));\n> + if (match->wc.masks.dl_src.be16[0] ||\n> + match->wc.masks.dl_src.be16[1] ||\n> + match->wc.masks.dl_src.be16[2] ||\n> + match->wc.masks.dl_dst.be16[0] ||\n> + match->wc.masks.dl_dst.be16[1] ||\n> + match->wc.masks.dl_dst.be16[2]) {\n\nIt looks like eth_addr_is_zero() can be used in the above.\n\n> + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\n> + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\n> + eth_spec.type = match->flow.dl_type;\n> +\n> + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\n> + sizeof(eth_mask.dst));\n> + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\n> + sizeof(eth_mask.src));\n> + eth_mask.type = match->wc.masks.dl_type;\n> +\n> + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\n> + ð_spec, ð_mask);\n> + } else {\n> + /*\n> + * If user specifies a flow (like UDP flow) without L2 patterns,\n> + * OVS will at least set the dl_type. Normally, it's enough to\n> + * create an eth pattern just with it. Unluckily, some Intel's\n> + * NIC (such as XL710) doesn't support that. Below is a workaround,\n> + * which simply matches any L2 pkts.\n> + */\n> + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\n> + }\n\nThis feels a lot like a layer violation - making the core aware\nof specific implementation details of lower layers.\n\nFrom a functional point of view, is the idea that\na eth_type+5-tuple match is converted to a 5-tuple match?\n\n> +\n> + /* VLAN */\n> + struct rte_flow_item_vlan vlan_spec;\n> + struct rte_flow_item_vlan vlan_mask;\n\nPlease declare all local variables at the top of the context\n(in this case function).\n\n...\n\n> + struct rte_flow_item_udp udp_spec;\n> + struct rte_flow_item_udp udp_mask;\n> + memset(&udp_mask, 0, sizeof(udp_mask));\n> + if ((proto == IPPROTO_UDP) &&\n> + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\n> + udp_spec.hdr.src_port = match->flow.tp_src;\n> + udp_spec.hdr.dst_port = match->flow.tp_dst;\n> +\n> + udp_mask.hdr.src_port = match->wc.masks.tp_src;\n> + udp_mask.hdr.dst_port = match->wc.masks.tp_dst;\n> +\n> + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_UDP,\n> + &udp_spec, &udp_mask);\n> +\n> + /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match */\n> + if (ipv4_next_proto_mask) {\n> + *ipv4_next_proto_mask = 0;\n\nI think this should be:\n\n+ *ipv4_next_proto_mask = NULL;\n\n> + }\n> + }\n> +\n> + struct rte_flow_item_sctp sctp_spec;\n> + struct rte_flow_item_sctp sctp_mask;\n> + memset(&sctp_mask, 0, sizeof(sctp_mask));\n> + if ((proto == IPPROTO_SCTP) &&\n\nIt seems there are unnecessary () in the line above.\n\n...\n\n> +/*\n> + * Validate for later rte flow offload creation. If any unsupported\n> + * flow are specified, return -1.\n> + */\n> +static int\n> +netdev_dpdk_validate_flow(const struct match *match)\n> +{\n> + struct match match_zero_wc;\n> +\n> + /* Create a wc-zeroed version of flow */\n> + match_init(&match_zero_wc, &match->flow, &match->wc);\n> +\n> +#define CHECK_NONZERO_BYTES(addr, size) do { \\\n\nI think do should appear on a new line.\n\n> + uint8_t *padr = (uint8_t *)(addr); \\\n> + int i; \\\n> + for (i = 0; i < (size); i++) { \\\n> + if (padr[i] != 0) { \\\n> + goto err; \\\n> + } \\\n> + } \\\n> +} while (0)\n> +\n> +#define CHECK_NONZERO(var) do { \\\n\nHere too.\n\n> + if ((var) != 0) { \\\n> + goto err; \\\n> + } \\\n> +} while (0)\n\nI think its better if macros (and defines) aren't declared\ninside of functions. The top of the file seems like\nthe best place to me. If not above the first function\nthat uses the macros, presumably this function.\n\n...","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"Ouwwu4cK\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpjtR2hTSz9s7h\n\tfor ;\n\tSat, 9 Sep 2017 02:47:59 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 1B250B88;\n\tFri, 8 Sep 2017 16:47:57 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 64397B5A\n\tfor ; Fri, 8 Sep 2017 16:47:56 +0000 (UTC)","from mail-wr0-f180.google.com (mail-wr0-f180.google.com\n\t[209.85.128.180])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id A3F7D469\n\tfor ; Fri, 8 Sep 2017 16:47:55 +0000 (UTC)","by mail-wr0-f180.google.com with SMTP id k20so5636465wre.4\n\tfor ; Fri, 08 Sep 2017 09:47:55 -0700 (PDT)","from vergenet.net (53.red-80-24-208.staticip.rima-tde.net.\n\t[80.24.208.53]) by smtp.gmail.com with ESMTPSA id\n\ts9sm1452730wra.73.2017.09.08.09.47.53\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tFri, 08 Sep 2017 09:47:53 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=6mtB9SyIu/p4Gzoo5yerIw1MnTkhOeqhxNMxyrnrahw=;\n\tb=Ouwwu4cKcVh4DPPJG9DPDxnQ22br6Pf+hEqghr1PmJkF7hBLVC/Q1yH9JC+DOR797m\n\tuR/R1y/9Bf0e1VmS9J1cHbWQIGeZ7+p9jJUhHxYLowTVaGizkonHy3ovmIEaP21AX1JH\n\t4KpL6rOjCRoxQZsD2fUsidnlJAd6ttEYB/NfhOEgG4mbkkdzXxn93IlqPhWobOaWHs8N\n\tjVZgRKKWvoYoJxzP22+S3TP7MtjgVvrCNrnuBkjv1huNOiPwr355gkqTErlkaJesQ8Kr\n\tnptFm2ppjb2/5i4UC1jZLk8yZZxS8JfP6OsPC4ZQtH7+dS9g8IEqyAD3kRymM/2ntj1f\n\tqV9w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=6mtB9SyIu/p4Gzoo5yerIw1MnTkhOeqhxNMxyrnrahw=;\n\tb=n+tZOmg07YCIKi029Bv9zPkas6KJoE8iDj17JHSW1AVpePdJeTseXN/kMCGyc/JWqn\n\tiXlmEjOoYCMCC0j6B/oj4TU6OeEbli36PeVGMXOdUbAABhK1HrqSdsr0lxtgUmd9fFRw\n\tPufOx0Tt5/7fBj+pCP4Pu+mgQIn0YqJNaj17q4NhGLlSni9tQiAGdxS7ZdCxu9M3umnD\n\t+wwMLWsIUOcyub/vSjKWiO7OM0oFboV2aPFMyFm7JW+glIEPgMNMgwD1xvR+Hrs3eSrd\n\t9938qVGoYxHJ5VNsrEN3dV6LN2LwTIuwZAZxgCYOFGqrlOUWFLaQ3JIfyei4ZkfFLMHb\n\tsTZA==","X-Gm-Message-State":"AHPjjUhloqFClwJVVeHTMlUgH89rpJ5uCc+FZf18dTI4x2guO8vLNZC6\n\tVcyRlU/KCTJYRecZtcRPiQ==","X-Google-Smtp-Source":"ADKCNb6nzTbZATZG5+95zM4j91MHntNHE7OypUpxbe/rBEJwWYX6Wnh+FCMLRluZZwUVkpgRehX34g==","X-Received":"by 10.223.143.110 with SMTP id\n\tp101mr2440584wrb.182.1504889274194; \n\tFri, 08 Sep 2017 09:47:54 -0700 (PDT)","Date":"Fri, 8 Sep 2017 18:47:55 +0200","From":"Simon Horman ","To":"Yuanhan Liu ","Message-ID":"<20170908164754.GD7356@vergenet.net>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-Spam-Status":"No, score=0.5 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"dev@openvswitch.org","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1765594,"web_url":"http://patchwork.ozlabs.org/comment/1765594/","msgid":"","list_archive_url":null,"date":"2017-09-08T19:20:20","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"On 9/8/17, 9:47 AM, \"ovs-dev-bounces@openvswitch.org on behalf of Simon Horman\" wrote:\n\n On Tue, Sep 05, 2017 at 05:22:57PM +0800, Yuanhan Liu wrote:\n > From: Finn Christensen \n > \n > The basic yet the major part of this patch is to translate the \"match\"\n > to rte flow patterns. And then, we create a rte flow with a MARK action.\n > Afterwards, all pkts matches the flow will have the mark id in the mbuf.\n > \n > For any unsupported flows, such as MPLS, -1 is returned, meaning the\n > flow offload is failed and then skipped.\n \n ...\n \n > +static int\n > +netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,\n > + const struct match *match,\n > + struct nlattr *nl_actions OVS_UNUSED,\n > + size_t actions_len,\n > + const ovs_u128 *ufid,\n > + struct offload_info *info)\n > +{\n > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);\n > + const struct rte_flow_attr flow_attr = {\n > + .group = 0,\n > + .priority = 0,\n > + .ingress = 1,\n > + .egress = 0\n > + };\n > + struct flow_patterns patterns = { .items = NULL, .cnt = 0 };\n > + struct flow_actions actions = { .actions = NULL, .cnt = 0 };\n \n I believe the following will give the same result as the above,\n less verbosely.\n \n + const struct rte_flow_attr flow_attr = { .ingress = 1 };\n + struct flow_patterns patterns = { .cnt = 0 };\n + struct flow_actions actions = { .cnt = 0 };\n\n[Darrell] I understand your good point about succinctness.\n Prior art seems to be towards showing all used members because\n it makes it clear nothing was missed and initializing to zero was intentional.\n\n \n > + struct rte_flow *flow;\n > + struct rte_flow_error error;\n > + uint8_t *ipv4_next_proto_mask = NULL;\n > + int ret = 0;\n > +\n > + /* Eth */\n > + struct rte_flow_item_eth eth_spec;\n > + struct rte_flow_item_eth eth_mask;\n \n Empty line here please.\n \n > + memset(ð_mask, 0, sizeof(eth_mask));\n > + if (match->wc.masks.dl_src.be16[0] ||\n > + match->wc.masks.dl_src.be16[1] ||\n > + match->wc.masks.dl_src.be16[2] ||\n > + match->wc.masks.dl_dst.be16[0] ||\n > + match->wc.masks.dl_dst.be16[1] ||\n > + match->wc.masks.dl_dst.be16[2]) {\n \n It looks like eth_addr_is_zero() can be used in the above.\n \n > + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\n > + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\n > + eth_spec.type = match->flow.dl_type;\n > +\n > + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\n > + sizeof(eth_mask.dst));\n > + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\n > + sizeof(eth_mask.src));\n > + eth_mask.type = match->wc.masks.dl_type;\n > +\n > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\n > + ð_spec, ð_mask);\n > + } else {\n > + /*\n > + * If user specifies a flow (like UDP flow) without L2 patterns,\n > + * OVS will at least set the dl_type. Normally, it's enough to\n > + * create an eth pattern just with it. Unluckily, some Intel's\n > + * NIC (such as XL710) doesn't support that. Below is a workaround,\n > + * which simply matches any L2 pkts.\n > + */\n > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\n > + }\n \n This feels a lot like a layer violation - making the core aware\n of specific implementation details of lower layers.\n \n From a functional point of view, is the idea that\n a eth_type+5-tuple match is converted to a 5-tuple match?\n \n > +\n > + /* VLAN */\n > + struct rte_flow_item_vlan vlan_spec;\n > + struct rte_flow_item_vlan vlan_mask;\n \n Please declare all local variables at the top of the context\n (in this case function).\n\n[Darrell] In OVS, we try to keep the variable declarations as close to their \n usage as possible, including inside condition checks when appropriate.\n This is easily forgotten and sometimes missed in review.\n \n ...\n \n > + struct rte_flow_item_udp udp_spec;\n > + struct rte_flow_item_udp udp_mask;\n > + memset(&udp_mask, 0, sizeof(udp_mask));\n > + if ((proto == IPPROTO_UDP) &&\n > + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\n > + udp_spec.hdr.src_port = match->flow.tp_src;\n > + udp_spec.hdr.dst_port = match->flow.tp_dst;\n > +\n > + udp_mask.hdr.src_port = match->wc.masks.tp_src;\n > + udp_mask.hdr.dst_port = match->wc.masks.tp_dst;\n > +\n > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_UDP,\n > + &udp_spec, &udp_mask);\n > +\n > + /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match */\n > + if (ipv4_next_proto_mask) {\n > + *ipv4_next_proto_mask = 0;\n \n I think this should be:\n \n + *ipv4_next_proto_mask = NULL;\n \n > + }\n > + }\n > +\n > + struct rte_flow_item_sctp sctp_spec;\n > + struct rte_flow_item_sctp sctp_mask;\n > + memset(&sctp_mask, 0, sizeof(sctp_mask));\n > + if ((proto == IPPROTO_SCTP) &&\n \n It seems there are unnecessary () in the line above.\n \n ...\n \n > +/*\n > + * Validate for later rte flow offload creation. If any unsupported\n > + * flow are specified, return -1.\n > + */\n > +static int\n > +netdev_dpdk_validate_flow(const struct match *match)\n > +{\n > + struct match match_zero_wc;\n > +\n > + /* Create a wc-zeroed version of flow */\n > + match_init(&match_zero_wc, &match->flow, &match->wc);\n > +\n > +#define CHECK_NONZERO_BYTES(addr, size) do { \\\n \n I think do should appear on a new line.\n\n[Darrell] I would prefer to just convert to a function since I think it would be\n more maintainable and clear, in this case.\n\n \n > + uint8_t *padr = (uint8_t *)(addr); \\\n > + int i; \\\n > + for (i = 0; i < (size); i++) { \\\n > + if (padr[i] != 0) { \\\n > + goto err; \\\n > + } \\\n > + } \\\n > +} while (0)\n > +\n > +#define CHECK_NONZERO(var) do { \\\n \n Here too.\n\n[Darrell] I would prefer to just convert to a function since I think it would be\n more maintainable and clear, in this case.\n\n \n > + if ((var) != 0) { \\\n > + goto err; \\\n > + } \\\n > +} while (0)\n\n[Darrell] I would prefer to just convert to a function since I think it would be\n more maintainable and clear, in this case.\n \n I think its better if macros (and defines) aren't declared\n inside of functions. The top of the file seems like\n the best place to me. If not above the first function\n that uses the macros, presumably this function.\n \n ...\n _______________________________________________\n dev mailing list\n dev@openvswitch.org\n https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=Zu47bSXASTdRRjIBaZ-6LV4Njey_vX22Kzs_OCUMowE&s=Cb8_oND5SSzqrVdzjO-AQ5kT6gU4_OMa3I5-6QtJkO8&e=","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"b7N2kbDS\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpnGM6N1tz9sRV\n\tfor ;\n\tSat, 9 Sep 2017 05:20:27 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id B5876CBB;\n\tFri, 8 Sep 2017 19:20:25 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id A2231BC9\n\tfor ; Fri, 8 Sep 2017 19:20:23 +0000 (UTC)","from NAM03-DM3-obe.outbound.protection.outlook.com\n\t(mail-dm3nam03on0072.outbound.protection.outlook.com [104.47.41.72])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id DD25A271\n\tfor ; Fri, 8 Sep 2017 19:20:22 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB532.namprd05.prod.outlook.com (10.141.29.151) with Microsoft\n\tSMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.56.4; Fri, 8 Sep 2017 19:20:20 +0000","from BLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) by\n\tBLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) with mapi id\n\t15.20.0056.003; Fri, 8 Sep 2017 19:20:20 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=H6upCivYV9KagM26gdUY4EkfQ5umLSCZyOvgAKyuPlY=;\n\tb=b7N2kbDSohLAnz1RxASqK7XBQT0dJV7bei1toQ+CbBkCLchGbOq2P4IwMVP2ZRkNXvF4SHUcgpr0H5C0hs+TUbbQsDxsMYmZiKlttS6l3k6AZyte977fxRF3oDPreOuZDT96QY7MnTpSaEzyvcfurLkbCM0AchpDQ8nJn6SI+AI=","From":"Darrell Ball ","To":"Simon Horman , Yuanhan Liu\n\t","Thread-Topic":"[ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","Thread-Index":"AQHTJiiw1ZjL6RzFTUOQLht0V/vcqaKrOB6A//+1PIA=","Date":"Fri, 8 Sep 2017 19:20:20 +0000","Message-ID":"","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<20170908164754.GD7356@vergenet.net>","In-Reply-To":"<20170908164754.GD7356@vergenet.net>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.23.0.170610","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"b7N2kbDS\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"x-originating-ip":"[73.162.236.45]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB532;\n\t20:/q1KxlPpe0Z+xxqYY4QBWVIAhM5l5GMpyO/1Zuh5/7V5chx+TCFF8rpN4yddgNyc5HoR++M5rNcgAhshSE8aBZNiQhe5bWea7vdJGjCEaVxUp2Kzq+VNTcFeGcKNWSCL7sPpsjFnSKdggN9wsZzC6eh9JejjKzU5yfz5YrQx/9w=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"47cfdde3-f32b-4660-2e5e-08d4f6eea593","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:BLUPR05MB532; ","x-ms-traffictypediagnostic":"BLUPR05MB532:","x-exchange-antispam-report-test":"UriScan:(10436049006162)(216315784871565)(17755550239193); ","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(10201501046)(100000703101)(100105400095)(93006095)(93001095)(3002001)(6041248)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123555025)(20161123560025)(20161123564025)(20161123562025)(20161123558100)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB532; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB532; ","x-forefront-prvs":"04244E0DC5","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(39860400002)(24454002)(189002)(377454003)(199003)(4326008)(2906002)(82746002)(229853002)(8936002)(105586002)(39060400002)(83716003)(83506001)(2900100001)(6246003)(33656002)(97736004)(6486002)(4001350100001)(68736007)(3280700002)(8676002)(3660700001)(36756003)(86362001)(478600001)(77096006)(5660300001)(81156014)(6116002)(81166006)(102836003)(966005)(3846002)(6506006)(25786009)(66066001)(14454004)(106356001)(76176999)(54356999)(53936002)(101416001)(50986999)(305945005)(6306002)(189998001)(6436002)(6512007)(7736002)(53546010)(2950100002)(99286003);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB532;\n\tH:BLUPR05MB611.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"<4CE789696AC1F942BA4983FB584FF696@namprd05.prod.outlook.com>","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"08 Sep 2017 19:20:20.4274\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB532","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1765969,"web_url":"http://patchwork.ozlabs.org/comment/1765969/","msgid":"<2EF2F5C0CC56984AA024D0B180335FCB4221DC41@IRSMSX102.ger.corp.intel.com>","list_archive_url":null,"date":"2017-09-10T16:32:19","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","submitter":{"id":67440,"url":"http://patchwork.ozlabs.org/api/people/67440/","name":"Chandran, Sugesh","email":"sugesh.chandran@intel.com"},"content":"As mentioned earlier in the cover letter we also have a similar implementation to do the flow translate.\nI feel its good to make bit more modular for this translation. The reason being its easy to extend and add more protocols in the future.\n\n Please find below our similar implementation. We have started with single function implementation as in patch series, and eventually moved\nto this approach. \n I can share the code as a patch if you are interested. Again the approach below work well for our use case. Feel free to\ndiscard if it doesn't make any sense/it's an overhead.\n\n\nThe flow and actions translate were invoked from dpdkhw_rte_flow_xlate and ' dpdkhw_rte_action_xlate'.\nstruct flow_xlate_dic {\n enum rte_flow_item_type rte_flow_type;\n /*\n * Flow xlate function to translate specific header match into rtl format.\n * Each rte_flow_item_type, it is necessary to define a corresponding\n * xlate function in this structure. Return 0 if the flow is being translated\n * successfully and error code otherwise.\n */\n int (*flow_xlate)(struct match *match, struct rte_flow_item *hw_flow_item,\n const void *md);\n};\n\nstatic int do_inport_flow_xlate(struct match *match,\n struct rte_flow_item *hw_flow_item, const void *md);\nstatic int do_l2_flow_xlate(struct match *match,\n struct rte_flow_item *hw_flow_item, const void *md);\nstatic int do_l3_flow_xlate(struct match *match,\n struct rte_flow_item *hw_flow_item, const void *md);\nstatic int do_l4_flow_xlate(struct match *match,\n struct rte_flow_item *hw_flow_item, const void *md);\nstatic int do_end_flow_xlate(struct match *match,\n struct rte_flow_item *hw_flow_item, const void *md);\n\nstruct flow_xlate_dic PORT_FLOW_XLATE = {\n RTE_FLOW_ITEM_TYPE_PORT,\n do_inport_flow_xlate\n};\n\nstruct flow_xlate_dic L2_FLOW_XLATE = {\n RTE_FLOW_ITEM_TYPE_ETH,\n do_l2_flow_xlate\n};\n\nstruct flow_xlate_dic L3_FLOW_XLATE = {\n RTE_FLOW_ITEM_TYPE_VOID, /* L3 flow item can be different */\n do_l3_flow_xlate\n};\n\n\nstruct flow_xlate_dic L4_FLOW_XLATE = {\n RTE_FLOW_ITEM_TYPE_VOID, /* Can be UDP/TCP */\n do_l4_flow_xlate\n};\n\nstruct flow_xlate_dic END_FLOW_XLATE = {\n RTE_FLOW_ITEM_TYPE_END,\n do_end_flow_xlate\n};\n\n/*\n * Convert the mac address to little endian for flow programming\n */\nstatic void\nntoh_mac(struct eth_addr *src, struct eth_addr *dst)\n{\n int i;\n for (i = 0; i < 6; i++) {\n dst->ea[6 - i - 1] = src->ea[i];\n }\n}\n\nstatic int\ndo_inport_flow_xlate(struct match *match, struct rte_flow_item *hw_flow_item,\n const void *md)\n{\n struct flow *flow;\n struct rte_flow_item_port *port_flow_item;\n struct rte_flow_item_port *port_flow_item_mask;\n struct netdev *netdev;\n uint16_t hw_portno;\n struct offload_info *ofld_info = (struct offload_info *)md;\n\n flow = &match->flow;\n port_flow_item = xzalloc (sizeof *port_flow_item);\n port_flow_item_mask = xzalloc(sizeof *port_flow_item_mask);\n if(!port_flow_item) {\n VLOG_ERR(\"Failed to allocate the memory for hardware flow item\");\n return ENOMEM;\n }\n\n netdev = get_hw_netdev(flow->in_port.odp_port, ofld_info->port_hmap_obj);\n if (!netdev) {\n VLOG_WARN(\"Inport %u is not a valid hardware accelerated port.\",\n odp_to_u32(flow->in_port.odp_port));\n return EOPNOTSUPP;\n }\n /* The inport should be the hardware port number, not the ovs portno */\n hw_portno = netdev_get_hw_portno(netdev);\n port_flow_item->index = hw_portno;\n\n hw_flow_item->type = PORT_FLOW_XLATE.rte_flow_type;\n hw_flow_item->spec = port_flow_item;\n hw_flow_item->last = NULL;\n\n /* Set the mask for the rte port flow */\n port_flow_item_mask->index = 0xFFFFFFFF;\n hw_flow_item->mask = port_flow_item_mask;\n return 0;\n}\n\nstatic int\ndo_l2_flow_xlate(struct match *match, struct rte_flow_item *hw_flow_item,\n const void *md)\n{\n struct flow *flow, *mask;\n struct rte_flow_item_eth *eth_flow_item;\n struct rte_flow_item_eth *eth_flow_mask;\n struct eth_addr mac_le;\n flow = &match->flow;\n mask = &match->wc.masks;\n bool is_l2_zero = 0;\n is_l2_zero = eth_addr_is_zero(mask->dl_dst);\n is_l2_zero &= eth_addr_is_zero(mask->dl_src);\n if(is_l2_zero) {\n VLOG_INFO(\"Cannot install flow with zero eth addr\");\n return EOPNOTSUPP;\n }\n eth_flow_item = xzalloc(sizeof *eth_flow_item);\n eth_flow_mask = xzalloc(sizeof *eth_flow_mask);\n if(!eth_flow_item || !eth_flow_mask) {\n VLOG_ERR(\"Failed to allocate the memory for flow item\");\n return ENOMEM;\n }\n ntoh_mac(&flow->dl_dst, &mac_le);\n memcpy(ð_flow_item->dst, &mac_le, sizeof(eth_flow_item->dst));\n\n ntoh_mac(&flow->dl_src, &mac_le);\n memcpy(ð_flow_item->src, &mac_le, sizeof(eth_flow_item->src));\n\n eth_flow_item->type = ntohs(flow->dl_type);\n\n /* Copy the address mask too */\n ntoh_mac(&mask->dl_dst, &mac_le);\n memcpy(ð_flow_mask->dst, &mac_le, sizeof(eth_flow_mask->dst));\n\n ntoh_mac(&mask->dl_src, &mac_le);\n memcpy(ð_flow_mask->src, &mac_le, sizeof(eth_flow_mask->src));\n eth_flow_mask->type = ntohs(mask->dl_type);\n\n hw_flow_item->type = L2_FLOW_XLATE.rte_flow_type;\n hw_flow_item->spec = eth_flow_item;\n hw_flow_item->last = NULL;\n hw_flow_item->mask = eth_flow_mask;\n return 0;\n}\n\nstatic int\ndo_l3_flow_xlate(struct match *match, struct rte_flow_item *hw_flow_item,\n const void *md)\n{\n struct flow *flow, *mask;\n\n /*Currently supports only ipv4 */\n flow = &match->flow;\n mask = &match->wc.masks;\n\n if(flow->dl_type == htons(ETH_TYPE_IP)) {\n struct rte_flow_item_ipv4 *ipv4_flow_item, *ipv4_flow_mask;\n\n ipv4_flow_item = xzalloc(sizeof *ipv4_flow_item);\n ipv4_flow_mask = xzalloc(sizeof *ipv4_flow_mask);\n\n /* Xlate the ip flow entries */\n ipv4_flow_item->hdr.src_addr = ntohl(flow->nw_src);\n ipv4_flow_item->hdr.dst_addr = ntohl(flow->nw_dst);\n ipv4_flow_item->hdr.next_proto_id = flow->nw_proto;\n ipv4_flow_item->hdr.type_of_service = flow->nw_tos;\n ipv4_flow_item->hdr.version_ihl = 4;\n hw_flow_item->type = RTE_FLOW_ITEM_TYPE_IPV4;\n hw_flow_item->spec = ipv4_flow_item;\n hw_flow_item->last = NULL;\n\n /* Xlate ipv4 mask entries */\n ipv4_flow_mask->hdr.src_addr = ntohl(mask->nw_src);\n ipv4_flow_mask->hdr.dst_addr = ntohl(mask->nw_dst);\n ipv4_flow_mask->hdr.next_proto_id = mask->nw_proto;\n ipv4_flow_mask->hdr.type_of_service = mask->nw_tos;\n ipv4_flow_mask->hdr.version_ihl = 0xFF;\n hw_flow_item->mask = ipv4_flow_mask;\n }\n else {\n VLOG_INFO(\"Not a IP flow, %d\", flow->dl_type);\n return EOPNOTSUPP;\n }\n return 0;\n\n}\n\nstatic int\ndo_l4_flow_xlate(struct match *match, struct rte_flow_item *hw_flow_item,\n const void *md)\n{\n struct flow *flow, *mask;\n\n /*Currently supports only UDP/TCP */\n flow = &match->flow;\n mask = &match->wc.masks;\n\n if(flow->nw_proto == IPPROTO_TCP) {\n struct rte_flow_item_tcp *tcp_flow_item, *tcp_flow_mask;\n tcp_flow_item = xzalloc(sizeof *tcp_flow_item);\n tcp_flow_mask = xzalloc(sizeof *tcp_flow_mask);\n\n /* Xlate tcp flow entries */\n tcp_flow_item->hdr.src_port = ntohs(flow->tp_src);\n tcp_flow_item->hdr.dst_port = ntohs(flow->tp_dst);\n hw_flow_item->type = RTE_FLOW_ITEM_TYPE_TCP;\n hw_flow_item->spec = tcp_flow_item;\n hw_flow_item->last = NULL;\n\n /* Xlate tcp flow mask entries */\n tcp_flow_mask->hdr.src_port = ntohs(mask->tp_src);\n tcp_flow_mask->hdr.dst_port = ntohs(mask->tp_dst);\n hw_flow_item->mask = tcp_flow_mask;\n }\n else if (flow->nw_proto == IPPROTO_UDP) {\n struct rte_flow_item_udp *udp_flow_item, *udp_flow_mask;\n udp_flow_item = xzalloc(sizeof *udp_flow_item);\n udp_flow_mask = xzalloc(sizeof *udp_flow_mask);\n\n /* xlate UDP flow entries */\n udp_flow_item->hdr.src_port = ntohs(flow->tp_src);\n udp_flow_item->hdr.dst_port = ntohs(flow->tp_dst);\n hw_flow_item->type = RTE_FLOW_ITEM_TYPE_UDP;\n hw_flow_item->spec = udp_flow_item;\n hw_flow_item->last = NULL;\n\n /* Xlate UDP mask entries */\n udp_flow_mask->hdr.src_port = ntohs(mask->tp_src);\n udp_flow_mask->hdr.dst_port = ntohs(mask->tp_dst);\n hw_flow_item->mask = udp_flow_mask;\n }\n else {\n VLOG_INFO(\"Not a TCP/UDP flow\");\n return ENOTSUP;\n }\n return 0;\n}\n\nstatic int\ndo_end_flow_xlate(struct match *match, struct rte_flow_item *hw_flow_item,\n const void *md)\n{\n hw_flow_item->type = RTE_FLOW_ITEM_TYPE_END;\n return 0;\n}\n\nstatic int\ndo_flow_xlate_helper(struct flow_xlate_dic xlate_dic_entry,\n struct match *match,\n struct rte_flow_item *hw_flow_item,\n int *index,\n const void *md,\n int max_flow_entry)\n{\n int ret = 0;\n if(*index < max_flow_entry) {\n ret = xlate_dic_entry.flow_xlate(match, hw_flow_item, md);\n }\n (*index) = ret ? (*index) : *index + 1;\n return ret;\n}\n\n#define DO_FLOW_XLATE(XLATE_DIC_ENTRY, MATCH_ENTRY, FLOW_ITEM, IDX_PTR, MD_PTR,\\\n MAX_FLOW) \\\n do_flow_xlate_helper(XLATE_DIC_ENTRY, MATCH_ENTRY, FLOW_ITEM, IDX_PTR, \\\n MD_PTR, MAX_FLOW)\n\nint\ndpdkhw_rte_flow_xlate(struct match *match,\n struct rte_flow_attr *hw_flow_attr,\n struct rte_flow_item hw_flow_batch[],\n const struct offload_info *ofld_info)\n{\n int i = 0;\n /* Keep the last one for the END FLOW type */\n int max_flow_entry = MAX_DPDKHW_RTE_FLOW_SIZE - 1;\n hw_flow_attr->group = 0;\n hw_flow_attr->priority = 0;\n hw_flow_attr->ingress = 1; /* Supports only ingress flow rules now */\n int ret = 0;\n\n /*\n * List of supported rte flow entries are populated below. Each header\n * has its own xlate function to generate the corresponding rte_flow entry.\n * The translate functions operate on each header fields independently in\n * the stack. It is possible that the flow can have match entry for port and\n * ip address when the mac flow translation is failed to do.\n */\n ret |= DO_FLOW_XLATE(PORT_FLOW_XLATE, match, &hw_flow_batch[i], &i,\n ofld_info, max_flow_entry);\n ret |= DO_FLOW_XLATE(L2_FLOW_XLATE, match, &hw_flow_batch[i], &i, NULL,\n max_flow_entry);\n ret |= DO_FLOW_XLATE(L3_FLOW_XLATE, match, &hw_flow_batch[i], &i, NULL,\n max_flow_entry);\n ret |= DO_FLOW_XLATE(L4_FLOW_XLATE, match, &hw_flow_batch[i], &i, NULL,\n max_flow_entry);\n /* Always the END function is called as a last function,\n * DO NOT ADD ANY TRANSLATE FUNCTION POST END XLATE.\n */\n DO_FLOW_XLATE(END_FLOW_XLATE, match, &hw_flow_batch[i], &i, NULL,\n MAX_DPDKHW_RTE_FLOW_SIZE);\n return ret;\n}\n\nint\ndpdkhw_rte_action_xlate(struct rte_flow_action hw_action_batch[],\n const struct nlattr *actions,\n size_t actions_len,\n const struct offload_info *ofld_info)\n{\n const struct nlattr *a;\n unsigned int left;\n int i = 0;\n int max_action_entry = MAX_DPDKHW_RTE_FLOW_SIZE - 1;\n\n if (!actions_len || !actions) {\n VLOG_WARN_RL(&rl, \"No actions to offload, not installing flows\");\n return EPERM;\n }\n NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) {\n int type = nl_attr_type(a);\n if(i >= max_action_entry) {\n VLOG_ERR(\"Max action entry limit reached, cannot add more actions\");\n return EPERM;\n }\n switch ((enum ovs_action_attr) type) {\n case OVS_ACTION_ATTR_OUTPUT: {\n /*\n * Current POC only supports the output action to a port.\n * TODO :: rte_flow supports only output to a vf function. Need a\n * port id based output action. Lets use the vf output actions with\n * a port_id now.\n */\n struct rte_flow_action_vf *rte_action_vf =\n xmalloc(sizeof *rte_action_vf);\n odp_port_t out_port = nl_attr_get_odp_port(a);\n /* Output port should be hardware port number. */\n struct netdev *netdev = get_hw_netdev(out_port,\n ofld_info->port_hmap_obj);\n if (!netdev) {\n VLOG_WARN(\"Cannot offload a flow with non accelerated output\"\n \" port %u\", odp_to_u32(out_port));\n return EPERM;\n }\n\n uint16_t hw_portno = netdev_get_hw_portno(netdev);\n rte_action_vf->original = 1;\n rte_action_vf->id = hw_portno;\n\n hw_action_batch[i].type = RTE_FLOW_ACTION_TYPE_VF;\n hw_action_batch[i].conf = rte_action_vf;\n i++;\n break;\n }\n case OVS_ACTION_ATTR_TUNNEL_PUSH:\n case OVS_ACTION_ATTR_TUNNEL_POP:\n case OVS_ACTION_ATTR_SET:\n case OVS_ACTION_ATTR_PUSH_VLAN:\n case OVS_ACTION_ATTR_POP_VLAN:\n case OVS_ACTION_ATTR_PUSH_MPLS:\n case OVS_ACTION_ATTR_POP_MPLS:\n case OVS_ACTION_ATTR_SET_MASKED:\n case OVS_ACTION_ATTR_SAMPLE:\n case OVS_ACTION_ATTR_HASH:\n case OVS_ACTION_ATTR_UNSPEC:\n case OVS_ACTION_ATTR_TRUNC:\n case __OVS_ACTION_ATTR_MAX:\n case OVS_ACTION_ATTR_USERSPACE:\n case OVS_ACTION_ATTR_RECIRC:\n case OVS_ACTION_ATTR_CT:\n VLOG_INFO_RL(&rl, \"TODO actions\");\n OVS_NOT_REACHED();\n break;\n default:\n VLOG_INFO_RL(&rl, \"Unsupported action to offload\");\n break;\n }\n }\n /* Add end action as a last action */\n hw_action_batch[i].type = RTE_FLOW_ACTION_TYPE_END;\n hw_action_batch[i].conf = NULL;\n return 0;\n}\n\nRegards\n_Sugesh\n\n\n> -----Original Message-----\n> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-\n> bounces@openvswitch.org] On Behalf Of Yuanhan Liu\n> Sent: Tuesday, September 5, 2017 10:23 AM\n> To: dev@openvswitch.org\n> Subject: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with rte\n> flow\n> \n> From: Finn Christensen \n> \n> The basic yet the major part of this patch is to translate the \"match\"\n> to rte flow patterns. And then, we create a rte flow with a MARK action.\n> Afterwards, all pkts matches the flow will have the mark id in the mbuf.\n> \n> For any unsupported flows, such as MPLS, -1 is returned, meaning the flow\n> offload is failed and then skipped.\n> \n> Co-authored-by: Yuanhan Liu \n> Signed-off-by: Finn Christensen \n> Signed-off-by: Yuanhan Liu \n> ---\n> \n> v2: - convert some macros to functions\n> - do not hardcode the max number of flow/action\n> - fix L2 patterns for Intel nic\n> - add comments for not implemented offload methods\n> ---\n> lib/netdev-dpdk.c | 421\n> +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n> 1 file changed, 420 insertions(+), 1 deletion(-)\n> \n> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 46f9885..37b0f99\n> 100644\n> --- a/lib/netdev-dpdk.c\n> +++ b/lib/netdev-dpdk.c\n\n[Snip]\n> + if (!flow) {\n> + VLOG_ERR(\"rte flow creat error: %u : message : %s\\n\",\n> + error.type, error.message);\n> + ret = -1;\n> + goto out;\n> + }\n> + add_ufid_dpdk_flow_mapping(ufid, flow);\n> + VLOG_INFO(\"installed flow %p by ufid \"UUID_FMT\"\\n\",\n> + flow, UUID_ARGS((struct uuid *)ufid));\n> +\n> +out:\n> + free(patterns.items);\n> + free(actions.actions);\n> + return ret;\n> +}\n> +\n> +/*\n> + * Validate for later rte flow offload creation. If any unsupported\n> + * flow are specified, return -1.\n> + */\n[Sugesh] I feel this is very hardware centric. There are chances hardware can support\nIpv6 or other fields in a packet. This has to be based on what flow fields a hardware can support.\n\n> +static int\n> +netdev_dpdk_validate_flow(const struct match *match) {\n> + struct match match_zero_wc;\n> +\n> + /* Create a wc-zeroed version of flow */\n> + match_init(&match_zero_wc, &match->flow, &match->wc);\n> +\n> +#define CHECK_NONZERO_BYTES(addr, size) do { \\\n> + uint8_t *padr = (uint8_t *)(addr); \\\n> + int i; \\\n> + for (i = 0; i < (size); i++) { \\\n> + if (padr[i] != 0) { \\\n> + goto err; \\\n> + } \\\n> + } \\\n> +} while (0)\n> +\n> +#define CHECK_NONZERO(var) do { \\\n> + if ((var) != 0) { \\\n> + goto err; \\\n> + } \\\n> +} while (0)\n> +\n> + CHECK_NONZERO_BYTES(&match_zero_wc.flow.tunnel,\n> + sizeof(match_zero_wc.flow.tunnel));\n> + CHECK_NONZERO(match->wc.masks.metadata);\n> + CHECK_NONZERO(match->wc.masks.skb_priority);\n> + CHECK_NONZERO(match->wc.masks.pkt_mark);\n> + CHECK_NONZERO(match->wc.masks.dp_hash);\n> +\n> + /* recirc id must be zero */\n> + CHECK_NONZERO(match_zero_wc.flow.recirc_id);\n> +\n> + CHECK_NONZERO(match->wc.masks.ct_state);\n> + CHECK_NONZERO(match->wc.masks.ct_zone);\n> + CHECK_NONZERO(match->wc.masks.ct_mark);\n> + CHECK_NONZERO(match->wc.masks.ct_label.u64.hi);\n> + CHECK_NONZERO(match->wc.masks.ct_label.u64.lo);\n> + CHECK_NONZERO(match->wc.masks.ct_nw_proto);\n> + CHECK_NONZERO(match->wc.masks.ct_tp_src);\n> + CHECK_NONZERO(match->wc.masks.ct_tp_dst);\n> + CHECK_NONZERO(match->wc.masks.conj_id);\n> + CHECK_NONZERO(match->wc.masks.actset_output);\n> +\n> + /* unsupported L2 */\n> + CHECK_NONZERO_BYTES(&match->wc.masks.mpls_lse,\n> + sizeof(match_zero_wc.flow.mpls_lse) /\n> + sizeof(ovs_be32));\n> +\n> + /* unsupported L3 */\n> + CHECK_NONZERO_BYTES(&match->wc.masks.ipv6_src, sizeof(struct\n> in6_addr));\n> + CHECK_NONZERO_BYTES(&match->wc.masks.ipv6_dst, sizeof(struct\n> in6_addr));\n> + CHECK_NONZERO(match->wc.masks.ipv6_label);\n> + CHECK_NONZERO_BYTES(&match->wc.masks.nd_target, sizeof(struct\n> in6_addr));\n> + CHECK_NONZERO_BYTES(&match->wc.masks.arp_sha, sizeof(struct\n> eth_addr));\n> + CHECK_NONZERO_BYTES(&match->wc.masks.arp_tha, sizeof(struct\n> + eth_addr));\n> +\n> + /* If fragmented, then don't HW accelerate - for now */\n> + CHECK_NONZERO(match_zero_wc.flow.nw_frag);\n> +\n> + /* unsupported L4 */\n> + CHECK_NONZERO(match->wc.masks.igmp_group_ip4);\n> +\n> + return 0;\n> +\n> +err:\n> + VLOG_INFO(\"Cannot HW accelerate this flow\");\n> + return -1;\n> +}\n> +\n> +static int\n> +netdev_dpdk_destroy_rte_flow(struct netdev_dpdk *dev,\n> + const ovs_u128 *ufid,\n> + struct rte_flow *rte_flow) {\n> + struct rte_flow_error error;\n> + int ret;\n> +\n> + ret = rte_flow_destroy(dev->port_id, rte_flow, &error);\n> + if (ret == 0) {\n> + del_ufid_dpdk_flow_mapping(ufid);\n> + VLOG_INFO(\"removed rte flow %p associated with ufid \" UUID_FMT\n> \"\\n\",\n> + rte_flow, UUID_ARGS((struct uuid *)ufid));\n> + } else {\n> + VLOG_ERR(\"rte flow destroy error: %u : message : %s\\n\",\n> + error.type, error.message);\n> + }\n> +\n> + return ret;\n> +}\n> +\n> +static int\n> +netdev_dpdk_flow_put(struct netdev *netdev, struct match *match,\n> + struct nlattr *actions, size_t actions_len,\n> + const ovs_u128 *ufid, struct offload_info *info,\n> + struct dpif_flow_stats *stats OVS_UNUSED) {\n> + struct rte_flow *rte_flow;\n> + int ret;\n> +\n> + /*\n> + * If an old rte_flow exists, it means it's a flow modification.\n> + * Here destroy the old rte flow first before adding a new one.\n> + */\n> + rte_flow = get_rte_flow_by_ufid(ufid);\n> + if (rte_flow) {\n> + ret = netdev_dpdk_destroy_rte_flow(netdev_dpdk_cast(netdev),\n> + ufid, rte_flow);\n> + if (ret < 0)\n> + return ret;\n> + }\n> +\n> + ret = netdev_dpdk_validate_flow(match);\n> + if (ret < 0) {\n> + return ret;\n> + }\n> +\n> + return netdev_dpdk_add_rte_flow_offload(netdev, match, actions,\n> + actions_len, ufid, info); }\n> +\n> +#define DPDK_FLOW_OFFLOAD_API \\\n> + NULL, /* flow_flush */ \\\n> + NULL, /* flow_dump_create */ \\\n> + NULL, /* flow_dump_destroy */ \\\n> + NULL, /* flow_dump_next */ \\\n> + netdev_dpdk_flow_put, \\\n> + NULL, /* flow_get */ \\\n> + NULL, /* flow_del */ \\\n> + NULL /* init_flow_api */\n> +\n> +\n> #define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT, \\\n> SET_CONFIG, SET_TX_MULTIQ, SEND, \\\n> GET_CARRIER, GET_STATS, \\\n> @@ -3472,7 +3891,7 @@ get_rte_flow_by_ufid(const ovs_u128 *ufid)\n> RXQ_RECV, \\\n> NULL, /* rx_wait */ \\\n> NULL, /* rxq_drain */ \\\n> - NO_OFFLOAD_API \\\n> + DPDK_FLOW_OFFLOAD_API \\\n> }\n> \n> static const struct netdev_class dpdk_class =\n> --\n> 2.7.4\n> \n> _______________________________________________\n> dev mailing list\n> dev@openvswitch.org\n> https://mail.openvswitch.org/mailman/listinfo/ovs-dev","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xqxRc4Pb4z9s7g\n\tfor ;\n\tMon, 11 Sep 2017 02:32:27 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 5549DA81;\n\tSun, 10 Sep 2017 16:32:24 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id BD7D7A7F\n\tfor ; Sun, 10 Sep 2017 16:32:23 +0000 (UTC)","from mga07.intel.com (mga07.intel.com [134.134.136.100])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 83F01E5\n\tfor ; Sun, 10 Sep 2017 16:32:22 +0000 (UTC)","from orsmga005.jf.intel.com ([10.7.209.41])\n\tby orsmga105.jf.intel.com with ESMTP; 10 Sep 2017 09:32:21 -0700","from irsmsx106.ger.corp.intel.com ([163.33.3.31])\n\tby orsmga005.jf.intel.com with ESMTP; 10 Sep 2017 09:32:20 -0700","from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by\n\tIRSMSX106.ger.corp.intel.com ([169.254.8.36]) with mapi id\n\t14.03.0319.002; Sun, 10 Sep 2017 17:32:19 +0100"],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos;i=\"5.42,372,1500966000\"; d=\"scan'208\";a=\"147585771\"","From":"\"Chandran, Sugesh\" ","To":"Yuanhan Liu , \"dev@openvswitch.org\"\n\t","Thread-Topic":"[ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","Thread-Index":"AQHTJikq1p1dhEbNk0iD2XBUGg33RqKuF0SQ","Date":"Sun, 10 Sep 2017 16:32:19 +0000","Message-ID":"<2EF2F5C0CC56984AA024D0B180335FCB4221DC41@IRSMSX102.ger.corp.intel.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>","In-Reply-To":"<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","x-titus-metadata-40":"eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTc4OWNiZWMtZDM3Zi00NzZmLTgyZDktMDcyMTJiZWI5Y2UyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IlFGRkxjMWpaOXhnaE1VXC9xMnZIMEdPT3BKR29Ra0dXRm14R25ESHRmd0U0PSJ9","x-ctpclassification":"CTP_IC","dlp-product":"dlpe-windows","dlp-version":"11.0.0.116","dlp-reaction":"no-action","x-originating-ip":"[163.33.239.180]","MIME-Version":"1.0","X-Spam-Status":"No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED,\n\tRP_MATCHES_RCVD autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766052,"web_url":"http://patchwork.ozlabs.org/comment/1766052/","msgid":"<20170911061102.GM9736@yliu-home>","list_archive_url":null,"date":"2017-09-11T06:11:02","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Fri, Sep 08, 2017 at 06:47:55PM +0200, Simon Horman wrote:\n> On Tue, Sep 05, 2017 at 05:22:57PM +0800, Yuanhan Liu wrote:\n> > From: Finn Christensen \n> > \n> > The basic yet the major part of this patch is to translate the \"match\"\n> > to rte flow patterns. And then, we create a rte flow with a MARK action.\n> > Afterwards, all pkts matches the flow will have the mark id in the mbuf.\n> > \n> > For any unsupported flows, such as MPLS, -1 is returned, meaning the\n> > flow offload is failed and then skipped.\n> \n> ...\n> \n> > +static int\n> > +netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,\n> > + const struct match *match,\n> > + struct nlattr *nl_actions OVS_UNUSED,\n> > + size_t actions_len,\n> > + const ovs_u128 *ufid,\n> > + struct offload_info *info)\n> > +{\n> > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);\n> > + const struct rte_flow_attr flow_attr = {\n> > + .group = 0,\n> > + .priority = 0,\n> > + .ingress = 1,\n> > + .egress = 0\n> > + };\n> > + struct flow_patterns patterns = { .items = NULL, .cnt = 0 };\n> > + struct flow_actions actions = { .actions = NULL, .cnt = 0 };\n> \n> I believe the following will give the same result as the above,\n> less verbosely.\n> \n> + const struct rte_flow_attr flow_attr = { .ingress = 1 };\n> + struct flow_patterns patterns = { .cnt = 0 };\n> + struct flow_actions actions = { .cnt = 0 };\n\nI'm not quite sure on that. If the compiler doesn't do zero assigment\ncorrectly, something deadly wrong could happen. They all are short\nstructs, that I think it's fine, IMO. If they are big, I'd prefer an\nexplicit memset.\n\n> > + struct rte_flow *flow;\n> > + struct rte_flow_error error;\n> > + uint8_t *ipv4_next_proto_mask = NULL;\n> > + int ret = 0;\n> > +\n> > + /* Eth */\n> > + struct rte_flow_item_eth eth_spec;\n> > + struct rte_flow_item_eth eth_mask;\n> \n> Empty line here please.\n\nI actually want to bind the two declartions with the following code piece,\nto show that they are tightly related.\n\n> > + memset(ð_mask, 0, sizeof(eth_mask));\n> > + if (match->wc.masks.dl_src.be16[0] ||\n> > + match->wc.masks.dl_src.be16[1] ||\n> > + match->wc.masks.dl_src.be16[2] ||\n> > + match->wc.masks.dl_dst.be16[0] ||\n> > + match->wc.masks.dl_dst.be16[1] ||\n> > + match->wc.masks.dl_dst.be16[2]) {\n> \n> It looks like eth_addr_is_zero() can be used in the above.\n\nYes, we could. Thanks for the tip.\n\n> > + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\n> > + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\n> > + eth_spec.type = match->flow.dl_type;\n> > +\n> > + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\n> > + sizeof(eth_mask.dst));\n> > + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\n> > + sizeof(eth_mask.src));\n> > + eth_mask.type = match->wc.masks.dl_type;\n> > +\n> > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\n> > + ð_spec, ð_mask);\n> > + } else {\n> > + /*\n> > + * If user specifies a flow (like UDP flow) without L2 patterns,\n> > + * OVS will at least set the dl_type. Normally, it's enough to\n> > + * create an eth pattern just with it. Unluckily, some Intel's\n> > + * NIC (such as XL710) doesn't support that. Below is a workaround,\n> > + * which simply matches any L2 pkts.\n> > + */\n> > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\n> > + }\n> \n> This feels a lot like a layer violation - making the core aware\n> of specific implementation details of lower layers.\n\nI agree with you, but unlickily, without it, Intel NIC simply won't work\n(according to Finn), unless you have mac addr being provided.\n\n> >From a functional point of view, is the idea that\n> a eth_type+5-tuple match is converted to a 5-tuple match?\n\nUnluckily, yes.\n\n> > + /* VLAN */\n> > + struct rte_flow_item_vlan vlan_spec;\n> > + struct rte_flow_item_vlan vlan_mask;\n> \n> Please declare all local variables at the top of the context\n> (in this case function).\n> \n> ...\n> \n> > + struct rte_flow_item_udp udp_spec;\n> > + struct rte_flow_item_udp udp_mask;\n> > + memset(&udp_mask, 0, sizeof(udp_mask));\n> > + if ((proto == IPPROTO_UDP) &&\n> > + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\n> > + udp_spec.hdr.src_port = match->flow.tp_src;\n> > + udp_spec.hdr.dst_port = match->flow.tp_dst;\n> > +\n> > + udp_mask.hdr.src_port = match->wc.masks.tp_src;\n> > + udp_mask.hdr.dst_port = match->wc.masks.tp_dst;\n> > +\n> > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_UDP,\n> > + &udp_spec, &udp_mask);\n> > +\n> > + /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match */\n> > + if (ipv4_next_proto_mask) {\n> > + *ipv4_next_proto_mask = 0;\n> \n> I think this should be:\n> \n> + *ipv4_next_proto_mask = NULL;\n\nSeems not. ipv4_next_proto_mask is defined as \"uint8_t *\".\n\n> > + }\n> > + }\n> > +\n> > + struct rte_flow_item_sctp sctp_spec;\n> > + struct rte_flow_item_sctp sctp_mask;\n> > + memset(&sctp_mask, 0, sizeof(sctp_mask));\n> > + if ((proto == IPPROTO_SCTP) &&\n> \n> It seems there are unnecessary () in the line above.\n\nyes.\n\n> > +/*\n> > + * Validate for later rte flow offload creation. If any unsupported\n> > + * flow are specified, return -1.\n> > + */\n> > +static int\n> > +netdev_dpdk_validate_flow(const struct match *match)\n> > +{\n> > + struct match match_zero_wc;\n> > +\n> > + /* Create a wc-zeroed version of flow */\n> > + match_init(&match_zero_wc, &match->flow, &match->wc);\n> > +\n> > +#define CHECK_NONZERO_BYTES(addr, size) do { \\\n> \n> I think do should appear on a new line.\n> \n> > + uint8_t *padr = (uint8_t *)(addr); \\\n> > + int i; \\\n> > + for (i = 0; i < (size); i++) { \\\n> > + if (padr[i] != 0) { \\\n> > + goto err; \\\n> > + } \\\n> > + } \\\n> > +} while (0)\n> > +\n> > +#define CHECK_NONZERO(var) do { \\\n> \n> Here too.\n> \n> > + if ((var) != 0) { \\\n> > + goto err; \\\n> > + } \\\n> > +} while (0)\n> \n> I think its better if macros (and defines) aren't declared\n> inside of functions. The top of the file seems like\n> the best place to me. If not above the first function\n> that uses the macros, presumably this function.\n\nAgain, I just follow the OVS habit. I saw quite many defines like\nthis in other code pieces. I'm fine to put it outside the function,\nor even define it as a function though.\n\n\t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=fridaylinux-org.20150623.gappssmtp.com\n\theader.i=@fridaylinux-org.20150623.gappssmtp.com\n\theader.b=\"hLH+eh8u\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrHcQ1342z9s76\n\tfor ;\n\tMon, 11 Sep 2017 16:11:17 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id D5AC3AAB;\n\tMon, 11 Sep 2017 06:11:14 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 8F8A6AA6\n\tfor ; Mon, 11 Sep 2017 06:11:13 +0000 (UTC)","from mail-pg0-f44.google.com (mail-pg0-f44.google.com\n\t[74.125.83.44])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id EA060D3\n\tfor ; Mon, 11 Sep 2017 06:11:12 +0000 (UTC)","by mail-pg0-f44.google.com with SMTP id v66so13660969pgb.5\n\tfor ; Sun, 10 Sep 2017 23:11:12 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\tb90sm4911117pfm.17.2017.09.10.23.11.09\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tSun, 10 Sep 2017 23:11:11 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=fridaylinux-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=R3itX2ktdmBtDA3pCb5oygCeKfpF6/yUlP54haYfCcE=;\n\tb=hLH+eh8u0XPdH9DBdBEiiXUfPRVOzLUxe1sZKU7Qf3ooN6mlkWftqIzNzT8HKLB+NP\n\tl2ERRgb4okJaYzy55NzkXq8CXZ11rv57e7yPWzB2ktuXZ5Sa6fgz587vG16Odc9WH9QN\n\tsRQiFk9ZnswlLUHTGjygEdSBeLj6kLWLxvuVXbTb8fti1OQUV49pFXB49d47PEJS6zGB\n\tJ2VsaVxKDLJrbiwefPlMfaj2lYzGfmxFbsKIwE5sbiUiRx2frxSRwv+9FtDeJlbnRDOd\n\tMxi+HfkpzFyWoscwQ/tXAqbbXclulujfRt1V2EgdE/j7SBD84yVwxUIUvMJ/h1etmVig\n\teMLA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=R3itX2ktdmBtDA3pCb5oygCeKfpF6/yUlP54haYfCcE=;\n\tb=XqzgrATaPUtLmC+WVw0kZXyM9KJoyIhnlxWgTZ/Xm/5tmDI4R0KRVVBInqYZqVq6p1\n\t/wzTIyXeBHusqwDpAlBRxAAGhGeyUVzGauz0lxsICHuF3RASAGGn/iEjpWbkt16dEYlW\n\tFVUvhYSCLspHA5mS+GLpL0N+bnYgznXj+bZA7DdWIjofQrQjwPBgwWcD1mFoddHabhzi\n\t/yPAicGpun6luA8gquQTvaxLFBfSwO+dixE0gOizwL0Ecm4u30SWMtNhdSLkH40oKxAb\n\tZ5J45DGreT91+l7cuIVBUqfNNkYx48YaowEmFMKxI/ijFN5eZGrKOjdpr4Tr6Pjr2sIH\n\te5Mw==","X-Gm-Message-State":"AHPjjUiufUAYKNy3Uo8eGhvyht1P1/cRyriS2Gp58w/ZAO/Rin1RTYn9\n\tBlOoImQ7BYRWDyOA","X-Google-Smtp-Source":"ADKCNb4xC+9xSVhuZ6+IeEsrY63hdydSQO3k+tRzX4X8of9TsjePuhgO0aA7c/813PI4fLKeKKUlxQ==","X-Received":"by 10.98.98.196 with SMTP id w187mr10697013pfb.235.1505110272425;\n\tSun, 10 Sep 2017 23:11:12 -0700 (PDT)","Date":"Mon, 11 Sep 2017 14:11:02 +0800","From":"Yuanhan Liu ","To":"Simon Horman ","Message-ID":"<20170911061102.GM9736@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<20170908164754.GD7356@vergenet.net>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170908164754.GD7356@vergenet.net>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"dev@openvswitch.org","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766189,"web_url":"http://patchwork.ozlabs.org/comment/1766189/","msgid":"<20170911095745.GZ9736@yliu-home>","list_archive_url":null,"date":"2017-09-11T09:57:45","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Sun, Sep 10, 2017 at 04:32:19PM +0000, Chandran, Sugesh wrote:\n> As mentioned earlier in the cover letter we also have a similar implementation to do the flow translate.\n> I feel its good to make bit more modular for this translation. The reason being its easy to extend and add more protocols in the future.\n\nHonestly, I don't see a strong need to make it that complex first. Yes,\nit's a big function with a chunk of codes. And yes, I'm also a fun of\nsplitting big monsters to many small functions. However, if you look\nat it closer, you will see it's nicely organized: the function is split\nnicely with chunks: something like one chunk for one protocol.\n\nExtending is also not that hard: just adding another chunk of the code.\n\nBesides, if you see tc offloads, they do it in a same way.\n\n[...]\n> > +\n> > +/*\n> > + * Validate for later rte flow offload creation. If any unsupported\n> > + * flow are specified, return -1.\n> > + */\n> [Sugesh] I feel this is very hardware centric. There are chances hardware can support\n> Ipv6 or other fields in a packet. This has to be based on what flow fields a hardware can support.\n\nYes, you are right. Again, we need capabilities feedback from DPDK.\n\n\t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=fridaylinux-org.20150623.gappssmtp.com\n\theader.i=@fridaylinux-org.20150623.gappssmtp.com\n\theader.b=\"oYvrIISR\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrNf049FBz9s1h\n\tfor ;\n\tMon, 11 Sep 2017 19:57:59 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id A3F27B8A;\n\tMon, 11 Sep 2017 09:57:56 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id AF797B76\n\tfor ; Mon, 11 Sep 2017 09:57:54 +0000 (UTC)","from mail-pf0-f177.google.com (mail-pf0-f177.google.com\n\t[209.85.192.177])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 41F6FA4\n\tfor ; Mon, 11 Sep 2017 09:57:54 +0000 (UTC)","by mail-pf0-f177.google.com with SMTP id y29so12507719pff.0\n\tfor ; Mon, 11 Sep 2017 02:57:54 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\tl12sm11757229pgr.10.2017.09.11.02.57.50\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tMon, 11 Sep 2017 02:57:52 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=fridaylinux-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=ZhZxsrXWVInn+PdunxDNiekSYTd4wW6j98Vc/mP52mU=;\n\tb=oYvrIISRGyBdbF3iAdKe29JM4mP5KWF2H/lMYU9YewjpLXCrQHV+gOg7jFvCtpBAVU\n\tXZaATcSlcGJcmraTE8BXMVGmRKdCC6zXj5EBfnvTWj5qwY1FM/K9V7gq/QgM170XdAr3\n\t1ocXvcJWa3sfb3i8bQPSWVOoJj34UFNWodmJkYiXLvZvy9lQuJCokN7dpTQMk0+mHA9x\n\to4HRkTvSXrryvj84EupvH5MZwcDFbGkKsMrD37Q4qjktc8nqiWHh8gY8fI+M6IHl/FS4\n\thJeYLW2IWM2bs67jy8wOv4K4Zp/gtVUilH2Tv6GIpfbh1dW3Si5x623Or1Q/oNpL/iNE\n\tWVtw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=ZhZxsrXWVInn+PdunxDNiekSYTd4wW6j98Vc/mP52mU=;\n\tb=t2ivpyqYCa9PEMkywD+XAWZaXmdc0TiuGwyXj8lMpHjl2IUJq3hn8PO2+iBIhHtNWs\n\tDQ6X3B2q9k12Aj7GsDpLHAm1e40xU+5GODHlTNikoABDzzbOGKMMY98kK1WrvdsXZ8jn\n\tQ7lF4eiIJYePqV9+tl2x0Sglcjmp1w9FEUvdK1hjZEuWXnrb/kbeFKJLj8td2xCurvwg\n\tUX+yV9CBI1DIsqLzgFEZmVmyvGUgI2oDAXxlvEQpS3h0BmbnTWGq5KbMsnH3DbGhGvez\n\tK104sAC5llmPvRynzOi8+E1p/NtrMLNbyG6187TA5/MZD6xhvShzQCA4oJY7L/7W+Gx+\n\tpP6A==","X-Gm-Message-State":"AHPjjUjsqe+546niwn1bQG81qTM/m1Z7RrA1TaMgQtVKrQg7CcgQk170\n\tPPpNdi+s9avVD95J","X-Google-Smtp-Source":"ADKCNb431Y/xp0d35JheqM25wYDzbeBtiiJ45zd5oiuINlKw0x7ZcqPXUGKB46EfmY3RsCj+q+C/3w==","X-Received":"by 10.98.141.77 with SMTP id z74mr11346393pfd.179.1505123873890; \n\tMon, 11 Sep 2017 02:57:53 -0700 (PDT)","Date":"Mon, 11 Sep 2017 17:57:45 +0800","From":"Yuanhan Liu ","To":"\"Chandran, Sugesh\" ","Message-ID":"<20170911095745.GZ9736@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<2EF2F5C0CC56984AA024D0B180335FCB4221DC41@IRSMSX102.ger.corp.intel.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<2EF2F5C0CC56984AA024D0B180335FCB4221DC41@IRSMSX102.ger.corp.intel.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766193,"web_url":"http://patchwork.ozlabs.org/comment/1766193/","msgid":"<2EF2F5C0CC56984AA024D0B180335FCB4221E0CB@IRSMSX102.ger.corp.intel.com>","list_archive_url":null,"date":"2017-09-11T10:04:21","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","submitter":{"id":67440,"url":"http://patchwork.ozlabs.org/api/people/67440/","name":"Chandran, Sugesh","email":"sugesh.chandran@intel.com"},"content":"Regards\n_Sugesh\n\n\n> -----Original Message-----\n> From: Yuanhan Liu [mailto:yliu@fridaylinux.org]\n> Sent: Monday, September 11, 2017 10:58 AM\n> To: Chandran, Sugesh \n> Cc: dev@openvswitch.org\n> Subject: Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n> rte flow\n> \n> On Sun, Sep 10, 2017 at 04:32:19PM +0000, Chandran, Sugesh wrote:\n> > As mentioned earlier in the cover letter we also have a similar\n> implementation to do the flow translate.\n> > I feel its good to make bit more modular for this translation. The reason\n> being its easy to extend and add more protocols in the future.\n> \n> Honestly, I don't see a strong need to make it that complex first. Yes, it's a big\n> function with a chunk of codes. And yes, I'm also a fun of splitting big\n> monsters to many small functions. However, if you look at it closer, you will\n> see it's nicely organized: the function is split nicely with chunks: something\n> like one chunk for one protocol.\n> \n> Extending is also not that hard: just adding another chunk of the code.\n[Sugesh] Sure, I just wanted to share our proposal which works fine for our usecase. :)\n> \n> Besides, if you see tc offloads, they do it in a same way.\n[Sugesh] yes. \n> \n> [...]\n> > > +\n> > > +/*\n> > > + * Validate for later rte flow offload creation. If any unsupported\n> > > + * flow are specified, return -1.\n> > > + */\n> > [Sugesh] I feel this is very hardware centric. There are chances\n> > hardware can support\n> > Ipv6 or other fields in a packet. This has to be based on what flow fields a\n> hardware can support.\n> \n> Yes, you are right. Again, we need capabilities feedback from DPDK.\n[Sugesh] Ok.\n> \n> \t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrNnS6qXjz9s4q\n\tfor ;\n\tMon, 11 Sep 2017 20:04:28 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id E24A8BAE;\n\tMon, 11 Sep 2017 10:04:26 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 3E1DAB94\n\tfor ; Mon, 11 Sep 2017 10:04:26 +0000 (UTC)","from mga03.intel.com (mga03.intel.com [134.134.136.65])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id CA647D3\n\tfor ; Mon, 11 Sep 2017 10:04:25 +0000 (UTC)","from fmsmga005.fm.intel.com ([10.253.24.32])\n\tby orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t11 Sep 2017 03:04:25 -0700","from irsmsx151.ger.corp.intel.com ([163.33.192.59])\n\tby fmsmga005.fm.intel.com with ESMTP; 11 Sep 2017 03:04:22 -0700","from irsmsx112.ger.corp.intel.com (10.108.20.5) by\n\tIRSMSX151.ger.corp.intel.com (163.33.192.59) with Microsoft SMTP\n\tServer (TLS) id 14.3.319.2; Mon, 11 Sep 2017 11:04:22 +0100","from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by\n\tirsmsx112.ger.corp.intel.com ([169.254.1.110]) with mapi id\n\t14.03.0319.002; Mon, 11 Sep 2017 11:04:21 +0100"],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos;i=\"5.42,377,1500966000\"; d=\"scan'208\";a=\"149842955\"","From":"\"Chandran, Sugesh\" ","To":"Yuanhan Liu ","Thread-Topic":"[ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","Thread-Index":"AQHTJikq1p1dhEbNk0iD2XBUGg33RqKuF0SQgAFUeoCAABI9wA==","Date":"Mon, 11 Sep 2017 10:04:21 +0000","Message-ID":"<2EF2F5C0CC56984AA024D0B180335FCB4221E0CB@IRSMSX102.ger.corp.intel.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<2EF2F5C0CC56984AA024D0B180335FCB4221DC41@IRSMSX102.ger.corp.intel.com>\n\t<20170911095745.GZ9736@yliu-home>","In-Reply-To":"<20170911095745.GZ9736@yliu-home>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","x-titus-metadata-40":"eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOWNiYzM5NjctNzY3MS00ZDRiLTg1NmEtOGQxNDJhZTNhZGExIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImR1b2NxdXFKWWtSV0F1RjZYaTBMRm5UM0tyZWVjTkM1aHVocVg0YlJHcnc9In0=","x-ctpclassification":"CTP_IC","dlp-product":"dlpe-windows","dlp-version":"11.0.0.116","dlp-reaction":"no-action","x-originating-ip":"[163.33.239.181]","MIME-Version":"1.0","X-Spam-Status":"No, score=-5.0 required=5.0 tests=RCVD_IN_DNSWL_HI,\n\tRP_MATCHES_RCVD autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766199,"web_url":"http://patchwork.ozlabs.org/comment/1766199/","msgid":"<20170911101257.GB9736@yliu-home>","list_archive_url":null,"date":"2017-09-11T10:12:57","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Mon, Sep 11, 2017 at 10:04:21AM +0000, Chandran, Sugesh wrote:\n> \n> \n> Regards\n> _Sugesh\n> \n> \n> > -----Original Message-----\n> > From: Yuanhan Liu [mailto:yliu@fridaylinux.org]\n> > Sent: Monday, September 11, 2017 10:58 AM\n> > To: Chandran, Sugesh \n> > Cc: dev@openvswitch.org\n> > Subject: Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n> > rte flow\n> > \n> > On Sun, Sep 10, 2017 at 04:32:19PM +0000, Chandran, Sugesh wrote:\n> > > As mentioned earlier in the cover letter we also have a similar\n> > implementation to do the flow translate.\n> > > I feel its good to make bit more modular for this translation. The reason\n> > being its easy to extend and add more protocols in the future.\n> > \n> > Honestly, I don't see a strong need to make it that complex first. Yes, it's a big\n> > function with a chunk of codes. And yes, I'm also a fun of splitting big\n> > monsters to many small functions. However, if you look at it closer, you will\n> > see it's nicely organized: the function is split nicely with chunks: something\n> > like one chunk for one protocol.\n> > \n> > Extending is also not that hard: just adding another chunk of the code.\n> [Sugesh] Sure, I just wanted to share our proposal which works fine for our usecase. :)\n\nThanks for sharing!\n\n\t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=fridaylinux-org.20150623.gappssmtp.com\n\theader.i=@fridaylinux-org.20150623.gappssmtp.com\n\theader.b=\"0wc9suhU\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrNzT4hRSz9sBd\n\tfor ;\n\tMon, 11 Sep 2017 20:13:09 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 8EC5BBD7;\n\tMon, 11 Sep 2017 10:13:07 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 4C662BD0\n\tfor ; Mon, 11 Sep 2017 10:13:06 +0000 (UTC)","from mail-pg0-f47.google.com (mail-pg0-f47.google.com\n\t[74.125.83.47])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id CDCB1D3\n\tfor ; Mon, 11 Sep 2017 10:13:05 +0000 (UTC)","by mail-pg0-f47.google.com with SMTP id d8so14484261pgt.4\n\tfor ; Mon, 11 Sep 2017 03:13:05 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\tr68sm15938716pfi.7.2017.09.11.03.13.02\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tMon, 11 Sep 2017 03:13:04 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=fridaylinux-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=VTD0JEmXMeweNafERIpE0SsKjyjPK6PN0/m6qLqbZOA=;\n\tb=0wc9suhU65xI6mTJxT/O5IxVniqbglqkBg8qpUJxjoXAbJOEc+UQ2nG5CArE1NP5Ga\n\tWQtwC9ysMWNBJ14jDVd8gIxe7EZpJbYXVSYGWVXXVwXXV7ov57eVyUzn489eY/b6hth6\n\tBO5MLu30pxlmjNxzv1k7wMJX6AUlYNWqGreu/uosiGdfLi7Hy/eyRYlTxj+YW64CNTrc\n\tYOIHRiOtOIXkp3GLkOyUaQ4tjN5XXQkpmO+LbO0Dz5TKdikcy5/E9dxHkiUmkIfrmNXP\n\ttrRNB1AFH3rlZrND68VCi1fsXMVKRLJA8hpKPbrnypGJv0EUqIVmNuP3pAPRStkgNG4z\n\tOg+g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=VTD0JEmXMeweNafERIpE0SsKjyjPK6PN0/m6qLqbZOA=;\n\tb=Ryr/MiyUnoNNaOGAYHUGw4qgjsrr4ck9Q/63jr3zZRjQvM5zN0NLzYvP4ZjYXv9qzd\n\tNgKjRZPd9ontEPCF3+mYBBNIGS7cLVXATSD7mN4YxOzIVKLSfLja6/R1XA0AKkbKKeFI\n\tbd/Ft2L2tr9qfxtTqglOKG6DGakTgZkCyRYezdqdeJV1T3yZL8C8yOM1IoYleOp35rwa\n\t4jkD0tu93Q9AO6JL2tnfOvPlyqju2ERd9sHLvVGV7eixHtg5b9rANSAJlTDKOCMqBpGP\n\tOAW5wX3oMviiMem9I/+uR1j7MEV+3UaKVMIlVfDoscAEnATgluuVr5S6KJfdyfgZVVnS\n\t888A==","X-Gm-Message-State":"AHPjjUjTMc+knwlWHBVQMyYJfkQkXu0Vkq8MXUkVlhzWyjAJAcaXBMXp\n\tlPE7udVDcf+1EaP/DoOgtg==","X-Google-Smtp-Source":"ADKCNb4yhsoiLahbDC+AqUJYPrMrAkg2+XJoMknyaHP2r71uDGE4SY5fSy/MCUsC6OqOsRkxg+UL9A==","X-Received":"by 10.98.57.129 with SMTP id u1mr11360428pfj.197.1505124785497; \n\tMon, 11 Sep 2017 03:13:05 -0700 (PDT)","Date":"Mon, 11 Sep 2017 18:12:57 +0800","From":"Yuanhan Liu ","To":"\"Chandran, Sugesh\" ","Message-ID":"<20170911101257.GB9736@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<2EF2F5C0CC56984AA024D0B180335FCB4221DC41@IRSMSX102.ger.corp.intel.com>\n\t<20170911095745.GZ9736@yliu-home>\n\t<2EF2F5C0CC56984AA024D0B180335FCB4221E0CB@IRSMSX102.ger.corp.intel.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<2EF2F5C0CC56984AA024D0B180335FCB4221E0CB@IRSMSX102.ger.corp.intel.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Spam-Status":"No, score=0.5 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte\tflow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766754,"web_url":"http://patchwork.ozlabs.org/comment/1766754/","msgid":"","list_archive_url":null,"date":"2017-09-12T08:34:44","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"On 9/10/17, 11:11 PM, \"ovs-dev-bounces@openvswitch.org on behalf of Yuanhan Liu\" wrote:\n\n On Fri, Sep 08, 2017 at 06:47:55PM +0200, Simon Horman wrote:\n > On Tue, Sep 05, 2017 at 05:22:57PM +0800, Yuanhan Liu wrote:\n > > From: Finn Christensen \n > > \n > > The basic yet the major part of this patch is to translate the \"match\"\n > > to rte flow patterns. And then, we create a rte flow with a MARK action.\n > > Afterwards, all pkts matches the flow will have the mark id in the mbuf.\n > > \n > > For any unsupported flows, such as MPLS, -1 is returned, meaning the\n > > flow offload is failed and then skipped.\n > \n > ...\n > \n > > +static int\n > > +netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,\n > > + const struct match *match,\n > > + struct nlattr *nl_actions OVS_UNUSED,\n > > + size_t actions_len,\n > > + const ovs_u128 *ufid,\n > > + struct offload_info *info)\n > > +{\n > > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);\n > > + const struct rte_flow_attr flow_attr = {\n > > + .group = 0,\n > > + .priority = 0,\n > > + .ingress = 1,\n > > + .egress = 0\n > > + };\n > > + struct flow_patterns patterns = { .items = NULL, .cnt = 0 };\n > > + struct flow_actions actions = { .actions = NULL, .cnt = 0 };\n > \n > I believe the following will give the same result as the above,\n > less verbosely.\n > \n > + const struct rte_flow_attr flow_attr = { .ingress = 1 };\n > + struct flow_patterns patterns = { .cnt = 0 };\n > + struct flow_actions actions = { .cnt = 0 };\n \n I'm not quite sure on that. If the compiler doesn't do zero assigment\n correctly, something deadly wrong could happen. They all are short\n structs, that I think it's fine, IMO. If they are big, I'd prefer an\n explicit memset.\n \n > > + struct rte_flow *flow;\n > > + struct rte_flow_error error;\n > > + uint8_t *ipv4_next_proto_mask = NULL;\n > > + int ret = 0;\n > > +\n > > + /* Eth */\n > > + struct rte_flow_item_eth eth_spec;\n > > + struct rte_flow_item_eth eth_mask;\n > \n > Empty line here please.\n \n I actually want to bind the two declartions with the following code piece,\n to show that they are tightly related.\n \n > > + memset(ð_mask, 0, sizeof(eth_mask));\n > > + if (match->wc.masks.dl_src.be16[0] ||\n > > + match->wc.masks.dl_src.be16[1] ||\n > > + match->wc.masks.dl_src.be16[2] ||\n > > + match->wc.masks.dl_dst.be16[0] ||\n > > + match->wc.masks.dl_dst.be16[1] ||\n > > + match->wc.masks.dl_dst.be16[2]) {\n > \n > It looks like eth_addr_is_zero() can be used in the above.\n \n Yes, we could. Thanks for the tip.\n \n > > + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\n > > + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\n > > + eth_spec.type = match->flow.dl_type;\n > > +\n > > + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\n > > + sizeof(eth_mask.dst));\n > > + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\n > > + sizeof(eth_mask.src));\n > > + eth_mask.type = match->wc.masks.dl_type;\n > > +\n > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\n > > + ð_spec, ð_mask);\n > > + } else {\n > > + /*\n > > + * If user specifies a flow (like UDP flow) without L2 patterns,\n > > + * OVS will at least set the dl_type. Normally, it's enough to\n > > + * create an eth pattern just with it. Unluckily, some Intel's\n > > + * NIC (such as XL710) doesn't support that. Below is a workaround,\n > > + * which simply matches any L2 pkts.\n > > + */\n > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\n > > + }\n > \n > This feels a lot like a layer violation - making the core aware\n > of specific implementation details of lower layers.\n \n I agree with you, but unlickily, without it, Intel NIC simply won't work\n (according to Finn), unless you have mac addr being provided.\n \n > >From a functional point of view, is the idea that\n > a eth_type+5-tuple match is converted to a 5-tuple match?\n \n Unluckily, yes.\n \n > > + /* VLAN */\n > > + struct rte_flow_item_vlan vlan_spec;\n > > + struct rte_flow_item_vlan vlan_mask;\n > \n > Please declare all local variables at the top of the context\n > (in this case function).\n > \n > ...\n > \n > > + struct rte_flow_item_udp udp_spec;\n > > + struct rte_flow_item_udp udp_mask;\n > > + memset(&udp_mask, 0, sizeof(udp_mask));\n > > + if ((proto == IPPROTO_UDP) &&\n > > + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\n > > + udp_spec.hdr.src_port = match->flow.tp_src;\n > > + udp_spec.hdr.dst_port = match->flow.tp_dst;\n > > +\n > > + udp_mask.hdr.src_port = match->wc.masks.tp_src;\n > > + udp_mask.hdr.dst_port = match->wc.masks.tp_dst;\n > > +\n > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_UDP,\n > > + &udp_spec, &udp_mask);\n > > +\n > > + /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match */\n > > + if (ipv4_next_proto_mask) {\n > > + *ipv4_next_proto_mask = 0;\n > \n > I think this should be:\n > \n > + *ipv4_next_proto_mask = NULL;\n \n Seems not. ipv4_next_proto_mask is defined as \"uint8_t *\".\n \n > > + }\n > > + }\n > > +\n > > + struct rte_flow_item_sctp sctp_spec;\n > > + struct rte_flow_item_sctp sctp_mask;\n > > + memset(&sctp_mask, 0, sizeof(sctp_mask));\n > > + if ((proto == IPPROTO_SCTP) &&\n > \n > It seems there are unnecessary () in the line above.\n \n yes.\n \n > > +/*\n > > + * Validate for later rte flow offload creation. If any unsupported\n > > + * flow are specified, return -1.\n > > + */\n > > +static int\n > > +netdev_dpdk_validate_flow(const struct match *match)\n > > +{\n > > + struct match match_zero_wc;\n > > +\n > > + /* Create a wc-zeroed version of flow */\n > > + match_init(&match_zero_wc, &match->flow, &match->wc);\n > > +\n > > +#define CHECK_NONZERO_BYTES(addr, size) do { \\\n > \n > I think do should appear on a new line.\n > \n > > + uint8_t *padr = (uint8_t *)(addr); \\\n > > + int i; \\\n > > + for (i = 0; i < (size); i++) { \\\n > > + if (padr[i] != 0) { \\\n > > + goto err; \\\n > > + } \\\n > > + } \\\n > > +} while (0)\n > > +\n > > +#define CHECK_NONZERO(var) do { \\\n > \n > Here too.\n > \n > > + if ((var) != 0) { \\\n > > + goto err; \\\n > > + } \\\n > > +} while (0)\n > \n > I think its better if macros (and defines) aren't declared\n > inside of functions. The top of the file seems like\n > the best place to me. If not above the first function\n > that uses the macros, presumably this function.\n \n Again, I just follow the OVS habit. I saw quite many defines like\n this in other code pieces. I'm fine to put it outside the function,\n or even define it as a function though.\t\n\n[Darrell] Pls convert to functions as discussed earlier.\n\n \n \t--yliu\n _______________________________________________\n dev mailing list\n dev@openvswitch.org\n https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=mVDsTNxOCpGJWdPt_BIHsPGlF-FSjSCIMFXRntq4ww4&s=QIQvaFzRDNUYTMXxRET9DIRtkVfULAPQYgNcuo8620o&e=","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"p4qhOfuB\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrylf1wd6z9s7g\n\tfor ;\n\tTue, 12 Sep 2017 18:34:52 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 3806CA91;\n\tTue, 12 Sep 2017 08:34:49 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 138F58EE\n\tfor ; Tue, 12 Sep 2017 08:34:48 +0000 (UTC)","from NAM03-CO1-obe.outbound.protection.outlook.com\n\t(mail-co1nam03on0062.outbound.protection.outlook.com [104.47.40.62])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 05261127\n\tfor ; Tue, 12 Sep 2017 08:34:46 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB306.namprd05.prod.outlook.com (10.141.23.148) with Microsoft\n\tSMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.56.4; Tue, 12 Sep 2017 08:34:44 +0000","from BLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) by\n\tBLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) with mapi id\n\t15.20.0056.010; Tue, 12 Sep 2017 08:34:44 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=9/tGgWXfsXoNedIsI1pP2pGswOBhESeHfPIVK0wwdds=;\n\tb=p4qhOfuBLEGSFAwEYn3x+XVFpQfRG3AG5DTXavFZgQRJTWD0x745CGZT9ge47N8CRzIbs8KLYwnAxQhv0wKIdt6Ui821jerpLmqKhqGvDjHEbG9Zw5a0NsGPJgabFXFGG1sD8ubmczVUqybPcYm3u+Dv13p9/bfdVPX1/ltZyAw=","From":"Darrell Ball ","To":"Yuanhan Liu , Simon Horman\n\t","Thread-Topic":"[ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","Thread-Index":"AQHTJiiw1ZjL6RzFTUOQLht0V/vcqaKrOB6AgAQFDQCAAUUhgA==","Date":"Tue, 12 Sep 2017 08:34:44 +0000","Message-ID":"","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<20170908164754.GD7356@vergenet.net>\n\t<20170911061102.GM9736@yliu-home>","In-Reply-To":"<20170911061102.GM9736@yliu-home>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.25.0.170815","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"p4qhOfuB\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"x-originating-ip":"[73.162.236.45]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB306;\n\t20:FM6uWzvKnXho1i7NT2JI6POh3ZPYhKkscFSrJvcHzS7iSOQ6zWSILyKd3EE2f/6RGfs4VD5OO3dNqUPzimkvtvy+IUzWIHBAQsv+Mz9M4AcJUJLsqzsJMYBp2umGSl4SfHaCkayBGhejp8Xb/iUUhbwhnrjapWGpKYX+sAzSpIs=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"2f467a42-7f9b-49be-ae29-08d4f9b91ed6","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:BLUPR05MB306; ","x-ms-traffictypediagnostic":"BLUPR05MB306:","x-exchange-antispam-report-test":"UriScan:(10436049006162)(216315784871565)(17755550239193); ","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(93006095)(93001095)(10201501046)(100000703101)(100105400095)(3002001)(6041248)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123555025)(20161123564025)(20161123558100)(20161123560025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB306; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB306; ","x-forefront-prvs":"042857DBB5","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(51914003)(24454002)(377454003)(189002)(199003)(8676002)(3660700001)(81166006)(3280700002)(6436002)(33656002)(68736007)(97736004)(105586002)(39060400002)(6506006)(14454004)(966005)(25786009)(101416001)(189998001)(4326008)(316002)(106356001)(2900100001)(81156014)(6306002)(93886005)(8936002)(6512007)(82746002)(53936002)(7736002)(2906002)(6246003)(77096006)(6116002)(2950100002)(5660300001)(102836003)(3846002)(229853002)(76176999)(478600001)(99286003)(50986999)(53546010)(83716003)(6486002)(83506001)(305945005)(66066001)(54356999)(4001350100001)(86362001)(36756003);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB306;\n\tH:BLUPR05MB611.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"<279ACC70898DCE4F92387878A4061E54@namprd05.prod.outlook.com>","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"12 Sep 2017 08:34:44.6640\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB306","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1767533,"web_url":"http://patchwork.ozlabs.org/comment/1767533/","msgid":"<6A6AA86E-D93C-41B1-A587-51C451BBA6B2@vmware.com>","list_archive_url":null,"date":"2017-09-13T03:47:55","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"On 9/11/17, 3:02 AM, \"ovs-dev-bounces@openvswitch.org on behalf of Yuanhan Liu\" wrote:\r\n\r\n On Sun, Sep 10, 2017 at 04:32:19PM +0000, Chandran, Sugesh wrote:\r\n > As mentioned earlier in the cover letter we also have a similar implementation to do the flow translate.\r\n > I feel its good to make bit more modular for this translation. The reason being its easy to extend and add more protocols in the future.\r\n \r\n Honestly, I don't see a strong need to make it that complex first. Yes,\r\n it's a big function with a chunk of codes. And yes, I'm also a fun of\r\n splitting big monsters to many small functions. However, if you look\r\n at it closer, you will see it's nicely organized: the function is split\r\n nicely with chunks: something like one chunk for one protocol.\r\n \r\n Extending is also not that hard: just adding another chunk of the code.\r\n \r\n Besides, if you see tc offloads, they do it in a same way.\r\n \r\n [...]\r\n > > +\r\n > > +/*\r\n > > + * Validate for later rte flow offload creation. If any unsupported\r\n > > + * flow are specified, return -1.\r\n > > + */\r\n > [Sugesh] I feel this is very hardware centric. There are chances hardware can support\r\n > Ipv6 or other fields in a packet. This has to be based on what flow fields a hardware can support.\r\n \r\n Yes, you are right. Again, we need capabilities feedback from DPDK.\r\n\r\n[Darrell] There have been several mentions of capability queries in version 1 and version 2 of the patchset.\r\n\r\n1/ We asked for HW scaling checks to better support manageability and predictability.\r\n\r\n2/ We asked for a ‘queue action workaround requirement’ query, which would be a nice enhancement,\r\n to avoid an unnecessary first failed attempt at flow create for nics that don’t allow the mark action in isolation.\r\n\r\n3/ Here we ask for flow matching capability checking, with fallback to what we already support today.\r\n Similar kinds of queries were also mentioned in regard to the patchset “prioritizing latency sensitive traffic”.\r\n TCP flags matching mentioned in Patch 2 also falls into this category.\r\n\r\n\r\n \r\n \t--yliu\r\n _______________________________________________\r\n dev mailing list\r\n dev@openvswitch.org\r\n https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=SbdNOA5djgzhAIm49EWuBLui1KsaSl-tOsvhgb685Js&s=oKRfV8aE2G8A_Te761VR9n29SJuhr4-SJshaFetgICw&e=","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"qCgiByKW\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsSLM711Vz9sNr\n\tfor ;\n\tWed, 13 Sep 2017 13:48:11 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 00FEFA7C;\n\tWed, 13 Sep 2017 03:48:01 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 2F247A7A\n\tfor ; Wed, 13 Sep 2017 03:48:00 +0000 (UTC)","from NAM03-CO1-obe.outbound.protection.outlook.com\n\t(mail-co1nam03on0054.outbound.protection.outlook.com [104.47.40.54])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 06D3D180\n\tfor ; Wed, 13 Sep 2017 03:47:58 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB547.namprd05.prod.outlook.com (10.141.202.18) with Microsoft\n\tSMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.56.4; Wed, 13 Sep 2017 03:47:55 +0000","from BLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) by\n\tBLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) with mapi id\n\t15.20.0056.010; Wed, 13 Sep 2017 03:47:55 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=jSbuBgdkY5dr4fKYZrMUFHSGIMuMzS+hL/JuLzqJ1x4=;\n\tb=qCgiByKW3iXrWKFS3DGowIGB6F9z+aUCeNHwGJp5c1N691c1MUkYM/ND202wewnxHFtvuYth4UEQUai00AgLGGe73U1D9yX9NpPb7F73SZu6GhfLj+xrGhhg5EsyGl7dsXwRMhkz3g3SmJ//eL5NndoFr29tqth8RaCkUca5ONs=","From":"Darrell Ball ","To":"Yuanhan Liu , \"Chandran, Sugesh\"\n\t","Thread-Topic":"[ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","Thread-Index":"AQHTLEMV9boGbJ3UbUq6wsvfba81Ig==","Date":"Wed, 13 Sep 2017 03:47:55 +0000","Message-ID":"<6A6AA86E-D93C-41B1-A587-51C451BBA6B2@vmware.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<2EF2F5C0CC56984AA024D0B180335FCB4221DC41@IRSMSX102.ger.corp.intel.com>\n\t<20170911095745.GZ9736@yliu-home>","In-Reply-To":"<20170911095745.GZ9736@yliu-home>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.25.0.170815","x-originating-ip":"[73.162.236.45]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB547;\n\t20:EMFif7R8VgB2opcKkm5ZOHKcMXWtC79UiU6IwZmVB+EdE7BSu79rJpOBo7Kgg5+fgVDFee3mXUWpzvdcdA98/Cyr37osE88wyHz4pGl2nxTm67Rb1J74B/o5JflVQ0ydRLGn2qHs3sDL02ueHjSsWmGIu2O5lRFXn4Mdy0/fjVU=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"b0238ad1-ae89-4d04-f744-08d4fa5a37d1","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:BLUPR05MB547; ","x-ms-traffictypediagnostic":"BLUPR05MB547:","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"qCgiByKW\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"x-exchange-antispam-report-test":"UriScan:(10436049006162)(216315784871565);","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(100000703101)(100105400095)(93006095)(93001095)(10201501046)(3002001)(6041248)(20161123555025)(20161123564025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123558100)(20161123560025)(20161123562025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB547; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB547; ","x-forefront-prvs":"042957ACD7","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(376002)(366002)(346002)(39860400002)(24454002)(199003)(189002)(377454003)(6506006)(2900100001)(77096006)(6486002)(2906002)(106356001)(6436002)(86362001)(2950100002)(5660300001)(3660700001)(105586002)(3280700002)(189998001)(33656002)(68736007)(229853002)(66066001)(316002)(53936002)(25786009)(6246003)(478600001)(4326008)(14454004)(82746002)(966005)(305945005)(7736002)(53546010)(83716003)(101416001)(36756003)(99286003)(97736004)(54356999)(8936002)(93886005)(4001350100001)(102836003)(6512007)(3846002)(6116002)(76176999)(50986999)(8676002)(6306002)(81166006)(83506001)(81156014);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB547;\n\tH:BLUPR05MB611.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; A:1; MX:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"<931E57AB4675CB4E9DBD18DBD0DAE0A8@namprd05.prod.outlook.com>","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"13 Sep 2017 03:47:55.4559\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB547","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1767543,"web_url":"http://patchwork.ozlabs.org/comment/1767543/","msgid":"","list_archive_url":null,"date":"2017-09-13T04:15:20","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"Still working my way through this patch, but I had a couple comments inline\r\n\r\nOn 9/5/17, 2:23 AM, \"Yuanhan Liu\" wrote:\r\n\r\n From: Finn Christensen \r\n \r\n The basic yet the major part of this patch is to translate the \"match\"\r\n to rte flow patterns. And then, we create a rte flow with a MARK action.\r\n Afterwards, all pkts matches the flow will have the mark id in the mbuf.\r\n \r\n For any unsupported flows, such as MPLS, -1 is returned, meaning the\r\n flow offload is failed and then skipped.\r\n \r\n Co-authored-by: Yuanhan Liu \r\n Signed-off-by: Finn Christensen \r\n Signed-off-by: Yuanhan Liu \r\n ---\r\n \r\n v2: - convert some macros to functions\r\n - do not hardcode the max number of flow/action\r\n - fix L2 patterns for Intel nic\r\n - add comments for not implemented offload methods\r\n ---\r\n lib/netdev-dpdk.c | 421 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\r\n 1 file changed, 420 insertions(+), 1 deletion(-)\r\n \r\n diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c\r\n index 46f9885..37b0f99 100644\r\n --- a/lib/netdev-dpdk.c\r\n +++ b/lib/netdev-dpdk.c\r\n @@ -58,6 +58,7 @@\r\n #include \"smap.h\"\r\n #include \"sset.h\"\r\n #include \"unaligned.h\"\r\n +#include \"uuid.h\"\r\n #include \"timeval.h\"\r\n #include \"unixctl.h\"\r\n \r\n @@ -3400,6 +3401,424 @@ get_rte_flow_by_ufid(const ovs_u128 *ufid)\r\n }\r\n \r\n \r\n +struct flow_patterns {\r\n + struct rte_flow_item *items;\r\n + int cnt;\r\n + int max;\r\n +};\r\n +\r\n +struct flow_actions {\r\n + struct rte_flow_action *actions;\r\n + int cnt;\r\n + int max;\r\n +};\r\n +\r\n +static void\r\n +add_flow_pattern(struct flow_patterns *patterns, enum rte_flow_item_type type,\r\n + const void *spec, const void *mask)\r\n +{\r\n + int cnt = patterns->cnt;\r\n +\r\n + if (cnt == 0) {\r\n + patterns->max = 8;\r\n + patterns->items = xcalloc(patterns->max, sizeof(struct rte_flow_item));\r\n + } else if (cnt == patterns->max) {\r\n + patterns->max *= 2;\r\n + patterns->items = xrealloc(patterns->items, patterns->max *\r\n + sizeof(struct rte_flow_item));\r\n + }\r\n +\r\n + patterns->items[cnt].type = type;\r\n + patterns->items[cnt].spec = spec;\r\n + patterns->items[cnt].mask = mask;\r\n + patterns->items[cnt].last = NULL;\r\n + patterns->cnt++;\r\n +}\r\n +\r\n +static void\r\n +add_flow_action(struct flow_actions *actions, enum rte_flow_action_type type,\r\n + const void *conf)\r\n +{\r\n + int cnt = actions->cnt;\r\n +\r\n + if (cnt == 0) {\r\n + actions->max = 8;\r\n + actions->actions = xcalloc(actions->max,\r\n + sizeof(struct rte_flow_action));\r\n + } else if (cnt == actions->max) {\r\n + actions->max *= 2;\r\n + actions->actions = xrealloc(actions->actions, actions->max *\r\n + sizeof(struct rte_flow_action));\r\n + }\r\n +\r\n + actions->actions[cnt].type = type;\r\n + actions->actions[cnt].conf = conf;\r\n + actions->cnt++;\r\n +}\r\n +\r\n +static int\r\n +netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,\r\n + const struct match *match,\r\n + struct nlattr *nl_actions OVS_UNUSED,\r\n + size_t actions_len,\r\n + const ovs_u128 *ufid,\r\n + struct offload_info *info)\r\n +{\r\n + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);\r\n + const struct rte_flow_attr flow_attr = {\r\n + .group = 0,\r\n + .priority = 0,\r\n + .ingress = 1,\r\n + .egress = 0\r\n + };\r\n + struct flow_patterns patterns = { .items = NULL, .cnt = 0 };\r\n + struct flow_actions actions = { .actions = NULL, .cnt = 0 };\r\n + struct rte_flow *flow;\r\n + struct rte_flow_error error;\r\n + uint8_t *ipv4_next_proto_mask = NULL;\r\n + int ret = 0;\r\n +\r\n + /* Eth */\r\n + struct rte_flow_item_eth eth_spec;\r\n + struct rte_flow_item_eth eth_mask;\r\n + memset(ð_mask, 0, sizeof(eth_mask));\r\n + if (match->wc.masks.dl_src.be16[0] ||\r\n + match->wc.masks.dl_src.be16[1] ||\r\n + match->wc.masks.dl_src.be16[2] ||\r\n + match->wc.masks.dl_dst.be16[0] ||\r\n + match->wc.masks.dl_dst.be16[1] ||\r\n + match->wc.masks.dl_dst.be16[2]) {\r\n + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\r\n + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\r\n + eth_spec.type = match->flow.dl_type;\r\n +\r\n + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\r\n + sizeof(eth_mask.dst));\r\n + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\r\n + sizeof(eth_mask.src));\r\n + eth_mask.type = match->wc.masks.dl_type;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\r\n + ð_spec, ð_mask);\r\n + } else {\r\n + /*\r\n + * If user specifies a flow (like UDP flow) without L2 patterns,\r\n + * OVS will at least set the dl_type. Normally, it's enough to\r\n + * create an eth pattern just with it. Unluckily, some Intel's\r\n + * NIC (such as XL710) doesn't support that. Below is a workaround,\r\n + * which simply matches any L2 pkts.\r\n + */\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\r\n + }\r\n +\r\n + /* VLAN */\r\n + struct rte_flow_item_vlan vlan_spec;\r\n + struct rte_flow_item_vlan vlan_mask;\r\n + memset(&vlan_mask, 0, sizeof(vlan_mask));\r\n + if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {\r\n + vlan_spec.tci = match->flow.vlans[0].tci;\r\n + vlan_mask.tci = match->wc.masks.vlans[0].tci;\r\n +\r\n + /* match any protocols */\r\n + vlan_mask.tpid = 0;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN,\r\n + &vlan_spec, &vlan_mask);\r\n + }\r\n +\r\n + /* IP v4 */\r\n + uint8_t proto = 0;\r\n + struct rte_flow_item_ipv4 ipv4_spec;\r\n + struct rte_flow_item_ipv4 ipv4_mask;\r\n + memset(&ipv4_mask, 0, sizeof(ipv4_mask));\r\n + if ((match->flow.dl_type == ntohs(ETH_TYPE_IP)) &&\r\n + (match->wc.masks.nw_src || match->wc.masks.nw_dst ||\r\n + match->wc.masks.nw_tos || match->wc.masks.nw_ttl ||\r\n + match->wc.masks.nw_proto)) {\r\n + ipv4_spec.hdr.type_of_service = match->flow.nw_tos;\r\n + ipv4_spec.hdr.time_to_live = match->flow.nw_tos;\r\n\r\n[Darrell] should be TTL; lots of e-mails but I don’t remember seeing this mentioned\r\n\r\n\r\n + ipv4_spec.hdr.next_proto_id = match->flow.nw_proto;\r\n + ipv4_spec.hdr.src_addr = match->flow.nw_src;\r\n + ipv4_spec.hdr.dst_addr = match->flow.nw_dst;\r\n +\r\n + ipv4_mask.hdr.type_of_service = match->wc.masks.nw_tos;\r\n + ipv4_mask.hdr.time_to_live = match->wc.masks.nw_tos;\r\n\r\n[Darrell] should be TTL; lots of e-mails but I don’t remember seeing this mentioned\r\n\r\n\r\n + ipv4_mask.hdr.next_proto_id = match->wc.masks.nw_proto;\r\n + ipv4_mask.hdr.src_addr = match->wc.masks.nw_src;\r\n + ipv4_mask.hdr.dst_addr = match->wc.masks.nw_dst;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_IPV4,\r\n + &ipv4_spec, &ipv4_mask);\r\n +\r\n + /* Save proto for L4 protocol setup */\r\n + proto = ipv4_spec.hdr.next_proto_id & ipv4_mask.hdr.next_proto_id;\r\n +\r\n + /* Remember proto mask address for later modification */\r\n + ipv4_next_proto_mask = &ipv4_mask.hdr.next_proto_id;\r\n + }\r\n +\r\n + if (proto != IPPROTO_ICMP && proto != IPPROTO_UDP &&\r\n + proto != IPPROTO_SCTP && proto != IPPROTO_TCP &&\r\n + (match->wc.masks.tp_src ||\r\n + match->wc.masks.tp_dst ||\r\n + match->wc.masks.tcp_flags)) {\r\n + VLOG_INFO(\"L4 Protocol (%u) not supported\", proto);\r\n + ret = -1;\r\n + goto out;\r\n + }\r\n +\r\n + struct rte_flow_item_udp udp_spec;\r\n + struct rte_flow_item_udp udp_mask;\r\n + memset(&udp_mask, 0, sizeof(udp_mask));\r\n + if ((proto == IPPROTO_UDP) &&\r\n + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\r\n + udp_spec.hdr.src_port = match->flow.tp_src;\r\n + udp_spec.hdr.dst_port = match->flow.tp_dst;\r\n +\r\n + udp_mask.hdr.src_port = match->wc.masks.tp_src;\r\n + udp_mask.hdr.dst_port = match->wc.masks.tp_dst;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_UDP,\r\n + &udp_spec, &udp_mask);\r\n +\r\n + /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match */\r\n + if (ipv4_next_proto_mask) {\r\n + *ipv4_next_proto_mask = 0;\r\n + }\r\n + }\r\n +\r\n + struct rte_flow_item_sctp sctp_spec;\r\n + struct rte_flow_item_sctp sctp_mask;\r\n + memset(&sctp_mask, 0, sizeof(sctp_mask));\r\n + if ((proto == IPPROTO_SCTP) &&\r\n + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\r\n + sctp_spec.hdr.src_port = match->flow.tp_src;\r\n + sctp_spec.hdr.dst_port = match->flow.tp_dst;\r\n +\r\n + sctp_mask.hdr.src_port = match->wc.masks.tp_src;\r\n + sctp_mask.hdr.dst_port = match->wc.masks.tp_dst;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_SCTP,\r\n + &sctp_spec, &sctp_mask);\r\n +\r\n + /* proto == SCTP and ITEM_TYPE_SCTP, thus no need for proto match */\r\n + if (ipv4_next_proto_mask) {\r\n + *ipv4_next_proto_mask = 0;\r\n + }\r\n + }\r\n +\r\n + struct rte_flow_item_icmp icmp_spec;\r\n + struct rte_flow_item_icmp icmp_mask;\r\n + memset(&icmp_mask, 0, sizeof(icmp_mask));\r\n + if ((proto == IPPROTO_ICMP) &&\r\n + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\r\n + icmp_spec.hdr.icmp_type = (uint8_t)ntohs(match->flow.tp_src);\r\n + icmp_spec.hdr.icmp_code = (uint8_t)ntohs(match->flow.tp_dst);\r\n +\r\n + icmp_mask.hdr.icmp_type = (uint8_t)ntohs(match->wc.masks.tp_src);\r\n + icmp_mask.hdr.icmp_code = (uint8_t)ntohs(match->wc.masks.tp_dst);\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ICMP,\r\n + &icmp_spec, &icmp_mask);\r\n +\r\n + /* proto == ICMP and ITEM_TYPE_ICMP, thus no need for proto match */\r\n + if (ipv4_next_proto_mask) {\r\n + *ipv4_next_proto_mask = 0;\r\n + }\r\n + }\r\n +\r\n + struct rte_flow_item_tcp tcp_spec;\r\n + struct rte_flow_item_tcp tcp_mask;\r\n + memset(&tcp_mask, 0, sizeof(tcp_mask));\r\n + if ((proto == IPPROTO_TCP) &&\r\n + (match->wc.masks.tp_src ||\r\n + match->wc.masks.tp_dst ||\r\n + match->wc.masks.tcp_flags)) {\r\n + tcp_spec.hdr.src_port = match->flow.tp_src;\r\n + tcp_spec.hdr.dst_port = match->flow.tp_dst;\r\n + tcp_spec.hdr.data_off = ntohs(match->flow.tcp_flags) >> 8;\r\n + tcp_spec.hdr.tcp_flags = ntohs(match->flow.tcp_flags) & 0xff;\r\n +\r\n + tcp_mask.hdr.src_port = match->wc.masks.tp_src;\r\n + tcp_mask.hdr.dst_port = match->wc.masks.tp_dst;\r\n + tcp_mask.hdr.data_off = ntohs(match->wc.masks.tcp_flags) >> 8;\r\n + tcp_mask.hdr.tcp_flags = ntohs(match->wc.masks.tcp_flags) & 0xff;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_TCP,\r\n + &tcp_spec, &tcp_mask);\r\n +\r\n + /* proto == TCP and ITEM_TYPE_TCP, thus no need for proto match */\r\n + if (ipv4_next_proto_mask) {\r\n + *ipv4_next_proto_mask = 0;\r\n + }\r\n + }\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_END, NULL, NULL);\r\n +\r\n + struct rte_flow_action_mark mark;\r\n + if (actions_len) {\r\n + mark.id = info->flow_mark;\r\n + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark);\r\n + } else {\r\n + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_DROP, NULL);\r\n + VLOG_INFO(\"no action given; drop pkts in hardware\\n\");\r\n + }\r\n + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL);\r\n +\r\n + flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items,\r\n + actions.actions, &error);\r\n + if (!flow) {\r\n + VLOG_ERR(\"rte flow creat error: %u : message : %s\\n\",\r\n + error.type, error.message);\r\n + ret = -1;\r\n + goto out;\r\n + }\r\n + add_ufid_dpdk_flow_mapping(ufid, flow);\r\n + VLOG_INFO(\"installed flow %p by ufid \"UUID_FMT\"\\n\",\r\n + flow, UUID_ARGS((struct uuid *)ufid));\r\n +\r\n +out:\r\n + free(patterns.items);\r\n + free(actions.actions);\r\n + return ret;\r\n +}\r\n +\r\n +/*\r\n + * Validate for later rte flow offload creation. If any unsupported\r\n + * flow are specified, return -1.\r\n + */\r\n +static int\r\n +netdev_dpdk_validate_flow(const struct match *match)\r\n +{\r\n + struct match match_zero_wc;\r\n +\r\n + /* Create a wc-zeroed version of flow */\r\n + match_init(&match_zero_wc, &match->flow, &match->wc);\r\n +\r\n +#define CHECK_NONZERO_BYTES(addr, size) do { \\\r\n + uint8_t *padr = (uint8_t *)(addr); \\\r\n + int i; \\\r\n + for (i = 0; i < (size); i++) { \\\r\n + if (padr[i] != 0) { \\\r\n + goto err; \\\r\n + } \\\r\n + } \\\r\n +} while (0)\r\n +\r\n +#define CHECK_NONZERO(var) do { \\\r\n + if ((var) != 0) { \\\r\n + goto err; \\\r\n + } \\\r\n +} while (0)\r\n +\r\n + CHECK_NONZERO_BYTES(&match_zero_wc.flow.tunnel,\r\n + sizeof(match_zero_wc.flow.tunnel));\r\n + CHECK_NONZERO(match->wc.masks.metadata);\r\n + CHECK_NONZERO(match->wc.masks.skb_priority);\r\n + CHECK_NONZERO(match->wc.masks.pkt_mark);\r\n + CHECK_NONZERO(match->wc.masks.dp_hash);\r\n +\r\n + /* recirc id must be zero */\r\n + CHECK_NONZERO(match_zero_wc.flow.recirc_id);\r\n +\r\n + CHECK_NONZERO(match->wc.masks.ct_state);\r\n + CHECK_NONZERO(match->wc.masks.ct_zone);\r\n + CHECK_NONZERO(match->wc.masks.ct_mark);\r\n + CHECK_NONZERO(match->wc.masks.ct_label.u64.hi);\r\n + CHECK_NONZERO(match->wc.masks.ct_label.u64.lo);\r\n + CHECK_NONZERO(match->wc.masks.ct_nw_proto);\r\n + CHECK_NONZERO(match->wc.masks.ct_tp_src);\r\n + CHECK_NONZERO(match->wc.masks.ct_tp_dst);\r\n + CHECK_NONZERO(match->wc.masks.conj_id);\r\n + CHECK_NONZERO(match->wc.masks.actset_output);\r\n +\r\n + /* unsupported L2 */\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.mpls_lse,\r\n + sizeof(match_zero_wc.flow.mpls_lse) /\r\n + sizeof(ovs_be32));\r\n +\r\n + /* unsupported L3 */\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.ipv6_src, sizeof(struct in6_addr));\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.ipv6_dst, sizeof(struct in6_addr));\r\n + CHECK_NONZERO(match->wc.masks.ipv6_label);\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.nd_target, sizeof(struct in6_addr));\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.arp_sha, sizeof(struct eth_addr));\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.arp_tha, sizeof(struct eth_addr));\r\n +\r\n + /* If fragmented, then don't HW accelerate - for now */\r\n + CHECK_NONZERO(match_zero_wc.flow.nw_frag);\r\n +\r\n + /* unsupported L4 */\r\n + CHECK_NONZERO(match->wc.masks.igmp_group_ip4);\r\n +\r\n + return 0;\r\n +\r\n +err:\r\n + VLOG_INFO(\"Cannot HW accelerate this flow\");\r\n + return -1;\r\n +}\r\n +\r\n +static int\r\n +netdev_dpdk_destroy_rte_flow(struct netdev_dpdk *dev,\r\n + const ovs_u128 *ufid,\r\n + struct rte_flow *rte_flow)\r\n +{\r\n + struct rte_flow_error error;\r\n + int ret;\r\n +\r\n + ret = rte_flow_destroy(dev->port_id, rte_flow, &error);\r\n + if (ret == 0) {\r\n + del_ufid_dpdk_flow_mapping(ufid);\r\n + VLOG_INFO(\"removed rte flow %p associated with ufid \" UUID_FMT \"\\n\",\r\n + rte_flow, UUID_ARGS((struct uuid *)ufid));\r\n + } else {\r\n + VLOG_ERR(\"rte flow destroy error: %u : message : %s\\n\",\r\n + error.type, error.message);\r\n + }\r\n +\r\n + return ret;\r\n +}\r\n +\r\n +static int\r\n +netdev_dpdk_flow_put(struct netdev *netdev, struct match *match,\r\n + struct nlattr *actions, size_t actions_len,\r\n + const ovs_u128 *ufid, struct offload_info *info,\r\n + struct dpif_flow_stats *stats OVS_UNUSED)\r\n +{\r\n + struct rte_flow *rte_flow;\r\n + int ret;\r\n +\r\n + /*\r\n + * If an old rte_flow exists, it means it's a flow modification.\r\n + * Here destroy the old rte flow first before adding a new one.\r\n + */\r\n + rte_flow = get_rte_flow_by_ufid(ufid);\r\n + if (rte_flow) {\r\n + ret = netdev_dpdk_destroy_rte_flow(netdev_dpdk_cast(netdev),\r\n + ufid, rte_flow);\r\n + if (ret < 0)\r\n + return ret;\r\n + }\r\n +\r\n + ret = netdev_dpdk_validate_flow(match);\r\n + if (ret < 0) {\r\n + return ret;\r\n + }\r\n +\r\n + return netdev_dpdk_add_rte_flow_offload(netdev, match, actions,\r\n + actions_len, ufid, info);\r\n +}\r\n +\r\n +#define DPDK_FLOW_OFFLOAD_API \\\r\n + NULL, /* flow_flush */ \\\r\n + NULL, /* flow_dump_create */ \\\r\n + NULL, /* flow_dump_destroy */ \\\r\n + NULL, /* flow_dump_next */ \\\r\n + netdev_dpdk_flow_put, \\\r\n + NULL, /* flow_get */ \\\r\n + NULL, /* flow_del */ \\\r\n + NULL /* init_flow_api */\r\n +\r\n +\r\n #define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT, \\\r\n SET_CONFIG, SET_TX_MULTIQ, SEND, \\\r\n GET_CARRIER, GET_STATS, \\\r\n @@ -3472,7 +3891,7 @@ get_rte_flow_by_ufid(const ovs_u128 *ufid)\r\n RXQ_RECV, \\\r\n NULL, /* rx_wait */ \\\r\n NULL, /* rxq_drain */ \\\r\n - NO_OFFLOAD_API \\\r\n + DPDK_FLOW_OFFLOAD_API \\\r\n }\r\n \r\n static const struct netdev_class dpdk_class =\r\n -- \r\n 2.7.4","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"Cri7tMlJ\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsSxt5ww0z9sPt\n\tfor ;\n\tWed, 13 Sep 2017 14:15:30 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 85CAEAF4;\n\tWed, 13 Sep 2017 04:15:26 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 83508A86\n\tfor ; Wed, 13 Sep 2017 04:15:24 +0000 (UTC)","from NAM02-CY1-obe.outbound.protection.outlook.com\n\t(mail-cys01nam02on0049.outbound.protection.outlook.com\n\t[104.47.37.49])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id A9DDA13E\n\tfor ; Wed, 13 Sep 2017 04:15:22 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB053.namprd05.prod.outlook.com (10.255.210.139) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.56.4; Wed, 13 Sep 2017 04:15:20 +0000","from BLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) by\n\tBLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) with mapi id\n\t15.20.0056.010; Wed, 13 Sep 2017 04:15:20 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=kG/28SkjO5REhrMG9XzwpojOVGgjLkF6JcBcP/zQ3Lg=;\n\tb=Cri7tMlJ7CDdfe+P1AmY43xqDS/pwOJeo1hS5Tr/9Vbwf1lW54rCkTY0gaJFAVfRDtWVFSAk4r2XdUuUUCY10MDSEJeVF6Lsy+OPI01Ild3zsbGqK4P9qy7UfHhw6hOjZdAYFpiajDtPdNuybnYB3iRxsV3h6ZEVJO/fZIMSArk=","From":"Darrell Ball ","To":"Yuanhan Liu , \"dev@openvswitch.org\"\n\t","Thread-Topic":"[PATCH v2 4/8] netdev-dpdk: implement flow put with rte flow","Thread-Index":"AQHTJiiw1ZjL6RzFTUOQLht0V/vcqaKxzCeA","Date":"Wed, 13 Sep 2017 04:15:20 +0000","Message-ID":"","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>","In-Reply-To":"<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.25.0.170815","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"Cri7tMlJ\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"x-originating-ip":"[73.162.236.45]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB053;\n\t20:kyckP8IMAPuhEHdXpYOkPGvcPhbVzI6DWXYrRQidapuUsfT9wyTX6llsidlBnUrRKDRHBO6qY1JITeLszagDZDdZGoWbpAkAdlc3vbe2PHwS9MZ5AEtMvz0XQ+wlwcz3f5wP7mTZ/lCS+EMztqCRilwtMEsIyb/HpvP9+OIWDTE=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"3e5ac21b-fe7c-45a2-b95f-08d4fa5e0c1d","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(300000502095)(300135100095)(22001)(2017030254152)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:BLUPR05MB053; ","x-ms-traffictypediagnostic":"BLUPR05MB053:","x-exchange-antispam-report-test":"UriScan:;","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(10201501046)(100000703101)(100105400095)(3002001)(93006095)(93001095)(6041248)(20161123555025)(20161123560025)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123558100)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB053; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB053; ","x-forefront-prvs":"042957ACD7","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(366002)(39860400002)(376002)(346002)(189002)(199003)(24454002)(377454003)(86362001)(53546010)(50986999)(68736007)(76176999)(54356999)(101416001)(305945005)(7736002)(575784001)(6246003)(2501003)(5660300001)(478600001)(105586002)(99286003)(6512007)(53946003)(54906002)(53936002)(106356001)(4326008)(82746002)(33656002)(8676002)(81156014)(81166006)(8936002)(36756003)(97736004)(6436002)(2900100001)(4001350100001)(189998001)(77096006)(83506001)(66066001)(83716003)(316002)(2906002)(6486002)(25786009)(229853002)(3280700002)(6506006)(3846002)(14454004)(102836003)(6116002)(3660700001)(2950100002);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB053;\n\tH:BLUPR05MB611.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"13 Sep 2017 04:15:20.1508\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB053","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1767576,"web_url":"http://patchwork.ozlabs.org/comment/1767576/","msgid":"","list_archive_url":null,"date":"2017-09-13T05:18:03","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"More comments\r\n\r\nOn 9/12/17, 9:15 PM, \"Darrell Ball\" wrote:\r\n\r\n Still working my way through this patch, but I had a couple comments inline\r\n \r\n On 9/5/17, 2:23 AM, \"Yuanhan Liu\" wrote:\r\n \r\n From: Finn Christensen \r\n \r\n The basic yet the major part of this patch is to translate the \"match\"\r\n to rte flow patterns. And then, we create a rte flow with a MARK action.\r\n Afterwards, all pkts matches the flow will have the mark id in the mbuf.\r\n \r\n For any unsupported flows, such as MPLS, -1 is returned, meaning the\r\n flow offload is failed and then skipped.\r\n \r\n Co-authored-by: Yuanhan Liu \r\n Signed-off-by: Finn Christensen \r\n Signed-off-by: Yuanhan Liu \r\n ---\r\n \r\n v2: - convert some macros to functions\r\n - do not hardcode the max number of flow/action\r\n - fix L2 patterns for Intel nic\r\n - add comments for not implemented offload methods\r\n ---\r\n lib/netdev-dpdk.c | 421 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\r\n 1 file changed, 420 insertions(+), 1 deletion(-)\r\n \r\n diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c\r\n index 46f9885..37b0f99 100644\r\n --- a/lib/netdev-dpdk.c\r\n +++ b/lib/netdev-dpdk.c\r\n @@ -58,6 +58,7 @@\r\n #include \"smap.h\"\r\n #include \"sset.h\"\r\n #include \"unaligned.h\"\r\n +#include \"uuid.h\"\r\n #include \"timeval.h\"\r\n #include \"unixctl.h\"\r\n \r\n @@ -3400,6 +3401,424 @@ get_rte_flow_by_ufid(const ovs_u128 *ufid)\r\n }\r\n \r\n \r\n +struct flow_patterns {\r\n + struct rte_flow_item *items;\r\n + int cnt;\r\n + int max;\r\n +};\r\n +\r\n +struct flow_actions {\r\n + struct rte_flow_action *actions;\r\n + int cnt;\r\n + int max;\r\n +};\r\n +\r\n +static void\r\n +add_flow_pattern(struct flow_patterns *patterns, enum rte_flow_item_type type,\r\n + const void *spec, const void *mask)\r\n +{\r\n + int cnt = patterns->cnt;\r\n +\r\n + if (cnt == 0) {\r\n + patterns->max = 8;\r\n + patterns->items = xcalloc(patterns->max, sizeof(struct rte_flow_item));\r\n + } else if (cnt == patterns->max) {\r\n + patterns->max *= 2;\r\n + patterns->items = xrealloc(patterns->items, patterns->max *\r\n + sizeof(struct rte_flow_item));\r\n + }\r\n +\r\n + patterns->items[cnt].type = type;\r\n + patterns->items[cnt].spec = spec;\r\n + patterns->items[cnt].mask = mask;\r\n + patterns->items[cnt].last = NULL;\r\n + patterns->cnt++;\r\n +}\r\n +\r\n +static void\r\n +add_flow_action(struct flow_actions *actions, enum rte_flow_action_type type,\r\n + const void *conf)\r\n +{\r\n + int cnt = actions->cnt;\r\n +\r\n + if (cnt == 0) {\r\n + actions->max = 8;\r\n + actions->actions = xcalloc(actions->max,\r\n + sizeof(struct rte_flow_action));\r\n + } else if (cnt == actions->max) {\r\n + actions->max *= 2;\r\n + actions->actions = xrealloc(actions->actions, actions->max *\r\n + sizeof(struct rte_flow_action));\r\n + }\r\n +\r\n + actions->actions[cnt].type = type;\r\n + actions->actions[cnt].conf = conf;\r\n + actions->cnt++;\r\n +}\r\n +\r\n +static int\r\n +netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,\r\n + const struct match *match,\r\n + struct nlattr *nl_actions OVS_UNUSED,\r\n + size_t actions_len,\r\n + const ovs_u128 *ufid,\r\n + struct offload_info *info)\r\n +{\r\n + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);\r\n + const struct rte_flow_attr flow_attr = {\r\n + .group = 0,\r\n + .priority = 0,\r\n + .ingress = 1,\r\n + .egress = 0\r\n + };\r\n + struct flow_patterns patterns = { .items = NULL, .cnt = 0 };\r\n + struct flow_actions actions = { .actions = NULL, .cnt = 0 };\r\n + struct rte_flow *flow;\r\n + struct rte_flow_error error;\r\n + uint8_t *ipv4_next_proto_mask = NULL;\r\n + int ret = 0;\r\n +\r\n + /* Eth */\r\n + struct rte_flow_item_eth eth_spec;\r\n + struct rte_flow_item_eth eth_mask;\r\n + memset(ð_mask, 0, sizeof(eth_mask));\r\n + if (match->wc.masks.dl_src.be16[0] ||\r\n + match->wc.masks.dl_src.be16[1] ||\r\n + match->wc.masks.dl_src.be16[2] ||\r\n + match->wc.masks.dl_dst.be16[0] ||\r\n + match->wc.masks.dl_dst.be16[1] ||\r\n + match->wc.masks.dl_dst.be16[2]) {\r\n + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\r\n + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\r\n + eth_spec.type = match->flow.dl_type;\r\n +\r\n + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\r\n + sizeof(eth_mask.dst));\r\n + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\r\n + sizeof(eth_mask.src));\r\n + eth_mask.type = match->wc.masks.dl_type;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\r\n + ð_spec, ð_mask);\r\n + } else {\r\n + /*\r\n + * If user specifies a flow (like UDP flow) without L2 patterns,\r\n + * OVS will at least set the dl_type. Normally, it's enough to\r\n + * create an eth pattern just with it. Unluckily, some Intel's\r\n + * NIC (such as XL710) doesn't support that. Below is a workaround,\r\n + * which simply matches any L2 pkts.\r\n + */\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\r\n + }\r\n +\r\n + /* VLAN */\r\n + struct rte_flow_item_vlan vlan_spec;\r\n + struct rte_flow_item_vlan vlan_mask;\r\n + memset(&vlan_mask, 0, sizeof(vlan_mask));\r\n + if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {\r\n + vlan_spec.tci = match->flow.vlans[0].tci;\r\n + vlan_mask.tci = match->wc.masks.vlans[0].tci;\r\n +\r\n + /* match any protocols */\r\n + vlan_mask.tpid = 0;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN,\r\n + &vlan_spec, &vlan_mask);\r\n + }\r\n +\r\n + /* IP v4 */\r\n + uint8_t proto = 0;\r\n + struct rte_flow_item_ipv4 ipv4_spec;\r\n + struct rte_flow_item_ipv4 ipv4_mask;\r\n + memset(&ipv4_mask, 0, sizeof(ipv4_mask));\r\n + if ((match->flow.dl_type == ntohs(ETH_TYPE_IP)) &&\r\n + (match->wc.masks.nw_src || match->wc.masks.nw_dst ||\r\n + match->wc.masks.nw_tos || match->wc.masks.nw_ttl ||\r\n + match->wc.masks.nw_proto)) {\r\n + ipv4_spec.hdr.type_of_service = match->flow.nw_tos;\r\n + ipv4_spec.hdr.time_to_live = match->flow.nw_tos;\r\n \r\n [Darrell] should be TTL; lots of e-mails but I don’t remember seeing this mentioned\r\n \r\n \r\n + ipv4_spec.hdr.next_proto_id = match->flow.nw_proto;\r\n + ipv4_spec.hdr.src_addr = match->flow.nw_src;\r\n + ipv4_spec.hdr.dst_addr = match->flow.nw_dst;\r\n +\r\n + ipv4_mask.hdr.type_of_service = match->wc.masks.nw_tos;\r\n + ipv4_mask.hdr.time_to_live = match->wc.masks.nw_tos;\r\n \r\n [Darrell] should be TTL; lots of e-mails but I don’t remember seeing this mentioned\r\n \r\n \r\n + ipv4_mask.hdr.next_proto_id = match->wc.masks.nw_proto;\r\n + ipv4_mask.hdr.src_addr = match->wc.masks.nw_src;\r\n + ipv4_mask.hdr.dst_addr = match->wc.masks.nw_dst;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_IPV4,\r\n + &ipv4_spec, &ipv4_mask);\r\n +\r\n + /* Save proto for L4 protocol setup */\r\n + proto = ipv4_spec.hdr.next_proto_id & ipv4_mask.hdr.next_proto_id;\r\n +\r\n + /* Remember proto mask address for later modification */\r\n + ipv4_next_proto_mask = &ipv4_mask.hdr.next_proto_id;\r\n + }\r\n +\r\n + if (proto != IPPROTO_ICMP && proto != IPPROTO_UDP &&\r\n + proto != IPPROTO_SCTP && proto != IPPROTO_TCP &&\r\n + (match->wc.masks.tp_src ||\r\n + match->wc.masks.tp_dst ||\r\n + match->wc.masks.tcp_flags)) {\r\n + VLOG_INFO(\"L4 Protocol (%u) not supported\", proto);\r\n + ret = -1;\r\n + goto out;\r\n + }\r\n +\r\n + struct rte_flow_item_udp udp_spec;\r\n + struct rte_flow_item_udp udp_mask;\r\n + memset(&udp_mask, 0, sizeof(udp_mask));\r\n + if ((proto == IPPROTO_UDP) &&\r\n + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\r\n + udp_spec.hdr.src_port = match->flow.tp_src;\r\n + udp_spec.hdr.dst_port = match->flow.tp_dst;\r\n +\r\n + udp_mask.hdr.src_port = match->wc.masks.tp_src;\r\n + udp_mask.hdr.dst_port = match->wc.masks.tp_dst;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_UDP,\r\n + &udp_spec, &udp_mask);\r\n +\r\n + /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match */\r\n + if (ipv4_next_proto_mask) {\r\n + *ipv4_next_proto_mask = 0;\r\n + }\r\n + }\r\n +\r\n + struct rte_flow_item_sctp sctp_spec;\r\n + struct rte_flow_item_sctp sctp_mask;\r\n + memset(&sctp_mask, 0, sizeof(sctp_mask));\r\n + if ((proto == IPPROTO_SCTP) &&\r\n + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\r\n + sctp_spec.hdr.src_port = match->flow.tp_src;\r\n + sctp_spec.hdr.dst_port = match->flow.tp_dst;\r\n +\r\n + sctp_mask.hdr.src_port = match->wc.masks.tp_src;\r\n + sctp_mask.hdr.dst_port = match->wc.masks.tp_dst;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_SCTP,\r\n + &sctp_spec, &sctp_mask);\r\n +\r\n + /* proto == SCTP and ITEM_TYPE_SCTP, thus no need for proto match */\r\n + if (ipv4_next_proto_mask) {\r\n + *ipv4_next_proto_mask = 0;\r\n + }\r\n + }\r\n +\r\n + struct rte_flow_item_icmp icmp_spec;\r\n + struct rte_flow_item_icmp icmp_mask;\r\n + memset(&icmp_mask, 0, sizeof(icmp_mask));\r\n + if ((proto == IPPROTO_ICMP) &&\r\n + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\r\n + icmp_spec.hdr.icmp_type = (uint8_t)ntohs(match->flow.tp_src);\r\n + icmp_spec.hdr.icmp_code = (uint8_t)ntohs(match->flow.tp_dst);\r\n +\r\n + icmp_mask.hdr.icmp_type = (uint8_t)ntohs(match->wc.masks.tp_src);\r\n + icmp_mask.hdr.icmp_code = (uint8_t)ntohs(match->wc.masks.tp_dst);\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ICMP,\r\n + &icmp_spec, &icmp_mask);\r\n +\r\n + /* proto == ICMP and ITEM_TYPE_ICMP, thus no need for proto match */\r\n + if (ipv4_next_proto_mask) {\r\n + *ipv4_next_proto_mask = 0;\r\n + }\r\n + }\r\n +\r\n + struct rte_flow_item_tcp tcp_spec;\r\n + struct rte_flow_item_tcp tcp_mask;\r\n + memset(&tcp_mask, 0, sizeof(tcp_mask));\r\n + if ((proto == IPPROTO_TCP) &&\r\n + (match->wc.masks.tp_src ||\r\n + match->wc.masks.tp_dst ||\r\n + match->wc.masks.tcp_flags)) {\r\n + tcp_spec.hdr.src_port = match->flow.tp_src;\r\n + tcp_spec.hdr.dst_port = match->flow.tp_dst;\r\n + tcp_spec.hdr.data_off = ntohs(match->flow.tcp_flags) >> 8;\r\n + tcp_spec.hdr.tcp_flags = ntohs(match->flow.tcp_flags) & 0xff;\r\n +\r\n + tcp_mask.hdr.src_port = match->wc.masks.tp_src;\r\n + tcp_mask.hdr.dst_port = match->wc.masks.tp_dst;\r\n + tcp_mask.hdr.data_off = ntohs(match->wc.masks.tcp_flags) >> 8;\r\n + tcp_mask.hdr.tcp_flags = ntohs(match->wc.masks.tcp_flags) & 0xff;\r\n +\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_TCP,\r\n + &tcp_spec, &tcp_mask);\r\n +\r\n + /* proto == TCP and ITEM_TYPE_TCP, thus no need for proto match */\r\n + if (ipv4_next_proto_mask) {\r\n + *ipv4_next_proto_mask = 0;\r\n + }\r\n + }\r\n + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_END, NULL, NULL);\r\n +\r\n + struct rte_flow_action_mark mark;\r\n + if (actions_len) {\r\n + mark.id = info->flow_mark;\r\n + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark);\r\n + } else {\r\n + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_DROP, NULL);\r\n + VLOG_INFO(\"no action given; drop pkts in hardware\\n\");\r\n + }\r\n + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL);\r\n +\r\n + flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items,\r\n + actions.actions, &error);\r\n + if (!flow) {\r\n + VLOG_ERR(\"rte flow creat error: %u : message : %s\\n\",\r\n + error.type, error.message);\r\n + ret = -1;\r\n + goto out;\r\n + }\r\n + add_ufid_dpdk_flow_mapping(ufid, flow);\r\n + VLOG_INFO(\"installed flow %p by ufid \"UUID_FMT\"\\n\",\r\n + flow, UUID_ARGS((struct uuid *)ufid));\r\n +\r\n +out:\r\n + free(patterns.items);\r\n + free(actions.actions);\r\n + return ret;\r\n +}\r\n +\r\n +/*\r\n + * Validate for later rte flow offload creation. If any unsupported\r\n + * flow are specified, return -1.\r\n + */\r\n +static int\r\n +netdev_dpdk_validate_flow(const struct match *match)\r\n +{\r\n + struct match match_zero_wc;\r\n +\r\n + /* Create a wc-zeroed version of flow */\r\n + match_init(&match_zero_wc, &match->flow, &match->wc);\r\n +\r\n +#define CHECK_NONZERO_BYTES(addr, size) do { \\\r\n + uint8_t *padr = (uint8_t *)(addr); \\\r\n + int i; \\\r\n + for (i = 0; i < (size); i++) { \\\r\n + if (padr[i] != 0) { \\\r\n + goto err; \\\r\n + } \\\r\n + } \\\r\n +} while (0)\r\n +\r\n +#define CHECK_NONZERO(var) do { \\\r\n + if ((var) != 0) { \\\r\n + goto err; \\\r\n + } \\\r\n +} while (0)\r\n +\r\n + CHECK_NONZERO_BYTES(&match_zero_wc.flow.tunnel,\r\n + sizeof(match_zero_wc.flow.tunnel));\r\n + CHECK_NONZERO(match->wc.masks.metadata);\r\n + CHECK_NONZERO(match->wc.masks.skb_priority);\r\n + CHECK_NONZERO(match->wc.masks.pkt_mark);\r\n + CHECK_NONZERO(match->wc.masks.dp_hash);\r\n +\r\n + /* recirc id must be zero */\r\n + CHECK_NONZERO(match_zero_wc.flow.recirc_id);\r\n +\r\n + CHECK_NONZERO(match->wc.masks.ct_state);\r\n + CHECK_NONZERO(match->wc.masks.ct_zone);\r\n + CHECK_NONZERO(match->wc.masks.ct_mark);\r\n\r\n[Darrell]\r\ncheck the other pkt_metadata fields; some are missing here.\r\nNeed to check this exhaustively.\r\n\r\n\r\n\r\n + CHECK_NONZERO(match->wc.masks.ct_label.u64.hi);\r\n + CHECK_NONZERO(match->wc.masks.ct_label.u64.lo);\r\n + CHECK_NONZERO(match->wc.masks.ct_nw_proto);\r\n + CHECK_NONZERO(match->wc.masks.ct_tp_src);\r\n + CHECK_NONZERO(match->wc.masks.ct_tp_dst);\r\n + CHECK_NONZERO(match->wc.masks.conj_id);\r\n + CHECK_NONZERO(match->wc.masks.actset_output);\r\n +\r\n + /* unsupported L2 */\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.mpls_lse,\r\n + sizeof(match_zero_wc.flow.mpls_lse) /\r\n + sizeof(ovs_be32));\r\n +\r\n + /* unsupported L3 */\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.ipv6_src, sizeof(struct in6_addr));\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.ipv6_dst, sizeof(struct in6_addr));\r\n + CHECK_NONZERO(match->wc.masks.ipv6_label);\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.nd_target, sizeof(struct in6_addr));\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.arp_sha, sizeof(struct eth_addr));\r\n + CHECK_NONZERO_BYTES(&match->wc.masks.arp_tha, sizeof(struct eth_addr));\r\n +\r\n + /* If fragmented, then don't HW accelerate - for now */\r\n + CHECK_NONZERO(match_zero_wc.flow.nw_frag);\r\n +\r\n + /* unsupported L4 */\r\n + CHECK_NONZERO(match->wc.masks.igmp_group_ip4);\r\n +\r\n + return 0;\r\n +\r\n +err:\r\n + VLOG_INFO(\"Cannot HW accelerate this flow\");\r\n + return -1;\r\n +}\r\n +\r\n +static int\r\n +netdev_dpdk_destroy_rte_flow(struct netdev_dpdk *dev,\r\n + const ovs_u128 *ufid,\r\n + struct rte_flow *rte_flow)\r\n +{\r\n + struct rte_flow_error error;\r\n + int ret;\r\n +\r\n + ret = rte_flow_destroy(dev->port_id, rte_flow, &error);\r\n + if (ret == 0) {\r\n + del_ufid_dpdk_flow_mapping(ufid);\r\n + VLOG_INFO(\"removed rte flow %p associated with ufid \" UUID_FMT \"\\n\",\r\n + rte_flow, UUID_ARGS((struct uuid *)ufid));\r\n + } else {\r\n + VLOG_ERR(\"rte flow destroy error: %u : message : %s\\n\",\r\n + error.type, error.message);\r\n + }\r\n +\r\n + return ret;\r\n +}\r\n +\r\n +static int\r\n +netdev_dpdk_flow_put(struct netdev *netdev, struct match *match,\r\n + struct nlattr *actions, size_t actions_len,\r\n + const ovs_u128 *ufid, struct offload_info *info,\r\n + struct dpif_flow_stats *stats OVS_UNUSED)\r\n +{\r\n + struct rte_flow *rte_flow;\r\n + int ret;\r\n +\r\n + /*\r\n + * If an old rte_flow exists, it means it's a flow modification.\r\n + * Here destroy the old rte flow first before adding a new one.\r\n + */\r\n + rte_flow = get_rte_flow_by_ufid(ufid);\r\n + if (rte_flow) {\r\n + ret = netdev_dpdk_destroy_rte_flow(netdev_dpdk_cast(netdev),\r\n + ufid, rte_flow);\r\n + if (ret < 0)\r\n + return ret;\r\n + }\r\n +\r\n + ret = netdev_dpdk_validate_flow(match);\r\n + if (ret < 0) {\r\n + return ret;\r\n + }\r\n +\r\n + return netdev_dpdk_add_rte_flow_offload(netdev, match, actions,\r\n + actions_len, ufid, info);\r\n +}\r\n +\r\n +#define DPDK_FLOW_OFFLOAD_API \\\r\n + NULL, /* flow_flush */ \\\r\n + NULL, /* flow_dump_create */ \\\r\n + NULL, /* flow_dump_destroy */ \\\r\n + NULL, /* flow_dump_next */ \\\r\n + netdev_dpdk_flow_put, \\\r\n + NULL, /* flow_get */ \\\r\n + NULL, /* flow_del */ \\\r\n + NULL /* init_flow_api */\r\n +\r\n +\r\n #define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT, \\\r\n SET_CONFIG, SET_TX_MULTIQ, SEND, \\\r\n GET_CARRIER, GET_STATS, \\\r\n @@ -3472,7 +3891,7 @@ get_rte_flow_by_ufid(const ovs_u128 *ufid)\r\n RXQ_RECV, \\\r\n NULL, /* rx_wait */ \\\r\n NULL, /* rxq_drain */ \\\r\n - NO_OFFLOAD_API \\\r\n + DPDK_FLOW_OFFLOAD_API \\\r\n }\r\n \r\n static const struct netdev_class dpdk_class =\r\n -- \r\n 2.7.4","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"e3lXFOaa\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsVLG35mwz9s76\n\tfor ;\n\tWed, 13 Sep 2017 15:18:12 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id F0C4BAB6;\n\tWed, 13 Sep 2017 05:18:08 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 2FEF9A73\n\tfor ; Wed, 13 Sep 2017 05:18:08 +0000 (UTC)","from NAM03-CO1-obe.outbound.protection.outlook.com\n\t(mail-co1nam03on0042.outbound.protection.outlook.com [104.47.40.42])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 0891DD3\n\tfor ; Wed, 13 Sep 2017 05:18:05 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB246.namprd05.prod.outlook.com (10.255.191.19) with Microsoft\n\tSMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.56.4; Wed, 13 Sep 2017 05:18:03 +0000","from BLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) by\n\tBLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) with mapi id\n\t15.20.0056.010; Wed, 13 Sep 2017 05:18:03 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=TK3GURp7ATb7BFiHcK8BTfmDL2mYr+MYsyCzghSQmTU=;\n\tb=e3lXFOaanDRUtXnDJrqlx9CRVn4m/0nWRTcvOK0+/jV+at/1m6ydxayozQ9teyzTi37fKIpEclNSu/5GJFFcx89oqL+LfquZ5wKlD5CqRZmeJw029h8slVNscN6hdGzbVQJ6EyoIsolO3Nx0kyV3cennxqvbT6ZVmEYuDiKRHAc=","From":"Darrell Ball ","To":"Yuanhan Liu , \"dev@openvswitch.org\"\n\t","Thread-Topic":"[PATCH v2 4/8] netdev-dpdk: implement flow put with rte flow","Thread-Index":"AQHTJiiw1ZjL6RzFTUOQLht0V/vcqaKxzCeAgAARhgA=","Date":"Wed, 13 Sep 2017 05:18:03 +0000","Message-ID":"","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t","In-Reply-To":"","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.25.0.170815","x-originating-ip":"[73.162.236.45]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB246;\n\t20:QRN353mqr12qfHIvLQNpyZ9drRC6fMdBLYlcCYIX0WLn692/JnGFr9y1tW11x0Uk+wwQnnzSmZIeBLf8mdd4ZYJa35ZkVs1V4WCbPjdo79PUloWg9vs75veOmKaKhyP1m0iWSNVsJLWC5s9dFocWbMsULvVcJkQNDyG74PfpPG8=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"8f71226c-9171-4c0b-072d-08d4fa66cf19","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:BLUPR05MB246; ","x-ms-traffictypediagnostic":"BLUPR05MB246:","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"e3lXFOaa\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"x-exchange-antispam-report-test":"UriScan:(61668805478150);","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(100000703101)(100105400095)(93006095)(93001095)(3002001)(10201501046)(6041248)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123562025)(20161123555025)(20161123564025)(20161123560025)(20161123558100)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB246; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB246; ","x-forefront-prvs":"042957ACD7","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(376002)(366002)(346002)(377454003)(24454002)(189002)(199003)(5660300001)(33656002)(575784001)(3280700002)(54356999)(86362001)(68736007)(189998001)(2906002)(53946003)(2900100001)(54906002)(50986999)(305945005)(4326008)(101416001)(99286003)(6512007)(6246003)(76176999)(229853002)(14454004)(316002)(7736002)(3660700001)(478600001)(36756003)(53936002)(8936002)(25786009)(97736004)(6436002)(77096006)(2501003)(6486002)(6506006)(81156014)(2950100002)(66066001)(81166006)(53546010)(8676002)(106356001)(83506001)(82746002)(102836003)(3846002)(105586002)(6116002)(83716003)(4001350100001)(579004);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB246;\n\tH:BLUPR05MB611.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"<1902E66ACCB83441856C4CD4D93D4134@namprd05.prod.outlook.com>","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"13 Sep 2017 05:18:03.1089\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB246","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1767721,"web_url":"http://patchwork.ozlabs.org/comment/1767721/","msgid":"<20170913094529.GC27555@vergenet.net>","list_archive_url":null,"date":"2017-09-13T09:45:30","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Tue, Sep 12, 2017 at 08:34:44AM +0000, Darrell Ball wrote:\n> \n> \n> On 9/10/17, 11:11 PM, \"ovs-dev-bounces@openvswitch.org on behalf of Yuanhan Liu\" wrote:\n> \n> On Fri, Sep 08, 2017 at 06:47:55PM +0200, Simon Horman wrote:\n> > On Tue, Sep 05, 2017 at 05:22:57PM +0800, Yuanhan Liu wrote:\n> > > From: Finn Christensen \n> > > \n> > > The basic yet the major part of this patch is to translate the \"match\"\n> > > to rte flow patterns. And then, we create a rte flow with a MARK action.\n> > > Afterwards, all pkts matches the flow will have the mark id in the mbuf.\n> > > \n> > > For any unsupported flows, such as MPLS, -1 is returned, meaning the\n> > > flow offload is failed and then skipped.\n> > \n> > ...\n> > \n> > > +static int\n> > > +netdev_dpdk_add_rte_flow_offload(struct netdev *netdev,\n> > > + const struct match *match,\n> > > + struct nlattr *nl_actions OVS_UNUSED,\n> > > + size_t actions_len,\n> > > + const ovs_u128 *ufid,\n> > > + struct offload_info *info)\n> > > +{\n> > > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);\n> > > + const struct rte_flow_attr flow_attr = {\n> > > + .group = 0,\n> > > + .priority = 0,\n> > > + .ingress = 1,\n> > > + .egress = 0\n> > > + };\n> > > + struct flow_patterns patterns = { .items = NULL, .cnt = 0 };\n> > > + struct flow_actions actions = { .actions = NULL, .cnt = 0 };\n> > \n> > I believe the following will give the same result as the above,\n> > less verbosely.\n> > \n> > + const struct rte_flow_attr flow_attr = { .ingress = 1 };\n> > + struct flow_patterns patterns = { .cnt = 0 };\n> > + struct flow_actions actions = { .cnt = 0 };\n> \n> I'm not quite sure on that. If the compiler doesn't do zero assigment\n> correctly, something deadly wrong could happen. They all are short\n> structs, that I think it's fine, IMO. If they are big, I'd prefer an\n> explicit memset.\n\nI'm pretty sure that the compiler will zero all other fields when the\nscheme I described above is used. However, its a moot point if the approach\nyou used is preferred. So I think what you have is fine.\n\n> \n> > > + struct rte_flow *flow;\n> > > + struct rte_flow_error error;\n> > > + uint8_t *ipv4_next_proto_mask = NULL;\n> > > + int ret = 0;\n> > > +\n> > > + /* Eth */\n> > > + struct rte_flow_item_eth eth_spec;\n> > > + struct rte_flow_item_eth eth_mask;\n> > \n> > Empty line here please.\n> \n> I actually want to bind the two declartions with the following code piece,\n> to show that they are tightly related.\n\nAs I think Darrell pointed out elsewhere it is common practice to\nput declarations close to code that uses them. This is different from\nmy assumption that they should be grouped at the top of a context.\nWith this in mind what you already have makes sense to me.\n\n> \n> > > + memset(ð_mask, 0, sizeof(eth_mask));\n> > > + if (match->wc.masks.dl_src.be16[0] ||\n> > > + match->wc.masks.dl_src.be16[1] ||\n> > > + match->wc.masks.dl_src.be16[2] ||\n> > > + match->wc.masks.dl_dst.be16[0] ||\n> > > + match->wc.masks.dl_dst.be16[1] ||\n> > > + match->wc.masks.dl_dst.be16[2]) {\n> > \n> > It looks like eth_addr_is_zero() can be used in the above.\n> \n> Yes, we could. Thanks for the tip.\n> \n> > > + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\n> > > + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\n> > > + eth_spec.type = match->flow.dl_type;\n> > > +\n> > > + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\n> > > + sizeof(eth_mask.dst));\n> > > + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\n> > > + sizeof(eth_mask.src));\n> > > + eth_mask.type = match->wc.masks.dl_type;\n> > > +\n> > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\n> > > + ð_spec, ð_mask);\n> > > + } else {\n> > > + /*\n> > > + * If user specifies a flow (like UDP flow) without L2 patterns,\n> > > + * OVS will at least set the dl_type. Normally, it's enough to\n> > > + * create an eth pattern just with it. Unluckily, some Intel's\n> > > + * NIC (such as XL710) doesn't support that. Below is a workaround,\n> > > + * which simply matches any L2 pkts.\n> > > + */\n> > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\n> > > + }\n> > \n> > This feels a lot like a layer violation - making the core aware\n> > of specific implementation details of lower layers.\n> \n> I agree with you, but unlickily, without it, Intel NIC simply won't work\n> (according to Finn), unless you have mac addr being provided.\n\nI think its reasonable to provide hardware-workarounds. But is it\nappropriate to enable it for all hardware and is this the most appropriate\nplace for a hardware work-around?\n\n> > >From a functional point of view, is the idea that\n> > a eth_type+5-tuple match is converted to a 5-tuple match?\n> \n> Unluckily, yes.\n> \n> > > + /* VLAN */\n> > > + struct rte_flow_item_vlan vlan_spec;\n> > > + struct rte_flow_item_vlan vlan_mask;\n> > \n> > Please declare all local variables at the top of the context\n> > (in this case function).\n> > \n> > ...\n> > \n> > > + struct rte_flow_item_udp udp_spec;\n> > > + struct rte_flow_item_udp udp_mask;\n> > > + memset(&udp_mask, 0, sizeof(udp_mask));\n> > > + if ((proto == IPPROTO_UDP) &&\n> > > + (match->wc.masks.tp_src || match->wc.masks.tp_dst)) {\n> > > + udp_spec.hdr.src_port = match->flow.tp_src;\n> > > + udp_spec.hdr.dst_port = match->flow.tp_dst;\n> > > +\n> > > + udp_mask.hdr.src_port = match->wc.masks.tp_src;\n> > > + udp_mask.hdr.dst_port = match->wc.masks.tp_dst;\n> > > +\n> > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_UDP,\n> > > + &udp_spec, &udp_mask);\n> > > +\n> > > + /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match */\n> > > + if (ipv4_next_proto_mask) {\n> > > + *ipv4_next_proto_mask = 0;\n> > \n> > I think this should be:\n> > \n> > + *ipv4_next_proto_mask = NULL;\n> \n> Seems not. ipv4_next_proto_mask is defined as \"uint8_t *\".\n\n\tSorry, I misread the code.\n\n> \n> > > + }\n> > > + }\n> > > +\n> > > + struct rte_flow_item_sctp sctp_spec;\n> > > + struct rte_flow_item_sctp sctp_mask;\n> > > + memset(&sctp_mask, 0, sizeof(sctp_mask));\n> > > + if ((proto == IPPROTO_SCTP) &&\n> > \n> > It seems there are unnecessary () in the line above.\n> \n> yes.\n> \n> > > +/*\n> > > + * Validate for later rte flow offload creation. If any unsupported\n> > > + * flow are specified, return -1.\n> > > + */\n> > > +static int\n> > > +netdev_dpdk_validate_flow(const struct match *match)\n> > > +{\n> > > + struct match match_zero_wc;\n> > > +\n> > > + /* Create a wc-zeroed version of flow */\n> > > + match_init(&match_zero_wc, &match->flow, &match->wc);\n> > > +\n> > > +#define CHECK_NONZERO_BYTES(addr, size) do { \\\n> > \n> > I think do should appear on a new line.\n> > \n> > > + uint8_t *padr = (uint8_t *)(addr); \\\n> > > + int i; \\\n> > > + for (i = 0; i < (size); i++) { \\\n> > > + if (padr[i] != 0) { \\\n> > > + goto err; \\\n> > > + } \\\n> > > + } \\\n> > > +} while (0)\n> > > +\n> > > +#define CHECK_NONZERO(var) do { \\\n> > \n> > Here too.\n> > \n> > > + if ((var) != 0) { \\\n> > > + goto err; \\\n> > > + } \\\n> > > +} while (0)\n> > \n> > I think its better if macros (and defines) aren't declared\n> > inside of functions. The top of the file seems like\n> > the best place to me. If not above the first function\n> > that uses the macros, presumably this function.\n> \n> Again, I just follow the OVS habit. I saw quite many defines like\n> this in other code pieces. I'm fine to put it outside the function,\n> or even define it as a function though.\t\n\nThanks, my assumption was incorrect here too.\n\n> [Darrell] Pls convert to functions as discussed earlier.\n\nThat sounds nice to me.","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"fW25NhtR\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xscGm30G8z9sPm\n\tfor ;\n\tWed, 13 Sep 2017 19:45:36 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 91989B9B;\n\tWed, 13 Sep 2017 09:45:34 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 8D5AAB8B\n\tfor ; Wed, 13 Sep 2017 09:45:33 +0000 (UTC)","from mail-wm0-f48.google.com (mail-wm0-f48.google.com\n\t[74.125.82.48])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 8CB67A7\n\tfor ; Wed, 13 Sep 2017 09:45:32 +0000 (UTC)","by mail-wm0-f48.google.com with SMTP id g206so2201509wme.0\n\tfor ; Wed, 13 Sep 2017 02:45:32 -0700 (PDT)","from vergenet.net (reginn.horms.nl.\n\t[2001:470:7eb3:403:d63d:7eff:fe99:ac9d])\n\tby smtp.gmail.com with ESMTPSA id\n\tf29sm5067360edf.87.2017.09.13.02.45.30\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 13 Sep 2017 02:45:30 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=8DWvGcttWb0stL61MAQKAd/mQhB+KV4gwufwPmBuKhU=;\n\tb=fW25NhtRag2Q+cppMZm+WMaZfbxnfBvpfPRJqCDjQZ7iK9HWAyjQSyrmEh8GERx++R\n\tSqXioDl8sjGia7bsK5VXEtFrw7csJO8aGLA5BTUhPqwLRgEven/sIhTpk2EbH7r1uU8v\n\tP9I8km4jzIKsPvZqsB3zC/L336jUEVZmJCyl5l1wYR9p0CNSgkGvTr3Jd7IS9corBONe\n\tkp7uLovPzMJ6jco9iN9xOyWWYfKTn1Ht3k/lWAWTYGR1xbQfG9uQbUeAG7jsNVgeZOI9\n\tQOu5OEgO1Dr+CLuhqE83O1sgOI5cZ0egTxCk9ncpLA9jW5K3iQcHQN9UKc1gNQMo2vVz\n\tsNXQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=8DWvGcttWb0stL61MAQKAd/mQhB+KV4gwufwPmBuKhU=;\n\tb=WaaXGC1l6UIUicqkzFqM/Ok0BkGVsgLiV0qmyQqQ1K/kQEO7JLAmP/AR7jxQqOlfAv\n\tWLAAgdHWGO/Od3E3WHNbKwuASM4U1qmA65mAUuwq8ktIblOhQznCH5qixo1rHE/TqkGW\n\tmO0EzweMeTxngTk29NSBNBDe7voSAD6I1AAU0emksO3/h8PhF6oTkyn4JkzJUYQQVrDs\n\tH2hD2yVGwi2VSWjFR9SIK/Fj1OG8FB1C5bb2UkPjjy3pY0hHWKRIxkUDq6zo1w0g4WYE\n\tnuBowzS+fue3ZAHLgYpzU1zOVRW6YTZ/O5TM+V/reWiRX7OukVHodQHXumXMo2feEM6F\n\tvRcA==","X-Gm-Message-State":"AHPjjUh6d5ZK2d3cilHv/YvPRCHHAdP+3SyEM2DAXKxUpQLysKe6QA8A\n\tHMjVCgUriwjlgFCj","X-Google-Smtp-Source":"ADKCNb79+x0DJuMMWCFJpyX8USPs6axfRylS7ZGx6C7FgDxrQ1QtK4B6pCr5k9iS+CPYGRq5kWvXag==","X-Received":"by 10.80.243.17 with SMTP id p17mr12714599edm.38.1505295931095; \n\tWed, 13 Sep 2017 02:45:31 -0700 (PDT)","Date":"Wed, 13 Sep 2017 11:45:30 +0200","From":"Simon Horman ","To":"Darrell Ball ","Message-ID":"<20170913094529.GC27555@vergenet.net>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<20170908164754.GD7356@vergenet.net>\n\t<20170911061102.GM9736@yliu-home>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-Spam-Status":"No, score=0.5 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1768308,"web_url":"http://patchwork.ozlabs.org/comment/1768308/","msgid":"<20170914023510.GC2050@yliu-home>","list_archive_url":null,"date":"2017-09-14T02:35:10","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Wed, Sep 13, 2017 at 11:45:30AM +0200, Simon Horman wrote:\n> > > > + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\n> > > > + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\n> > > > + eth_spec.type = match->flow.dl_type;\n> > > > +\n> > > > + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\n> > > > + sizeof(eth_mask.dst));\n> > > > + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\n> > > > + sizeof(eth_mask.src));\n> > > > + eth_mask.type = match->wc.masks.dl_type;\n> > > > +\n> > > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\n> > > > + ð_spec, ð_mask);\n> > > > + } else {\n> > > > + /*\n> > > > + * If user specifies a flow (like UDP flow) without L2 patterns,\n> > > > + * OVS will at least set the dl_type. Normally, it's enough to\n> > > > + * create an eth pattern just with it. Unluckily, some Intel's\n> > > > + * NIC (such as XL710) doesn't support that. Below is a workaround,\n> > > > + * which simply matches any L2 pkts.\n> > > > + */\n> > > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\n> > > > + }\n> > > \n> > > This feels a lot like a layer violation - making the core aware\n> > > of specific implementation details of lower layers.\n> > \n> > I agree with you, but unlickily, without it, Intel NIC simply won't work\n> > (according to Finn), unless you have mac addr being provided.\n> \n> I think its reasonable to provide hardware-workarounds. But is it\n> appropriate to enable it for all hardware\n\nWithout the underlaying hardware info/cap given, yes, I'm afraid that\nis what we can get best now.\n\n> and is this the most appropriate\n> place for a hardware work-around?\n\nMaybe not. Maybe this should be workarounded inside the DPDK PMD driver.\n\n\t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=fridaylinux-org.20150623.gappssmtp.com\n\theader.i=@fridaylinux-org.20150623.gappssmtp.com\n\theader.b=\"BxitsUuQ\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xt2gw4HHBz9t3V\n\tfor ;\n\tThu, 14 Sep 2017 12:35:24 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id D7368949;\n\tThu, 14 Sep 2017 02:35:20 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id E06488D9\n\tfor ; Thu, 14 Sep 2017 02:35:19 +0000 (UTC)","from mail-pg0-f53.google.com (mail-pg0-f53.google.com\n\t[74.125.83.53])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 73D5F113\n\tfor ; Thu, 14 Sep 2017 02:35:19 +0000 (UTC)","by mail-pg0-f53.google.com with SMTP id j16so3875952pga.1\n\tfor ; Wed, 13 Sep 2017 19:35:19 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\tx189sm25664744pfx.188.2017.09.13.19.35.15\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tWed, 13 Sep 2017 19:35:17 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=fridaylinux-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=0sFfQ/8SabZvnW8UPfoS1lHIt0ocEiAtbPVVXqSTUt4=;\n\tb=BxitsUuQZVEQXFcjp676pJu42SYLyapnZizXTl5O6ZkGUJ6xYq0xa0A+MNwnIsisGz\n\tQ67UaudYL+YQL1laxIOQozz545E8peZkBxEi4f17lHjVCcrz/O1DvKXjYrp4XHNLxNhs\n\tJar1qHvuyWL7ZkX7exEx88XD3tx2CpNpWnpUPepegn8aUnutL7qYfLpi6awL5aAVpluz\n\tP1PIdYwFv1HAbi7HpHA2lcQ59FzhPYUsUrtFRZ941qnHaEZZb49AaEGX5HwxcbNObtmL\n\tWsbwvX/0vFD1JymABhX2Ogeiks2gwP+LoMJESjo+jKGITIWUZxwRqtNwkEGhEqC7ATdr\n\tk1KA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=0sFfQ/8SabZvnW8UPfoS1lHIt0ocEiAtbPVVXqSTUt4=;\n\tb=GOfTpgwN/+4xvmCEsQ60bkRhVpc1RYtc0PW+RcIigTuhWO0Nv7JLgglLw/8oJMqeEv\n\tngR7zDxnunG9j4qRyWeBdUmPA3MSMGJ7Bu/5jHaQVGlbDrXl4Xc0Ul2Fbs/z57eAEbB4\n\t22I+bpi/7I3+VzB9mvegNYfwCaLOkN7tcXwEMy/z/U7WwA5PKZhrHC4AzA3YaW7oI/9q\n\tn5WLxF5PJ6vnN00XxptvvaIZeGfzFqityq1z3+kqKAn+xnYnU/BX6WA+/IB/hzsQ9SC4\n\tiCtSIyVYeUvfCpAq/5SrgpQdbBfhl0/SnSPUfd/6TEt1+nwy+zMvjacmb+19z6WWpzwX\n\thAwA==","X-Gm-Message-State":"AHPjjUiA8SkN+SsN8j6Kj8VFVWfLElYTEJM8hthZuIy80LEbWMZ17h1K\n\tuGR2/jZfViE503geZX6HMQ==","X-Google-Smtp-Source":"ADKCNb6loL+E6GU4RiWjjphz1+06in8zIavwrKqxuga90DI2bm4u9kd5us02kgX69PLSSBdtEFr8Kg==","X-Received":"by 10.98.89.151 with SMTP id k23mr20497942pfj.69.1505356519083; \n\tWed, 13 Sep 2017 19:35:19 -0700 (PDT)","Date":"Thu, 14 Sep 2017 10:35:10 +0800","From":"Yuanhan Liu ","To":"Simon Horman ","Message-ID":"<20170914023510.GC2050@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<20170908164754.GD7356@vergenet.net>\n\t<20170911061102.GM9736@yliu-home>\n\t\n\t<20170913094529.GC27555@vergenet.net>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170913094529.GC27555@vergenet.net>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1768309,"web_url":"http://patchwork.ozlabs.org/comment/1768309/","msgid":"<20170914023737.GD2050@yliu-home>","list_archive_url":null,"date":"2017-09-14T02:37:37","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Wed, Sep 13, 2017 at 05:18:03AM +0000, Darrell Ball wrote:\n> + /* IP v4 */\n> + uint8_t proto = 0;\n> + struct rte_flow_item_ipv4 ipv4_spec;\n> + struct rte_flow_item_ipv4 ipv4_mask;\n> + memset(&ipv4_mask, 0, sizeof(ipv4_mask));\n> + if ((match->flow.dl_type == ntohs(ETH_TYPE_IP)) &&\n> + (match->wc.masks.nw_src || match->wc.masks.nw_dst ||\n> + match->wc.masks.nw_tos || match->wc.masks.nw_ttl ||\n> + match->wc.masks.nw_proto)) {\n> + ipv4_spec.hdr.type_of_service = match->flow.nw_tos;\n> + ipv4_spec.hdr.time_to_live = match->flow.nw_tos;\n> \n> [Darrell] should be TTL; lots of e-mails but I don’t remember seeing this mentioned\n\nThat's really a good catch! And actually, I have also noticed it few days,\nthat I have fixed in my local v3 branch.\n\n> +\n> + CHECK_NONZERO_BYTES(&match_zero_wc.flow.tunnel,\n> + sizeof(match_zero_wc.flow.tunnel));\n> + CHECK_NONZERO(match->wc.masks.metadata);\n> + CHECK_NONZERO(match->wc.masks.skb_priority);\n> + CHECK_NONZERO(match->wc.masks.pkt_mark);\n> + CHECK_NONZERO(match->wc.masks.dp_hash);\n> +\n> + /* recirc id must be zero */\n> + CHECK_NONZERO(match_zero_wc.flow.recirc_id);\n> +\n> + CHECK_NONZERO(match->wc.masks.ct_state);\n> + CHECK_NONZERO(match->wc.masks.ct_zone);\n> + CHECK_NONZERO(match->wc.masks.ct_mark);\n> \n> [Darrell]\n> check the other pkt_metadata fields; some are missing here.\n\nI will give it a try.\n\n\t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=fridaylinux-org.20150623.gappssmtp.com\n\theader.i=@fridaylinux-org.20150623.gappssmtp.com\n\theader.b=\"PLglCcY3\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xt2kn4r0dz9t3V\n\tfor ;\n\tThu, 14 Sep 2017 12:37:53 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 58F1D95D;\n\tThu, 14 Sep 2017 02:37:49 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 5842C941\n\tfor ; Thu, 14 Sep 2017 02:37:48 +0000 (UTC)","from mail-pg0-f44.google.com (mail-pg0-f44.google.com\n\t[74.125.83.44])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id F2C15113\n\tfor ; Thu, 14 Sep 2017 02:37:47 +0000 (UTC)","by mail-pg0-f44.google.com with SMTP id i130so3886717pgc.3\n\tfor ; Wed, 13 Sep 2017 19:37:47 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\tp71sm30764590pfl.56.2017.09.13.19.37.44\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tWed, 13 Sep 2017 19:37:46 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=fridaylinux-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=U1sQIX7px0Epo7NgkZA4SPZtywP7XK/JwHjzQoLMEyg=;\n\tb=PLglCcY3CEaRwOVSt2wck5dIIQmCNqcgf18OTKpXApvgW5GCTepZauwqo4ZSpp52q4\n\t6GEWUZicd4tJ+S2hymcPi6aZM6iqu2ZKeL3H1gyogvTa9+mj5qBorLUh+5ku5Qi5zh+E\n\tE6x8BqWfLJXu+C34OwijDUZ901Nk2qdT0vRwTEIduqxe/V/o81bU37keLz+BeipKeS19\n\t6ZMikCtKywZS9xMU6l6bok6z4mF0DaXuXvGjcd/wmXmGxRvVb2DFN4cX275OEVz4Y4Lu\n\tFkCub61aHLPOmJsPHFWIenvvKT5b7PNMVq0q8T/1XZNi4CKdZgK4ejO4RWGDI7oWP3Nd\n\twC0g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=U1sQIX7px0Epo7NgkZA4SPZtywP7XK/JwHjzQoLMEyg=;\n\tb=Ga2NVCplvmCvDosDjuiH/HuxNnfQQtPxS5Mj6auTQhPdffr0o1dlFY03WyHcCK1GEN\n\tn/LaWrs2P7MIAfgUQd4ARiBkDfbtLWJfj1LA7jHLR2XR49zaPS4WHVHzZLXngG7u+fxi\n\tpQbBCs48qPAuol6p4ticYwwXOSeNr0eU18rhTdXNBFl7VwpjLWSl0eMD8alh8cF4IiOi\n\tI8faYooGvBNllaGqz9wfxYzYFJvp/6Egm+dWPYMJpN1BxSLzGE7MttDOOrF429CLic+M\n\tsjV9Y1F+XmQkI5OgUcSy3PNq/xUDthVonNRfaAgXaiYBpgA0dl0e2yQ1r5DA4PP88pG7\n\t8iNQ==","X-Gm-Message-State":"AHPjjUj4WIUU8ymperna9mWPHdAIrf8+tFN1JX3b6+ZXQntW3rnMQtm4\n\tBDQjc6/wA2BPvEje2vjPwg==","X-Google-Smtp-Source":"ADKCNb4NF6cbL68tV80bUbySaixHdhpaRJz6lerlsrqcjZb4j4pp2P9hGX0zSkT1XIOyerzVs2Z+VA==","X-Received":"by 10.84.232.72 with SMTP id f8mr22691149pln.269.1505356667603; \n\tWed, 13 Sep 2017 19:37:47 -0700 (PDT)","Date":"Thu, 14 Sep 2017 10:37:37 +0800","From":"Yuanhan Liu ","To":"Darrell Ball ","Message-ID":"<20170914023737.GD2050@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1768413,"web_url":"http://patchwork.ozlabs.org/comment/1768413/","msgid":"<20170914081951.GB19617@vergenet.net>","list_archive_url":null,"date":"2017-09-14T08:19:52","subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Thu, Sep 14, 2017 at 10:35:10AM +0800, Yuanhan Liu wrote:\n> On Wed, Sep 13, 2017 at 11:45:30AM +0200, Simon Horman wrote:\n> > > > > + rte_memcpy(ð_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));\n> > > > > + rte_memcpy(ð_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));\n> > > > > + eth_spec.type = match->flow.dl_type;\n> > > > > +\n> > > > > + rte_memcpy(ð_mask.dst, &match->wc.masks.dl_dst,\n> > > > > + sizeof(eth_mask.dst));\n> > > > > + rte_memcpy(ð_mask.src, &match->wc.masks.dl_src,\n> > > > > + sizeof(eth_mask.src));\n> > > > > + eth_mask.type = match->wc.masks.dl_type;\n> > > > > +\n> > > > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,\n> > > > > + ð_spec, ð_mask);\n> > > > > + } else {\n> > > > > + /*\n> > > > > + * If user specifies a flow (like UDP flow) without L2 patterns,\n> > > > > + * OVS will at least set the dl_type. Normally, it's enough to\n> > > > > + * create an eth pattern just with it. Unluckily, some Intel's\n> > > > > + * NIC (such as XL710) doesn't support that. Below is a workaround,\n> > > > > + * which simply matches any L2 pkts.\n> > > > > + */\n> > > > > + add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);\n> > > > > + }\n> > > > \n> > > > This feels a lot like a layer violation - making the core aware\n> > > > of specific implementation details of lower layers.\n> > > \n> > > I agree with you, but unlickily, without it, Intel NIC simply won't work\n> > > (according to Finn), unless you have mac addr being provided.\n> > \n> > I think its reasonable to provide hardware-workarounds. But is it\n> > appropriate to enable it for all hardware\n> \n> Without the underlaying hardware info/cap given, yes, I'm afraid that\n> is what we can get best now.\n\nAgreed.\n\n> > and is this the most appropriate\n> > place for a hardware work-around?\n> \n> Maybe not. Maybe this should be workarounded inside the DPDK PMD driver.\n\nPushing it lower down sounds better to me.\nBut I'm not really in a position to judge how practical\nit is to handle this in the DPDK PMD driver.","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"eOHw48qN\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtBKY35tLz9s7v\n\tfor ;\n\tThu, 14 Sep 2017 18:20:01 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id BF2049F0;\n\tThu, 14 Sep 2017 08:19:58 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id F27808DC\n\tfor ; Thu, 14 Sep 2017 08:19:56 +0000 (UTC)","from mail-wm0-f46.google.com (mail-wm0-f46.google.com\n\t[74.125.82.46])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 33A3DA7\n\tfor ; Thu, 14 Sep 2017 08:19:56 +0000 (UTC)","by mail-wm0-f46.google.com with SMTP id g206so7483337wme.0\n\tfor ; Thu, 14 Sep 2017 01:19:56 -0700 (PDT)","from vergenet.net ([217.111.208.18])\n\tby smtp.gmail.com with ESMTPSA id v2sm399081wmf.8.2017.09.14.01.19.53\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 14 Sep 2017 01:19:54 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=SamY7JjDDknFfaRvSZL/yGzaeIRkOI/Y6oDVC/G+xlA=;\n\tb=eOHw48qNMWwA5Zh4BeLS/esw3E++pj5+BSvJG8o0saaSDYgeVjQwM8i97scBOMVrbo\n\tBExXBKwVNXZsm/te6U6L2xuFDMd5b94bUPVL/Dij45sgH5R0RwfraCV3l+wLafA6KW4s\n\tKvIKP9sRn/eLug2OHGv85dbjYz1a/BHRa8RX6BRgQL+gcYm+tqKN12TmEAHMEtOJP/wg\n\tBNy0gEOfqaL9AB75eSPlIPACrdAczVtIh7doBTqe6XpmfFbq+BHhgsG2ax3IxMp1Hu4A\n\tlvcnu6a2mSDdoV14l8cKlYF38wWT1oVstYMTPiT9CL2Ig/9oNL96P4EZ03qhKGEf5SrX\n\tBvDQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=SamY7JjDDknFfaRvSZL/yGzaeIRkOI/Y6oDVC/G+xlA=;\n\tb=o61GvmH+5b0GahZZOVWJ7PUTblLHj5hwAIFvCRshNatiOW8HJvcriVREQTCY6d04Ai\n\t2fAnhqhU0zy/hHclKEpiD11oLBqr6dPBVh+yTNY8y8HR6dA180BGOqSLGNvLCUZ8/UJo\n\ta95U5zAv9eGeUd6wn48+jsao5GFRPZDXEZe34iBaqV0HwZzlnBRpbL+CKCo3qrFmS1pY\n\tfXDhmiWnioROXUfNcaioSa8Fp9hq1bM8dMqF3x20cJJKghXzXaZEEuSmNHFkw+RgAnMI\n\tJcghyqGbBpHryohzUQ/oZOOx+1+zprMJLk7xoAxt5dhZz7MfWj1VZ+84HMQNNWASpKHq\n\tQS/g==","X-Gm-Message-State":"AHPjjUgnO0mYa1bJbsSHZCDhjmRUC8LONDVJH8bBy7rYx63c4vBkMbNe\n\tUJ3mnhX0Nntc+2ST","X-Google-Smtp-Source":"AOwi7QD8jJbNB9nk6jCpX8juBqeZ8ciDLO5qF29heHe61k2V/oePH6VQUV6dDDWtxJu8tuj7cwzLzw==","X-Received":"by 10.28.230.3 with SMTP id d3mr1047951wmh.54.1505377194835;\n\tThu, 14 Sep 2017 01:19:54 -0700 (PDT)","Date":"Thu, 14 Sep 2017 10:19:52 +0200","From":"Simon Horman ","To":"Yuanhan Liu ","Message-ID":"<20170914081951.GB19617@vergenet.net>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-5-git-send-email-yliu@fridaylinux.org>\n\t<20170908164754.GD7356@vergenet.net>\n\t<20170911061102.GM9736@yliu-home>\n\t\n\t<20170913094529.GC27555@vergenet.net>\n\t<20170914023510.GC2050@yliu-home>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170914023510.GC2050@yliu-home>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-Spam-Status":"No, score=0.5 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with\n\trte flow","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}}]