* Re: [PATCH net-next 1/4] net/flow_dissector: add support for dissection of misc ip header fields
From: Or Gerlitz @ 2017-05-27 16:31 UTC (permalink / raw)
To: Tom Herbert
Cc: Or Gerlitz, David S. Miller, Linux Kernel Network Developers,
Saeed Mahameed, Roi Dayan, Paul Blakey
In-Reply-To: <CALx6S36uJrFA+TO7RDaRAxZi0vuEEc90_TnWP8a2yNP+V3X2qQ@mail.gmail.com>
On Thu, May 25, 2017 at 7:22 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Thu, May 25, 2017 at 6:24 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> Add support for dissection of ip tos and ttl and ipv6 traffic-class
>> and hoplimit. Both are dissected into the same struct.
>> Uses similar call to ip dissection function as with tcp, arp and others.
>> +/**
>> + * struct flow_dissector_key_ip:
>> + * @tos: tos
>> + * @ttl: ttl
>> + */
>> +struct flow_dissector_key_ip {
>> + __u8 tos;
>> + __u8 ttl;
>> +};
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> +static void
>> +__skb_flow_dissect_ipv4(const struct sk_buff *skb,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data, const struct iphdr *iph)
>> +{
>> + struct flow_dissector_key_ip *key_ip;
>> +
>> + if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_IP))
>> + return;
>> +
>> + key_ip = skb_flow_dissector_target(flow_dissector,
>> + FLOW_DISSECTOR_KEY_IP,
>> + target_container);
>> + key_ip->tos = iph->tos;
>> + key_ip->ttl = iph->ttl;
>
> In an encapsulation this returns the tos and ttl of the encapsulated
> packet. Is that really useful to the caller? Seems more likely that
> they need the outer tos and ttl for forwarding.
In what we are dealing with, classification is carried after the
packet is decapsulated by the shared tunnel device. So even today,e.g
for the src/dst IP, the dissection is carried on what were the inner
fields before decap.
When we did the the flower patches for being able to classify on both
the inner and outer fields (say outer src/dst ip, tunnel key) for what
related to the macs/ips/ports/etc -- I don't think we touched the
existing dissection, I will look on that to see if I am wrong..
^ permalink raw reply
* Re: [PATCH net-next 1/4] net/flow_dissector: add support for dissection of misc ip header fields
From: Or Gerlitz @ 2017-05-27 16:29 UTC (permalink / raw)
To: Tom Herbert
Cc: Or Gerlitz, David S. Miller, Linux Kernel Network Developers,
Saeed Mahameed, Roi Dayan, Paul Blakey
In-Reply-To: <CALx6S379RzD+t=mDHgs+u-BrWgd7Jt4W_C-kNarA7t+HwEJ3jQ@mail.gmail.com>
On Thu, May 25, 2017 at 6:42 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Thu, May 25, 2017 at 6:24 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> Add support for dissection of ip tos and ttl and ipv6 traffic-class
>> and hoplimit. Both are dissected into the same struct.
>> --- a/include/net/flow_dissector.h
>> +++ b/include/net/flow_dissector.h
>> +/**
>> + * struct flow_dissector_key_ip:
>> + * @tos: tos
>> + * @ttl: ttl
>> + */
>> +struct flow_dissector_key_ip {
>> + __u8 tos;
>> + __u8 ttl;
>> +};
>> +
> Looks like yet more complexity be piled onto flow dissector. Instead
> of splitting out individual fields can we just return a pointer to the
> IP header and let the caller extract the fields they're interested in?
Do you mean that struct flow_dissector_key_ip will only contain
(union?) const struct iphdr * and const struct ipv6hdr * ? I wasn't
sure how would that further look on the kernel SW classification path
(the non offloaded case)
^ permalink raw reply
* Re: [PATCH] net/core: remove explicit do_softirq() from busy_poll_stop()
From: Sebastian Andrzej Siewior @ 2017-05-27 16:23 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David S. Miller, Thomas Gleixner
In-Reply-To: <CANn89iJx0-z2KZWMRZdqmYUNAWcqQo4nBbRwK7pLz2-WAymqrQ@mail.gmail.com>
On 2017-05-22 14:26:44 [-0700], Eric Dumazet wrote:
> On Mon, May 22, 2017 at 12:26 PM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > Since commit 217f69743681 ("net: busy-poll: allow preemption in
> > sk_busy_loop()") there is an explicit do_softirq() invocation after
> > local_bh_enable() has been invoked.
> > I don't understand why we need this because local_bh_enable() will
> > invoke do_softirq() once the softirq counter reached zero and we have
> > softirq-related work pending.
> >
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> > net/core/dev.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index fca407b4a6ea..e84eb0ec5529 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -5199,8 +5199,6 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock)
> > if (rc == BUSY_POLL_BUDGET)
> > __napi_schedule(napi);
> > local_bh_enable();
> > - if (local_softirq_pending())
> > - do_softirq();
> > }
>
> preemption is disabled.
so? This patch:
diff --git a/init/main.c b/init/main.c
--- a/init/main.c
+++ b/init/main.c
@@ -1001,6 +1001,21 @@ static int __ref kernel_init(void *unused)
"See Linux Documentation/admin-guide/init.rst for guidance.");
}
+static void delay_thingy_func(struct work_struct *x)
+{
+ preempt_disable();
+ local_bh_disable();
+ pr_err("one %s\n", current->comm);
+ raise_softirq(TASKLET_SOFTIRQ);
+ pr_err("two %s\n", current->comm);
+ local_bh_enable();
+ pr_err("three %s\n", current->comm);
+ preempt_enable();
+ pr_err("four %s\n", current->comm);
+}
+
+static DECLARE_DELAYED_WORK(delay_thingy, delay_thingy_func);
+
static noinline void __init kernel_init_freeable(void)
{
/*
@@ -1038,6 +1053,7 @@ static noinline void __init kernel_init_freeable(void)
do_basic_setup();
+ schedule_delayed_work(&delay_thingy, HZ * 5);
/* Open the /dev/console on the rootfs, this should never fail */
if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
pr_err("Warning: unable to open an initial console.\n");
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 4e09821f9d9e..b8dcb9dc5692 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -500,6 +500,7 @@ static __latent_entropy void tasklet_action(struct softirq_action *a)
{
struct tasklet_struct *list;
+ pr_err("%s() %s\n", __func__, current->comm);
local_irq_disable();
list = __this_cpu_read(tasklet_vec.head);
__this_cpu_write(tasklet_vec.head, NULL);
gives me this output:
[ 7.132439] one kworker/4:2
[ 7.132806] two kworker/4:2
[ 7.133120] softirq: tasklet_action() kworker/4:2
[ 7.133623] three kworker/4:2
[ 7.133940] four kworker/4:2
which means after the last local_bh_enable() we already invoke the
raised softirq handler. It does not matter that we are in a
preempt_disable() region.
>
> Look at netif_rx_ni() for a similar construct.
correct, this is old and it is already patched in -RT. And I have no
clue why this is required by because netif_rx_internal() itself does
preempt_disable() / get_cpu() so this one already disables preemption.
Looking at it, I *think* you want local_bh_disable() instead of
preempt_disable() and do_softirq() removed, too.
Let me browse at the musuem a little bit… ahhh, here -> "[PATCH] Make
netif_rx_ni preempt-safe" [0]. There we got the preempt_disable() from
which protects against parallel invocations of do_softirq() in user
context. And we only do the check for pending softirqs (and invoke
do_softirq()) because netif_rx() sets the pending bits without raising
the softirq and it is done in a BH-enabled section. And in interrupt
context we check for those in the interrupt-exit path. So in this case
we have to do it manually.
[0] http://oss.sgi.com/projects/netdev/archive/2004-10/msg02211.html
> What exact problem do you have with existing code, that is worth
> adding this change ?
I need to workaround the non-existing do_softirq() function in RT and my
current workaround is the patch I posted. I don't see the need for the
two lines. And it seems that the other construct in netif_rx_ni() can be
simplified / removed, too.
> Thanks.
Sebastian
^ permalink raw reply related
* [PATCH net-next] liquidio: add support for OVS offload
From: Felix Manlunas @ 2017-05-27 15:56 UTC (permalink / raw)
To: davem
Cc: netdev, veerasenareddy.burru, raghu.vatsavayi, derek.chickles,
satananda.burla
From: VSR Burru <veerasenareddy.burru@cavium.com>
Add support for OVS offload. By default PF driver runs in basic NIC mode
as usual. To run in OVS mode, use the insmod parameter "fw_type=ovs".
For OVS mode, create a management interface for communication with NIC
firmware. This communication channel uses PF0's I/O rings.
Bump up driver version to 1.6.0 to match newer firmware.
Signed-off-by: VSR Burru <veerasenareddy.burru@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/Makefile | 1 +
drivers/net/ethernet/cavium/liquidio/lio_main.c | 27 +-
.../net/ethernet/cavium/liquidio/liquidio_common.h | 23 +-
.../net/ethernet/cavium/liquidio/liquidio_image.h | 1 +
.../net/ethernet/cavium/liquidio/liquidio_mgmt.c | 439 +++++++++++++++++++++
.../net/ethernet/cavium/liquidio/octeon_console.c | 27 +-
drivers/net/ethernet/cavium/liquidio/octeon_main.h | 9 +
7 files changed, 516 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/Makefile b/drivers/net/ethernet/cavium/liquidio/Makefile
index c4d411d..2064157 100644
--- a/drivers/net/ethernet/cavium/liquidio/Makefile
+++ b/drivers/net/ethernet/cavium/liquidio/Makefile
@@ -15,6 +15,7 @@ liquidio-$(CONFIG_LIQUIDIO) += lio_ethtool.o \
octeon_mailbox.o \
octeon_mem_ops.o \
octeon_droq.o \
+ liquidio_mgmt.o \
octeon_nic.o
liquidio-objs := lio_main.o octeon_console.o $(liquidio-y)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index ba01242..b22eb74 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -43,6 +43,8 @@ MODULE_FIRMWARE(LIO_FW_DIR LIO_FW_BASE_NAME LIO_210SV_NAME LIO_FW_NAME_SUFFIX);
MODULE_FIRMWARE(LIO_FW_DIR LIO_FW_BASE_NAME LIO_210NV_NAME LIO_FW_NAME_SUFFIX);
MODULE_FIRMWARE(LIO_FW_DIR LIO_FW_BASE_NAME LIO_410NV_NAME LIO_FW_NAME_SUFFIX);
MODULE_FIRMWARE(LIO_FW_DIR LIO_FW_BASE_NAME LIO_23XX_NAME LIO_FW_NAME_SUFFIX);
+MODULE_FIRMWARE(LIO_FW_DIR LIO_FW_BASE_NAME LIO_23XX_NAME "_"
+ LIO_FW_NAME_TYPE_OVS LIO_FW_NAME_SUFFIX);
static int ddr_timeout = 10000;
module_param(ddr_timeout, int, 0644);
@@ -57,7 +59,7 @@ MODULE_PARM_DESC(debug, "NETIF_MSG debug bits");
static char fw_type[LIO_MAX_FW_TYPE_LEN];
module_param_string(fw_type, fw_type, sizeof(fw_type), 0000);
-MODULE_PARM_DESC(fw_type, "Type of firmware to be loaded. Default \"nic\"");
+MODULE_PARM_DESC(fw_type, "Type of firmware to be loaded (nic,ovs,none). Default \"nic\". Use \"none\" to load firmware from flash on LiquidIO adapter.");
static int ptp_enable = 1;
@@ -1414,6 +1416,12 @@ static bool fw_type_is_none(void)
sizeof(LIO_FW_NAME_TYPE_NONE)) == 0;
}
+static bool is_fw_type_ovs(void)
+{
+ return strncmp(fw_type, LIO_FW_NAME_TYPE_OVS,
+ sizeof(LIO_FW_NAME_TYPE_OVS)) == 0;
+}
+
/**
*\brief Destroy resources associated with octeon device
* @param pdev PCI device structure
@@ -1776,6 +1784,9 @@ static void liquidio_remove(struct pci_dev *pdev)
dev_dbg(&oct_dev->pci_dev->dev, "Stopping device\n");
+ if (is_fw_type_ovs())
+ lio_mgmt_exit();
+
if (oct_dev->watchdog_task)
kthread_stop(oct_dev->watchdog_task);
@@ -3933,6 +3944,8 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
u32 resp_size, ctx_size, data_size;
u32 ifidx_or_pfnum;
struct lio_version *vdata;
+ union oct_nic_vf_info vf_info;
+
/* This is to handle link status changes */
octeon_register_dispatch_fn(octeon_dev, OPCODE_NIC,
@@ -4001,9 +4014,16 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
sc->iq_no = 0;
+ /* Populate VF info for OVS firmware */
+ vf_info.u64 = 0;
+
+ vf_info.s.bus_num = octeon_dev->pci_dev->bus->number;
+ vf_info.s.dev_fn = octeon_dev->pci_dev->devfn;
+ vf_info.s.max_vfs = octeon_dev->sriov_info.max_vfs;
+
octeon_prepare_soft_command(octeon_dev, sc, OPCODE_NIC,
OPCODE_NIC_IF_CFG, 0,
- if_cfg.u64, 0);
+ if_cfg.u64, vf_info.u64);
sc->callback = if_cfg_callback;
sc->callback_arg = sc;
@@ -4382,6 +4402,9 @@ static int liquidio_init_nic_module(struct octeon_device *oct)
goto octnet_init_failure;
}
+ if (is_fw_type_ovs())
+ lio_mgmt_init(oct);
+
liquidio_ptp_init(oct);
dev_dbg(&oct->pci_dev->dev, "Network interfaces ready\n");
diff --git a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
index 8ea2323..d8a1a1d 100644
--- a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
+++ b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
@@ -27,8 +27,8 @@
#define LIQUIDIO_PACKAGE ""
#define LIQUIDIO_BASE_MAJOR_VERSION 1
-#define LIQUIDIO_BASE_MINOR_VERSION 5
-#define LIQUIDIO_BASE_MICRO_VERSION 1
+#define LIQUIDIO_BASE_MINOR_VERSION 6
+#define LIQUIDIO_BASE_MICRO_VERSION 0
#define LIQUIDIO_BASE_VERSION __stringify(LIQUIDIO_BASE_MAJOR_VERSION) "." \
__stringify(LIQUIDIO_BASE_MINOR_VERSION)
#define LIQUIDIO_MICRO_VERSION "." __stringify(LIQUIDIO_BASE_MICRO_VERSION)
@@ -63,6 +63,8 @@ enum octeon_tag_type {
*/
#define OPCODE_CORE 0 /* used for generic core operations */
#define OPCODE_NIC 1 /* used for NIC operations */
+#define OPCODE_OVS 2 /* used for OVS operations */
+
/* Subcodes are used by host driver/apps to identify the sub-operation
* for the core. They only need to by unique for a given subsystem.
*/
@@ -890,4 +892,21 @@ union oct_nic_if_cfg {
} s;
};
+union oct_nic_vf_info {
+ u64 u64;
+ struct {
+#ifdef __BIG_ENDIAN_BITFIELD
+ u64 max_vfs:32;
+ u64 bus_num:8;
+ u64 dev_fn:8;
+ u64 reserved:16;
+#else
+ u64 reserved:16;
+ u64 dev_fn:8;
+ u64 bus_num:8;
+ u64 max_vfs:32;
+#endif
+ } s;
+};
+
#endif
diff --git a/drivers/net/ethernet/cavium/liquidio/liquidio_image.h b/drivers/net/ethernet/cavium/liquidio/liquidio_image.h
index 78a3685..cbd03f5 100644
--- a/drivers/net/ethernet/cavium/liquidio/liquidio_image.h
+++ b/drivers/net/ethernet/cavium/liquidio/liquidio_image.h
@@ -24,6 +24,7 @@
#define LIO_FW_BASE_NAME "lio_"
#define LIO_FW_NAME_SUFFIX ".bin"
#define LIO_FW_NAME_TYPE_NIC "nic"
+#define LIO_FW_NAME_TYPE_OVS "ovs"
#define LIO_FW_NAME_TYPE_NONE "none"
#define LIO_MAX_FIRMWARE_VERSION_LEN 16
diff --git a/drivers/net/ethernet/cavium/liquidio/liquidio_mgmt.c b/drivers/net/ethernet/cavium/liquidio/liquidio_mgmt.c
new file mode 100644
index 0000000..1fd7497
--- /dev/null
+++ b/drivers/net/ethernet/cavium/liquidio/liquidio_mgmt.c
@@ -0,0 +1,439 @@
+/**********************************************************************
+ * Author: Cavium, Inc.
+ *
+ * Contact: support@cavium.com
+ * Please include "LiquidIO" in the subject.
+ *
+ * Copyright (c) 2003-2017 Cavium, Inc.
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, Version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This file is distributed in the hope that it will be useful, but
+ * AS-IS and WITHOUT ANY WARRANTY; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE, TITLE, or
+ * NONINFRINGEMENT. See the GNU General Public License for more details.
+ ***********************************************************************/
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/crc32.h>
+#include <linux/dma-mapping.h>
+#include <linux/pci.h>
+#include <linux/pci_ids.h>
+#include <linux/ip.h>
+#include <net/ip.h>
+#include <linux/ipv6.h>
+#include <linux/net_tstamp.h>
+#include <linux/if_vlan.h>
+#include <linux/firmware.h>
+#include <linux/ethtool.h>
+#include <linux/types.h>
+#include <linux/list.h>
+#include <linux/workqueue.h>
+#include <linux/interrupt.h>
+#include "octeon_config.h"
+#include "liquidio_common.h"
+#include "octeon_droq.h"
+#include "octeon_iq.h"
+#include "response_manager.h"
+#include "octeon_device.h"
+#include "octeon_nic.h"
+#include "octeon_main.h"
+#include "octeon_network.h"
+
+/* VSW specific opcodes. these should match with those in cvmcs-vsw-msg.h */
+#define OPCODE_VSW_HOST_COMM_PKT_DATA 0x10
+
+static struct net_device *netdev;
+struct lio_mgmt {
+ atomic_t ifstate;
+ struct tasklet_struct rx_tasklet;
+ struct list_head rx_pkts_list;
+ struct net_device *parent_netdev;
+ struct octeon_device *oct_dev;
+ struct net_device *netdev;
+ u64 dev_capability;
+ u64 enc_dev_capability;
+ struct oct_link_info linfo;
+ u32 intf_open;
+};
+
+struct lio_mgmt_rx_pkt {
+ struct list_head list;
+ struct sk_buff *skb;
+};
+
+#define LIO_MGMT_SIZE (sizeof(struct lio_mgmt))
+#define GET_LIO_MGMT(netdev) ((struct lio_mgmt *)netdev_priv(netdev))
+
+/**
+ * \brief Stop Tx queues
+ * @param netdev network device
+ */
+static inline void txqs_stop(struct net_device *netdev)
+{
+ if (netif_is_multiqueue(netdev)) {
+ int i;
+
+ for (i = 0; i < netdev->num_tx_queues; i++)
+ netif_stop_subqueue(netdev, i);
+ } else {
+ netif_stop_queue(netdev);
+ }
+}
+
+/**
+ * \brief Start Tx queues
+ * @param netdev network device
+ */
+static inline void txqs_start(struct net_device *netdev)
+{
+ if (netif_is_multiqueue(netdev)) {
+ int i;
+
+ for (i = 0; i < netdev->num_tx_queues; i++)
+ netif_start_subqueue(netdev, i);
+ } else {
+ netif_start_queue(netdev);
+ }
+}
+
+/**
+ * \brief Stop Tx queue
+ * @param netdev network device
+ */
+static void stop_txq(struct net_device *netdev)
+{
+ txqs_stop(netdev);
+}
+
+/**
+ * \brief Start Tx queue
+ * @param netdev network device
+ */
+static void start_txq(struct net_device *netdev)
+{
+ txqs_start(netdev);
+}
+
+static int lio_mgmt_open(struct net_device *netdev)
+{
+ struct lio_mgmt *lio_mgmt = GET_LIO_MGMT(netdev);
+
+ ifstate_set((struct lio *)lio_mgmt, LIO_IFSTATE_RUNNING);
+ netif_carrier_on(netdev);
+
+ start_txq(netdev);
+
+ /* Ready for link status updates */
+ lio_mgmt->intf_open = 1;
+
+ return 0;
+}
+
+/**
+ * \brief Net device stop for LiquidIO
+ * @param netdev network device
+ */
+static int lio_mgmt_stop(struct net_device *netdev)
+{
+ struct lio_mgmt *lio_mgmt = GET_LIO_MGMT(netdev);
+
+ ifstate_reset((struct lio *)lio_mgmt, LIO_IFSTATE_RUNNING);
+
+ netif_tx_disable(netdev);
+
+ /* Inform that netif carrier is down */
+ netif_carrier_off(netdev);
+ lio_mgmt->intf_open = 0;
+
+ return 0;
+}
+
+static void packet_sent_callback(struct octeon_device *oct,
+ u32 status, void *buf)
+{
+ struct octeon_soft_command *sc = (struct octeon_soft_command *)buf;
+ struct sk_buff *skb = sc->ctxptr;
+
+ dma_unmap_single(&oct->pci_dev->dev, sc->dmadptr,
+ sc->datasize, DMA_TO_DEVICE);
+ dev_kfree_skb_any(skb);
+ kfree(sc);
+}
+
+/** \brief Transmit networks packets to the Octeon interface
+ * @param skbuff skbuff struct to be passed to network layer.
+ * @param netdev pointer to network device
+ * @returns whether the packet was transmitted to the device okay or not
+ * (NETDEV_TX_OK or NETDEV_TX_BUSY)
+ */
+static int lio_mgmt_xmit(struct sk_buff *skb, struct net_device *netdev)
+{
+ struct lio_mgmt *lio_mgmt;
+ struct lio *parent_lio;
+ struct octeon_soft_command *sc;
+ int status;
+ struct octeon_instr_pki_ih3 *pki_ih3;
+
+ lio_mgmt = GET_LIO_MGMT(netdev);
+ parent_lio = GET_LIO(lio_mgmt->parent_netdev);
+
+ /* Check for all conditions in which the current packet cannot be
+ * transmitted.
+ */
+ if (!(atomic_read(&lio_mgmt->ifstate) & LIO_IFSTATE_RUNNING) ||
+ (skb->len <= 0)) {
+ goto lio_xmit_failed;
+ }
+
+ if (octnet_iq_is_full(lio_mgmt->oct_dev, parent_lio->txq)) {
+ /* defer sending if queue is full */
+ return NETDEV_TX_BUSY;
+ }
+
+ sc = kzalloc(sizeof(*sc), GFP_ATOMIC);
+ if (!sc)
+ goto lio_xmit_failed;
+
+ if (skb_shinfo(skb)->nr_frags == 0) {
+ sc->dmadptr = dma_map_single(&lio_mgmt->oct_dev->pci_dev->dev,
+ skb->data,
+ skb->len, DMA_TO_DEVICE);
+ if (dma_mapping_error
+ (&lio_mgmt->oct_dev->pci_dev->dev, sc->dmadptr)) {
+ return NETDEV_TX_BUSY;
+ }
+ sc->virtdptr = skb->data;
+ sc->datasize = skb->len;
+ sc->ctxptr = skb; /* to be freed in sent callback */
+ sc->dmarptr = 0;
+ sc->rdatasize = 0;
+ sc->iq_no = parent_lio->txq; /* default input queue */
+ octeon_prepare_soft_command(lio_mgmt->oct_dev, sc, OPCODE_OVS,
+ OPCODE_VSW_HOST_COMM_PKT_DATA, 0, 0,
+ 0);
+
+ /*prepare softcommand uses ATOMIC TAG, change it to ORDERED */
+ pki_ih3 = (struct octeon_instr_pki_ih3 *)&sc->cmd.cmd3.pki_ih3;
+ pki_ih3->tag = LIO_DATA((lio_mgmt->oct_dev->instr_queue
+ [sc->iq_no]->txpciq.s.port));
+ pki_ih3->tagtype = ORDERED_TAG;
+
+ sc->callback = packet_sent_callback;
+ sc->callback_arg = sc;
+ status = octeon_send_soft_command(lio_mgmt->oct_dev, sc);
+ if (status == IQ_SEND_FAILED) {
+ dma_unmap_single(&lio_mgmt->oct_dev->pci_dev->dev,
+ sc->dmadptr, sc->datasize,
+ DMA_TO_DEVICE);
+ goto lio_xmit_failed;
+ }
+
+ if (status == IQ_SEND_STOP)
+ stop_txq(netdev);
+ } else {
+ goto lio_xmit_failed;
+ }
+
+ netdev->stats.tx_packets++;
+ netdev->stats.tx_bytes += skb->len;
+
+ return NETDEV_TX_OK;
+
+lio_xmit_failed:
+ netdev->stats.tx_dropped++;
+ dev_kfree_skb_any(skb);
+ return NETDEV_TX_OK;
+}
+
+static void rx_tasklet(unsigned long data)
+{
+ struct net_device *netdev = (struct net_device *)(data);
+ struct lio_mgmt *lio_mgmt = GET_LIO_MGMT(netdev);
+ struct lio_mgmt_rx_pkt *rxpkt, *tmp;
+
+ list_for_each_entry_safe(rxpkt, tmp, &lio_mgmt->rx_pkts_list, list) {
+ netif_rx(rxpkt->skb);
+ list_del(&rxpkt->list);
+ kfree(rxpkt);
+ }
+}
+
+static int lio_mgmt_rx(struct octeon_recv_info *recv_info, void *buf)
+{
+ struct octeon_recv_pkt *recv_pkt = recv_info->recv_pkt;
+ struct sk_buff *skb;
+ struct lio_mgmt *lio_mgmt = GET_LIO_MGMT(netdev);
+ unsigned int pkt_size = 0;
+ unsigned char *pkt_ptr;
+ struct lio_mgmt_rx_pkt *rxpkt;
+ int i;
+
+ /* Do not proceed if the interface is not in RUNNING state. */
+ if (!ifstate_check((struct lio *)lio_mgmt, LIO_IFSTATE_RUNNING)) {
+ for (i = 0; i < recv_pkt->buffer_count; i++)
+ recv_buffer_free(recv_pkt->buffer_ptr[i]);
+
+ octeon_free_recv_info(recv_info);
+ return 0;
+ }
+
+ pkt_size = recv_pkt->buffer_size[0];
+ pkt_ptr = get_rbd(recv_pkt->buffer_ptr[0]);
+
+ skb = netdev_alloc_skb_ip_align(netdev, pkt_size);
+ if (likely(skb))
+ skb_copy_to_linear_data(skb, pkt_ptr, pkt_size);
+
+ skb_put(skb, pkt_size);
+ netdev->stats.rx_packets++;
+ netdev->stats.rx_bytes += skb->len;
+
+ skb->dev = netdev;
+ skb->protocol = eth_type_trans(skb, skb->dev);
+ /* checksum has already been verified */
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+ rxpkt = kmalloc(sizeof(*rxpkt), GFP_ATOMIC);
+ if (rxpkt) {
+ INIT_LIST_HEAD(&rxpkt->list);
+ rxpkt->skb = skb;
+ list_add_tail(&rxpkt->list, &lio_mgmt->rx_pkts_list);
+ }
+
+ tasklet_schedule(&lio_mgmt->rx_tasklet);
+
+ for (i = 0; i < recv_pkt->buffer_count; i++)
+ recv_buffer_free(recv_pkt->buffer_ptr[i]);
+
+ octeon_free_recv_info(recv_info);
+ return 0;
+}
+
+const struct net_device_ops liocomdevops = {
+ .ndo_open = lio_mgmt_open,
+ .ndo_stop = lio_mgmt_stop,
+ .ndo_start_xmit = lio_mgmt_xmit,
+};
+
+static int __lio_mgmt_init(struct octeon_device *octdev)
+{
+ struct lio_mgmt *lio_mgmt = NULL;
+ struct lio *parent_lio;
+
+ /* Register netdev only for pf 0 */
+ if (octdev->pf_num == 0) {
+ netdev = alloc_etherdev(LIO_MGMT_SIZE);
+ if (!netdev) {
+ dev_err(&octdev->pci_dev->dev, "Mgmt: Device allocation failed\n");
+ goto nic_dev_fail;
+ }
+
+ /* SET_NETDEV_DEV(netdev, &octdev->pci_dev->dev); */
+ netdev->netdev_ops = &liocomdevops;
+
+ lio_mgmt = GET_LIO_MGMT(netdev);
+ memset(lio_mgmt, 0, LIO_MGMT_SIZE);
+ lio_mgmt->oct_dev = octdev;
+
+ /*use ifidx zero of pf */
+ lio_mgmt->parent_netdev = octdev->props[0].netdev;
+ parent_lio = GET_LIO(lio_mgmt->parent_netdev);
+
+ lio_mgmt->dev_capability = NETIF_F_HIGHDMA
+ | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_RXCSUM;
+ lio_mgmt->enc_dev_capability = NETIF_F_IP_CSUM
+ | NETIF_F_IPV6_CSUM | NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
+
+ netdev->vlan_features = lio_mgmt->dev_capability;
+ netdev->features = lio_mgmt->dev_capability;
+ netdev->hw_features = lio_mgmt->dev_capability;
+
+ lio_mgmt->linfo = parent_lio->linfo;
+
+ eth_hw_addr_random(netdev);
+ }
+ if (octdev->pf_num == 0) {
+ char name[IFNAMSIZ];
+ int dev_id, fn;
+
+ tasklet_init(&lio_mgmt->rx_tasklet, rx_tasklet, (u64)netdev);
+ INIT_LIST_HEAD(&lio_mgmt->rx_pkts_list);
+
+ dev_id = (octdev->pci_dev->devfn & 0xff) >> 3;
+ fn = octdev->pci_dev->devfn & 0x7;
+
+ if (fn) {
+ snprintf(name, IFNAMSIZ, "lio-p%ds%df%d-mgmt",
+ octdev->pci_dev->bus->number, dev_id, fn);
+ } else {
+ snprintf(name, IFNAMSIZ, "lio-p%ds%d-mgmt",
+ octdev->pci_dev->bus->number, dev_id);
+ }
+
+ strncpy(netdev->name, name, sizeof(netdev->name) - 1);
+
+ /* Register the network device with the OS */
+ if (register_netdev(netdev)) {
+ dev_err(&octdev->pci_dev->dev, "Mgmt: Device registration failed\n");
+ goto nic_dev_fail;
+ }
+
+ netif_carrier_on(netdev);
+ ifstate_set((struct lio *)lio_mgmt, LIO_IFSTATE_REGISTERED);
+ /* Register RX dispatch function */
+ if (octeon_register_dispatch_fn(octdev, OPCODE_OVS,
+ OPCODE_VSW_HOST_COMM_PKT_DATA,
+ lio_mgmt_rx, octdev)) {
+ goto nic_dev_fail;
+ }
+ }
+
+ return 0;
+
+nic_dev_fail:
+ if (netdev) {
+ struct lio_mgmt *lio_mgmt = GET_LIO_MGMT(netdev);
+
+ if (atomic_read(&lio_mgmt->ifstate) &
+ LIO_IFSTATE_REGISTERED)
+ unregister_netdev(netdev);
+
+ free_netdev(netdev);
+ netdev = NULL;
+ }
+
+ netdev = NULL;
+ return -ENOMEM;
+}
+
+static void __lio_mgmt_exit(void)
+{
+ pr_info("LiquidIO Communication module is now unloaded\n");
+
+ if (netdev) {
+ struct lio_mgmt *lio_mgmt = GET_LIO_MGMT(netdev);
+
+ if (atomic_read(&lio_mgmt->ifstate) & LIO_IFSTATE_RUNNING)
+ txqs_stop(netdev);
+
+ if (atomic_read(&lio_mgmt->ifstate) &
+ LIO_IFSTATE_REGISTERED)
+ unregister_netdev(netdev);
+
+ free_netdev(netdev);
+ netdev = NULL;
+ }
+}
+
+int lio_mgmt_init(struct octeon_device *octdev)
+{
+ return __lio_mgmt_init(octdev);
+}
+
+void lio_mgmt_exit(void)
+{
+ __lio_mgmt_exit();
+}
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_console.c b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
index 53f38d0..c7b05b7 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_console.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
@@ -724,18 +724,22 @@ static int octeon_console_read(struct octeon_device *oct, u32 console_num,
}
#define FBUF_SIZE (4 * 1024 * 1024)
-u8 fbuf[FBUF_SIZE];
int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
size_t size)
{
int ret = 0;
- u8 *p = fbuf;
u32 crc32_result;
u64 load_addr;
u32 image_len;
struct octeon_firmware_file_header *h;
u32 i, rem;
+ char *bootcmd;
+ char date[30];
+ struct timeval time;
+ struct tm tm;
+ size_t cmdlen;
+
if (size < sizeof(struct octeon_firmware_file_header)) {
dev_err(&oct->pci_dev->dev, "Firmware file too small (%d < %d).\n",
@@ -805,10 +809,9 @@ int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
else
size = FBUF_SIZE;
- memcpy(p, data, size);
-
/* download the image */
- octeon_pci_write_core_mem(oct, load_addr, p, (u32)size);
+ octeon_pci_write_core_mem(
+ oct, load_addr, (u8 *)data, (u32)size);
data += size;
rem -= (u32)size;
@@ -818,8 +821,18 @@ int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
dev_info(&oct->pci_dev->dev, "Writing boot command: %s\n",
h->bootcmd);
- /* Invoke the bootcmd */
- ret = octeon_console_send_cmd(oct, h->bootcmd, 50);
+ /*Get time of the day*/
+ do_gettimeofday(&time);
+ time_to_tm(time.tv_sec, (-sys_tz.tz_minuteswest) * 60, &tm);
+ snprintf(date, 30, " date=%04ld.%02d.%02d-%02d:%02d:%02d",
+ tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
+ tm.tm_hour, tm.tm_min, tm.tm_sec);
+ cmdlen = strlen(h->bootcmd) + sizeof(date) + 1;
+ bootcmd = kmalloc(cmdlen, GFP_KERNEL);
+ strncpy(bootcmd, h->bootcmd, cmdlen);
+ strcat(bootcmd, date);
+ ret = octeon_console_send_cmd(oct, bootcmd, 50);
+ kfree(bootcmd);
return 0;
}
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_main.h b/drivers/net/ethernet/cavium/liquidio/octeon_main.h
index bed9ef1..3a36105 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_main.h
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_main.h
@@ -198,4 +198,13 @@ sleep_timeout_cond(wait_queue_head_t *wait_queue,
#define ROUNDUP128(val) (((val) + 127) & 0xffffff80)
#endif
+/* Initializes the LiquidIO management interface module
+ * @param octdev - octeon device pointer
+ * @returns 0 if init is success, -1 otherwise
+ */
+int lio_mgmt_init(struct octeon_device *octdev);
+
+/* De-initializes the LiquidIO management interface module */
+void lio_mgmt_exit(void);
+
#endif /* _OCTEON_MAIN_H_ */
^ permalink raw reply related
* Re: [PATCH net-next] net: phy: Relax error checking on sysfs_create_link()
From: Florian Fainelli @ 2017-05-27 15:40 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, Woojung.Huh
In-Reply-To: <20170527033418.3583-1-f.fainelli@gmail.com>
On May 26, 2017 8:34:18 PM PDT, Florian Fainelli <f.fainelli@gmail.com> wrote:
>Some Ethernet drivers will attach/connect to a PHY device before
>calling
>register_netdevice() which is responsible for calling
>netdev_register_kobject()
>which would do the network device's kobject initialization. In such a
>case,
>sysfs_create_link() would return -ENOENT because the network device's
>kobject
>is not ready yet, and we would fail to connect to the PHY device.
>
>In order to keep things simple and symetrical, we just take the success
>path as
>indicative of the ability to access the network device's kobject, and
>create
>the second link if that's the case.
>
>Fixes: 5568363f0cb3 ("net: phy: Create sysfs reciprocal links for
>attached_dev/phydev")
>Reported-by: Woojung Hung <Woojung.Huh@microchip.com>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>---
> drivers/net/phy/phy_device.c | 28 ++++++++++++++++++++--------
> include/linux/phy.h | 2 ++
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/phy/phy_device.c
>b/drivers/net/phy/phy_device.c
>index f84414b8f2ee..523366bd705a 100644
>--- a/drivers/net/phy/phy_device.c
>+++ b/drivers/net/phy/phy_device.c
>@@ -960,15 +960,25 @@ int phy_attach_direct(struct net_device *dev,
>struct phy_device *phydev,
>
> phydev->attached_dev = dev;
> dev->phydev = phydev;
>+
>+ /* Some Ethernet drivers try to connect to a PHY device before
>+ * calling register_netdevice() -> netdev_register_kobject() and
>+ * does the dev->dev.kobj initialization. Here we only check for
>+ * success which indicates that the network device kobject is
>+ * ready. Once we do that we still need to keep track of whether
>+ * links were successfully set up or not for phy_detach() to
>+ * remove them accordingly.
>+ */
> err = sysfs_create_link(&phydev->mdio.dev.kobj, &dev->dev.kobj,
> "attached_dev");
>- if (err)
>- goto error;
>+ if (!err) {
>+ err = sysfs_create_link(&dev->dev.kobj, &phydev->mdio.dev.kobj,
>+ "phydev");
>+ if (err)
>+ goto error;
>
>- err = sysfs_create_link(&dev->dev.kobj, &phydev->mdio.dev.kobj,
>- "phydev");
>- if (err)
>- goto error;
>+ phydev->sysfs_links = true;
>+ }
>
We should not assume that this Boolean will be true or false if we enter this function a second time, v2 coming.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] net: ndisc.c: reduce size of __ndisc_fill_addr_option()
From: Alexey Dobriyan @ 2017-05-27 15:15 UTC (permalink / raw)
To: Linyu.Yuan; +Cc: netdev, davem
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -148,17 +148,18 @@ void __ndisc_fill_addr_option(struct sk_buff *skb, int type, void *data,
> space -= data_len;
> - if (space > 0)
> - memset(opt, 0, space);
> +
> + memset(opt, 0, space);
This can't be right.
And what size are you reducing?
^ permalink raw reply
* Re: [PATCH] dsa: mv88e6xxx: fix returnvar.cocci warnings
From: Andrew Lunn @ 2017-05-27 14:51 UTC (permalink / raw)
To: Julia Lawall
Cc: netdev, Vivien Didelot, Florian Fainelli, linux-kernel,
kbuild-all
In-Reply-To: <alpine.DEB.2.20.1705270635210.2907@hadrien>
On Sat, May 27, 2017 at 06:38:14AM +0200, Julia Lawall wrote:
> Remove unneeded variable used to store return value.
>
> Generated by: scripts/coccinelle/misc/returnvar.cocci
Hi Julia
Thanks for the patch. However, Vivien already submitted a patch.
Andrew
^ permalink raw reply
* [PATCH V6 net-next iproute] ip: Add support for netdev events to monitor
From: Vladislav Yasevich @ 2017-05-27 14:14 UTC (permalink / raw)
To: netdev; +Cc: dsahern, roopa, jiri, vfalico, andy, Vladislav Yasevich
In-Reply-To: <1495894476-9726-1-git-send-email-vyasevic@redhat.com>
Add IFLA_EVENT handling so that event types can be viewed with
'monitor' command. This gives a little more information for why
a given message was receivied.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
include/linux/if_link.h | 11 +++++++++++
ip/ipaddress.c | 21 +++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 5a3a048..c0a6769 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -157,6 +157,7 @@ enum {
IFLA_GSO_MAX_SIZE,
IFLA_PAD,
IFLA_XDP,
+ IFLA_EVENT,
__IFLA_MAX
};
@@ -909,4 +910,14 @@ enum {
#define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
+enum {
+ IFLA_EVENT_NONE,
+ IFLA_EVENT_REBOOT, /* internal reset / reboot */
+ IFLA_EVENT_FEATURES, /* change in offload features */
+ IFLA_EVENT_BONDING_FAILOVER, /* hange in active slave */
+ IFLA_EVENT_NOTIFY_PEERS, /* re-sent grat. arp/ndisc */
+ IFLA_EVENT_IGMP_RESEND, /* re-sent IGMP JOIN */
+ IFLA_EVENT_BONDING_OPTIONS, /* change in bonding options */
+};
+
#endif /* _LINUX_IF_LINK_H */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b8d9c7d..c6e7413 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -753,6 +753,24 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
return 0;
}
+static const char *netdev_events[] = {"NONE",
+ "REBOOT",
+ "FEATURE CHANGE",
+ "BONDING FAILOVER",
+ "NOTIFY PEERS",
+ "RESEND IGMP",
+ "BONDING OPTION"};
+
+static void print_dev_event(FILE *f, __u32 event)
+{
+ if (event >= ARRAY_SIZE(netdev_events))
+ fprintf(f, "event %d ", event);
+ else {
+ if (event)
+ fprintf(f, "event %s ", netdev_events[event]);
+ }
+}
+
int print_linkinfo(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg)
{
@@ -858,6 +876,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
if (filter.showqueue)
print_queuelen(fp, tb);
+ if (tb[IFLA_EVENT])
+ print_dev_event(fp, rta_getattr_u32(tb[IFLA_EVENT]));
+
if (!filter.family || filter.family == AF_PACKET || show_details) {
SPRINT_BUF(b1);
fprintf(fp, "%s", _SL_);
--
2.7.4
^ permalink raw reply related
* [PATCH V6 net-next 2/2] bonding: Prevent duplicate userspace notification
From: Vladislav Yasevich @ 2017-05-27 14:14 UTC (permalink / raw)
To: netdev; +Cc: dsahern, roopa, jiri, vfalico, andy, Vladislav Yasevich,
David Ahern
In-Reply-To: <1495894476-9726-1-git-send-email-vyasevic@redhat.com>
Whenever a user changes bonding options, a NETDEV_CHANGEINFODATA
notificatin is generated which results in a rtnelink message to
be sent. While runnig 'ip monitor', we can actually see 2 messages,
one a result of the event, and the other a result of state change
that is generated bo netdev_state_change(). However, this is not
always the case. If bonding changes were done via sysfs or ifenslave
(old ioctl interface), then only 1 message is seen.
This patch removes duplicate messages in the case of using netlink
to configure bonding. It introduceds a separte function that
triggers a netdev event and uses that function in the syfs and ioctl
cases.
This was discovered while auditing all the different envents and
continues the effort of cleaning up duplicated netlink messages.
CC: David Ahern <dsa@cumulusnetworks.com>
CC: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
drivers/net/bonding/bond_main.c | 3 ++-
drivers/net/bonding/bond_options.c | 27 +++++++++++++++++++++++++--
include/net/bond_options.h | 2 ++
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7331331..d7aa137 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3481,7 +3481,8 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
case BOND_CHANGE_ACTIVE_OLD:
case SIOCBONDCHANGEACTIVE:
bond_opt_initstr(&newval, slave_dev->name);
- res = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval);
+ res = __bond_opt_set_notify(bond, BOND_OPT_ACTIVE_SLAVE,
+ &newval);
break;
default:
res = -EOPNOTSUPP;
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 1bcbb89..8ca6833 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -673,7 +673,30 @@ int __bond_opt_set(struct bonding *bond,
out:
if (ret)
bond_opt_error_interpret(bond, opt, ret, val);
- else if (bond->dev->reg_state == NETREG_REGISTERED)
+
+ return ret;
+}
+/**
+ * __bond_opt_set_notify - set a bonding option
+ * @bond: target bond device
+ * @option: option to set
+ * @val: value to set it to
+ *
+ * This function is used to change the bond's option value and trigger
+ * a notification to user sapce. It can be used for both enabling/changing
+ * an option and for disabling it. RTNL lock must be obtained before calling
+ * this function.
+ */
+int __bond_opt_set_notify(struct bonding *bond,
+ unsigned int option, struct bond_opt_value *val)
+{
+ int ret = -ENOENT;
+
+ ASSERT_RTNL();
+
+ ret = __bond_opt_set(bond, option, val);
+
+ if (!ret && (bond->dev->reg_state == NETREG_REGISTERED))
call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
return ret;
@@ -696,7 +719,7 @@ int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf)
if (!rtnl_trylock())
return restart_syscall();
bond_opt_initstr(&optval, buf);
- ret = __bond_opt_set(bond, option, &optval);
+ ret = __bond_opt_set_notify(bond, option, &optval);
rtnl_unlock();
return ret;
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 1797235..d79d28f 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -104,6 +104,8 @@ struct bond_option {
int __bond_opt_set(struct bonding *bond, unsigned int option,
struct bond_opt_value *val);
+int __bond_opt_set_notify(struct bonding *bond, unsigned int option,
+ struct bond_opt_value *val);
int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf);
const struct bond_opt_value *bond_opt_parse(const struct bond_option *opt,
--
2.7.4
^ permalink raw reply related
* [PATCH V6 net-next 1/2] rtnl: Add support for netdev event to link messages
From: Vladislav Yasevich @ 2017-05-27 14:14 UTC (permalink / raw)
To: netdev; +Cc: dsahern, roopa, jiri, vfalico, andy, Vladislav Yasevich
In-Reply-To: <1495894476-9726-1-git-send-email-vyasevic@redhat.com>
When netdev events happen, a rtnetlink_event() handler will send
messages for every event in it's white list. These messages contain
current information about a particular device, but they do not include
the iformation about which event just happened. So, it is impossible
to tell what just happend for these events.
This patch adds a new extension to RTM_NEWLINK message called IFLA_EVENT
that would have an encoding of event that triggered this
message. This would allow the the message consumer to easily determine
if it needs to perform certain actions.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
include/linux/rtnetlink.h | 3 +-
include/uapi/linux/if_link.h | 11 ++++++++
net/core/dev.c | 2 +-
net/core/rtnetlink.c | 65 ++++++++++++++++++++++++++++++++++++++------
4 files changed, 70 insertions(+), 11 deletions(-)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 57e5484..dea59c8 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -18,7 +18,8 @@ extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change, gfp_t flags);
struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
- unsigned change, gfp_t flags);
+ unsigned change, u32 event,
+ gfp_t flags);
void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev,
gfp_t flags);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 15ac203..8ed679f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -157,6 +157,7 @@ enum {
IFLA_GSO_MAX_SIZE,
IFLA_PAD,
IFLA_XDP,
+ IFLA_EVENT,
__IFLA_MAX
};
@@ -911,4 +912,14 @@ enum {
#define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
+enum {
+ IFLA_EVENT_NONE,
+ IFLA_EVENT_REBOOT, /* internal reset / reboot */
+ IFLA_EVENT_FEATURES, /* change in offload features */
+ IFLA_EVENT_BONDING_FAILOVER, /* change in active slave */
+ IFLA_EVENT_NOTIFY_PEERS, /* re-sent grat. arp/ndisc */
+ IFLA_EVENT_IGMP_RESEND, /* re-sent IGMP JOIN */
+ IFLA_EVENT_BONDING_OPTIONS, /* change in bonding options */
+};
+
#endif /* _UAPI_LINUX_IF_LINK_H */
diff --git a/net/core/dev.c b/net/core/dev.c
index 3d98fbf..06e0a74 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7084,7 +7084,7 @@ static void rollback_registered_many(struct list_head *head)
if (!dev->rtnl_link_ops ||
dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U,
+ skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U, 0,
GFP_KERNEL);
/*
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index dab2834..07218eb 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -941,6 +941,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
+ nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_SWITCH_ID */
+ nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
+ rtnl_xdp_size() /* IFLA_XDP */
+ + nla_total_size(4) /* IFLA_EVENT */
+ nla_total_size(1); /* IFLA_PROTO_DOWN */
}
@@ -1282,9 +1283,40 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
return err;
}
+static u32 rtnl_get_event(unsigned long event)
+{
+ u32 rtnl_event_type = IFLA_EVENT_NONE;
+
+ switch (event) {
+ case NETDEV_REBOOT:
+ rtnl_event_type = IFLA_EVENT_REBOOT;
+ break;
+ case NETDEV_FEAT_CHANGE:
+ rtnl_event_type = IFLA_EVENT_FEATURES;
+ break;
+ case NETDEV_BONDING_FAILOVER:
+ rtnl_event_type = IFLA_EVENT_BONDING_FAILOVER;
+ break;
+ case NETDEV_NOTIFY_PEERS:
+ rtnl_event_type = IFLA_EVENT_NOTIFY_PEERS;
+ break;
+ case NETDEV_RESEND_IGMP:
+ rtnl_event_type = IFLA_EVENT_IGMP_RESEND;
+ break;
+ case NETDEV_CHANGEINFODATA:
+ rtnl_event_type = IFLA_EVENT_BONDING_OPTIONS;
+ break;
+ default:
+ break;
+ }
+
+ return rtnl_event_type;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
- unsigned int flags, u32 ext_filter_mask)
+ unsigned int flags, u32 ext_filter_mask,
+ u32 event)
{
struct ifinfomsg *ifm;
struct nlmsghdr *nlh;
@@ -1333,6 +1365,11 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down))
goto nla_put_failure;
+ if (event != IFLA_EVENT_NONE) {
+ if (nla_put_u32(skb, IFLA_EVENT, event))
+ goto nla_put_failure;
+ }
+
if (rtnl_fill_link_ifmap(skb, dev))
goto nla_put_failure;
@@ -1467,6 +1504,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
[IFLA_LINK_NETNSID] = { .type = NLA_S32 },
[IFLA_PROTO_DOWN] = { .type = NLA_U8 },
[IFLA_XDP] = { .type = NLA_NESTED },
+ [IFLA_EVENT] = { .type = NLA_U32 },
};
static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -1626,7 +1664,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, 0,
flags,
- ext_filter_mask);
+ ext_filter_mask, 0);
if (err < 0) {
if (likely(skb->len))
@@ -2736,7 +2774,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
return -ENOBUFS;
err = rtnl_fill_ifinfo(nskb, dev, RTM_NEWLINK, NETLINK_CB(skb).portid,
- nlh->nlmsg_seq, 0, 0, ext_filter_mask);
+ nlh->nlmsg_seq, 0, 0, ext_filter_mask, 0);
if (err < 0) {
/* -EMSGSIZE implies BUG in if_nlmsg_size */
WARN_ON(err == -EMSGSIZE);
@@ -2808,7 +2846,8 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
}
struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
- unsigned int change, gfp_t flags)
+ unsigned int change,
+ u32 event, gfp_t flags)
{
struct net *net = dev_net(dev);
struct sk_buff *skb;
@@ -2819,7 +2858,7 @@ struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
if (skb == NULL)
goto errout;
- err = rtnl_fill_ifinfo(skb, dev, type, 0, 0, change, 0, 0);
+ err = rtnl_fill_ifinfo(skb, dev, type, 0, 0, change, 0, 0, event);
if (err < 0) {
/* -EMSGSIZE implies BUG in if_nlmsg_size() */
WARN_ON(err == -EMSGSIZE);
@@ -2840,18 +2879,25 @@ void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags)
rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
}
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
- gfp_t flags)
+static void rtmsg_ifinfo_event(int type, struct net_device *dev,
+ unsigned int change, u32 event,
+ gfp_t flags)
{
struct sk_buff *skb;
if (dev->reg_state != NETREG_REGISTERED)
return;
- skb = rtmsg_ifinfo_build_skb(type, dev, change, flags);
+ skb = rtmsg_ifinfo_build_skb(type, dev, change, event, flags);
if (skb)
rtmsg_ifinfo_send(skb, dev, flags);
}
+
+void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
+ gfp_t flags)
+{
+ rtmsg_ifinfo_event(type, dev, change, IFLA_EVENT_NONE, flags);
+}
EXPORT_SYMBOL(rtmsg_ifinfo);
static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
@@ -4165,7 +4211,8 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_NOTIFY_PEERS:
case NETDEV_RESEND_IGMP:
case NETDEV_CHANGEINFODATA:
- rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
+ rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, rtnl_get_event(event),
+ GFP_KERNEL);
break;
default:
break;
--
2.7.4
^ permalink raw reply related
* [PATCH V6 net-next 0/2] rtnetlink: Updates to rtnetlink_event()
From: Vladislav Yasevich @ 2017-05-27 14:14 UTC (permalink / raw)
To: netdev; +Cc: dsahern, roopa, jiri, vfalico, andy, Vladislav Yasevich
First is the patch to add IFLA_EVENT attribute to the netlink message. It
supports only currently white-listed events.
Like before, this is just an attribute that gets added to the rtnetlink
message only when the messaged was generated as a result of a netdev event.
In my case, this is necessary since I want to trap NETDEV_NOTIFY_PEERS
event (also possibly NETDEV_RESEND_IGMP event) and perform certain actions
in user space. This is not possible since the messages generated as
a result of netdev events do not usually contain any changed data. They
are just notifications. This patch exposes this notification type to
userspace.
Second, I remove duplicate messages that a result of a change to bonding
options. If netlink is used to configure bonding options, 2 messages
are generated, one as a result NETDEV_CHANGEINFODATA event triggered by
bonding code and one a result of device state changes triggered by
netdev_state_change (called from do_setlink).
V6: Updated names and refactored to make it less tied to netdev events.
(From David Ahern)
V5: Rebased. Added iproute2 patch to the series.
V4:
* Removed the patch the removed NETDEV_CHANGENAME from event whitelist.
It doesn't trigger duplicate messages since name changes can only be
done while device is down and netdev_state_change() doesn't report
changes while device is down.
* Added a patch to clean-up duplicate messages on bonding option changes.
V3: Rebased. Cleaned-up duplicate event.
V2: Added missed events (from David Ahern)
Vladislav Yasevich (2):
rtnl: Add support for netdev event to link messages
bonding: Prevent duplicate userspace notification
drivers/net/bonding/bond_main.c | 3 +-
drivers/net/bonding/bond_options.c | 27 ++++++++++++++--
include/linux/rtnetlink.h | 3 +-
include/net/bond_options.h | 2 ++
include/uapi/linux/if_link.h | 11 +++++++
net/core/dev.c | 2 +-
net/core/rtnetlink.c | 65 ++++++++++++++++++++++++++++++++------
7 files changed, 99 insertions(+), 14 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH net-next v2 0/8] net: extend RTM_GETROUTE to return fib result
From: David Ahern @ 2017-05-27 14:02 UTC (permalink / raw)
To: Roopa Prabhu, David Miller
Cc: Rami Rosen, netdev@vger.kernel.org, Nikolay Aleksandrov
In-Reply-To: <CAJieiUiK=G1B-LnNH_=MYjFjqQVWu2n15a9Tg5fi_fyvSDpcFQ@mail.gmail.com>
On 5/27/17 12:00 AM, Roopa Prabhu wrote:
> On Fri, May 26, 2017 at 11:18 AM, David Miller <davem@davemloft.net> wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>> Date: Thu, 25 May 2017 10:42:32 -0700
>>
>>> This series adds a new RTM_F_FIB_MATCH flag to return matched fib result
>>> with RTM_GETROUTE. This is useful for applications and protocols in
>>> userspace wanting to query the selected route.
>>
>> Looks good, series applied, thanks.
>>
>
> thank you.
>
>> Have you considered taking this further and allowing one to see which
>> nexthop a route lookup picked?
>
> since the default RTM_GETROUTE output gives most of the attributes
> from the resolved dst,
> have not considered adding more to it yet...but certainly can if
> needed in the future.
>
One extension is to pass in prefix and length (plus any options to such
as metric to uniquely discriminate a route) and get back the route
entry. It is needed to retrieve the BPF code for routes with a bpf
encap. This patch set makes it easier.
^ permalink raw reply
* Re: [PATCH v2 6/6] stmmac: pci: Remove setup handler indirection via stmmac_pci_info
From: Andy Shevchenko @ 2017-05-27 13:38 UTC (permalink / raw)
To: Jan Kiszka
Cc: Giuseppe Cavallaro, Alexandre Torgue, David Miller, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <6183ce221f01384f4d50b0cfd62beabe337f5b39.1495814872.git.jan.kiszka@siemens.com>
On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> By now, stmmac_pci_info only contains a single entry.
_For now_.
> Register this
> directly with the PCI device table, removing one indirection.
I am not sure this patch is needed.
Next time something comes up we would need to extend this and
effectively revert this change.
So, my vote is to leave it as is for now.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 5/6] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
From: Andy Shevchenko @ 2017-05-27 13:28 UTC (permalink / raw)
To: Jan Kiszka
Cc: Giuseppe Cavallaro, Alexandre Torgue, David Miller, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <6e12b2e63c7eaab6db937a99fedf79ec806d176f.1495814872.git.jan.kiszka@siemens.com>
On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.
> struct stmmac_pci_dmi_data {
> - const char *name;
> - const char *asset_tag;
> - unsigned int func;
> + int func;
> int phy_addr;
> };
Can we leave unsigned type here...
> -static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
> +static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data[] = {
> + {-1, -1},
> +};
> +static const struct stmmac_pci_dmi_data iot2040_stmmac_dmi_data[] = {
> + {-1, -1},
> +};
...and avoid this not so standard terminators?
> + .matches = {
> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "GalileoGen2"),
> + },
> + .driver_data = (void *)galileo_stmmac_dmi_data,
Can't be slightly better
.driver_data = &galileo_stmmac_dmi_data,
?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 3/6] stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
From: Andy Shevchenko @ 2017-05-27 13:25 UTC (permalink / raw)
To: Jan Kiszka
Cc: Giuseppe Cavallaro, Alexandre Torgue, David Miller, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <61af7f05bb92da3220ed5799dfa5bb84e102f067.1495814872.git.jan.kiszka@siemens.com>
On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Move the special case for the early Galileo firmware into
> quark_default_setup. This allows to use stmmac_pci_find_phy_addr for
> non-quark cases.
> ret = stmmac_pci_find_phy_addr(pdev, info);
> - if (ret < 0)
> - return ret;
> + if (ret < 0) {
> + /*
> + * Galileo boards with old firmware don't support DMI. We always
> + * use 1 here as PHY address, so at least the first found MAC
> + * controller would be probed.
> + */
> + if (!dmi_get_system_info(DMI_BOARD_NAME))
> + ret = 1;
> + else
> + return ret;
Perhaps
/* Return error to the caller on DMI enabled boards */
if (dmi_...)
return ret;
/*
* Comment goes here, I suppose.
*/
ret = 1;
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 2/6] stmmac: pci: Use stmmac_pci_info for all devices
From: Andy Shevchenko @ 2017-05-27 13:16 UTC (permalink / raw)
To: Jan Kiszka
Cc: Giuseppe Cavallaro, Alexandre Torgue, David Miller, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75Ve=CJ-ZRmaAFX+tezMJ4u11v=ptrDDORF485f3_iFEWeg@mail.gmail.com>
On Sat, May 27, 2017 at 4:13 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Make stmmac_default_data compatible with stmmac_pci_info.setup and use
>> an info structure for all devices. This allows to make the probing more
>> regular.
> Or converting defines first to PCI_DEVICE_ID_*
It looks even for previously mentioned approach we need to rename
constants first.
> and
>
> #define STMMAC_DEVICE(_vid, _did, info) { \
> PCI_DEVICE(PCI_VENDOR_ID_##_vid, PCI_DEVICE_ID_##_did),
> \
>
> which I like even better.
Or even
#define STMMAC_DEVICE(_vid, _did, info) { \
PCI_VDEVICE(_vid, PCI_DEVICE_ID_##_did), \
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 2/6] stmmac: pci: Use stmmac_pci_info for all devices
From: Andy Shevchenko @ 2017-05-27 13:13 UTC (permalink / raw)
To: Jan Kiszka
Cc: Giuseppe Cavallaro, Alexandre Torgue, David Miller, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <dc0eaa024838f4ef34913aed63315771ac81dcc1.1495814872.git.jan.kiszka@siemens.com>
On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Make stmmac_default_data compatible with stmmac_pci_info.setup and use
> an info structure for all devices. This allows to make the probing more
> regular.
> +#define STMMAC_DEVICE(vendor_id, dev_id, info) { \
> + PCI_DEVICE(vendor_id, dev_id), \
Perhaps
#define STMMAC_DEVICE(_vid, _did, info) { \
PCI_DEVICE(PCI_VENDOR_ID_##_vid, _did), \
Or converting defines first to PCI_DEVICE_ID_*
and
#define STMMAC_DEVICE(_vid, _did, info) { \
PCI_DEVICE(PCI_VENDOR_ID_##_vid, PCI_DEVICE_ID_##_did),
\
which I like even better.
> + .driver_data = (kernel_ulong_t)&info \
> + }
> +
> static const struct pci_device_id stmmac_id_table[] = {
> - {PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)},
> - {PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)},
> - {PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info},
> + STMMAC_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID, stmmac_pci_info),
> + STMMAC_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC,
> + stmmac_pci_info),
> + STMMAC_DEVICE(PCI_VENDOR_ID_INTEL, STMMAC_QUARK_ID, quark_pci_info),
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* TCP get SND_CWND change on loss event
From: Lars Erik Storbukås @ 2017-05-27 12:12 UTC (permalink / raw)
To: Netdev
I want to store the value of snd_cwnd when a congestion event occurs
(value before snd_cwnd is reduced), and the new value of snd_cwnd (the
value it has been reduced to). In other words: the congestion window
before and after a congestion event occurs.
I'm uncertain where (and how) it would be logical to implement this. I
have found two possible locations in the tcp_input.c where (I think)
it could be implemented:
static void tcp_cong_control(...) {
...
if (tcp_in_cwnd_reduction(sk)) {
struct tcp_sock *tp = tcp_sk(sk);
prior_congestion_window = tp->snd_cwnd;
/* Reduce cwnd if state mandates */
tcp_cwnd_reduction(sk, acked_sacked, flag);
reduced_congestion_window = tp->snd_cwnd;
}
...
}
or
static void tcp_fastretrans_alert(...) {
...
default:
...
struct tcp_sock *tp = tcp_sk(sk);
prior_congestion_window = tp->snd_cwnd;
/* Otherwise enter Recovery state */
tcp_enter_recovery(sk, (flag & FLAG_ECE));
fast_rexmit = 1;
reduced_congestion_window = tp->snd_cwnd;
...
}
Does anyone have advice on where (and how) to implement this? Does any
of the proposed solutions above seem logical?
/ Lars Erik Storbukås
^ permalink raw reply
* Re: [PATCH 1/2] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Ding Tianhong @ 2017-05-27 10:34 UTC (permalink / raw)
To: Alexander Duyck
Cc: Mark Rutland, Gabriele Paoloni, Asit K Mallick, Catalin Marinas,
Will Deacon, LinuxArm, Raj, Ashok, Bjorn Helgaas, Ganesh GR,
Jeff Kirsher, Bob Shaw, Casey Leedom, Patrick J Cramer, Arjun V.,
Michael Werner, linux-arm-kernel@lists.infradead.org, Amir Ancel,
Netdev, David Laight, Suravee Suthikulpanit, Robin Murphy,
David Miller, h
In-Reply-To: <CAKgT0UfwnGEf2yok5E2KeZPP2JkqhTpCf+25+A8j5jfupLRJ5A@mail.gmail.com>
On 2017/5/26 3:49, Alexander Duyck wrote:
> On Thu, May 25, 2017 at 6:35 AM, Ding Tianhong <dingtianhong@huawei.com> wrote:
>>
>> On 2017/5/9 8:48, Casey Leedom wrote:
>>>
>>> | From: Alexander Duyck <alexander.duyck@gmail.com>
>>> | Date: Saturday, May 6, 2017 11:07 AM
>>> |
>>> | | From: Ding Tianhong <dingtianhong@huawei.com>
>>> | | Date: Fri, May 5, 2017 at 8:08 PM
>>> | |
>>> | | According the suggestion, I could only think of this code:
>>> | | ..
>>> |
>>> | This is a bit simplistic but it is a start.
>>>
>>> Yes, something tells me that this is going to be more complicated than any
>>> of us like ...
>>>
>>> | The other bit I was getting at is that we need to update the core PCIe
>>> | code so that when we configure devices and the root complex reports no
>>> | support for relaxed ordering it should be clearing the relaxed
>>> | ordering bits in the PCIe configuration registers on the upstream
>>> | facing devices.
>>>
>>> Of course, this can be written to by the Driver at any time ... and is in
>>> the case of the cxgb4 Driver ...
>>>
>>> After a lot of rummaging around, it does look like KVM prohibits writes to
>>> the PCIe Capability Device Control register in drivers/xen/xen-pciback/
>>> conf_space_capability.c and conf_space.c simply because writes aren't
>>> allowed unless "permissive" is set. So it ~looks~ like a driver running in
>>> a Virtual Machine can't turn Enable Relaxed Ordering back on ...
>>>
>>> | The last bit we need in all this is a way to allow for setups where
>>> | peer-to-peer wants to perform relaxed ordering but for writes to the
>>> | host we have to not use relaxed ordering. For that we need to enable a
>>> | special case and that isn't handled right now in any of the solutions
>>> | we have coded up so far.
>>>
>>> Yes, we do need this.
>>>
>>>
>>> | From: Alexander Duyck <alexander.duyck@gmail.com>
>>> | Date: Saturday, May 8, 2017 08:22 AM
>>> |
>>> | The problem is we need to have something that can be communicated
>>> | through a VM. Your change doesn't work in that regard. That was why I
>>> | suggested just updating the code so that we when we initialized PCIe
>>> | devices what we do is either set or clear the relaxed ordering bit in
>>> | the PCIe device control register. That way when we direct assign an
>>> | interface it could know just based on the bits int the PCIe
>>> | configuration if it could use relaxed ordering or not.
>>> |
>>> | At that point the driver code itself becomes very simple since you
>>> | could just enable the relaxed ordering by default in the igb/ixgbe
>>> | driver and if the bit is set or cleared in the PCIe configuration then
>>> | we are either sending with relaxed ordering requests or not and don't
>>> | have to try and locate the root complex.
>>> |
>>> | So from the sound of it Casey has a special use case where he doesn't
>>> | want to send relaxed ordering frames to the root complex, but instead
>>> | would like to send them to another PCIe device. To do that he needs to
>>> | have a way to enable the relaxed ordering bit in the PCIe
>>> | configuration but then not send any to the root complex. Odds are that
>>> | is something he might be able to just implement in the driver, but is
>>> | something that may become a more general case in the future. I don't
>>> | see our change here impacting it as long as we keep the solution
>>> | generic and mostly confined to when we instantiate the devices as the
>>> | driver could likely make the decision to change the behavior later.
>>>
>>> It's not just me. Intel has said that while RO directed at the Root
>>> Complex Host Coherent Memory has a performance bug (not Data Corruption),
>>> it's a performance win for Peer-to-Peer writes to MMIO Space. (I'll be very
>>> interested in hearing what the bug is if we get that much detail. The very
>>> same TLPs directed to the Root Complex Port without Relaxed Ordering set get
>>> good performance. So this is essentially a bug in the hardware that was
>>> ~trying~ to implement a performance win.)
>>>
>>> Meanwhile, I currently only know of a single PCIe End Point which causes
>>> catastrophic results: the AMD A1100 ARM SoC ("SEATTLE"). And it's not even
>>> clear that product is even alive anymore since I haven't been able to get
>>> any responses from them for several months.
>>>
>>> What I'm saying is: let's try to architect a solution which doesn't throw
>>> the baby out with the bath water ...
>>>
>>> I think that if a Device's Root Complex Port has problems with Relaxed
>>> Ordering, it ~probably~ makes sense to turn off the PCIe Capability Device
>>> Control[Enable Relaxed Ordering] when we assign a device to a Virtual
>>> Machine since the Device Driver can no longer query the Relaxed Ordering
>>> Support of the Root Complex Port. The only down side of this would be if we
>>> assigned two Peers to a VM in an application which wanted to do Peer-to-Peer
>>> transfers. But that seems like a hard application to support in any case
>>> since the PCI Bus:Slot.Function IDs for assigned Devices within a VM don't
>>> match the actual values.
>>>
>>> For Devices running in the base OS/Hypervisor, their Drivers can query the
>>> Relaxed Ordering Support for the Root Complex Port or a Peer Device. So a
>>> simple flag within the (struct pci_dev *)->dev_flags would serve for that
>>> along with a per-Architecture/Platform mechanism for setting it ...
>>>
>>> Casey
>>>
>>
>> I have take a time to talk to our kvm team about how to distinguish the relaxed
>> ordering in the VM for some vf just like 82599-vf, the probe routine looks like
>> could work like this:
>> 1) QEMU could emulate the platform by the Vender ID and device ID which could be
>> read from the host.
>> 2) The QEMU could create a virtual PCIe dev complex and recognize the PCIe bus address which
>> come and detach from the host to the guest.
>> 3) the PCI quirk could enable the Relaxed Ordering by the Vendor ID and Device ID.
>> 4) The VF drivers could read the flag and set to the hw.
>>
>> So I think we could set the PCI_DEV_FLAGS_RELAXED_ORDERING_ENABLED for some special platform
>> and don't enable by default, if I miss something, please not hesitate to enlighten me :)
>
> This isn't what I had in mind at all.
>
> So what Casey had originally submitted was a step in the direction of
> what I was thinking. Basically on platforms where it is not advisable
> to enable relaxed ordering we need to advertise that relaxed ordering
> is not safe. Then when we are initializing the devices underneath
> those we need to be clearing the relaxed ordering enable bits in the
> PCI configuration, that is the piece that was missing from Casey's
> original patch. In addition we then need to have a way for devices to
> optionally enable relaxed ordering for cases like Casey pointed out
> where they might want to use relaxed ordering for peer-to-peer
> transactions, but not for transactions to the root complex. Finally in
> the case of the Intel drivers we could then just drop the compile time
> checks entirely and just enable the device to configure the internal
> registers for relaxed ordering because the configuration space becomes
> the spot that controls if this gets enabled or not.
>
> So the initial set of patches Casey submitted only really consisted of
> 2 patches. What I am proposing is that we would be looking at
> expanding this out to about 4 patches. The first patch is the original
> 1 of 2, the second patch would be to modify the PCI initialization
> code to clear the relaxed ordering enable bit in the event that the
> root complex doesn't want relaxed ordering enabled, the third would be
> to make changes to the Chelsio driver as needed to allow for the
> peer-to-peer case to be enabled when the relaxed ordering bit in the
> configuration space is not enabled without triggering any relaxed
> ordering requests to the root complex, and the last one would be to
> drop the defines in ixgbe and whatever other Intel drivers are
> currently checking for either SPARC or the define that was added to
> support relaxed ordering and just act like we are going to do it
> always with the PCI configuration space controlling if we do or not.
>
> Ideally as a part of the second patch we should have a way of testing
> if a given path can support relaxed ordering. That way when we go to
> try to enable a peer-to-peer setup we can be certain that a given path
> will work and don't try enabling it in paths that would be unsupported
> for peer-to-peer.
>
> This ends up being somewhat of a risk for the Intel NICs, but if the
> Chelsio devices have been running with relaxed ordering enabled for
> some time and have identified the chipsets that should be issues, then
> odds are we should be fine as well.
>
According to your opinion, I try to build the second patch again,
1. in the pcie probe time, the pci_configure_relaxed_ordering will used to set the relaxed ordering
bit for configuration space.
2. export the pcie_get_relaxed_ordering for drivers and the drivers could decide whether should enable the relaxed ordering.
If I still go to the wrong path, please correct me, thanks.
---------------------------------------------------------------
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b01bd5b..0076e4a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4852,6 +4852,28 @@ int pcie_get_mps(struct pci_dev *dev)
}
EXPORT_SYMBOL(pcie_get_mps);
+int pcie_set_relaxed_ordering(struct pci_dev *dev)
+{
+ return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN);
+}
+EXPORT_SYMBOL(pcie_set_relaxed_ordering);
+
+int pcie_clear_relaxed_ordering(struct pci_dev *dev)
+{
+ return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN);
+}
+EXPORT_SYMBOL(pcie_clear_relaxed_ordering);
+
+int pcie_get_relaxed_ordering(struct pci_dev *dev)
+{
+ u16 v;
+
+ pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
+
+ return (v & PCI_EXP_DEVCTL_RELAX_EN) >> 4;
+}
+EXPORT_SYMBOL(pcie_get_relaxed_ordering);
+
/**
* pcie_set_mps - set PCI Express maximum payload size
* @dev: PCI device to query
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 19c8950..aeb22b5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1701,6 +1701,16 @@ static void pci_configure_extended_tags(struct pci_dev *dev)
PCI_EXP_DEVCTL_EXT_TAG);
}
+static void pci_configure_relaxed_ordering(struct pci_dev *dev)
+{
+ int ret;
+
+ if (dev && (dev->dev_flags & PCI_DEV_FLAGS_NO_RELAXED_ORDERING))
+ pcie_set_relaxed_ordering(dev);
+ else
+ pcie_clear_relaxed_ordering(dev);
+}
+
static void pci_configure_device(struct pci_dev *dev)
{
struct hotplug_params hpp;
@@ -1708,6 +1718,7 @@ static void pci_configure_device(struct pci_dev *dev)
pci_configure_mps(dev);
pci_configure_extended_tags(dev);
+ pci_configure_relaxed_ordering(dev);
memset(&hpp, 0, sizeof(hpp));
ret = pci_get_hp_params(dev, &hpp);
Thanks
Ding
>>> .
>>>
>>
>
> .
>
^ permalink raw reply related
* Re: [Patch net-next] net_sched: only create filter chains for new filters/actions
From: Jiri Pirko @ 2017-05-27 10:05 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko
In-Reply-To: <CAM_iQpUJcK0ZSqK8=a_D9dBw1OED++A=bC9-UUov4ng0mMpJUg@mail.gmail.com>
Fri, May 26, 2017 at 06:55:25PM CEST, xiyou.wangcong@gmail.com wrote:
>On Fri, May 26, 2017 at 7:54 AM, David Miller <davem@davemloft.net> wrote:
>> And I also didn't find the boolean logic hard to understand at all.
>>
>> It is in fact a very common pattern to pass a "create" boolean into
>> lookup functions, to tell them whether to create a new object on
>> lookup failure or not. And then also to control that boolean via
>> what kind of netlink request we are processing.
>
>+10
>
>It is a widely used pattern among the kernel source code.
>I'd be surprised if an experienced kernel developer is not
>aware of this pattern. ;)
Cong, as you wisely put, I'm not aware of this pattern and I'm also
unaware of existence of ternary operator. Are this notes necessary?
Does that make you feel better?
^ permalink raw reply
* Re: [Patch net-next] net_sched: only create filter chains for new filters/actions
From: Jiri Pirko @ 2017-05-27 10:02 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, netdev, jhs, jiri
In-Reply-To: <20170526.105443.1489276661727770629.davem@davemloft.net>
Fri, May 26, 2017 at 04:54:43PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri, 26 May 2017 07:53:52 +0200
>
>> Thu, May 25, 2017 at 06:14:56PM CEST, davem@davemloft.net wrote:
>>>From: Cong Wang <xiyou.wangcong@gmail.com>
>>>Date: Tue, 23 May 2017 09:42:37 -0700
>>>
>>>> tcf_chain_get() always creates a new filter chain if not found
>>>> in existing ones. This is totally unnecessary when we get or
>>>> delete filters, new chain should be only created for new filters
>>>> (or new actions).
>>>>
>>>> Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters")
>>>> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
>>>> Cc: Jiri Pirko <jiri@mellanox.com>
>>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>>
>>>Indeed, get and delete requests should not create new objects, ever.
>>>
>>>I have pretty much no idea why Jiri is making such a big fuss about
>>>this change, to be quite honest. :-)
>>
>> Because it makes already hard to read code even worse, for *no* benefit.
>> That's why.
>
>Jiri, if you say the same thing 100 times, it doesn't help anyone
>understand your arguments any better.
>
>Creating new objects when a GET or a DEL is requested is flat out
>wrong.
Allright. I ack that.
>
>And Cong is fixing that.
>
>And I also didn't find the boolean logic hard to understand at all.
>
>It is in fact a very common pattern to pass a "create" boolean into
>lookup functions, to tell them whether to create a new object on
>lookup failure or not. And then also to control that boolean via
>what kind of netlink request we are processing.
>
>So you tell me what's so bad about his change given the above?
>
>Give me details and real facts, like I just did, rather than vague
>statements about "benefit" and "hard to read".
What I don't like is the double "n->nlmsg_type == RTM_NEWTFILTER" check
and return value decusion according to the latter check. The code logic
is split into tcf_chain_get function and its caller. That is
at least odd.
Since you don't like the PTR_ERR approach, I'll try to figure out how to
do this another way.
^ permalink raw reply
* Re: [patch net-next] net/sched: let chain_get to figure out the return value
From: Jiri Pirko @ 2017-05-27 9:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jhs, xiyou.wangcong, mlxsw
In-Reply-To: <20170526.105912.229870645071385383.davem@davemloft.net>
Fri, May 26, 2017 at 04:59:12PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri, 26 May 2017 09:21:29 +0200
>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Alhough I believe that this create/nocreate dance is completelly
>> pointless, at least make it a bit nicer and easier to read.
>> Push the decision on what error value is returned to chain_get function
>> and use ERR macros.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>No, this is quite worse.
>
>You're leaving pointer error values in structures. That's extremely
>error prone.
Yet used everywhere in kernel.
>
>And as stated in the other thread, I don't think Cong's logic is strange
>or hard to understand at all.
That is why tc code looks how it does :/
But perhaps I'm slow and everything is crystal-clear to everyone else.
^ permalink raw reply
* Re: [PATCH 0/2] Document and use eeprom-length property
From: Shawn Guo @ 2017-05-27 8:13 UTC (permalink / raw)
To: David Miller; +Cc: andrew, netdev, linux-arm-kernel, vivien.didelot
In-Reply-To: <20170526.150242.515745885812232154.davem@davemloft.net>
On Fri, May 26, 2017 at 03:02:42PM -0400, David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Fri, 26 May 2017 01:44:42 +0200
>
> > The mv88e6xxx switch driver allows the size of the attached EEPROM to
> > be described in DT. This property is missing from the binding
> > documentation. Add it. And make use of it on the ZII Devel B board.
> >
> > David, Shawn, please could you talk amongs yourself to decide who
> > takes what.
>
> I can take this if it works for Shawn, otherwise I'm also fine if Shawn
> takes it and if so feel free to add my:
>
> Acked-by: David S. Miller <davem@davemloft.net>
Hi David,
I see these two patches can be applied separately, so I picked up 2/2
and left 1/2 to you.
Shawn
^ permalink raw reply
* Re: [PATCH 2/2] ARM: VF610: ZII devel b: Add switch eeprom-length properties
From: Shawn Guo @ 2017-05-27 8:10 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, linux ARM, Vivien Didelot
In-Reply-To: <1495755884-31341-3-git-send-email-andrew@lunn.ch>
On Fri, May 26, 2017 at 01:44:44AM +0200, Andrew Lunn wrote:
> Two of the Ethernet switches on this board have EEPROMs connected.
> Add the eeprom-length property to the device tree, making it possible
> to access the EEPROM using ethtool -e.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Applied with a bit update on subject prefix. Thanks.
Shawn
^ permalink raw reply
* Re: [PATCH net-next v2 0/8] net: extend RTM_GETROUTE to return fib result
From: Roopa Prabhu @ 2017-05-27 6:00 UTC (permalink / raw)
To: David Miller
Cc: David Ahern, Rami Rosen, netdev@vger.kernel.org,
Nikolay Aleksandrov
In-Reply-To: <20170526.141838.790479821297151569.davem@davemloft.net>
On Fri, May 26, 2017 at 11:18 AM, David Miller <davem@davemloft.net> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> Date: Thu, 25 May 2017 10:42:32 -0700
>
>> This series adds a new RTM_F_FIB_MATCH flag to return matched fib result
>> with RTM_GETROUTE. This is useful for applications and protocols in
>> userspace wanting to query the selected route.
>
> Looks good, series applied, thanks.
>
thank you.
> Have you considered taking this further and allowing one to see which
> nexthop a route lookup picked?
since the default RTM_GETROUTE output gives most of the attributes
from the resolved dst,
have not considered adding more to it yet...but certainly can if
needed in the future.
^ 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