* RE: netdev carrier changes is one even after ethernet link up.
From: Bhadram Varka @ 2017-09-04 5:10 UTC (permalink / raw)
To: Florian Fainelli, andrew@lunn.ch; +Cc: linux-netdev
In-Reply-To: <a7bf6cf0-c534-95f9-47f6-ee56a95fe961@gmail.com>
> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Saturday, September 02, 2017 7:25 AM
> To: Bhadram Varka <vbhadram@nvidia.com>; andrew@lunn.ch
> Cc: linux-netdev <netdev@vger.kernel.org>
> Subject: Re: netdev carrier changes is one even after ethernet link up.
>
> On 08/31/2017 10:49 PM, Bhadram Varka wrote:
> > Thanks for responding. Now responding inline
> >
> >> -----Original Message-----
> >> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> >> Sent: Friday, September 01, 2017 5:53 AM
> >> To: Bhadram Varka <vbhadram@nvidia.com>; andrew@lunn.ch
> >> Cc: linux-netdev <netdev@vger.kernel.org>
> >> Subject: Re: netdev carrier changes is one even after ethernet link up.
> >>
> >> On 08/30/2017 10:53 PM, Bhadram Varka wrote:
> >>> Hi,
> >>>
> >>>
> >>>
> >>> I have observed that carrier_changes is one even in case of the
> >>> ethernet link is up.
> >>>
> >>>
> >>>
> >>> After investigating the code below is my observation -
> >>>
> >>>
> >>>
> >>> ethernet_driver_probe()
> >>>
> >>> +--->phy_connect()
> >>>
> >>> | +--->phy_attach_direct()
> >>>
> >>> | +---> netif_carrier_off() : which increments
> >>> carrier_changes to one.
> >>>
> >>> +--->register_netdevice() : will the carrier_changes becomes zero here ?
> >>>
> >>> +--->netif_carrier_off(): not increment the carrier_changes since
> >>> __LINK_STATE_NOCARRIER already set.
> >>>
> >>>
> >>>
> >>> From ethernet driver open will start the PHY and trigger the
> >>> phy_state_machine.
> >>>
> >>> Phy_state_machine workqueue calling netif_carrier_on() once the link
> >>> is
> >> UP.
> >>>
> >>> netif_carrier_on() increments the carrier_changes by one.
> >>
> >> If the call trace is correct, then there is at least two problems here:
> >>
> >> - phy_connect() does start the PHY machine which means that as soon
> >> as it detects a link state of any kind (up or down) it can call
> >> netif_carrier_off() respectively netif_carrier_on()
> >>
> >> - as soon as you call register_netdevice() notifiers run and other
> >> parts of the kernel or user-space programs can see an inconsistent
> >> link state
> >>
> >> I would suggest doing the following sequence instead:
> >>
> >> netif_carrier_off()
> >> register_netdevice()
> >> phy_connect()
> >>
> >> Which should result in a consistent link state and carrier value.
> >>
> > Yes, It will address the issue.
> >
> > If we did the phy_conect in ndo_open it will make the carrier changes as
> two. But if we did in probe function then it's not working.
> >
> > In ethernet driver probe - (below sequence is not working)
> > phy_connect()
> > register_netdevice()
> > netif_carrier_off()
> >
> > working sequence:
> > In probe():
> > register_netdevice()
> > ndo_open:
> > phy_connect()
> >
> > After reverting - https://lkml.org/lkml/2016/1/9/173 this works if we do
> phy_connect in probe as well.
>
> But as mentioned before you should not be doing the PHY probe in your
> driver's probe function for different reasons:
>
> - the probe function's responsibility is to initialize the driver and the HW to a
> state where they both have everything needed but it should be in quiesced
> state. There is no guarantee that your network device may ever be used
> after probe unless something calls ndo_open(), you should therefore keep
> all resources to a minimum: memory allocated, HW powered down etc.
>
> - there is a race condition between the PHY state machine started in
> phy_connect(), and when register_netdevice() is called and notifiers running
> which can lead to an inconsistent state for the carrier
Agreed. But I could see lot of drivers performing the phy_connect() in driver_probe() function itself.
Also observed that these drivers are performing netif_carrier_off() after register_netdevice().
My point is that why do we need to do netif_carrier_off() in phy_connect() since we already doing the same after register_netdevice() in MAC driver ?
Thanks!
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply
* RE: netdev carrier changes is one even after ethernet link up.
From: Bhadram Varka @ 2017-09-04 5:07 UTC (permalink / raw)
To: Florian Fainelli, andrew@lunn.ch; +Cc: linux-netdev
In-Reply-To: <a7bf6cf0-c534-95f9-47f6-ee56a95fe961@gmail.com>
Hi Florian,
> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Saturday, September 02, 2017 7:25 AM
> To: Bhadram Varka <vbhadram@nvidia.com>; andrew@lunn.ch
> Cc: linux-netdev <netdev@vger.kernel.org>
> Subject: Re: netdev carrier changes is one even after ethernet link up.
>
> On 08/31/2017 10:49 PM, Bhadram Varka wrote:
> > Thanks for responding. Now responding inline
> >
> >> -----Original Message-----
> >> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> >> Sent: Friday, September 01, 2017 5:53 AM
> >> To: Bhadram Varka <vbhadram@nvidia.com>; andrew@lunn.ch
> >> Cc: linux-netdev <netdev@vger.kernel.org>
> >> Subject: Re: netdev carrier changes is one even after ethernet link up.
> >>
> >> On 08/30/2017 10:53 PM, Bhadram Varka wrote:
> >>> Hi,
> >>>
> >>>
> >>>
> >>> I have observed that carrier_changes is one even in case of the
> >>> ethernet link is up.
> >>>
> >>>
> >>>
> >>> After investigating the code below is my observation -
> >>>
> >>>
> >>>
> >>> ethernet_driver_probe()
> >>>
> >>> +--->phy_connect()
> >>>
> >>> | +--->phy_attach_direct()
> >>>
> >>> | +---> netif_carrier_off() : which increments
> >>> carrier_changes to one.
> >>>
> >>> +--->register_netdevice() : will the carrier_changes becomes zero here ?
> >>>
> >>> +--->netif_carrier_off(): not increment the carrier_changes since
> >>> __LINK_STATE_NOCARRIER already set.
> >>>
> >>>
> >>>
> >>> From ethernet driver open will start the PHY and trigger the
> >>> phy_state_machine.
> >>>
> >>> Phy_state_machine workqueue calling netif_carrier_on() once the link
> >>> is
> >> UP.
> >>>
> >>> netif_carrier_on() increments the carrier_changes by one.
> >>
> >> If the call trace is correct, then there is at least two problems here:
> >>
> >> - phy_connect() does start the PHY machine which means that as soon
> >> as it detects a link state of any kind (up or down) it can call
> >> netif_carrier_off() respectively netif_carrier_on()
> >>
> >> - as soon as you call register_netdevice() notifiers run and other
> >> parts of the kernel or user-space programs can see an inconsistent
> >> link state
> >>
> >> I would suggest doing the following sequence instead:
> >>
> >> netif_carrier_off()
> >> register_netdevice()
> >> phy_connect()
> >>
> >> Which should result in a consistent link state and carrier value.
> >>
> > Yes, It will address the issue.
> >
> > If we did the phy_conect in ndo_open it will make the carrier changes as
> two. But if we did in probe function then it's not working.
> >
> > In ethernet driver probe - (below sequence is not working)
> > phy_connect()
> > register_netdevice()
> > netif_carrier_off()
> >
> > working sequence:
> > In probe():
> > register_netdevice()
> > ndo_open:
> > phy_connect()
> >
> > After reverting - https://lkml.org/lkml/2016/1/9/173 this works if we do
> phy_connect in probe as well.
>
> But as mentioned before you should not be doing the PHY probe in your
> driver's probe function for different reasons:
>
> - the probe function's responsibility is to initialize the driver and the HW to a
> state where they both have everything needed but it should be in quiesced
> state. There is no guarantee that your network device may ever be used
> after probe unless something calls ndo_open(), you should therefore keep
> all resources to a minimum: memory allocated, HW powered down etc.
>
> - there is a race condition between the PHY state machine started in
> phy_connect(), and when register_netdevice() is called and notifiers running
> which can lead to an inconsistent state for the carrier
>
Agreed. But I could see lot of drivers performing the phy_connect() in driver_probe() function itself.
Also observed that these drivers are performing netif_carrier_off() after register_netdevice().
My point is that we do we need to do netif_carrier_off() in phy_connect since we already doing the same after register_netdevice() ?
Thanks!
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply
* Re: linux-next: manual merge of the pci tree with the net tree
From: Stephen Rothwell @ 2017-09-04 4:54 UTC (permalink / raw)
To: Bjorn Helgaas, David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Ding Tianhong,
Casey Leedom, Sinan Kaya
In-Reply-To: <20170816095128.7205e4f1@canb.auug.org.au>
Hi all,
On Wed, 16 Aug 2017 09:51:28 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the pci tree got a conflict in:
>
> drivers/pci/probe.c
>
> between commit:
>
> a99b646afa8a ("PCI: Disable PCIe Relaxed Ordering if unsupported")
>
> from the net tree and commit:
>
> 62ce94a7a5a5 ("PCI: Mark Broadcom HT2100 Root Port Extended Tags as broken")
>
> from the pci tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc drivers/pci/probe.c
> index e6a917b4acd3,d11fede6bd53..000000000000
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@@ -1751,67 -1753,51 +1753,94 @@@ int pci_configure_extended_tags(struct
> int ret;
>
> if (!pci_is_pcie(dev))
> - return;
> + return 0;
>
> - ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
> + ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
> if (ret)
> - return;
> + return 0;
> +
> + if (!(cap & PCI_EXP_DEVCAP_EXT_TAG))
> + return 0;
> +
> + ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
> + if (ret)
> + return 0;
> +
> + host = pci_find_host_bridge(dev->bus);
> + if (!host)
> + return 0;
> +
> + /*
> + * If some device in the hierarchy doesn't handle Extended Tags
> + * correctly, make sure they're disabled.
> + */
> + if (host->no_ext_tags) {
> + if (ctl & PCI_EXP_DEVCTL_EXT_TAG) {
> + dev_info(&dev->dev, "disabling Extended Tags\n");
> + pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> + PCI_EXP_DEVCTL_EXT_TAG);
> + }
> + return 0;
> + }
>
> - if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> + if (!(ctl & PCI_EXP_DEVCTL_EXT_TAG)) {
> + dev_info(&dev->dev, "enabling Extended Tags\n");
> pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> PCI_EXP_DEVCTL_EXT_TAG);
> + }
> + return 0;
> }
>
> +/**
> + * pcie_relaxed_ordering_enabled - Probe for PCIe relaxed ordering enable
> + * @dev: PCI device to query
> + *
> + * Returns true if the device has enabled relaxed ordering attribute.
> + */
> +bool pcie_relaxed_ordering_enabled(struct pci_dev *dev)
> +{
> + u16 v;
> +
> + pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
> +
> + return !!(v & PCI_EXP_DEVCTL_RELAX_EN);
> +}
> +EXPORT_SYMBOL(pcie_relaxed_ordering_enabled);
> +
> +static void pci_configure_relaxed_ordering(struct pci_dev *dev)
> +{
> + struct pci_dev *root;
> +
> + /* PCI_EXP_DEVICE_RELAX_EN is RsvdP in VFs */
> + if (dev->is_virtfn)
> + return;
> +
> + if (!pcie_relaxed_ordering_enabled(dev))
> + return;
> +
> + /*
> + * For now, we only deal with Relaxed Ordering issues with Root
> + * Ports. Peer-to-Peer DMA is another can of worms.
> + */
> + root = pci_find_pcie_root_port(dev);
> + if (!root)
> + return;
> +
> + if (root->dev_flags & PCI_DEV_FLAGS_NO_RELAXED_ORDERING) {
> + pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> + PCI_EXP_DEVCTL_RELAX_EN);
> + dev_info(&dev->dev, "Disable Relaxed Ordering because the Root Port didn't support it\n");
> + }
> +}
> +
> static void pci_configure_device(struct pci_dev *dev)
> {
> struct hotplug_params hpp;
> int ret;
>
> pci_configure_mps(dev);
> - pci_configure_extended_tags(dev);
> + pci_configure_extended_tags(dev, NULL);
> + pci_configure_relaxed_ordering(dev);
>
> memset(&hpp, 0, sizeof(hpp));
> ret = pci_get_hp_params(dev, &hpp);
Just a reminder that this conflict still exists.
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: pull request: bluetooth-next 2017-09-03
From: David Miller @ 2017-09-04 4:28 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth, netdev
In-Reply-To: <20170903062055.GA9569@x1c.home>
From: Johan Hedberg <johan.hedberg@gmail.com>
Date: Sun, 3 Sep 2017 09:20:55 +0300
> Here's one last bluetooth-next pull request for the 4.14 kernel:
>
> - NULL pointer fix in ca8210 802.15.4 driver
> - A few "const" fixes
> - New Kconfig option for disabling legacy interfaces
>
> Please let me know if there are any issues pulling. Thanks.
Pulled.
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: qualcomm: rmnet: Fix comments on initial patchset
From: David Miller @ 2017-09-04 4:27 UTC (permalink / raw)
To: subashab; +Cc: netdev
In-Reply-To: <1504416646-9527-1-git-send-email-subashab@codeaurora.org>
From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Sat, 2 Sep 2017 23:30:40 -0600
> This series fixes the comments from Dan on the first patch series.
>
> Fixes a memory corruption which could occur if mux_id was higher than 32.
> Remove the RMNET_LOCAL_LOGICAL_ENDPOINT which is no longer used.
> Make a log message more useful.
> Combine __rmnet_set_endpoint_config() with rmnet_set_endpoint_config().
> Set the mux_id in rmnet_vnd_newlink().
> Set the ingress and egress data format directly in newlink.
> Implement ndo_get_iflink to find the real_dev.
> Rename the real_dev_info to port to make it similar to other drivers.
>
> The conversion of rmnet_devices to a list and hash lookup will be sent
> as part of a seperate patch.
Series applied.
^ permalink raw reply
* Re: [PATCH net-next 0/6] nfp: refactor app init, and minor flower fixes
From: David Miller @ 2017-09-04 4:22 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20170903012605.7435-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Sat, 2 Sep 2017 18:25:59 -0700
> This series is a part 2 to what went into net as a simpler fix.
> In net we simply moved when existing callbacks are invoked to
> ensure flower app does not still use representors when lower
> netdev has already been destroyed. In this series we add a
> callback to notify apps when vNIC netdevs are fully initialized
> and they are about to be destroyed. This allows flower to spawn
> representors at the right time, while keeping the start/stop
> callbacks for what they are intended to be used - FW initialization
> over control channel.
>
> Patch 4 improves drop monitor interaction and patch 5 changes
> the default Kconfig selection of flower offload. Patch 6 fixes
> locking around representor updates which got lost in net-next.
Series applied, thanks.
^ permalink raw reply
* Re: [pull request][net-next 00/17] Mellanox, mlx5 updates 2017-09-03
From: David Miller @ 2017-09-04 4:19 UTC (permalink / raw)
To: saeedm; +Cc: netdev, kernel-team
In-Reply-To: <20170903042117.28923-1-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Sun, 3 Sep 2017 07:21:00 +0300
> This series from Tariq includes micro data path optimization for mlx5e
> netdevice driver.
>
> Sorry about the late submission but most of the patches are really
> small and trivial.
>
> For more details please see tag log message below.
> Please pull and let me know if there's any problem.
Pulled, thanks.
^ permalink raw reply
* [PATCH net-next 2/2] tun: rename generic_xdp to skb_xdp
From: Jason Wang @ 2017-09-04 3:36 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: mst, Jason Wang, Daniel Borkmann
In-Reply-To: <1504496169-31190-1-git-send-email-jasowang@redhat.com>
Rename "generic_xdp" to "skb_xdp" to avoid confusing it with the
generic XDP which will be done at netif_receive_skb().
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/tun.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 80ac18f..3c9985f 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1267,7 +1267,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
struct tun_file *tfile,
struct iov_iter *from,
struct virtio_net_hdr *hdr,
- int len, int *generic_xdp)
+ int len, int *skb_xdp)
{
struct page_frag *alloc_frag = ¤t->task_frag;
struct sk_buff *skb;
@@ -1301,13 +1301,13 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
* we do XDP on skb in case the headroom is not enough.
*/
if (hdr->gso_type || !xdp_prog)
- *generic_xdp = 1;
+ *skb_xdp = 1;
else
- *generic_xdp = 0;
+ *skb_xdp = 0;
rcu_read_lock();
xdp_prog = rcu_dereference(tun->xdp_prog);
- if (xdp_prog && !*generic_xdp) {
+ if (xdp_prog && !*skb_xdp) {
struct xdp_buff xdp;
void *orig_data;
u32 act;
@@ -1389,7 +1389,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
bool zerocopy = false;
int err;
u32 rxhash;
- int generic_xdp = 1;
+ int skb_xdp = 1;
if (!(tun->dev->flags & IFF_UP))
return -EIO;
@@ -1448,7 +1448,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
}
if (tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
- skb = tun_build_skb(tun, tfile, from, &gso, len, &generic_xdp);
+ /* For the packet that is not easy to be processed
+ * (e.g gso or jumbo packet), we will do it at after
+ * skb was created with generic XDP routine.
+ */
+ skb = tun_build_skb(tun, tfile, from, &gso, len, &skb_xdp);
if (IS_ERR(skb)) {
this_cpu_inc(tun->pcpu_stats->rx_dropped);
return PTR_ERR(skb);
@@ -1528,7 +1532,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
skb_reset_network_header(skb);
skb_probe_transport_header(skb, 0);
- if (generic_xdp) {
+ if (skb_xdp) {
struct bpf_prog *xdp_prog;
int ret;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 1/2] tun: reserve extra headroom only when XDP is set
From: Jason Wang @ 2017-09-04 3:36 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: mst, Jason Wang, Jakub Kicinski
We reserve headroom unconditionally which could cause unnecessary
stress on socket memory accounting because of increased trusesize. Fix
this by only reserve extra headroom when XDP is set.
Cc: Jakub Kicinski <kubakici@wp.pl>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/tun.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 06e8f0b..80ac18f 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -108,7 +108,7 @@ do { \
#endif
#define TUN_HEADROOM 256
-#define TUN_RX_PAD (NET_IP_ALIGN + NET_SKB_PAD + TUN_HEADROOM)
+#define TUN_RX_PAD (NET_IP_ALIGN + NET_SKB_PAD)
/* TUN device flags */
@@ -1272,25 +1272,35 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
struct page_frag *alloc_frag = ¤t->task_frag;
struct sk_buff *skb;
struct bpf_prog *xdp_prog;
- int buflen = SKB_DATA_ALIGN(len + TUN_RX_PAD) +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ int buflen = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
unsigned int delta = 0;
char *buf;
size_t copied;
bool xdp_xmit = false;
- int err;
+ int err, pad = TUN_RX_PAD;
+
+ rcu_read_lock();
+ xdp_prog = rcu_dereference(tun->xdp_prog);
+ if (xdp_prog)
+ pad += TUN_HEADROOM;
+ buflen += SKB_DATA_ALIGN(len + pad);
+ rcu_read_unlock();
if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, GFP_KERNEL)))
return ERR_PTR(-ENOMEM);
buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
copied = copy_page_from_iter(alloc_frag->page,
- alloc_frag->offset + TUN_RX_PAD,
+ alloc_frag->offset + pad,
len, from);
if (copied != len)
return ERR_PTR(-EFAULT);
- if (hdr->gso_type)
+ /* There's a small window that XDP may be set after the check
+ * of xdp_prog above, this should be rare and for simplicity
+ * we do XDP on skb in case the headroom is not enough.
+ */
+ if (hdr->gso_type || !xdp_prog)
*generic_xdp = 1;
else
*generic_xdp = 0;
@@ -1303,7 +1313,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
u32 act;
xdp.data_hard_start = buf;
- xdp.data = buf + TUN_RX_PAD;
+ xdp.data = buf + pad;
xdp.data_end = xdp.data + len;
orig_data = xdp.data;
act = bpf_prog_run_xdp(xdp_prog, &xdp);
@@ -1339,7 +1349,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
return ERR_PTR(-ENOMEM);
}
- skb_reserve(skb, TUN_RX_PAD - delta);
+ skb_reserve(skb, pad - delta);
skb_put(skb, len + delta);
get_page(alloc_frag->page);
alloc_frag->offset += buflen;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v2 0/4] net: dsa: Allow switch drivers to indicate number of TX queues
From: Florian Fainelli @ 2017-09-04 3:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, andrew, vivien.didelot, jiri, jhs, xiyou.wangcong
In-Reply-To: <20170903.202009.2104340600797726296.davem@davemloft.net>
Le 09/03/17 à 20:20, David Miller a écrit :
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Sat, 2 Sep 2017 11:06:05 -0700
>
>> This patch series extracts the parts of the patch set that are likely not to be
>> controversial and actually bringing multi-queue support to DSA-created network
>> devices.
>>
>> With these patches, we can now use sch_multiq as documented under
>> Documentation/networking/multique.txt and let applications dedice the switch
>> port output queue they want to use. Currently only Broadcom tags utilize that
>> information.
>>
>> Changes in v2:
>> - use a proper define for the number of TX queues in bcm_sf2.c (Andrew)
>
> Hello Florian.
>
> For some reason this series didn't make it completely into patchwork, I only
> see patch #1 there.
>
> Can you try submitting again? If it doesn't work this time I'll just apply
> it by hand from my inbox.
No problem, I just resent this patch series, thanks for the heads-up.
--
Florian
^ permalink raw reply
* [PATCH net-next v2 RESEND 4/4] net: dsa: bcm_sf2: Configure IMP port TC2QOS mapping
From: Florian Fainelli @ 2017-09-04 3:27 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170904032703.4593-1-f.fainelli@gmail.com>
Even though TC2QOS mapping is for switch egress queues, we need to
configure it correclty in order for the Broadcom tag ingress (CPU ->
switch) queue selection to work correctly since there is a 1:1 mapping
between switch egress queues and ingress queues.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 6b184bafa235..d7b53d53c116 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -103,6 +103,7 @@ static void bcm_sf2_brcm_hdr_setup(struct bcm_sf2_priv *priv, int port)
static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
+ unsigned int i;
u32 reg, offset;
if (priv->type == BCM7445_DEVICE_ID)
@@ -129,6 +130,14 @@ static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
reg |= MII_DUMB_FWDG_EN;
core_writel(priv, reg, CORE_SWITCH_CTRL);
+ /* Configure Traffic Class to QoS mapping, allow each priority to map
+ * to a different queue number
+ */
+ reg = core_readl(priv, CORE_PORT_TC2_QOS_MAP_PORT(port));
+ for (i = 0; i < SF2_NUM_EGRESS_QUEUES; i++)
+ reg |= i << (PRT_TO_QID_SHIFT * i);
+ core_writel(priv, reg, CORE_PORT_TC2_QOS_MAP_PORT(port));
+
bcm_sf2_brcm_hdr_setup(priv, port);
/* Force link status for IMP port */
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v2 RESEND 3/4] net: dsa: bcm_sf2: Advertise number of egress queues
From: Florian Fainelli @ 2017-09-04 3:27 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170904032703.4593-1-f.fainelli@gmail.com>
The switch supports 8 egress queues per port, so indicate that such that
net/dsa/slave.c::dsa_slave_create can allocate the right number of TX queues.
While at it use SF2_NUM_EGRESS_QUEUE as a define for the number of queues we
support.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 5 ++++-
drivers/net/dsa/bcm_sf2_regs.h | 3 +++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 554fe2df9365..6b184bafa235 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -244,7 +244,7 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
* to a different queue number
*/
reg = core_readl(priv, CORE_PORT_TC2_QOS_MAP_PORT(port));
- for (i = 0; i < 8; i++)
+ for (i = 0; i < SF2_NUM_EGRESS_QUEUES; i++)
reg |= i << (PRT_TO_QID_SHIFT * i);
core_writel(priv, reg, CORE_PORT_TC2_QOS_MAP_PORT(port));
@@ -1151,6 +1151,9 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
ds = dev->ds;
ds->ops = &bcm_sf2_ops;
+ /* Advertise the 8 egress queues */
+ ds->num_tx_queues = SF2_NUM_EGRESS_QUEUES;
+
dev_set_drvdata(&pdev->dev, priv);
spin_lock_init(&priv->indir_lock);
diff --git a/drivers/net/dsa/bcm_sf2_regs.h b/drivers/net/dsa/bcm_sf2_regs.h
index 26052450091e..49695fcc2ea8 100644
--- a/drivers/net/dsa/bcm_sf2_regs.h
+++ b/drivers/net/dsa/bcm_sf2_regs.h
@@ -401,4 +401,7 @@ enum bcm_sf2_reg_offs {
#define CFP_NUM_RULES 256
+/* Number of egress queues per port */
+#define SF2_NUM_EGRESS_QUEUES 8
+
#endif /* __BCM_SF2_REGS_H */
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v2 RESEND 2/4] net: dsa: tag_brcm: Set output queue from skb queue mapping
From: Florian Fainelli @ 2017-09-04 3:27 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170904032703.4593-1-f.fainelli@gmail.com>
We originally used skb->priority but that was not quite correct as this
bitfield needs to contain the egress switch queue we intend to send this
SKB to.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/tag_brcm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index de74c3f77818..dbb016434ace 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -62,6 +62,7 @@
static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
+ u16 queue = skb_get_queue_mapping(skb);
u8 *brcm_tag;
if (skb_cow_head(skb, BRCM_TAG_LEN) < 0)
@@ -78,7 +79,7 @@ static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev
* deprecated
*/
brcm_tag[0] = (1 << BRCM_OPCODE_SHIFT) |
- ((skb->priority << BRCM_IG_TC_SHIFT) & BRCM_IG_TC_MASK);
+ ((queue & BRCM_IG_TC_MASK) << BRCM_IG_TC_SHIFT);
brcm_tag[1] = 0;
brcm_tag[2] = 0;
if (p->dp->index == 8)
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v2 RESEND 1/4] net: dsa: Allow switch drivers to indicate number of TX queues
From: Florian Fainelli @ 2017-09-04 3:27 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170904032703.4593-1-f.fainelli@gmail.com>
Let switch drivers indicate how many TX queues they support. Some
switches, such as Broadcom Starfighter 2 are designed with 8 egress
queues. Future changes will allow us to leverage the queue mapping and
direct the transmission towards a particular queue.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
include/net/dsa.h | 3 +++
net/dsa/slave.c | 8 ++++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 398ca8d70ccd..dd44d6ce1097 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -243,6 +243,9 @@ struct dsa_switch {
/* devlink used to represent this switch device */
struct devlink *devlink;
+ /* Number of switch port queues */
+ unsigned int num_tx_queues;
+
/* Dynamically allocated ports, keep last */
size_t num_ports;
struct dsa_port ports[];
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 78e78a6e6833..2afa99506f8b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1259,8 +1259,12 @@ int dsa_slave_create(struct dsa_port *port, const char *name)
cpu_dp = ds->dst->cpu_dp;
master = cpu_dp->netdev;
- slave_dev = alloc_netdev(sizeof(struct dsa_slave_priv), name,
- NET_NAME_UNKNOWN, ether_setup);
+ if (!ds->num_tx_queues)
+ ds->num_tx_queues = 1;
+
+ slave_dev = alloc_netdev_mqs(sizeof(struct dsa_slave_priv), name,
+ NET_NAME_UNKNOWN, ether_setup,
+ ds->num_tx_queues, 1);
if (slave_dev == NULL)
return -ENOMEM;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v2 RESEND 0/4] net: dsa: Allow switch drivers to indicate number of TX queues
From: Florian Fainelli @ 2017-09-04 3:26 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
Hi all,
This patch series extracts the parts of the patch set that are likely not to be
controversial and actually bringing multi-queue support to DSA-created network
devices.
With these patches, we can now use sch_multiq as documented under
Documentation/networking/multique.txt and let applications dedice the switch
port output queue they want to use. Currently only Broadcom tags utilize that
information.
Resending based on David's feedback regarding the patches not in patchwork.
Changes in v2:
- use a proper define for the number of TX queues in bcm_sf2.c (Andrew)
Changes from RFC:
- dropped the ability to configure RX queues since we don't do anything with
those just yet
- dropped the patches that dealt with binding the DSA slave network devices
queues with their master network devices queues this will be worked on
separately.
Florian Fainelli (4):
net: dsa: Allow switch drivers to indicate number of TX queues
net: dsa: tag_brcm: Set output queue from skb queue mapping
net: dsa: bcm_sf2: Advertise number of egress queues
net: dsa: bcm_sf2: Configure IMP port TC2QOS mapping
drivers/net/dsa/bcm_sf2.c | 14 +++++++++++++-
drivers/net/dsa/bcm_sf2_regs.h | 3 +++
include/net/dsa.h | 3 +++
net/dsa/slave.c | 8 ++++++--
net/dsa/tag_brcm.c | 3 ++-
5 files changed, 27 insertions(+), 4 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [patch net-next v2 00/21] mlxsw: Offloading GRE tunnels
From: David Miller @ 2017-09-04 3:23 UTC (permalink / raw)
To: jiri; +Cc: netdev, petrm, idosch, mlxsw
In-Reply-To: <20170902214929.2890-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Sat, 2 Sep 2017 23:49:08 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Petr says:
>
> This patch series introduces to mlxsw driver support for offloading
> IP-in-IP tunnels in general, and for (subset of) GRE in particular.
...
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next v2 0/4] net: dsa: Allow switch drivers to indicate number of TX queues
From: David Miller @ 2017-09-04 3:20 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot, jiri, jhs, xiyou.wangcong
In-Reply-To: <20170902180609.23122-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sat, 2 Sep 2017 11:06:05 -0700
> This patch series extracts the parts of the patch set that are likely not to be
> controversial and actually bringing multi-queue support to DSA-created network
> devices.
>
> With these patches, we can now use sch_multiq as documented under
> Documentation/networking/multique.txt and let applications dedice the switch
> port output queue they want to use. Currently only Broadcom tags utilize that
> information.
>
> Changes in v2:
> - use a proper define for the number of TX queues in bcm_sf2.c (Andrew)
Hello Florian.
For some reason this series didn't make it completely into patchwork, I only
see patch #1 there.
Can you try submitting again? If it doesn't work this time I'll just apply
it by hand from my inbox.
Thanks.
^ permalink raw reply
* Re: [PATCH net] net: dsa: loop: Do not unregister invalid fixed PHY
From: David Miller @ 2017-09-04 3:19 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot, linux-kernel
In-Reply-To: <20170902155645.19071-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sat, 2 Sep 2017 08:56:45 -0700
> During error injection it was possible to crash in dsa_loop_exit() because of
> an attempt to unregister an invalid PHY. We actually want to the driver probing
> in dsa_loop_init() even though fixed_phy_register() may return an error to
> exercise how DSA deals with such cases, but we should not be crashing during
> driver removal.
>
> Fixes: 98cd1552ea27 ("net: dsa: Mock-up driver")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next v3 0/3] net: mvpp2: improve the mac address retrieval logic
From: David Miller @ 2017-09-04 3:17 UTC (permalink / raw)
To: antoine.tenart
Cc: andrew, gregory.clement, thomas.petazzoni, nadavh, linux,
linux-kernel, mw, stefanc, netdev
In-Reply-To: <20170902090649.28426-1-antoine.tenart@free-electrons.com>
From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Sat, 2 Sep 2017 11:06:46 +0200
> This series aims at fixing the logic behind the MAC address retrieval in the
> PPv2 driver. A possible issue is also fixed in patch 3/3 to introduce fallbacks
> when the address given in the device tree isn't valid.
...
> Since v2:
> - Patch 1/4 from v2 was applied on net (and net was merged in net-next).
> - Rebased on net-next.
>
> Since v1:
> - Rebased onto net (was on net-next).
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: invoke zerocopy callback on xmit path if no tx napi
From: Jason Wang @ 2017-09-04 3:03 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, Koichiro Den, virtualization,
Network Development
In-Reply-To: <CAF=yD-LZ4Abn8CFSQyuNnYSRZkHx4FTzK4hSi9WQaHO-28rsHQ@mail.gmail.com>
On 2017年09月02日 00:17, Willem de Bruijn wrote:
>>>> This is not a 50/50 split, which impliesTw that some packets from the
>>>> large
>>>> packet flow are still converted to copying. Without the change the rate
>>>> without queue was 80k zerocopy vs 80k copy, so this choice of
>>>> (vq->num >> 2) appears too conservative.
>>>>
>>>> However, testing with (vq->num >> 1) was not as effective at mitigating
>>>> stalls. I did not save that data, unfortunately. Can run more tests on
>>>> fine
>>>> tuning this variable, if the idea sounds good.
>>>
>>> Looks like there're still two cases were left:
>> To be clear, this patch is not intended to fix all issues. It is a small
>> improvement to avoid HoL blocking due to queued zerocopy skbs.
Right, just want to see if there's anything left.
>>
>> The trade-off is that reverting to copying in these cases increases
>> cycle cost. I think that that is a trade-off worth making compared to
>> the alternative drop in throughput. It probably would be good to be
>> able to measure this without kernel instrumentation: export
>> counters similar to net->tx_zcopy_err and net->tx_packets (though
>> without reset to zero, as in vhost_net_tx_packet).
I think it's acceptable if extra cycles were spent if we detect HOL anyhow.
>>
>>> 1) sndbuf is not INT_MAX
>> You mean the case where the device stalls, later zerocopy notifications
>> are queued, but these are never cleaned in free_old_xmit_skbs,
>> because it requires a start_xmit and by now the (only) socket is out of
>> descriptors?
> Typo, sorry. I meant out of sndbuf.
I mean e.g for tun. If its sndbuf is smaller than e.g (vq->num >> 1) *
$pkt_size and if all packet were held by some modules, limitation like
vq->num >> 1 won't work since we hit sudbuf before it.
>
>> A watchdog would help somewhat. With tx-napi, this case cannot occur,
>> either, as free_old_xmit_skbs no longer depends on a call to start_xmit.
>>
>>> 2) tx napi is used for virtio-net
>> I am not aware of any issue specific to the use of tx-napi?
Might not be clear here, I mean e.g virtio_net (tx-napi) in guest +
vhost_net (zerocopy) in host. In this case, even if we switch to
datacopy if ubuf counts exceeds vq->num >> 1, we still complete tx
buffers in order, tx interrupt could be delayed for indefinite time.
>>
>>> 1) could be a corner case, and for 2) what your suggest here may not solve
>>> the issue since it still do in order completion.
>> Somewhat tangential, but it might also help to break the in-order
>> completion processing in vhost_zerocopy_signal_used. Complete
>> all descriptors between done_idx and upend_idx. done_idx should
>> then only be forward to the oldest still not-completed descriptor.
>>
>> In the test I ran, where the oldest descriptors are held in a queue and
>> all newer ones are tail-dropped,
Do you mean the descriptors were tail-dropped by vhost?
>> this would avoid blocking a full ring
>> of completions, when only a small number (or 1) is actually delayed.
>>
>> Dynamic switching between copy and zerocopy using zcopy_used
>> already returns completions out-of-order, so this is not a huge leap.
Yes.
Thanks
^ permalink raw reply
* Re: [PATCH net] vhost_net: correctly check tx avail during rx busy polling
From: Jason Wang @ 2017-09-04 2:51 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20170901184531-mutt-send-email-mst@kernel.org>
On 2017年09月01日 23:51, Michael S. Tsirkin wrote:
> On Fri, Sep 01, 2017 at 05:02:50PM +0800, Jason Wang wrote:
>> We check tx avail through vhost_enable_notify() in the past which is
>> wrong since it only checks whether or not guest has filled more
>> available buffer since last avail idx synchronization which was just
>> done by vhost_vq_avail_empty() before. What we really want is checking
>> pending buffers in the avail ring.
> These are rx buffers, right? I'm not even sure why do we need to poll
> for them. Running out of rx buffers is a slow path.
Actually it polls for tx buffer here. I admit the code (or probably the
variable name) is confusing here.
>
>> Fix this by calling
>> vhost_vq_avail_empty() instead.
>>
>> This issue could be noticed by doing netperf TCP_RR benchmark as
>> client from guest (but not host). With this fix, TCP_RR from guest to
>> localhost restores from 1375.91 trans per sec to 55235.28 trans per
>> sec on my laptop (Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz).
>>
>> Fixes: 030881372460 ("vhost_net: basic polling support")
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> - The patch is needed for -stable
>> ---
>> drivers/vhost/net.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 06d0448..1b68253 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -634,7 +634,7 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
> In fact why does it poll the ring at all? I thought this function's
> job is to poll the socket, isn't it?
Tx notification is disabled to try to avoid vmexits, so we poll tx avail
buffers too.
>
>
>>
>> preempt_enable();
>>
>> - if (vhost_enable_notify(&net->dev, vq))
>> + if (!vhost_vq_avail_empty(&net->dev, vq))
>> vhost_poll_queue(&vq->poll);
>> mutex_unlock(&vq->mutex);
>
> Adding more contex:
>
> mutex_lock(&vq->mutex);
> vhost_disable_notify(&net->dev, vq);
>
> preempt_disable();
> endtime = busy_clock() + vq->busyloop_timeout;
>
> while (vhost_can_busy_poll(&net->dev, endtime) &&
> !sk_has_rx_data(sk) &&
> vhost_vq_avail_empty(&net->dev, vq))
> cpu_relax();
>
> preempt_enable();
>
> if (vhost_enable_notify(&net->dev, vq))
> vhost_poll_queue(&vq->poll);
> mutex_unlock(&vq->mutex);
>
> len = peek_head_len(rvq, sk);
>
>
> If you drop this we'll exit the function with notifications
> disabled. Seems wrong to me.
Yes, will fix this in V2.
Thanks
>
>>
>> --
>> 2.7.4
^ permalink raw reply
* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Yang, Yi @ 2017-09-04 2:38 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, e@erig.me
In-Reply-To: <87wp5l7560.fsf-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org>
On Wed, Aug 30, 2017 at 05:53:27PM +0800, Hannes Frederic Sowa wrote:
> Hello,
>
> Yi Yang <yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> writes:
>
> [...]
>
> > +struct ovs_key_nsh {
> > + u8 flags;
> > + u8 ttl;
> > + u8 mdtype;
> > + u8 np;
> > + __be32 path_hdr;
> > + __be32 context[NSH_MD1_CONTEXT_SIZE];
> > +};
> > +
> > struct sw_flow_key {
> > u8 tun_opts[IP_TUNNEL_OPTS_MAX];
> > u8 tun_opts_len;
> > @@ -144,6 +154,7 @@ struct sw_flow_key {
> > };
> > } ipv6;
> > };
> > + struct ovs_key_nsh nsh; /* network service header */
> > struct {
> > /* Connection tracking fields not packed above. */
> > struct {
>
> Does it makes sense to keep the context headers as part of the flow?
> What is the reasoning behind it? With mdtype 2 headers this might either
> not work very well or will increase sw_flow_key size causing slowdowns
> for all protocols.
For userspace, miniflow can handle such issue, but kernel data path
didn't provide such mechanism, so I don't think we can think of a better
way to fix this.
We have to have context headers in flow for match and set, every hop in
service function chaining can put its metedata into context headers on
demand, MD type 2 is much more complicated than you can imagine, it is
impossible to use an uniform way to handle MD type 1 and MD type 2, our
way is to keep MD type 1 keys in struct ovs_key_nsh and to reuse struct
flow_tnl for MD type 2 metadata, in case of MD type 2, we can set
context headers to 0 in struct ovs_key_nsh.
I beleive every newly-added key will result in similiar issue you
concern, maybe it will be better to introduce miniflow in kernel data
path, but it isn't in scope of this patch series.
It will be great if you can have a better proposal to fix your concern.
>
> [...]
^ permalink raw reply
* (unknown),
From: marketing @ 2017-09-04 2:33 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 14036757427509.doc --]
[-- Type: application/msword, Size: 40698 bytes --]
^ permalink raw reply
* Re: [PATCH nf-next 3/5] netlink: add NLM_F_NONREC flag for deletion requests
From: Pablo Neira Ayuso @ 2017-09-04 0:22 UTC (permalink / raw)
To: David Miller; +Cc: netfilter-devel, netdev
In-Reply-To: <20170903.171418.350656093498203113.davem@davemloft.net>
On Sun, Sep 03, 2017 at 05:14:18PM -0700, David Miller wrote:
>
> I only see patches 3, 4, and 5 of this series.
>
> If this is meant for net-next inclusion, you'll have to submit it such that
> I see the entire series on netdev and thus in patchwork.
I'm posting this new NLM_F_NONREC for acknowledgment, if possible. I
have a few more patches that follow up so I can take them through
nf-next in the next batch.
But I can just re-send this through your net-next tree, as you prefer.
^ permalink raw reply
* Re: [PATCH nf-next 3/5] netlink: add NLM_F_NONREC flag for deletion requests
From: David Miller @ 2017-09-04 0:14 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1504475761-11454-1-git-send-email-pablo@netfilter.org>
I only see patches 3, 4, and 5 of this series.
If this is meant for net-next inclusion, you'll have to submit it such that
I see the entire series on netdev and thus in patchwork.
Thanks.
^ 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