Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/1.2/patches/808034/?format=api
{ "id": 808034, "url": "http://patchwork.ozlabs.org/api/1.2/patches/808034/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/patch/20170831054642.13721-1-dja@axtens.net/", "project": { "id": 7, "url": "http://patchwork.ozlabs.org/api/1.2/projects/7/?format=api", "name": "Linux network development", "link_name": "netdev", "list_id": "netdev.vger.kernel.org", "list_email": "netdev@vger.kernel.org", "web_url": null, "scm_url": null, "webscm_url": null, "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20170831054642.13721-1-dja@axtens.net>", "list_archive_url": null, "date": "2017-08-31T05:46:42", "name": "bnx2x: drop packets where gso_size is too big for hardware", "commit_ref": null, "pull_url": null, "state": "changes-requested", "archived": true, "hash": "60fa5135716150a01f8837da704f0285e244fb15", "submitter": { "id": 65792, "url": "http://patchwork.ozlabs.org/api/1.2/people/65792/?format=api", "name": "Daniel Axtens", "email": "dja@axtens.net" }, "delegate": { "id": 34, "url": "http://patchwork.ozlabs.org/api/1.2/users/34/?format=api", "username": "davem", "first_name": "David", "last_name": "Miller", "email": "davem@davemloft.net" }, "mbox": "http://patchwork.ozlabs.org/project/netdev/patch/20170831054642.13721-1-dja@axtens.net/mbox/", "series": [ { "id": 750, "url": "http://patchwork.ozlabs.org/api/1.2/series/750/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/list/?series=750", "date": "2017-08-31T05:46:42", "name": "bnx2x: drop packets where gso_size is too big for hardware", "version": 1, "mbox": "http://patchwork.ozlabs.org/series/750/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/808034/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/808034/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<netdev-owner@vger.kernel.org>", "X-Original-To": "patchwork-incoming@ozlabs.org", "Delivered-To": "patchwork-incoming@ozlabs.org", "Authentication-Results": [ "ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)", "ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=axtens.net header.i=@axtens.net\n\theader.b=\"oLL4cpFm\"; dkim-atps=neutral" ], "Received": [ "from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjWbQ2XMBz9s83\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 31 Aug 2017 15:46:58 +1000 (AEST)", "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750897AbdHaFq4 (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 31 Aug 2017 01:46:56 -0400", "from mail-pf0-f196.google.com ([209.85.192.196]:32867 \"EHLO\n\tmail-pf0-f196.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750781AbdHaFqy (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 31 Aug 2017 01:46:54 -0400", "by mail-pf0-f196.google.com with SMTP id r62so2892312pfj.0\n\tfor <netdev@vger.kernel.org>; Wed, 30 Aug 2017 22:46:54 -0700 (PDT)", "from localhost.localdomain (124-171-202-56.dyn.iinet.net.au.\n\t[124.171.202.56]) by smtp.gmail.com with ESMTPSA id\n\t7sm12588870pfn.185.2017.08.30.22.46.49\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 30 Aug 2017 22:46:53 -0700 (PDT)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google;\n\th=from:to:cc:subject:date:message-id;\n\tbh=4xtPE8PLEynXKhQmhE6cH74Zkx7UckYUZKkLw3+1FCc=;\n\tb=oLL4cpFmlKshBtY9aZD2dK1LFjhuj4WvJGibzuUujuaaM+YFLti5SW8/dN/+BaamxH\n\tfvzgRR6yFUEMNkCcTujBq29O8GfLPQqDGmU9ighV7SpMuuvc0TiWyYrH1YRySLlGEF5O\n\tXIiVAOE+6SFhU9B0acSE0+EOkxcnBnPwzvU94=", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id;\n\tbh=4xtPE8PLEynXKhQmhE6cH74Zkx7UckYUZKkLw3+1FCc=;\n\tb=QM4nax3HROBwJbxABBzqo8guz9fljvwL6Zwv888mJR0bhsHg610kQlrnxriRlv4myt\n\tw8elzv3bPcA+cWSIL4rRlbXsM6+RUU44mIWa4wbMY59B+yE+gsSjuACHc6fiD5odTJIC\n\tGFhKnzMmcQ4w6zmNXKN2y+AKaVqyURNf86w51mut30bLtrB8e+kkkYzeNqswRIEx6fYu\n\tc4Yp/yaYuZrtqCREWHL/eZK7dsa0UZW0X+mGjP+Zp7ES8DRYApGjtFtPKCVCS09BpgIv\n\tvTcHOfLjyaKf+B89pUUgJXxxIbQOIgB2+cafwynfUlJz2r6qxAwZfKayxoCyuB2U0LFs\n\tZSvQ==", "X-Gm-Message-State": "AHYfb5hD+tvvdsn57SwV/X3Gu7gp8SrvhgemOWuesRlrZPB7ETkELYFo\n\tKVQV8RG/Q0uw76BauhvhaQ==", "X-Received": "by 10.99.3.3 with SMTP id 3mr1152189pgd.240.1504158413826;\n\tWed, 30 Aug 2017 22:46:53 -0700 (PDT)", "From": "Daniel Axtens <dja@axtens.net>", "To": "netdev@vger.kernel.org", "Cc": "tlfalcon@linux.vnet.ibm.com, Yuval.Mintz@cavium.com,\n\tariel.elior@cavium.com, everest-linux-l2@cavium.com,\n\tjay.vosburgh@canonical.com, Daniel Axtens <dja@axtens.net>", "Subject": "[PATCH] bnx2x: drop packets where gso_size is too big for hardware", "Date": "Thu, 31 Aug 2017 15:46:42 +1000", "Message-Id": "<20170831054642.13721-1-dja@axtens.net>", "X-Mailer": "git-send-email 2.11.0", "Sender": "netdev-owner@vger.kernel.org", "Precedence": "bulk", "List-ID": "<netdev.vger.kernel.org>", "X-Mailing-List": "netdev@vger.kernel.org" }, "content": "If a bnx2x card is passed a GSO packet with a gso_size larger than\n~9700 bytes, it will cause a firmware error that will bring the card\ndown:\n\nbnx2x: [bnx2x_attn_int_deasserted3:4323(enP24p1s0f0)]MC assert!\nbnx2x: [bnx2x_mc_assert:720(enP24p1s0f0)]XSTORM_ASSERT_LIST_INDEX 0x2\nbnx2x: [bnx2x_mc_assert:736(enP24p1s0f0)]XSTORM_ASSERT_INDEX 0x0 = 0x00000000 0x25e43e47 0x00463e01 0x00010052\nbnx2x: [bnx2x_mc_assert:750(enP24p1s0f0)]Chip Revision: everest3, FW Version: 7_13_1\n... (dump of values continues) ...\n\nDetect when gso_size + header length is greater than the maximum\npacket size (9700 bytes) and drop the packet.\n\nThis raises the obvious question - how do we end up with a packet with\na gso_size that's greater than 9700? This has been observed on an\npowerpc system when Open vSwitch is forwarding a packet from an\nibmveth device.\n\nibmveth is a bit special. It's the driver for communication between\nvirtual machines (aka 'partitions'/LPARs) running under IBM's\nproprietary hypervisor on ppc machines. It allows sending very large\npackets (up to 64kB) between LPARs. This involves some quite\n'interesting' things: for example, when talking TCP, the MSS is stored\nthe checksum field (see ibmveth_rx_mss_helper() in ibmveth.c).\n\nNormally on a box like this, there would be a Virtual I/O Server\n(VIOS) partition that owns the physical network card. VIOS lets the\nAIX partitions know when they're talking to a real network and that\nthey should drop their MSS. This works fine if VIOS owns the physical\nnetwork card.\n\nHowever, in this case, a Linux partition owns the card (this is known\nas a NovaLink setup). The negotiation between VIOS and AIX uses a\nnon-standard TCP option, so Linux has never supported that. Instead,\nLinux just supports receiving large packets. It doesn't support any\nform of messaging/MSS negotiation back to other LPARs.\n\nTo get some clarity about where the large MSS was coming from, I asked\nThomas Falcon, the maintainer of ibmveth, for some background:\n\n\"In most cases, large segments are an aggregation of smaller packets\nby the Virtual I/O Server (VIOS) partition and then are forwarded to\nthe Linux LPAR / ibmveth driver. These segments can be as large as\n64KB. In this case, since the customer is using Novalink, I believe\nwhat is happening is pretty straightforward: the large segments are\ncreated by the AIX partition and then forwarded to the Linux\npartition, ... The ibmveth driver doesn't do any aggregation itself\nbut just ensures the proper bits are set before sending the frame up\nto avoid giving the upper layers indigestion.\"\n\nIt is possible to stop AIX from sending these large segments, but it\nrequires configuration on each LPAR. While ibmveth's behaviour is\nadmittedly weird, we should fix this here: it shouldn't be possible\nfor it to cause a firmware panic on another card.\n\nCc: Thomas Falcon <tlfalcon@linux.vnet.ibm.com> # ibmveth\nCc: Yuval Mintz <Yuval.Mintz@cavium.com> # bnx2x\nThanks-to: Jay Vosburgh <jay.vosburgh@canonical.com> # veth info\nSigned-off-by: Daniel Axtens <dja@axtens.net>\n---\n drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 2 ++\n drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 33 +++++++++++++++---------\n drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c | 1 -\n 3 files changed, 23 insertions(+), 13 deletions(-)", "diff": "diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h\nindex 352beff796ae..b36d54737d70 100644\n--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h\n+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h\n@@ -2517,4 +2517,6 @@ void bnx2x_set_rx_ts(struct bnx2x *bp, struct sk_buff *skb);\n */\n int bnx2x_vlan_reconfigure_vid(struct bnx2x *bp);\n \n+#define MAX_PACKET_SIZE\t(9700)\n+\n #endif /* bnx2x.h */\ndiff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c\nindex 1216c1f1e052..1c5517a9348c 100644\n--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c\n+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c\n@@ -3742,6 +3742,7 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)\n \t__le16 pkt_size = 0;\n \tstruct ethhdr *eth;\n \tu8 mac_type = UNICAST_ADDRESS;\n+\tunsigned int pkts_compl = 0, bytes_compl = 0;\n \n #ifdef BNX2X_STOP_ON_ERROR\n \tif (unlikely(bp->panic))\n@@ -4029,6 +4030,14 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)\n \t\t skb->len, hlen, skb_headlen(skb),\n \t\t skb_shinfo(skb)->gso_size);\n \n+\t\tif (unlikely(skb_shinfo(skb)->gso_size + hlen > MAX_PACKET_SIZE)) {\n+\t\t\tBNX2X_ERR(\"reported gso segment size plus headers \"\n+\t\t\t\t \"(%d + %d) > MAX_PACKET_SIZE; dropping pkt!\",\n+\t\t\t\t skb_shinfo(skb)->gso_size, hlen);\n+\n+\t\t\tgoto free_and_drop;\n+\t\t}\n+\n \t\ttx_start_bd->bd_flags.as_bitfield |= ETH_TX_BD_FLAGS_SW_LSO;\n \n \t\tif (unlikely(skb_headlen(skb) > hlen)) {\n@@ -4061,21 +4070,10 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)\n \t\tmapping = skb_frag_dma_map(&bp->pdev->dev, frag, 0,\n \t\t\t\t\t skb_frag_size(frag), DMA_TO_DEVICE);\n \t\tif (unlikely(dma_mapping_error(&bp->pdev->dev, mapping))) {\n-\t\t\tunsigned int pkts_compl = 0, bytes_compl = 0;\n-\n \t\t\tDP(NETIF_MSG_TX_QUEUED,\n \t\t\t \"Unable to map page - dropping packet...\\n\");\n \n-\t\t\t/* we need unmap all buffers already mapped\n-\t\t\t * for this SKB;\n-\t\t\t * first_bd->nbd need to be properly updated\n-\t\t\t * before call to bnx2x_free_tx_pkt\n-\t\t\t */\n-\t\t\tfirst_bd->nbd = cpu_to_le16(nbd);\n-\t\t\tbnx2x_free_tx_pkt(bp, txdata,\n-\t\t\t\t\t TX_BD(txdata->tx_pkt_prod),\n-\t\t\t\t\t &pkts_compl, &bytes_compl);\n-\t\t\treturn NETDEV_TX_OK;\n+\t\t\tgoto free_and_drop;\n \t\t}\n \n \t\tbd_prod = TX_BD(NEXT_TX_IDX(bd_prod));\n@@ -4176,6 +4174,17 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)\n \ttxdata->tx_pkt++;\n \n \treturn NETDEV_TX_OK;\n+\n+free_and_drop:\n+\t/*\n+\t * we need unmap all buffers already mapped for this SKB;\n+\t * first_bd->nbd need to be properly updated before call to\n+\t * bnx2x_free_tx_pkt\n+\t */\n+\tfirst_bd->nbd = cpu_to_le16(nbd);\n+\tbnx2x_free_tx_pkt(bp, txdata, TX_BD(txdata->tx_pkt_prod),\n+\t\t\t &pkts_compl, &bytes_compl);\n+\treturn NETDEV_TX_OK;\n }\n \n void bnx2x_get_c2s_mapping(struct bnx2x *bp, u8 *c2s_map, u8 *c2s_default)\ndiff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c\nindex 7dd83d0ef0a0..59a3a9419cde 100644\n--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c\n+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c\n@@ -189,7 +189,6 @@ typedef int (*read_sfp_module_eeprom_func_p)(struct bnx2x_phy *phy,\n #define ETS_E3B0_NIG_MIN_W_VAL_20GBPS\t\t\t(2720)\n #define ETS_E3B0_PBF_MIN_W_VAL\t\t\t\t(10000)\n \n-#define MAX_PACKET_SIZE\t\t\t\t\t(9700)\n #define MAX_KR_LINK_RETRY\t\t\t\t4\n #define DEFAULT_TX_DRV_BRDCT\t\t2\n #define DEFAULT_TX_DRV_IFIR\t\t0\n", "prefixes": [] }