* [PATCH] net: r6040: add in missing white space in error message text
From: Colin King @ 2016-09-12 12:08 UTC (permalink / raw)
To: Florian Fainelli, netdev, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
A couple of dev_err messages span two lines and the literal
string is missing a white space between words. Add the white
space.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/rdc/r6040.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index cb29ee2..5dc655b 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -1062,13 +1062,13 @@ static int r6040_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
/* this should always be supported */
err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
if (err) {
- dev_err(&pdev->dev, "32-bit PCI DMA addresses"
+ dev_err(&pdev->dev, "32-bit PCI DMA addresses "
"not supported by the card\n");
goto err_out_disable_dev;
}
err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
if (err) {
- dev_err(&pdev->dev, "32-bit PCI DMA addresses"
+ dev_err(&pdev->dev, "32-bit PCI DMA addresses "
"not supported by the card\n");
goto err_out_disable_dev;
}
--
2.9.3
^ permalink raw reply related
* Re: [net-next PATCH v2 2/2] e1000: bundle xdp xmit routines
From: Jesper Dangaard Brouer @ 2016-09-12 12:17 UTC (permalink / raw)
To: John Fastabend
Cc: bblanco, alexei.starovoitov, jeffrey.t.kirsher, davem,
xiyou.wangcong, intel-wired-lan, u9012063, netdev, brouer
In-Reply-To: <20160909212938.4001.40540.stgit@john-Precision-Tower-5810>
On Fri, 09 Sep 2016 14:29:38 -0700
John Fastabend <john.fastabend@gmail.com> wrote:
> e1000 supports a single TX queue so it is being shared with the stack
> when XDP runs XDP_TX action. This requires taking the xmit lock to
> ensure we don't corrupt the tx ring. To avoid taking and dropping the
> lock per packet this patch adds a bundling implementation to submit
> a bundle of packets to the xmit routine.
>
> I tested this patch running e1000 in a VM using KVM over a tap
> device using pktgen to generate traffic along with 'ping -f -l 100'.
>
> Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Thank you for actually implementing this! :-)
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
[...]
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
> index 91d5c87..b985271 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
>
[...]
> @@ -3369,33 +3381,52 @@ static void e1000_tx_map_rxpage(struct e1000_tx_ring *tx_ring,
> }
>
> static void e1000_xmit_raw_frame(struct e1000_rx_buffer *rx_buffer_info,
> - unsigned int len,
> - struct net_device *netdev,
> - struct e1000_adapter *adapter)
> + __u32 len,
> + struct e1000_adapter *adapter,
> + struct e1000_tx_ring *tx_ring)
> {
> - struct netdev_queue *txq = netdev_get_tx_queue(netdev, 0);
> - struct e1000_hw *hw = &adapter->hw;
> - struct e1000_tx_ring *tx_ring;
> -
> if (len > E1000_MAX_DATA_PER_TXD)
> return;
>
> + if (E1000_DESC_UNUSED(tx_ring) < 2)
> + return;
> +
> + e1000_tx_map_rxpage(tx_ring, rx_buffer_info, len);
> + e1000_tx_queue(adapter, tx_ring, 0/*tx_flags*/, 1);
> +}
> +
> +static void e1000_xdp_xmit_bundle(struct e1000_rx_buffer_bundle *buffer_info,
> + struct net_device *netdev,
> + struct e1000_adapter *adapter)
> +{
> + struct netdev_queue *txq = netdev_get_tx_queue(netdev, 0);
> + struct e1000_tx_ring *tx_ring = adapter->tx_ring;
> + struct e1000_hw *hw = &adapter->hw;
> + int i = 0;
> +
> /* e1000 only support a single txq at the moment so the queue is being
> * shared with stack. To support this requires locking to ensure the
> * stack and XDP are not running at the same time. Devices with
> * multiple queues should allocate a separate queue space.
> + *
> + * To amortize the locking cost e1000 bundles the xmits and sends as
> + * many as possible until either running out of descriptors or failing.
> */
> HARD_TX_LOCK(netdev, txq, smp_processor_id());
>
> - tx_ring = adapter->tx_ring;
> -
> - if (E1000_DESC_UNUSED(tx_ring) < 2) {
> - HARD_TX_UNLOCK(netdev, txq);
> - return;
> + for (; i < E1000_XDP_XMIT_BUNDLE_MAX && buffer_info[i].buffer; i++) {
^^^
> + e1000_xmit_raw_frame(buffer_info[i].buffer,
> + buffer_info[i].length,
> + adapter, tx_ring);
> + buffer_info[i].buffer->rxbuf.page = NULL;
> + buffer_info[i].buffer = NULL;
> + buffer_info[i].length = 0;
> + i++;
^^^
Looks like "i" is incremented twice, is that correct?
> }
>
> - e1000_tx_map_rxpage(tx_ring, rx_buffer_info, len);
> - e1000_tx_queue(adapter, tx_ring, 0/*tx_flags*/, 1);
> + /* kick hardware to send bundle and return control back to the stack */
> + writel(tx_ring->next_to_use, hw->hw_addr + tx_ring->tdt);
> + mmiowb();
>
> writel(tx_ring->next_to_use, hw->hw_addr + tx_ring->tdt);
> mmiowb();
> @@ -4281,9 +4312,10 @@ static bool e1000_clean_jumbo_rx_irq(struct e1000_adapter *adapter,
> struct bpf_prog *prog;
> u32 length;
> unsigned int i;
> - int cleaned_count = 0;
> + int cleaned_count = 0, xdp_xmit = 0;
> bool cleaned = false;
> unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> + struct e1000_rx_buffer_bundle *xdp_bundle = rx_ring->xdp_buffer;
>
> rcu_read_lock(); /* rcu lock needed here to protect xdp programs */
> prog = READ_ONCE(adapter->prog);
> @@ -4338,13 +4370,13 @@ static bool e1000_clean_jumbo_rx_irq(struct e1000_adapter *adapter,
> case XDP_PASS:
> break;
> case XDP_TX:
> + xdp_bundle[xdp_xmit].buffer = buffer_info;
> + xdp_bundle[xdp_xmit].length = length;
> dma_sync_single_for_device(&pdev->dev,
> dma,
> length,
> DMA_TO_DEVICE);
> - e1000_xmit_raw_frame(buffer_info, length,
> - netdev, adapter);
> - buffer_info->rxbuf.page = NULL;
> + xdp_xmit++;
> case XDP_DROP:
> default:
> /* re-use mapped page. keep buffer_info->dma
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net-next] net/sched: act_tunnel_key: Remove rcu_read_lock protection
From: Hadar Hen Zion @ 2016-09-12 12:19 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jiri Pirko, Jiri Benc, Jamal Hadi Salim, Shmulik Ladkani,
Tom Herbert, Eric Dumazet, Cong Wang, Amir Vadai, Or Gerlitz,
Hadar Hen Zion
Remove rcu_read_lock protection from tunnel_key_dump and use
rtnl_dereference, dump operation is protected by rtnl lock.
Also, remove rcu_read_lock from tunnel_key_release and use
rcu_dereference_protected.
Both operations are running exclusively and a writer couldn't modify
t->params while those functions are executed.
Fixes: 54d94fd89d90 ('net/sched: Introduce act_tunnel_key')
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
---
net/sched/act_tunnel_key.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index dceff74..af47bdf 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -194,15 +194,12 @@ static void tunnel_key_release(struct tc_action *a, int bind)
struct tcf_tunnel_key *t = to_tunnel_key(a);
struct tcf_tunnel_key_params *params;
- rcu_read_lock();
- params = rcu_dereference(t->params);
+ params = rcu_dereference_protected(t->params, 1);
if (params->tcft_action == TCA_TUNNEL_KEY_ACT_SET)
dst_release(¶ms->tcft_enc_metadata->dst);
kfree_rcu(params, rcu);
-
- rcu_read_unlock();
}
static int tunnel_key_dump_addresses(struct sk_buff *skb,
@@ -245,10 +242,8 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
.bindcnt = t->tcf_bindcnt - bind,
};
struct tcf_t tm;
- int ret = -1;
- rcu_read_lock();
- params = rcu_dereference(t->params);
+ params = rtnl_dereference(t->params);
opt.t_action = params->tcft_action;
opt.action = params->action;
@@ -272,15 +267,11 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
&tm, TCA_TUNNEL_KEY_PAD))
goto nla_put_failure;
- ret = skb->len;
- goto out;
+ return skb->len;
nla_put_failure:
nlmsg_trim(skb, b);
-out:
- rcu_read_unlock();
-
- return ret;
+ return -1;
}
static int tunnel_key_walker(struct net *net, struct sk_buff *skb,
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 25/26] pch_gbe: constify local structures
From: Julia Lawall @ 2016-09-12 12:25 UTC (permalink / raw)
To: Julia Lawall; +Cc: netdev, joe, kernel-janitors, linux-kernel
In-Reply-To: <1473599168-30561-26-git-send-email-Julia.Lawall@lip6.fr>
On Sun, 11 Sep 2016, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
Actually, this patch should be dropped. Coccinelle did not recognize
kernel_ulong_t as a type, so it interpreted
(kernel_ulong_t)&pch_gbe_minnow_privdata
as a bit and operation.
julia
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> The semantic patch seems too long for a commit log, but is in the cover
> letter.
>
> drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> index 3cd87a4..6f33258 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> @@ -2729,7 +2729,7 @@ static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
> return ret;
> }
>
> -static struct pch_gbe_privdata pch_gbe_minnow_privdata = {
> +static const struct pch_gbe_privdata pch_gbe_minnow_privdata = {
> .phy_tx_clk_delay = true,
> .phy_disable_hibernate = true,
> .platform_init = pch_gbe_minnow_platform_init,
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 2/3] net-next: dsa: add Qualcomm tag RX/TX handler
From: Andrew Lunn @ 2016-09-12 12:26 UTC (permalink / raw)
To: John Crispin
Cc: David S. Miller, Florian Fainelli, netdev, linux-kernel,
qsdk-review
In-Reply-To: <1473669337-21221-3-git-send-email-john@phrozen.org>
Hi John
> +
> +static inline int reg_to_port(int reg)
> +{
> + if (reg < 5)
> + return reg + 1;
> +
> + return -1;
> +}
> +
> +static inline int port_to_reg(int port)
> +{
> + if (port >= 1 && port <= 6)
> + return port - 1;
> +
> + return -1;
> +}
No need for inline, leave the compiler to decide.
> +
> +static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct dsa_slave_priv *p = netdev_priv(dev);
> + u16 *phdr, hdr;
> +
> + dev->stats.tx_packets++;
> + dev->stats.tx_bytes += skb->len;
> +
> + if (skb_cow_head(skb, 0) < 0)
> + goto out_free;
> +
> + skb_push(skb, QCA_HDR_LEN);
> +
> + memmove(skb->data, skb->data + QCA_HDR_LEN, 2 * ETH_ALEN);
> + phdr = (u16 *)(skb->data + 2 * ETH_ALEN);
> +
> + /* Set the version field, and set destination port information */
> + hdr = QCA_HDR_VERSION << QCA_HDR_XMIT_VERSION_S |
> + QCA_HDR_XMIT_FROM_CPU |
> + 1 << reg_to_port(p->port);
> +
> + *phdr = htons(hdr);
> +
> + //skb->dev = p->parent->dst->master_netdev;
> + //dev_queue_xmit(skb);
No commented out code please.
> +
> + return skb;
> +
> +out_free:
> + kfree_skb(skb);
> + return NULL;
> +}
> +
> +static int qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
> + struct packet_type *pt, struct net_device *orig_dev)
> +{
> + struct dsa_switch_tree *dst = dev->dsa_ptr;
> + struct dsa_switch *ds;
> + u8 ver;
> + int port, phy;
> + __be16 *phdr, hdr;
> +
> + if (unlikely(!dst))
> + goto out_drop;
> +
> + skb = skb_unshare(skb, GFP_ATOMIC);
> + if (!skb)
> + goto out;
> +
> + if (unlikely(!pskb_may_pull(skb, QCA_HDR_LEN)))
> + goto out_drop;
> +
> + /* Ethernet is added by the switch between src addr and Ethertype
'Ethernet' seems to be the wrong word here.
> +
> + /* Get source port information */
> + port = (hdr & QCA_HDR_RECV_SOURCE_PORT_MASK);
> + phy = port_to_reg(port);
> + if (unlikely(phy < 0) || !ds->ports[phy].netdev)
> + goto out_drop;
Could you use a different variable name than phy. phy has a well known
meaning, and this usage is not it.
Otherwise, this looks good.
Andrew
^ permalink raw reply
* Re: [PATCH 05/26] ARCNET: constify local structures
From: Julia Lawall @ 2016-09-12 12:31 UTC (permalink / raw)
To: Michael Grzeschik; +Cc: joe, kernel-janitors, netdev, linux-kernel
In-Reply-To: <1473599168-30561-6-git-send-email-Julia.Lawall@lip6.fr>
On Sun, 11 Sep 2016, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
Actually, this patch should be dropped. Coccinelle did not recognize
kernel_ulong_t as a type, so it interpreted things like
(kernel_ulong_t)&card_info_10mbit
as a bit and operation.
julia
>
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> The semantic patch seems too long for a commit log, but is in the cover
> letter.
>
> drivers/net/arcnet/com20020-pci.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c
> index 239de38..32b8406 100644
> --- a/drivers/net/arcnet/com20020-pci.c
> +++ b/drivers/net/arcnet/com20020-pci.c
> @@ -264,7 +264,7 @@ static void com20020pci_remove(struct pci_dev *pdev)
> }
> }
>
> -static struct com20020_pci_card_info card_info_10mbit = {
> +static const struct com20020_pci_card_info card_info_10mbit = {
> .name = "ARC-PCI",
> .devcount = 1,
> .chan_map_tbl = {
> @@ -277,7 +277,7 @@ static struct com20020_pci_card_info card_info_10mbit = {
> .flags = ARC_CAN_10MBIT,
> };
>
> -static struct com20020_pci_card_info card_info_5mbit = {
> +static const struct com20020_pci_card_info card_info_5mbit = {
> .name = "ARC-PCI",
> .devcount = 1,
> .chan_map_tbl = {
> @@ -290,7 +290,7 @@ static struct com20020_pci_card_info card_info_5mbit = {
> .flags = ARC_IS_5MBIT,
> };
>
> -static struct com20020_pci_card_info card_info_sohard = {
> +static const struct com20020_pci_card_info card_info_sohard = {
> .name = "PLX-PCI",
> .devcount = 1,
> /* SOHARD needs PCI base addr 4 */
> @@ -304,7 +304,7 @@ static struct com20020_pci_card_info card_info_sohard = {
> .flags = ARC_CAN_10MBIT,
> };
>
> -static struct com20020_pci_card_info card_info_eae_arc1 = {
> +static const struct com20020_pci_card_info card_info_eae_arc1 = {
> .name = "EAE PLX-PCI ARC1",
> .devcount = 1,
> .chan_map_tbl = {
> @@ -329,7 +329,7 @@ static struct com20020_pci_card_info card_info_eae_arc1 = {
> .flags = ARC_CAN_10MBIT,
> };
>
> -static struct com20020_pci_card_info card_info_eae_ma1 = {
> +static const struct com20020_pci_card_info card_info_eae_ma1 = {
> .name = "EAE PLX-PCI MA1",
> .devcount = 2,
> .chan_map_tbl = {
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 11/26] can: constify local structures
From: Julia Lawall @ 2016-09-12 12:33 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: joe, kernel-janitors, Marc Kleine-Budde, linux-can, netdev,
linux-kernel
In-Reply-To: <1473599168-30561-12-git-send-email-Julia.Lawall@lip6.fr>
On Sun, 11 Sep 2016, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
Actually, this patch should be dropped. Coccinelle did not recognize
kernel_ulong_t as a type, so it interpreted eg
(kernel_ulong_t)&plx_pci_card_info_adlink
as a bit and operation.
julia
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> The semantic patch seems too long for a commit log, but is in the cover
> letter.
>
> drivers/net/can/c_can/c_can_pci.c | 4 ++--
> drivers/net/can/sja1000/plx_pci.c | 20 ++++++++++----------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can_pci.c b/drivers/net/can/c_can/c_can_pci.c
> index 7be393c..4bc345d 100644
> --- a/drivers/net/can/c_can/c_can_pci.c
> +++ b/drivers/net/can/c_can/c_can_pci.c
> @@ -251,14 +251,14 @@ static void c_can_pci_remove(struct pci_dev *pdev)
> pci_disable_device(pdev);
> }
>
> -static struct c_can_pci_data c_can_sta2x11= {
> +static const struct c_can_pci_data c_can_sta2x11 = {
> .type = BOSCH_C_CAN,
> .reg_align = C_CAN_REG_ALIGN_32,
> .freq = 52000000, /* 52 Mhz */
> .bar = 0,
> };
>
> -static struct c_can_pci_data c_can_pch = {
> +static const struct c_can_pci_data c_can_pch = {
> .type = BOSCH_C_CAN,
> .reg_align = C_CAN_REG_32,
> .freq = 50000000, /* 50 MHz */
> diff --git a/drivers/net/can/sja1000/plx_pci.c b/drivers/net/can/sja1000/plx_pci.c
> index 3eb7430..59bc378 100644
> --- a/drivers/net/can/sja1000/plx_pci.c
> +++ b/drivers/net/can/sja1000/plx_pci.c
> @@ -170,7 +170,7 @@ struct plx_pci_card_info {
> void (*reset_func)(struct pci_dev *pdev);
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_adlink = {
> +static const struct plx_pci_card_info plx_pci_card_info_adlink = {
> "Adlink PCI-7841/cPCI-7841", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {1, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x80, 0x80} },
> @@ -178,7 +178,7 @@ static struct plx_pci_card_info plx_pci_card_info_adlink = {
> /* based on PLX9052 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_adlink_se = {
> +static const struct plx_pci_card_info plx_pci_card_info_adlink_se = {
> "Adlink PCI-7841/cPCI-7841 SE", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x80, 0x80} },
> @@ -186,7 +186,7 @@ static struct plx_pci_card_info plx_pci_card_info_adlink_se = {
> /* based on PLX9052 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_esd200 = {
> +static const struct plx_pci_card_info plx_pci_card_info_esd200 = {
> "esd CAN-PCI/CPCI/PCI104/200", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
> @@ -194,7 +194,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd200 = {
> /* based on PLX9030/9050 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_esd266 = {
> +static const struct plx_pci_card_info plx_pci_card_info_esd266 = {
> "esd CAN-PCI/PMC/266", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
> @@ -202,7 +202,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd266 = {
> /* based on PLX9056 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_esd2000 = {
> +static const struct plx_pci_card_info plx_pci_card_info_esd2000 = {
> "esd CAN-PCIe/2000", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
> @@ -210,7 +210,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd2000 = {
> /* based on PEX8311 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_ixxat = {
> +static const struct plx_pci_card_info plx_pci_card_info_ixxat = {
> "IXXAT PC-I 04/PCI", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x200, 0x80} },
> @@ -218,7 +218,7 @@ static struct plx_pci_card_info plx_pci_card_info_ixxat = {
> /* based on PLX9050 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_marathon_pci = {
> +static const struct plx_pci_card_info plx_pci_card_info_marathon_pci = {
> "Marathon CAN-bus-PCI", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x00}, {4, 0x00, 0x00} },
> @@ -234,7 +234,7 @@ static struct plx_pci_card_info plx_pci_card_info_marathon_pcie = {
> /* based on PEX8311 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_tews = {
> +static const struct plx_pci_card_info plx_pci_card_info_tews = {
> "TEWS TECHNOLOGIES TPMC810", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x000, 0x80}, {2, 0x100, 0x80} },
> @@ -242,7 +242,7 @@ static struct plx_pci_card_info plx_pci_card_info_tews = {
> /* based on PLX9030 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_cti = {
> +static const struct plx_pci_card_info plx_pci_card_info_cti = {
> "Connect Tech Inc. CANpro/104-Plus Opto (CRG001)", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x000, 0x80}, {2, 0x100, 0x80} },
> @@ -250,7 +250,7 @@ static struct plx_pci_card_info plx_pci_card_info_cti = {
> /* based on PLX9030 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_elcus = {
> +static const struct plx_pci_card_info plx_pci_card_info_elcus = {
> "Eclus CAN-200-PCI", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {1, 0x00, 0x00}, { {2, 0x00, 0x80}, {3, 0x00, 0x80} },
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] net: r6040: add in missing white space in error message text
From: Sergei Shtylyov @ 2016-09-12 13:04 UTC (permalink / raw)
To: Colin King, Florian Fainelli, netdev, linux-kernel
In-Reply-To: <20160912120857.31600-1-colin.king@canonical.com>
Hello.
On 9/12/2016 3:08 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> A couple of dev_err messages span two lines and the literal
> string is missing a white space between words. Add the white
> space.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/rdc/r6040.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
> index cb29ee2..5dc655b 100644
> --- a/drivers/net/ethernet/rdc/r6040.c
> +++ b/drivers/net/ethernet/rdc/r6040.c
> @@ -1062,13 +1062,13 @@ static int r6040_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> /* this should always be supported */
> err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> if (err) {
> - dev_err(&pdev->dev, "32-bit PCI DMA addresses"
> + dev_err(&pdev->dev, "32-bit PCI DMA addresses "
> "not supported by the card\n");
> goto err_out_disable_dev;
> }
> err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
> if (err) {
> - dev_err(&pdev->dev, "32-bit PCI DMA addresses"
> + dev_err(&pdev->dev, "32-bit PCI DMA addresses "
> "not supported by the card\n");
> goto err_out_disable_dev;
> }
The message strings simply shouldn't be broken up, and checkpatch.pl knows
about that.
MBR, Sergei
^ permalink raw reply
* RE: [patch net-next v8 2/3] net: core: Add offload stats to if_stats_msg
From: Nogah Frankel @ 2016-09-12 11:29 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Jiri Pirko, Linux Kernel Network Developers, David S. Miller,
Ido Schimmel, Elad Raz, Yotam Gigi, Or Gerlitz,
roopa@cumulusnetworks.com, linville@tuxdriver.com, tgraf@suug.ch,
gospo@cumulusnetworks.com, sfeldma@gmail.com, sd@queasysnail.net,
Eran Ben Elisha, ast@plumgrid.com, edumazet@google.com,
hannes@stressinduktion.org,
"f.fainelli@gmail.com" <f.f
In-Reply-To: <72691810-B520-4E46-8B45-83C540039173@cumulusnetworks.com>
> -----Original Message-----
> From: Nikolay Aleksandrov [mailto:nikolay@cumulusnetworks.com]
> Sent: Friday, September 09, 2016 4:39 PM
> To: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Cc: Jiri Pirko <jiri@resnulli.us>; Linux Kernel Network Developers <netdev@vger.kernel.org>; David S. Miller
> <davem@davemloft.net>; Nogah Frankel <nogahf@mellanox.com>; Ido Schimmel <idosch@mellanox.com>; Elad Raz
> <eladr@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>; Or Gerlitz <ogerlitz@mellanox.com>; roopa@cumulusnetworks.com;
> linville@tuxdriver.com; tgraf@suug.ch; gospo@cumulusnetworks.com; sfeldma@gmail.com; sd@queasysnail.net; Eran Ben Elisha
> <eranbe@mellanox.com>; ast@plumgrid.com; edumazet@google.com; hannes@stressinduktion.org; f.fainelli@gmail.com;
> dsa@cumulusnetworks.com
> Subject: Re: [patch net-next v8 2/3] net: core: Add offload stats to if_stats_msg
>
>
> > On Sep 9, 2016, at 4:36 PM, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> >
> >>
> >> On Sep 8, 2016, at 9:19 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> From: Nogah Frankel <nogahf@mellanox.com>
> >>
> >> Add a nested attribute of offload stats to if_stats_msg
> >> named IFLA_STATS_LINK_OFFLOAD_XSTATS.
> >> Under it, add SW stats, meaning stats only per packets that went via
> >> slowpath to the cpu, named IFLA_OFFLOAD_XSTATS_CPU_HIT.
> >>
> >> Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> >> ---
> >> include/uapi/linux/if_link.h | 9 ++++
> >> net/core/rtnetlink.c | 97 ++++++++++++++++++++++++++++++++++++++++++--
> >> 2 files changed, 102 insertions(+), 4 deletions(-)
> >>
> > [snip]
> >> @@ -3655,6 +3725,22 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
> >> }
> >> }
> >>
> >> + if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_OFFLOAD_XSTATS,
> >> + *idxattr)) {
> >> + attr = nla_nest_start(skb, IFLA_STATS_LINK_OFFLOAD_XSTATS);
> >> + if (!attr)
> >> + goto nla_put_failure;
> >> +
> >> + err = rtnl_get_offload_stats(skb, dev);
> >> + if (err == -ENODATA)
> >> + nla_nest_cancel(skb, attr);
> >> + else
> >> + nla_nest_end(skb, attr);
> >> +
> >> + if ((err) && (err != -ENODATA))
> >> + goto nla_put_failure;
> >> + }
> >> +
> >
> > Hmm, actually on a second read I think there’s a potential problem here. Since you don’t set *idxattr and if the space
> > isn’t enough the dump will get restarted and this will lead to an infinite loop.
>
> Err, poor choice of words. I meant the loop will be for userspace since the dump will err out at the same spot all the time so it
> won’t be able to ever finish. :-)
>
> > I.e. if the previous attributes filled the skb and there’s not enough room for this one, it will return an error
> > but *idxattr will be == 0 from the previous attribute thus the whole dump will be restarted (this is in case someone
> > requests this attribute with some of the others of course).
> >
> > Cheers,
> > Nik
> >
> >
I'll fix it. Thanks.
> >> nlmsg_end(skb, nlh);
> >>
> >> return 0;
> >> @@ -3712,6 +3798,9 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
> >> }
> >> }
> >>
> >> + if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_OFFLOAD_XSTATS, 0))
> >> + size += rtnl_get_offload_stats_size(dev);
> >> +
> >> return size;
> >> }
> >>
> >> --
> >> 2.5.5
^ permalink raw reply
* Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family
From: Andrew Lunn @ 2016-09-12 13:16 UTC (permalink / raw)
To: John Crispin
Cc: David S. Miller, Florian Fainelli, netdev, linux-kernel,
qsdk-review
In-Reply-To: <1473669337-21221-4-git-send-email-john@phrozen.org>
> +static u32
> +qca8k_mii_read32(struct mii_bus *bus, int phy_id, u32 regnum)
> +{
> + u16 lo, hi;
> +
> + lo = bus->read(bus, phy_id, regnum);
> + hi = bus->read(bus, phy_id, regnum + 1);
> +
> + return (hi << 16) | lo;
> +}
> +
> +static void
> +qca8k_mii_write32(struct mii_bus *bus, int phy_id, u32 regnum, u32 val)
> +{
> + u16 lo, hi;
> +
> + lo = val & 0xffff;
> + hi = (u16)(val >> 16);
> +
> + bus->write(bus, phy_id, regnum, lo);
> + bus->write(bus, phy_id, regnum + 1, hi);
> +}
Hi John
Thanks for picking up this driver and continuing its journey towards
mainline.
bus->read() and bus->write() can return an error code. There is a lot
of error handling in the mv88e6xxx driver looking at the error code
from these two functions. And it very rarely happens. So it seems
overkill to me. However, i have had errors, when bringing up a new
board and the device tree is wrong somehow.
But ignoring potential error altogether does not seem wise. I think at
minimum you should look at the return code in these functions and do a
rate limited dev_err().
> +
> +static void
> +qca8k_set_page(struct mii_bus *bus, u16 page)
> +{
> + if (page == qca8k_current_page)
> + return;
> +
> + bus->write(bus, 0x18, 0, page);
> + udelay(5);
Is that delay in the data sheet? What about other accesses, like
reading the stats which is going to generate a lot of back to back
reads?
> + qca8k_current_page = page;
> +}
> +
> +static u32
> +qca8k_read(struct qca8k_priv *priv, u32 reg)
> +{
> + u16 r1, r2, page;
> + u32 val;
> +
> + qca8k_split_addr(reg, &r1, &r2, &page);
> +
> + mutex_lock(&priv->bus->mdio_lock);
> +
> + qca8k_set_page(priv->bus, page);
> + val = qca8k_mii_read32(priv->bus, 0x10 | r2, r1);
> +
> + mutex_unlock(&priv->bus->mdio_lock);
> +
> + return val;
> +}
> +
> +static void
> +qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
> +{
> + u16 r1, r2, page;
> +
> + qca8k_split_addr(reg, &r1, &r2, &page);
> +
> + mutex_lock(&priv->bus->mdio_lock);
> +
> + qca8k_set_page(priv->bus, page);
> + qca8k_mii_write32(priv->bus, 0x10 | r2, r1, val);
> +
> + mutex_unlock(&priv->bus->mdio_lock);
> +}
> +
> +static u32
> +qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 val)
> +{
> + u16 r1, r2, page;
> + u32 ret;
> +
> + qca8k_split_addr(reg, &r1, &r2, &page);
> +
> + mutex_lock(&priv->bus->mdio_lock);
> +
> + qca8k_set_page(priv->bus, page);
> + ret = qca8k_mii_read32(priv->bus, 0x10 | r2, r1);
> + ret &= ~mask;
> + ret |= val;
> + qca8k_mii_write32(priv->bus, 0x10 | r2, r1, ret);
> +
> + mutex_unlock(&priv->bus->mdio_lock);
> +
> + return ret;
> +}
> +
> +static inline void
> +qca8k_reg_set(struct qca8k_priv *priv, u32 reg, u32 val)
> +{
> + qca8k_rmw(priv, reg, 0, val);
> +}
> +
> +static inline void
> +qca8k_reg_clear(struct qca8k_priv *priv, u32 reg, u32 val)
> +{
> + qca8k_rmw(priv, reg, val, 0);
> +}
Drop the inline please.
> +static u16
> +qca8k_phy_mmd_read(struct qca8k_priv *priv, int phy_addr, u16 addr, u16 reg)
> +{
> + u16 data;
> +
> + mutex_lock(&priv->bus->mdio_lock);
> +
> + priv->bus->write(priv->bus, phy_addr, MII_ATH_MMD_ADDR, addr);
> + priv->bus->write(priv->bus, phy_addr, MII_ATH_MMD_DATA, reg);
> + priv->bus->write(priv->bus, phy_addr, MII_ATH_MMD_ADDR, addr | 0x4000);
> + data = priv->bus->read(priv->bus, phy_addr, MII_ATH_MMD_DATA);
> +
> + mutex_unlock(&priv->bus->mdio_lock);
Have you run lockdep on this? The mv88e6xxx has something similar, and
were getying false positive splats. We needed to use
mdiobus_write_nested(). Since you are using bus->write directly, not
using the wrapper, maybe this is not an issue. But i'm wondering if
ignoring the wrapped is the right thing to do, with nested MDIO
busses. Something i need to think about.
> +static int
> +qca8k_fdb_busy_wait(struct qca8k_priv *priv)
> +{
> + unsigned long timeout;
> +
> + timeout = jiffies + msecs_to_jiffies(20);
> +
> + /* loop until the busy flag has cleared */
> + do {
> + u32 reg = qca8k_read(priv, QCA8K_REG_ATU_FUNC);
> + int busy = reg & QCA8K_ATU_FUNC_BUSY;
> +
> + if (!busy)
> + break;
> + } while (!time_after_eq(jiffies, timeout));
> +
> + return time_after_eq(jiffies, timeout);
> +}
No sleep? You busy loop for 20ms?
> +
> +static void
> +qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb)
> +{
> + u32 reg[4];
> + int i;
> +
> + /* load the ARL table into an array */
> + for (i = 0; i < 4; i++)
> + reg[i] = qca8k_read(priv, QCA8K_REG_ATU_DATA0 + (i * 4));
> +
> + /* vid - 83:72 */
> + fdb->vid = (reg[2] >> QCA8K_ATU_VID_S) & QCA8K_ATU_VID_M;
> + /* aging - 67:64 */
> + fdb->aging = reg[2] & QCA8K_ATU_STATUS_M;
> + /* portmask - 54:48 */
> + fdb->port_mask = (reg[1] >> QCA8K_ATU_PORT_S) & QCA8K_ATU_PORT_M;
> + /* mac - 47:0 */
> + fdb->mac[0] = (reg[1] >> QCA8K_ATU_ADDR0_S) & 0xff;
> + fdb->mac[1] = reg[1] & 0xff;
> + fdb->mac[2] = (reg[0] >> QCA8K_ATU_ADDR2_S) & 0xff;
> + fdb->mac[3] = (reg[0] >> QCA8K_ATU_ADDR3_S) & 0xff;
> + fdb->mac[4] = (reg[0] >> QCA8K_ATU_ADDR4_S) & 0xff;
> + fdb->mac[5] = reg[0] & 0xff;
> +}
> +
> +static void
> +qca8k_fdb_write(struct qca8k_priv *priv, u16 vid, u8 port_mask, const u8 *mac,
> + u8 aging)
> +{
> + u32 reg[3] = { 0 };
> + int i;
> +
> + /* vid - 83:72 */
> + reg[2] = (vid & QCA8K_ATU_VID_M) << QCA8K_ATU_VID_S;
> + /* aging - 67:64 */
> + reg[2] |= aging & QCA8K_ATU_STATUS_M;
> + /* portmask - 54:48 */
> + reg[1] = (port_mask & QCA8K_ATU_PORT_M) << QCA8K_ATU_PORT_S;
> + /* mac - 47:0 */
> + reg[1] |= mac[0] << QCA8K_ATU_ADDR0_S;
> + reg[1] |= mac[1];
> + reg[0] |= mac[2] << QCA8K_ATU_ADDR2_S;
> + reg[0] |= mac[3] << QCA8K_ATU_ADDR3_S;
> + reg[0] |= mac[4] << QCA8K_ATU_ADDR4_S;
> + reg[0] |= mac[5];
> +
> + /* load the array into the ARL table */
> + for (i = 0; i < 3; i++)
> + qca8k_write(priv, QCA8K_REG_ATU_DATA0 + (i * 4), reg[i]);
> +}
> +
> +static int
> +qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port)
> +{
> + u32 reg;
> +
> + /* Set the command and FDB index */
> + reg = QCA8K_ATU_FUNC_BUSY;
> + reg |= cmd;
> + if (port >= 0) {
> + reg |= QCA8K_ATU_FUNC_PORT_EN;
> + reg |= (port && QCA8K_ATU_FUNC_PORT_M) << QCA8K_ATU_FUNC_PORT_S;
> + }
> +
> + /* Write the function register triggering the table access */
> + qca8k_write(priv, QCA8K_REG_ATU_FUNC, reg);
> +
> + /* wait for completion */
> + if (qca8k_fdb_busy_wait(priv))
> + return -1;
> +
> + return 0;
> +}
> +
> +static int
> +qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port)
> +{
> + int ret;
> +
> + qca8k_fdb_write(priv, fdb->vid, fdb->port_mask, fdb->mac, fdb->aging);
> + ret = qca8k_fdb_access(priv, QCA8K_FDB_NEXT, port);
> + if (ret >= 0)
> + qca8k_fdb_read(priv, fdb);
> +
> + return ret;
> +}
So a big picture question. How does locking work?
qca8k_fdb_write() will take and release the MDIO bus
lock. qca8k_fdb_access() will also multiple times take and release the
lock and then qca8k_fdb_read() will take and release the lock a few
times. So it seems like there are multiple opportunities for a race
condition, multiple parallel calls to qca8k_fdb_next() for different
ports. Is this addresses somewhere?
I'm out of time for the moment. More comments later.
Andrew
^ permalink raw reply
* Re: [PATCH 00/26] constify local structures
From: Jarkko Sakkinen @ 2016-09-12 13:16 UTC (permalink / raw)
To: Julia Lawall
Cc: linux-renesas-soc, joe, kernel-janitors, Sergei Shtylyov,
linux-pm, platform-driver-x86, linux-media, linux-can,
Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail, Chien Tin Tung,
linux-rdma, netdev, devel, alsa-devel, linux-kernel, linux-fbdev,
linux-wireless, Jason Gunthorpe, tpmdd-devel, linux-scsi,
linux-spi, l
In-Reply-To: <alpine.DEB.2.10.1609121051050.3049@hadrien>
On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
>
>
> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
>
> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > > Constify local structures.
> > >
> > > The semantic patch that makes this change is as follows:
> > > (http://coccinelle.lip6.fr/)
> >
> > Just my two cents but:
> >
> > 1. You *can* use a static analysis too to find bugs or other issues.
> > 2. However, you should manually do the commits and proper commit
> > messages to subsystems based on your findings. And I generally think
> > that if one contributes code one should also at least smoke test changes
> > somehow.
> >
> > I don't know if I'm alone with my opinion. I just think that one should
> > also do the analysis part and not blindly create and submit patches.
>
> All of the patches are compile tested. And the individual patches are
Compile-testing is not testing. If you are not able to test a commit,
you should explain why.
> submitted to the relevant maintainers. The individual commit messages
> give a more detailed explanation of the strategy used to decide that the
> structure was constifiable. It seemed redundant to put that in the cover
> letter, which will not be committed anyway.
I don't mean to be harsh but I do not care about your thought process
*that much* when I review a commit (sometimes it might make sense to
explain that but it depends on the context).
I mostly only care why a particular change makes sense for this
particular subsystem. The report given by a static analysis tool can
be a starting point for making a commit but it's not sufficient.
Based on the report you should look subsystems as individuals.
> julia
/Jarkko
^ permalink raw reply
* [PATCH net-next 0/5] mlx4 misc fixes and improvements
From: Tariq Toukan @ 2016-09-12 13:20 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan
Hi Dave,
This patchset contains some bug fixes, a cleanup, and small improvements
from the team to the mlx4 Eth and core drivers.
Series generated against net-next commit:
02154927c115 "net: dsa: bcm_sf2: Get VLAN_PORT_MASK from b53_device"
Please push the following patch to -stable >= 4.6 as well:
"net/mlx4_core: Fix to clean devlink resources"
Thanks,
Tariq.
Jack Morgenstein (1):
net/mlx4_core: Fix deadlock when switching between polling and event
fw commands
Kamal Heib (2):
net/mlx4_en: Fix wrong indentation
net/mlx4_core: Fix to clean devlink resources
Leon Romanovsky (1):
net/mlx4_core: Use RCU to perform radix tree lookup for SRQ
Tariq Toukan (1):
net/mlx4_en: Add branch prediction hints in RX data-path
drivers/net/ethernet/mellanox/mlx4/cmd.c | 23 +++++++++++++++++------
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 28 +++++++++++++++-------------
drivers/net/ethernet/mellanox/mlx4/main.c | 3 +++
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 ++
drivers/net/ethernet/mellanox/mlx4/srq.c | 14 +++++---------
5 files changed, 42 insertions(+), 28 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next 2/5] net/mlx4_en: Fix wrong indentation
From: Tariq Toukan @ 2016-09-12 13:20 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Kamal Heib, Tariq Toukan
In-Reply-To: <1473686416-11779-1-git-send-email-tariqt@mellanox.com>
From: Kamal Heib <kamalh@mellanox.com>
Use tabs instead of spaces before if statement, no functional change.
Fixes: e7c1c2c46201 ("mlx4_en: Added self diagnostics test implementation")
Signed-off-by: Kamal Heib <kamalh@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index bb7ccb109aa4..51fa0aa8a49c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -1022,7 +1022,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
goto next;
}
- if (unlikely(priv->validate_loopback)) {
+ if (unlikely(priv->validate_loopback)) {
validate_loopback(priv, skb);
goto next;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 1/5] net/mlx4_en: Add branch prediction hints in RX data-path
From: Tariq Toukan @ 2016-09-12 13:20 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <1473686416-11779-1-git-send-email-tariqt@mellanox.com>
Add likely/unlikely hints to improve branch predictions
in the RX data-path.
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 6758292311f4..bb7ccb109aa4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -72,7 +72,7 @@ static int mlx4_alloc_pages(struct mlx4_en_priv *priv,
}
dma = dma_map_page(priv->ddev, page, 0, PAGE_SIZE << order,
frag_info->dma_dir);
- if (dma_mapping_error(priv->ddev, dma)) {
+ if (unlikely(dma_mapping_error(priv->ddev, dma))) {
put_page(page);
return -ENOMEM;
}
@@ -108,7 +108,8 @@ static int mlx4_en_alloc_frags(struct mlx4_en_priv *priv,
ring_alloc[i].page_size)
continue;
- if (mlx4_alloc_pages(priv, &page_alloc[i], frag_info, gfp))
+ if (unlikely(mlx4_alloc_pages(priv, &page_alloc[i],
+ frag_info, gfp)))
goto out;
}
@@ -585,7 +586,7 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
frag_info = &priv->frag_info[nr];
if (length <= frag_info->frag_prefix_size)
break;
- if (!frags[nr].page)
+ if (unlikely(!frags[nr].page))
goto fail;
dma = be64_to_cpu(rx_desc->data[nr].addr);
@@ -625,7 +626,7 @@ static struct sk_buff *mlx4_en_rx_skb(struct mlx4_en_priv *priv,
dma_addr_t dma;
skb = netdev_alloc_skb(priv->dev, SMALL_PACKET_SIZE + NET_IP_ALIGN);
- if (!skb) {
+ if (unlikely(!skb)) {
en_dbg(RX_ERR, priv, "Failed allocating skb\n");
return NULL;
}
@@ -736,7 +737,8 @@ static int get_fixed_ipv6_csum(__wsum hw_checksum, struct sk_buff *skb,
{
__wsum csum_pseudo_hdr = 0;
- if (ipv6h->nexthdr == IPPROTO_FRAGMENT || ipv6h->nexthdr == IPPROTO_HOPOPTS)
+ if (unlikely(ipv6h->nexthdr == IPPROTO_FRAGMENT ||
+ ipv6h->nexthdr == IPPROTO_HOPOPTS))
return -1;
hw_checksum = csum_add(hw_checksum, (__force __wsum)htons(ipv6h->nexthdr));
@@ -769,7 +771,7 @@ static int check_csum(struct mlx4_cqe *cqe, struct sk_buff *skb, void *va,
get_fixed_ipv4_csum(hw_checksum, skb, hdr);
#if IS_ENABLED(CONFIG_IPV6)
else if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV6))
- if (get_fixed_ipv6_csum(hw_checksum, skb, hdr))
+ if (unlikely(get_fixed_ipv6_csum(hw_checksum, skb, hdr)))
return -1;
#endif
return 0;
@@ -796,10 +798,10 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
u64 timestamp;
bool l2_tunnel;
- if (!priv->port_up)
+ if (unlikely(!priv->port_up))
return 0;
- if (budget <= 0)
+ if (unlikely(budget <= 0))
return polled;
/* Protect accesses to: ring->xdp_prog, priv->mac_hash list */
@@ -902,16 +904,16 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
case XDP_PASS:
break;
case XDP_TX:
- if (!mlx4_en_xmit_frame(frags, dev,
+ if (likely(!mlx4_en_xmit_frame(frags, dev,
length, tx_index,
- &doorbell_pending))
+ &doorbell_pending)))
goto consumed;
break;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
case XDP_DROP:
- if (mlx4_en_rx_recycle(ring, frags))
+ if (likely(mlx4_en_rx_recycle(ring, frags)))
goto consumed;
goto next;
}
@@ -1015,7 +1017,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
/* GRO not possible, complete processing here */
skb = mlx4_en_rx_skb(priv, rx_desc, frags, length);
- if (!skb) {
+ if (unlikely(!skb)) {
ring->dropped++;
goto next;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 3/5] net/mlx4_core: Use RCU to perform radix tree lookup for SRQ
From: Tariq Toukan @ 2016-09-12 13:20 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Leon Romanovsky, Tariq Toukan
In-Reply-To: <1473686416-11779-1-git-send-email-tariqt@mellanox.com>
From: Leon Romanovsky <leonro@mellanox.com>
Radix tree lookup can be performed without locking.
Fixes: 225c7b1feef1 ("IB/mlx4: Add a driver Mellanox ConnectX InfiniBand adapters")
Suggested-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/srq.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/srq.c b/drivers/net/ethernet/mellanox/mlx4/srq.c
index 67146624eb58..f44d089e2ca6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/srq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/srq.c
@@ -45,15 +45,12 @@ void mlx4_srq_event(struct mlx4_dev *dev, u32 srqn, int event_type)
struct mlx4_srq_table *srq_table = &mlx4_priv(dev)->srq_table;
struct mlx4_srq *srq;
- spin_lock(&srq_table->lock);
-
+ rcu_read_lock();
srq = radix_tree_lookup(&srq_table->tree, srqn & (dev->caps.num_srqs - 1));
+ rcu_read_unlock();
if (srq)
atomic_inc(&srq->refcount);
-
- spin_unlock(&srq_table->lock);
-
- if (!srq) {
+ else {
mlx4_warn(dev, "Async event for bogus SRQ %08x\n", srqn);
return;
}
@@ -301,12 +298,11 @@ struct mlx4_srq *mlx4_srq_lookup(struct mlx4_dev *dev, u32 srqn)
{
struct mlx4_srq_table *srq_table = &mlx4_priv(dev)->srq_table;
struct mlx4_srq *srq;
- unsigned long flags;
- spin_lock_irqsave(&srq_table->lock, flags);
+ rcu_read_lock();
srq = radix_tree_lookup(&srq_table->tree,
srqn & (dev->caps.num_srqs - 1));
- spin_unlock_irqrestore(&srq_table->lock, flags);
+ rcu_read_unlock();
return srq;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 5/5] net/mlx4_core: Fix to clean devlink resources
From: Tariq Toukan @ 2016-09-12 13:20 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Kamal Heib, Tariq Toukan
In-Reply-To: <1473686416-11779-1-git-send-email-tariqt@mellanox.com>
From: Kamal Heib <kamalh@mellanox.com>
This patch cleans devlink resources by calling devlink_port_unregister()
to avoid the following issues:
- Kernel panic when triggering reset flow.
- Memory leak due to unfreed resources in mlx4_init_port_info().
Fixes: 09d4d087cd48 ("mlx4: Implement devlink interface")
Signed-off-by: Kamal Heib <kamalh@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 75dd2e3d3059..7183ac4135d2 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2970,6 +2970,7 @@ static int mlx4_init_port_info(struct mlx4_dev *dev, int port)
mlx4_err(dev, "Failed to create mtu file for port %d\n", port);
device_remove_file(&info->dev->persist->pdev->dev,
&info->port_attr);
+ devlink_port_unregister(&info->devlink_port);
info->port = -1;
}
@@ -2984,6 +2985,8 @@ static void mlx4_cleanup_port_info(struct mlx4_port_info *info)
device_remove_file(&info->dev->persist->pdev->dev, &info->port_attr);
device_remove_file(&info->dev->persist->pdev->dev,
&info->port_mtu_attr);
+ devlink_port_unregister(&info->devlink_port);
+
#ifdef CONFIG_RFS_ACCEL
free_irq_cpu_rmap(info->rmap);
info->rmap = NULL;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 4/5] net/mlx4_core: Fix deadlock when switching between polling and event fw commands
From: Tariq Toukan @ 2016-09-12 13:20 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Eran Ben Elisha, Jack Morgenstein, Matan Barak,
Tariq Toukan
In-Reply-To: <1473686416-11779-1-git-send-email-tariqt@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
When switching from polling-based fw commands to event-based fw
commands, there is a race condition which could cause a fw command
in another task to hang: that task will keep waiting for the polling
sempahore, but may never be able to acquire it. This is due to
mlx4_cmd_use_events, which "down"s the sempahore back to 0.
During driver initialization, this is not a problem, since no other
tasks which invoke FW commands are active.
However, there is a problem if the driver switches to polling mode
and then back to event mode during normal operation.
The "test_interrupts" feature does exactly that.
Running "ethtool -t <eth device> offline" causes the PF driver to
temporarily switch to polling mode, and then back to event mode.
(Note that for VF drivers, such switching is not performed).
Fix this by adding a read-write semaphore for protection when
switching between modes.
Fixes: 225c7b1feef1 ("IB/mlx4: Add a driver Mellanox ConnectX InfiniBand adapters")
Signed-off-by: Matan Barak <matanb@mellanox.com>
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/cmd.c | 23 +++++++++++++++++------
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 ++
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index f04a423ff79d..a58d96cf1ed1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -785,17 +785,23 @@ int __mlx4_cmd(struct mlx4_dev *dev, u64 in_param, u64 *out_param,
return mlx4_cmd_reset_flow(dev, op, op_modifier, -EIO);
if (!mlx4_is_mfunc(dev) || (native && mlx4_is_master(dev))) {
+ int ret;
+
if (dev->persist->state & MLX4_DEVICE_STATE_INTERNAL_ERROR)
return mlx4_internal_err_ret_value(dev, op,
op_modifier);
+ down_read(&mlx4_priv(dev)->cmd.switch_sem);
if (mlx4_priv(dev)->cmd.use_events)
- return mlx4_cmd_wait(dev, in_param, out_param,
- out_is_imm, in_modifier,
- op_modifier, op, timeout);
+ ret = mlx4_cmd_wait(dev, in_param, out_param,
+ out_is_imm, in_modifier,
+ op_modifier, op, timeout);
else
- return mlx4_cmd_poll(dev, in_param, out_param,
- out_is_imm, in_modifier,
- op_modifier, op, timeout);
+ ret = mlx4_cmd_poll(dev, in_param, out_param,
+ out_is_imm, in_modifier,
+ op_modifier, op, timeout);
+
+ up_read(&mlx4_priv(dev)->cmd.switch_sem);
+ return ret;
}
return mlx4_slave_cmd(dev, in_param, out_param, out_is_imm,
in_modifier, op_modifier, op, timeout);
@@ -2454,6 +2460,7 @@ int mlx4_cmd_init(struct mlx4_dev *dev)
int flags = 0;
if (!priv->cmd.initialized) {
+ init_rwsem(&priv->cmd.switch_sem);
mutex_init(&priv->cmd.slave_cmd_mutex);
sema_init(&priv->cmd.poll_sem, 1);
priv->cmd.use_events = 0;
@@ -2583,6 +2590,7 @@ int mlx4_cmd_use_events(struct mlx4_dev *dev)
if (!priv->cmd.context)
return -ENOMEM;
+ down_write(&priv->cmd.switch_sem);
for (i = 0; i < priv->cmd.max_cmds; ++i) {
priv->cmd.context[i].token = i;
priv->cmd.context[i].next = i + 1;
@@ -2606,6 +2614,7 @@ int mlx4_cmd_use_events(struct mlx4_dev *dev)
down(&priv->cmd.poll_sem);
priv->cmd.use_events = 1;
+ up_write(&priv->cmd.switch_sem);
return err;
}
@@ -2618,6 +2627,7 @@ void mlx4_cmd_use_polling(struct mlx4_dev *dev)
struct mlx4_priv *priv = mlx4_priv(dev);
int i;
+ down_write(&priv->cmd.switch_sem);
priv->cmd.use_events = 0;
for (i = 0; i < priv->cmd.max_cmds; ++i)
@@ -2626,6 +2636,7 @@ void mlx4_cmd_use_polling(struct mlx4_dev *dev)
kfree(priv->cmd.context);
up(&priv->cmd.poll_sem);
+ up_write(&priv->cmd.switch_sem);
}
struct mlx4_cmd_mailbox *mlx4_alloc_cmd_mailbox(struct mlx4_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index c9d7fc5159f2..c128ba3ef014 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -46,6 +46,7 @@
#include <linux/interrupt.h>
#include <linux/spinlock.h>
#include <net/devlink.h>
+#include <linux/rwsem.h>
#include <linux/mlx4/device.h>
#include <linux/mlx4/driver.h>
@@ -627,6 +628,7 @@ struct mlx4_cmd {
struct mutex slave_cmd_mutex;
struct semaphore poll_sem;
struct semaphore event_sem;
+ struct rw_semaphore switch_sem;
int max_cmds;
spinlock_t context_lock;
int free_head;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 00/26] constify local structures
From: Julia Lawall @ 2016-09-12 13:23 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: alsa-devel, Mustafa Ismail, Tatyana Nikolova, kernel-janitors,
linux-fbdev, platform-driver-x86, devel, linux-scsi, linux-rdma,
Jason Gunthorpe, linux-acpi, tpmdd-devel, linux-media, linux-pm,
linux-can, Shiraz Saleem, Sergei Shtylyov, netdev, Chien Tin Tung,
linux-wireless, linux-kernel, linux-spi, linux-renesas-soc,
linux-usb, joe
In-Reply-To: <20160912131625.GD957@intel.com>
On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:
> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >
> >
> > On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >
> > > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > > > Constify local structures.
> > > >
> > > > The semantic patch that makes this change is as follows:
> > > > (http://coccinelle.lip6.fr/)
> > >
> > > Just my two cents but:
> > >
> > > 1. You *can* use a static analysis too to find bugs or other issues.
> > > 2. However, you should manually do the commits and proper commit
> > > messages to subsystems based on your findings. And I generally think
> > > that if one contributes code one should also at least smoke test changes
> > > somehow.
> > >
> > > I don't know if I'm alone with my opinion. I just think that one should
> > > also do the analysis part and not blindly create and submit patches.
> >
> > All of the patches are compile tested. And the individual patches are
>
> Compile-testing is not testing. If you are not able to test a commit,
> you should explain why.
>
> > submitted to the relevant maintainers. The individual commit messages
> > give a more detailed explanation of the strategy used to decide that the
> > structure was constifiable. It seemed redundant to put that in the cover
> > letter, which will not be committed anyway.
>
> I don't mean to be harsh but I do not care about your thought process
> *that much* when I review a commit (sometimes it might make sense to
> explain that but it depends on the context).
>
> I mostly only care why a particular change makes sense for this
> particular subsystem. The report given by a static analysis tool can
> be a starting point for making a commit but it's not sufficient.
> Based on the report you should look subsystems as individuals.
OK, thanks for the feedback.
julia
^ permalink raw reply
* Re: [PATCH v4 1/6] net: stmmac: dwmac-rk: add rk3366 & rk3399 specific data
From: Rob Herring @ 2016-09-12 13:28 UTC (permalink / raw)
To: Caesar Wang
Cc: Heiko Stuebner, David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
Douglas Anderson, dbasehore-F7+t8E8rja9g9hUCZPvPmw, Roger Chen,
Mark Rutland, Giuseppe Cavallaro, Alexandre Torgue, Xing Zheng,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1472752204-8924-2-git-send-email-wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On Fri, Sep 02, 2016 at 01:49:59AM +0800, Caesar Wang wrote:
> From: Roger Chen <roger.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> Add constants and callback functions for the dwmac on rk3228/rk3229 socs.
> As can be seen, the base structure is the same, only registers and the
> bits in them moved slightly.
>
> Signed-off-by: Roger Chen <roger.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Reviewed-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
>
> ---
>
> Changes in v4:
> - Fixes from the original patch on https://patchwork.kernel.org/patch/9274557/
>
> Changes in v3: None
> Changes in v2: None
>
> .../devicetree/bindings/net/rockchip-dwmac.txt | 8 +-
> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 226 +++++++++++++++++++++
> 2 files changed, 232 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> index cccd945..95383c5 100644
> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> @@ -3,8 +3,12 @@ Rockchip SoC RK3288 10/100/1000 Ethernet driver(GMAC)
> The device node has following properties.
>
> Required properties:
> - - compatible: Can be one of "rockchip,rk3228-gmac", "rockchip,rk3288-gmac",
> - "rockchip,rk3368-gmac"
> + - compatible: should be "rockchip,<name>-gamc"
s/gamc/gmac/
Please send a follow-up patch to fix.
> + "rockchip,rk3228-gmac": found on RK322x SoCs
> + "rockchip,rk3288-gmac": found on RK3288 SoCs
> + "rockchip,rk3366-gmac": found on RK3366 SoCs
> + "rockchip,rk3368-gmac": found on RK3368 SoCs
> + "rockchip,rk3399-gmac": found on RK3399 SoCs
> - reg: addresses and length of the register sets for the device.
> - interrupts: Should contain the GMAC interrupts.
> - interrupt-names: Should contain the interrupt names "macirq".
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] test_bpf: fix the dummy skb after dissector changes
From: Daniel Borkmann @ 2016-09-12 13:31 UTC (permalink / raw)
To: Jakub Kicinski, Alexei Starovoitov; +Cc: netdev, Hadar Hen Zion
In-Reply-To: <1473681897-32260-1-git-send-email-jakub.kicinski@netronome.com>
On 09/12/2016 02:04 PM, Jakub Kicinski wrote:
> Commit d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan
> info from skb->vlan_tci") made flow dissector look at vlan_proto
> when vlan is present. Since test_bpf sets skb->vlan_tci to ~0
> (including VLAN_TAG_PRESENT) we have to populate skb->vlan_proto.
>
> Fixes false negative on test #24:
> test_bpf: #24 LD_PAYLOAD_OFF jited:0 175 ret 0 != 42 FAIL (1 times)
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH V3] dt: net: enhance DWC EQoS binding to support Tegra186
From: Rob Herring @ 2016-09-12 13:38 UTC (permalink / raw)
To: Stephen Warren
Cc: Mark Rutland, Lars Persson, devicetree, netdev, linux-tegra,
Stephen Warren
In-Reply-To: <038d710a-f8fe-b569-11ee-2dc4c3ec6d28@wwwdotorg.org>
On Thu, Sep 08, 2016 at 11:49:31AM -0600, Stephen Warren wrote:
> On 09/01/2016 01:02 PM, Stephen Warren wrote:
> >From: Stephen Warren <swarren@nvidia.com>
> >
> >The Synopsys DWC EQoS is a configurable IP block which supports multiple
> >options for bus type, clocking and reset structure, and feature list.
> >Extend the DT binding to define a "compatible value" for the configuration
> >contained in NVIDIA's Tegra186 SoC, and define some new properties and
> >list property entries required by that configuration.
> >
> >Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >---
> >v3:
> >* Document legacy clock-names entries separately, and make it obvious
> > they're deprecated.
> >* Reword the description of the "rx" clock to better describe the HW.
> >* Add some extra guidance for future extensions of the binding to cover
> > configurations where additional RX clocks are required.
> >* Explicitly document the list of clocks and resets for every compatible
> > value; don't miss any out.
>
> Rob, Mark,
>
> Does this version look good (Lars already acked it)? If so, if you could ack
> it that'd be great; I want to send some U-Boot drivers that use this
> binding, but don't want to do so before I know it's final.
Applied.
Rob
^ permalink raw reply
* Re: [PATCH 00/26] constify local structures
From: Felipe Balbi @ 2016-09-12 13:43 UTC (permalink / raw)
To: Jarkko Sakkinen, Julia Lawall
Cc: linux-renesas-soc, joe, kernel-janitors, Sergei Shtylyov,
linux-pm, platform-driver-x86, linux-media, linux-can,
Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail, Chien Tin Tung,
linux-rdma, netdev, devel, alsa-devel, linux-kernel, linux-fbdev,
linux-wireless, Jason Gunthorpe, tpmdd-devel, linux-scsi,
linux-spi, l
In-Reply-To: <20160912131625.GD957@intel.com>
[-- Attachment #1: Type: text/plain, Size: 1837 bytes --]
Hi,
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
>>
>>
>> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
>>
>> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
>> > > Constify local structures.
>> > >
>> > > The semantic patch that makes this change is as follows:
>> > > (http://coccinelle.lip6.fr/)
>> >
>> > Just my two cents but:
>> >
>> > 1. You *can* use a static analysis too to find bugs or other issues.
>> > 2. However, you should manually do the commits and proper commit
>> > messages to subsystems based on your findings. And I generally think
>> > that if one contributes code one should also at least smoke test changes
>> > somehow.
>> >
>> > I don't know if I'm alone with my opinion. I just think that one should
>> > also do the analysis part and not blindly create and submit patches.
>>
>> All of the patches are compile tested. And the individual patches are
>
> Compile-testing is not testing. If you are not able to test a commit,
> you should explain why.
Dude, Julia has been doing semantic patching for years already and
nobody has raised any concerns so far. There's already an expectation
that Coccinelle *works* and Julia's sematic patches are sound.
Besides, adding 'const' is something that causes virtually no functional
changes to the point that build-testing is really all you need. Any
problems caused by adding 'const' to a definition will be seen by build
errors or warnings.
Really, just stop with the pointless discussion and go read a bit about
Coccinelle and what semantic patches are giving you. The work done by
Julia and her peers are INRIA have measurable benefits.
You're really making a thunderstorm in a glass of water.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply
* Re: [PATCH] net: dsa: add FIB support
From: kbuild test robot @ 2016-09-12 13:49 UTC (permalink / raw)
To: John Crispin
Cc: kbuild-all, David S. Miller, Andrew Lunn, Florian Fainelli,
netdev, linux-kernel, John Crispin
In-Reply-To: <1473675233-36952-1-git-send-email-john@phrozen.org>
[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]
Hi John,
[auto build test ERROR on net-next/master]
[cannot apply to v4.8-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/John-Crispin/net-dsa-add-FIB-support/20160912-190416
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All errors (new ones prefixed by >>):
net/dsa/slave.c: In function 'dsa_slave_ipv4_fib_add':
>> net/dsa/slave.c:345:9: error: 'struct dsa_switch' has no member named 'drv'
if (!ds->drv->ipv4_fib_prepare || !ds->drv->ipv4_fib_add)
^
net/dsa/slave.c:345:39: error: 'struct dsa_switch' has no member named 'drv'
if (!ds->drv->ipv4_fib_prepare || !ds->drv->ipv4_fib_add)
^
net/dsa/slave.c:349:11: error: 'struct dsa_switch' has no member named 'drv'
ret = ds->drv->ipv4_fib_prepare(ds, p->port, fib4, trans);
^
net/dsa/slave.c:351:11: error: 'struct dsa_switch' has no member named 'drv'
ret = ds->drv->ipv4_fib_add(ds, p->port, fib4, trans);
^
net/dsa/slave.c: In function 'dsa_slave_ipv4_fib_del':
net/dsa/slave.c:363:8: error: 'struct dsa_switch' has no member named 'drv'
if (ds->drv->ipv4_fib_del)
^
net/dsa/slave.c:364:11: error: 'struct dsa_switch' has no member named 'drv'
ret = ds->drv->ipv4_fib_del(ds, p->port, fib4);
^
vim +345 net/dsa/slave.c
339 struct switchdev_trans *trans)
340 {
341 struct dsa_slave_priv *p = netdev_priv(dev);
342 struct dsa_switch *ds = p->parent;
343 int ret;
344
> 345 if (!ds->drv->ipv4_fib_prepare || !ds->drv->ipv4_fib_add)
346 return -EOPNOTSUPP;
347
348 if (switchdev_trans_ph_prepare(trans))
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 44352 bytes --]
^ permalink raw reply
* Re: [PATCH 00/26] constify local structures
From: Julia Lawall @ 2016-09-12 13:52 UTC (permalink / raw)
To: Felipe Balbi
Cc: alsa-devel, Mustafa Ismail, Tatyana Nikolova, kernel-janitors,
linux-fbdev, Jarkko Sakkinen, platform-driver-x86, devel,
linux-scsi, linux-rdma, Jason Gunthorpe, linux-acpi, tpmdd-devel,
linux-media, linux-pm, linux-can, Julia Lawall, Shiraz Saleem,
Sergei Shtylyov, netdev, Chien Tin Tung, linux-wireless,
linux-kernel, linux-spi, linux-renesas-soc, linux-usb
In-Reply-To: <877fah5j35.fsf@linux.intel.com>
On Mon, 12 Sep 2016, Felipe Balbi wrote:
>
> Hi,
>
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >>
> >>
> >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >>
> >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> >> > > Constify local structures.
> >> > >
> >> > > The semantic patch that makes this change is as follows:
> >> > > (http://coccinelle.lip6.fr/)
> >> >
> >> > Just my two cents but:
> >> >
> >> > 1. You *can* use a static analysis too to find bugs or other issues.
> >> > 2. However, you should manually do the commits and proper commit
> >> > messages to subsystems based on your findings. And I generally think
> >> > that if one contributes code one should also at least smoke test changes
> >> > somehow.
> >> >
> >> > I don't know if I'm alone with my opinion. I just think that one should
> >> > also do the analysis part and not blindly create and submit patches.
> >>
> >> All of the patches are compile tested. And the individual patches are
> >
> > Compile-testing is not testing. If you are not able to test a commit,
> > you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
>
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
>
> Really, just stop with the pointless discussion and go read a bit about
> Coccinelle and what semantic patches are giving you. The work done by
> Julia and her peers are INRIA have measurable benefits.
>
> You're really making a thunderstorm in a glass of water.
Thanks for the defense, but since a lot of these patches torned out to be
wrong, due to an incorrect parse by Coccinelle, combined with an
unpleasantly lax compiler, Jarkko does have a point that I should have
looked at the patches more carefully. In any case, I have written to the
maintainers relevant to the patches that turned out to be incorrect.
julia
^ permalink raw reply
* Re: [PATCH 00/26] constify local structures
From: Geert Uytterhoeven @ 2016-09-12 13:57 UTC (permalink / raw)
To: Felipe Balbi
Cc: Jarkko Sakkinen, Julia Lawall, Linux-Renesas, Joe Perches,
kernel-janitors@vger.kernel.org, Sergei Shtylyov, Linux PM list,
platform-driver-x86, Linux Media Mailing List, linux-can,
Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail, Chien Tin Tung,
linux-rdma, netdev@vger.kernel.org, driverdevel
In-Reply-To: <877fah5j35.fsf@linux.intel.com>
On Mon, Sep 12, 2016 at 3:43 PM, Felipe Balbi
<felipe.balbi@linux.intel.com> wrote:
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
>> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
>>> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
>>> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
>>> > > Constify local structures.
>>> > >
>>> > > The semantic patch that makes this change is as follows:
>>> > > (http://coccinelle.lip6.fr/)
>>> >
>>> > Just my two cents but:
>>> >
>>> > 1. You *can* use a static analysis too to find bugs or other issues.
>>> > 2. However, you should manually do the commits and proper commit
>>> > messages to subsystems based on your findings. And I generally think
>>> > that if one contributes code one should also at least smoke test changes
>>> > somehow.
>>> >
>>> > I don't know if I'm alone with my opinion. I just think that one should
>>> > also do the analysis part and not blindly create and submit patches.
>>>
>>> All of the patches are compile tested. And the individual patches are
>>
>> Compile-testing is not testing. If you are not able to test a commit,
>> you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
+1
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
Unfortunately in this particular case they could lead to failures that can only
be detected at runtime, when failing o write to a read-only piece of memory,
due to the casting away of the constness of the pointers later.
Fortunately this was detected during code review (doh...).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox