* Re: [PATCHv2 net] i40e: Implement ndo_gso_check()
From: Jesse Gross @ 2014-12-01 23:53 UTC (permalink / raw)
To: Tom Herbert
Cc: Joe Stringer, netdev, Shannon Nelson, Brandeburg, Jesse,
Jeff Kirsher, linux.nics, Linux Kernel Mailing List
In-Reply-To: <CA+mtBx-Q=Ebrx_8NVB5mWURj-oWB3gdNs5fSAL=Frr9sOmyvHw@mail.gmail.com>
On Mon, Dec 1, 2014 at 3:47 PM, Tom Herbert <therbert@google.com> wrote:
> On Mon, Dec 1, 2014 at 3:35 PM, Joe Stringer <joestringer@nicira.com> wrote:
>> On 21 November 2014 at 09:59, Joe Stringer <joestringer@nicira.com> wrote:
>>> On 20 November 2014 16:19, Jesse Gross <jesse@nicira.com> wrote:
>>>> I don't know if we need to have the check at all for IPIP though -
>>>> after all the driver doesn't expose support for it all (actually it
>>>> doesn't expose GRE either). This raises kind of an interesting
>>>> question about the checks though - it's pretty easy to add support to
>>>> the driver for a new GSO type (and I imagine that people will be
>>>> adding GRE soon) and forget to update the check.
>>>
>>> If the check is more conservative, then testing would show that it's
>>> not working and lead people to figure out why (and update the check).
>>
>> More concretely, one suggestion would be something like following at
>> the start of each gso_check():
>>
>> + const int supported = SKB_GSO_TCPV4 | SKB_GSO_TCPV6 | SKB_GSO_FCOE |
>> + SKB_GSO_UDP | SKB_GSO_UDP_TUNNEL;
>> +
>> + if (skb_shinfo(skb)->gso_type & ~supported)
>> + return false;
>
> This should already be handled by net_gso_ok.
My original point wasn't so much that this isn't handled at the moment
but that it's easy to add a supported GSO type but then forget to
update this check - i.e. if a driver already supports UDP_TUNNEL and
adds support for GRE with the same constraints. It seems not entirely
ideal that this function is acting as a blacklist rather than a
whitelist.
^ permalink raw reply
* Re: [PATCHv2 net] i40e: Implement ndo_gso_check()
From: Joe Stringer @ 2014-12-01 23:58 UTC (permalink / raw)
To: Jesse Gross
Cc: Tom Herbert, netdev, Shannon Nelson, Brandeburg, Jesse,
Jeff Kirsher, linux.nics, Linux Kernel Mailing List
In-Reply-To: <CAEP_g=979u0baWXGmCxo61a-KYEUwkruqBNof_zQpsvJxRFqSA@mail.gmail.com>
On 1 December 2014 at 15:53, Jesse Gross <jesse@nicira.com> wrote:
> On Mon, Dec 1, 2014 at 3:47 PM, Tom Herbert <therbert@google.com> wrote:
>> On Mon, Dec 1, 2014 at 3:35 PM, Joe Stringer <joestringer@nicira.com> wrote:
>>> On 21 November 2014 at 09:59, Joe Stringer <joestringer@nicira.com> wrote:
>>>> On 20 November 2014 16:19, Jesse Gross <jesse@nicira.com> wrote:
>>>>> I don't know if we need to have the check at all for IPIP though -
>>>>> after all the driver doesn't expose support for it all (actually it
>>>>> doesn't expose GRE either). This raises kind of an interesting
>>>>> question about the checks though - it's pretty easy to add support to
>>>>> the driver for a new GSO type (and I imagine that people will be
>>>>> adding GRE soon) and forget to update the check.
>>>>
>>>> If the check is more conservative, then testing would show that it's
>>>> not working and lead people to figure out why (and update the check).
>>>
>>> More concretely, one suggestion would be something like following at
>>> the start of each gso_check():
>>>
>>> + const int supported = SKB_GSO_TCPV4 | SKB_GSO_TCPV6 | SKB_GSO_FCOE |
>>> + SKB_GSO_UDP | SKB_GSO_UDP_TUNNEL;
>>> +
>>> + if (skb_shinfo(skb)->gso_type & ~supported)
>>> + return false;
>>
>> This should already be handled by net_gso_ok.
>
> My original point wasn't so much that this isn't handled at the moment
> but that it's easy to add a supported GSO type but then forget to
> update this check - i.e. if a driver already supports UDP_TUNNEL and
> adds support for GRE with the same constraints. It seems not entirely
> ideal that this function is acting as a blacklist rather than a
> whitelist.
How much less ideal is it to forget to update the check than to make
this a blacklist?
^ permalink raw reply
* Re: [PATCH 1/1] net-ipvlan: Deletion of an unnecessary check before the function call "free_percpu"
From: Mahesh Bandewar @ 2014-12-02 0:01 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: linux-netdev, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5479E693.3010200@users.sourceforge.net>
On Sat, Nov 29, 2014 at 7:30 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 29 Nov 2014 16:23:20 +0100
>
> The free_percpu() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Mahesh Bandewar <maheshb@google.com>
> ---
> drivers/net/ipvlan/ipvlan_main.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index 96b71b0..feb1853 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -125,8 +125,7 @@ static void ipvlan_uninit(struct net_device *dev)
> struct ipvl_dev *ipvlan = netdev_priv(dev);
> struct ipvl_port *port = ipvlan->port;
>
> - if (ipvlan->pcpu_stats)
> - free_percpu(ipvlan->pcpu_stats);
> + free_percpu(ipvlan->pcpu_stats);
>
> port->count -= 1;
> if (!port->count)
> --
> 2.1.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv2 net] i40e: Implement ndo_gso_check()
From: Tom Herbert @ 2014-12-02 0:09 UTC (permalink / raw)
To: Jesse Gross
Cc: Joe Stringer, netdev, Shannon Nelson, Brandeburg, Jesse,
Jeff Kirsher, linux.nics, Linux Kernel Mailing List
In-Reply-To: <CAEP_g=979u0baWXGmCxo61a-KYEUwkruqBNof_zQpsvJxRFqSA@mail.gmail.com>
On Mon, Dec 1, 2014 at 3:53 PM, Jesse Gross <jesse@nicira.com> wrote:
> On Mon, Dec 1, 2014 at 3:47 PM, Tom Herbert <therbert@google.com> wrote:
>> On Mon, Dec 1, 2014 at 3:35 PM, Joe Stringer <joestringer@nicira.com> wrote:
>>> On 21 November 2014 at 09:59, Joe Stringer <joestringer@nicira.com> wrote:
>>>> On 20 November 2014 16:19, Jesse Gross <jesse@nicira.com> wrote:
>>>>> I don't know if we need to have the check at all for IPIP though -
>>>>> after all the driver doesn't expose support for it all (actually it
>>>>> doesn't expose GRE either). This raises kind of an interesting
>>>>> question about the checks though - it's pretty easy to add support to
>>>>> the driver for a new GSO type (and I imagine that people will be
>>>>> adding GRE soon) and forget to update the check.
>>>>
>>>> If the check is more conservative, then testing would show that it's
>>>> not working and lead people to figure out why (and update the check).
>>>
>>> More concretely, one suggestion would be something like following at
>>> the start of each gso_check():
>>>
>>> + const int supported = SKB_GSO_TCPV4 | SKB_GSO_TCPV6 | SKB_GSO_FCOE |
>>> + SKB_GSO_UDP | SKB_GSO_UDP_TUNNEL;
>>> +
>>> + if (skb_shinfo(skb)->gso_type & ~supported)
>>> + return false;
>>
>> This should already be handled by net_gso_ok.
>
> My original point wasn't so much that this isn't handled at the moment
> but that it's easy to add a supported GSO type but then forget to
> update this check - i.e. if a driver already supports UDP_TUNNEL and
> adds support for GRE with the same constraints. It seems not entirely
> ideal that this function is acting as a blacklist rather than a
> whitelist.
Agreed, it would be nice to have all the checking logic in one place.
If all the drivers end up implementing ndo_gso_check then we could
potentially get rid of the GSO types as features. This probably
wouldn't be a bad thing since we already know that the features
mechanism doesn't scale (for instance there's no way to indicate that
certain combinations of GSO types are supported by a device).
^ permalink raw reply
* [PATCH v3 net-next] net: bcmgenet: enable driver to work without a device tree
From: Petri Gynther @ 2014-12-02 0:18 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli
Modify bcmgenet driver so that it can be used on Broadcom 7xxx
MIPS-based STB platforms without a device tree.
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/ethernet/broadcom/Kconfig | 1 -
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 31 ++++--
drivers/net/ethernet/broadcom/genet/bcmmii.c | 126 ++++++++++++++++++++-----
include/linux/platform_data/bcmgenet.h | 18 ++++
4 files changed, 143 insertions(+), 33 deletions(-)
create mode 100644 include/linux/platform_data/bcmgenet.h
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index c3e260c..888247a 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -62,7 +62,6 @@ config BCM63XX_ENET
config BCMGENET
tristate "Broadcom GENET internal MAC support"
- depends on OF
select MII
select PHYLIB
select FIXED_PHY if BCMGENET=y
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index f2fadb0..adfef5c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -42,6 +42,7 @@
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <linux/phy.h>
+#include <linux/platform_data/bcmgenet.h>
#include <asm/unaligned.h>
@@ -2586,8 +2587,9 @@ static const struct of_device_id bcmgenet_match[] = {
static int bcmgenet_probe(struct platform_device *pdev)
{
+ struct bcmgenet_platform_data *pd = pdev->dev.platform_data;
struct device_node *dn = pdev->dev.of_node;
- const struct of_device_id *of_id;
+ const struct of_device_id *of_id = NULL;
struct bcmgenet_priv *priv;
struct net_device *dev;
const void *macaddr;
@@ -2601,9 +2603,11 @@ static int bcmgenet_probe(struct platform_device *pdev)
return -ENOMEM;
}
- of_id = of_match_node(bcmgenet_match, dn);
- if (!of_id)
- return -EINVAL;
+ if (dn) {
+ of_id = of_match_node(bcmgenet_match, dn);
+ if (!of_id)
+ return -EINVAL;
+ }
priv = netdev_priv(dev);
priv->irq0 = platform_get_irq(pdev, 0);
@@ -2615,11 +2619,15 @@ static int bcmgenet_probe(struct platform_device *pdev)
goto err;
}
- macaddr = of_get_mac_address(dn);
- if (!macaddr) {
- dev_err(&pdev->dev, "can't find MAC address\n");
- err = -EINVAL;
- goto err;
+ if (dn) {
+ macaddr = of_get_mac_address(dn);
+ if (!macaddr) {
+ dev_err(&pdev->dev, "can't find MAC address\n");
+ err = -EINVAL;
+ goto err;
+ }
+ } else {
+ macaddr = pd->mac_address;
}
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2659,7 +2667,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
priv->dev = dev;
priv->pdev = pdev;
- priv->version = (enum bcmgenet_version)of_id->data;
+ if (of_id)
+ priv->version = (enum bcmgenet_version)of_id->data;
+ else
+ priv->version = pd->genet_version;
priv->clk = devm_clk_get(&priv->pdev->dev, "enet");
if (IS_ERR(priv->clk))
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 933cd7e..446889c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -23,6 +23,7 @@
#include <linux/of.h>
#include <linux/of_net.h>
#include <linux/of_mdio.h>
+#include <linux/platform_data/bcmgenet.h>
#include "bcmgenet.h"
@@ -312,22 +313,6 @@ static int bcmgenet_mii_probe(struct net_device *dev)
u32 phy_flags;
int ret;
- if (priv->phydev) {
- pr_info("PHY already attached\n");
- return 0;
- }
-
- /* In the case of a fixed PHY, the DT node associated
- * to the PHY is the Ethernet MAC DT node.
- */
- if (!priv->phy_dn && of_phy_is_fixed_link(dn)) {
- ret = of_phy_register_fixed_link(dn);
- if (ret)
- return ret;
-
- priv->phy_dn = of_node_get(dn);
- }
-
/* Communicate the integrated PHY revision */
phy_flags = priv->gphy_rev;
@@ -337,11 +322,39 @@ static int bcmgenet_mii_probe(struct net_device *dev)
priv->old_duplex = -1;
priv->old_pause = -1;
- phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup,
- phy_flags, priv->phy_interface);
- if (!phydev) {
- pr_err("could not attach to PHY\n");
- return -ENODEV;
+ if (dn) {
+ if (priv->phydev) {
+ pr_info("PHY already attached\n");
+ return 0;
+ }
+
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (!priv->phy_dn && of_phy_is_fixed_link(dn)) {
+ ret = of_phy_register_fixed_link(dn);
+ if (ret)
+ return ret;
+
+ priv->phy_dn = of_node_get(dn);
+ }
+
+ phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup,
+ phy_flags, priv->phy_interface);
+ if (!phydev) {
+ pr_err("could not attach to PHY\n");
+ return -ENODEV;
+ }
+ } else {
+ phydev = priv->phydev;
+ phydev->dev_flags = phy_flags;
+
+ ret = phy_connect_direct(dev, phydev, bcmgenet_mii_setup,
+ priv->phy_interface);
+ if (ret) {
+ pr_err("could not attach to PHY\n");
+ return -ENODEV;
+ }
}
priv->phydev = phydev;
@@ -438,6 +451,75 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
return 0;
}
+static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv)
+{
+ struct device *kdev = &priv->pdev->dev;
+ struct bcmgenet_platform_data *pd = kdev->platform_data;
+ struct mii_bus *mdio = priv->mii_bus;
+ struct phy_device *phydev;
+ int ret;
+
+ if (pd->phy_interface != PHY_INTERFACE_MODE_MOCA && pd->mdio_enabled) {
+ /*
+ * Internal or external PHY with MDIO access
+ */
+ if (pd->phy_address >= 0 && pd->phy_address < PHY_MAX_ADDR)
+ mdio->phy_mask = ~(1 << pd->phy_address);
+ else
+ mdio->phy_mask = 0;
+
+ ret = mdiobus_register(mdio);
+ if (ret) {
+ dev_err(kdev, "failed to register MDIO bus\n");
+ return ret;
+ }
+
+ if (pd->phy_address >= 0 && pd->phy_address < PHY_MAX_ADDR)
+ phydev = mdio->phy_map[pd->phy_address];
+ else
+ phydev = phy_find_first(mdio);
+
+ if (!phydev) {
+ dev_err(kdev, "failed to register PHY device\n");
+ mdiobus_unregister(mdio);
+ return -ENODEV;
+ }
+ } else {
+ /*
+ * MoCA port or no MDIO access.
+ * Use fixed PHY to represent the link layer.
+ */
+ struct fixed_phy_status fphy_status = {
+ .link = 1,
+ .speed = pd->phy_speed,
+ .duplex = pd->phy_duplex,
+ .pause = 0,
+ .asym_pause = 0,
+ };
+
+ phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
+ if (!phydev || IS_ERR(phydev)) {
+ dev_err(kdev, "failed to register fixed PHY device\n");
+ return -ENODEV;
+ }
+ }
+
+ priv->phydev = phydev;
+ priv->phy_interface = pd->phy_interface;
+
+ return 0;
+}
+
+static int bcmgenet_mii_bus_init(struct bcmgenet_priv *priv)
+{
+ struct device_node *dn = priv->pdev->dev.of_node;
+
+ if (dn)
+ return bcmgenet_mii_of_init(priv);
+ else
+ return bcmgenet_mii_pd_init(priv);
+}
+
int bcmgenet_mii_init(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
@@ -447,7 +529,7 @@ int bcmgenet_mii_init(struct net_device *dev)
if (ret)
return ret;
- ret = bcmgenet_mii_of_init(priv);
+ ret = bcmgenet_mii_bus_init(priv);
if (ret)
goto out_free;
diff --git a/include/linux/platform_data/bcmgenet.h b/include/linux/platform_data/bcmgenet.h
new file mode 100644
index 0000000..26af543
--- /dev/null
+++ b/include/linux/platform_data/bcmgenet.h
@@ -0,0 +1,18 @@
+#ifndef __LINUX_PLATFORM_DATA_BCMGENET_H__
+#define __LINUX_PLATFORM_DATA_BCMGENET_H__
+
+#include <linux/types.h>
+#include <linux/if_ether.h>
+#include <linux/phy.h>
+
+struct bcmgenet_platform_data {
+ bool mdio_enabled;
+ phy_interface_t phy_interface;
+ int phy_address;
+ int phy_speed;
+ int phy_duplex;
+ u8 mac_address[ETH_ALEN];
+ int genet_version;
+};
+
+#endif
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [RFC PATCH] netfilter: conntrack: cache route for forwarded connections
From: Florian Westphal @ 2014-12-02 0:28 UTC (permalink / raw)
To: netfilter-devel; +Cc: brouer, netdev, Florian Westphal
... to avoid per-packet FIB lookup if possible.
The cached dst is re-used provided the input interface
is the same as that of the previous packet in the same direction.
If not, the cached dst is invalidated.
This should speed up forwarding when conntrack is already in use
anyway, especially when using reverse path filtering -- active RPF
enforces two FIB lookups for each packet.
Before the routing cache removal this didn't matter since RPF
was performed only when route cache didn't yield a result; but without
route cache it comes at high price.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
Sending as RFC since I haven't tested this yet (aside from
single-forwarded-flow), so no performance data either.
- doesn't work when iif changes (it invalidates cached dst), don't
think its a problem
- auto-active when module is loaded (and always active when =y).
- cache becomes used after conntrack enters ASSURED state.
TODOs:
- integrate with netfilter ipv4/ipv6 rpfilter match (already working on this)
Things to consider:
- perhaps extend it for local connections too; could cache
socket to get sk from conntrack (and then fetchg dst from sk).
include/net/netfilter/nf_conntrack_extend.h | 4 +
include/net/netfilter/nf_conntrack_rtcache.h | 28 +++
net/netfilter/Kconfig | 11 +
net/netfilter/Makefile | 1 +
net/netfilter/nf_conntrack_rtcache.c | 327 +++++++++++++++++++++++++++
5 files changed, 371 insertions(+)
create mode 100644 include/net/netfilter/nf_conntrack_rtcache.h
create mode 100644 net/netfilter/nf_conntrack_rtcache.c
diff --git a/include/net/netfilter/nf_conntrack_extend.h b/include/net/netfilter/nf_conntrack_extend.h
index 55d1504..1b00d57 100644
--- a/include/net/netfilter/nf_conntrack_extend.h
+++ b/include/net/netfilter/nf_conntrack_extend.h
@@ -30,6 +30,9 @@ enum nf_ct_ext_id {
#if IS_ENABLED(CONFIG_NETFILTER_SYNPROXY)
NF_CT_EXT_SYNPROXY,
#endif
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_RTCACHE)
+ NF_CT_EXT_RTCACHE,
+#endif
NF_CT_EXT_NUM,
};
@@ -43,6 +46,7 @@ enum nf_ct_ext_id {
#define NF_CT_EXT_TIMEOUT_TYPE struct nf_conn_timeout
#define NF_CT_EXT_LABELS_TYPE struct nf_conn_labels
#define NF_CT_EXT_SYNPROXY_TYPE struct nf_conn_synproxy
+#define NF_CT_EXT_RTCACHE_TYPE struct nf_conn_rtcache
/* Extensions: optional stuff which isn't permanently in struct. */
struct nf_ct_ext {
diff --git a/include/net/netfilter/nf_conntrack_rtcache.h b/include/net/netfilter/nf_conntrack_rtcache.h
new file mode 100644
index 0000000..e8d215d
--- /dev/null
+++ b/include/net/netfilter/nf_conntrack_rtcache.h
@@ -0,0 +1,28 @@
+#include <linux/gfp.h>
+#include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_extend.h>
+
+struct dst_entry;
+union nf_conn_cache_ptr {
+ struct dst_entry *dst;
+};
+
+struct nf_conn_rtcache {
+ union nf_conn_cache_ptr ptr[IP_CT_DIR_MAX];
+ int iif[IP_CT_DIR_MAX];
+};
+
+static inline struct nf_conn_rtcache *nf_ct_rtcache_find(const struct nf_conn *ct)
+{
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_RTCACHE)
+ return nf_ct_ext_find(ct, NF_CT_EXT_RTCACHE);
+#else
+ return NULL;
+#endif
+}
+
+static inline int nf_conn_rtcache_iif_get(const struct nf_conn_rtcache *rtc,
+ enum ip_conntrack_dir dir)
+{
+ return rtc->iif[dir];
+}
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index b02660f..4a8b7e7 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -106,6 +106,17 @@ config NF_CONNTRACK_EVENTS
If unsure, say `N'.
+config NF_CONNTRACK_RTCACHE
+ tristate "Cache route entries in conntrack objects"
+ depends on NETFILTER_ADVANCED
+ depends on NF_CONNTRACK
+ help
+ If this option is enabled, the connection tracking code will
+ cache routing information for each connection that is being
+ forwarded.
+
+ If unsure, say `N'.
+
config NF_CONNTRACK_TIMEOUT
bool 'Connection tracking timeout'
depends on NETFILTER_ADVANCED
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 89f73a9..ac830c9 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -5,6 +5,7 @@ nf_conntrack-$(CONFIG_NF_CONNTRACK_TIMEOUT) += nf_conntrack_timeout.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_TIMESTAMP) += nf_conntrack_timestamp.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_EVENTS) += nf_conntrack_ecache.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_LABELS) += nf_conntrack_labels.o
+nf_conntrack-$(CONFIG_NF_CONNTRACK_RTCACHE) += nf_conntrack_rtcache.o
obj-$(CONFIG_NETFILTER) = netfilter.o
diff --git a/net/netfilter/nf_conntrack_rtcache.c b/net/netfilter/nf_conntrack_rtcache.c
new file mode 100644
index 0000000..1013987
--- /dev/null
+++ b/net/netfilter/nf_conntrack_rtcache.c
@@ -0,0 +1,327 @@
+/* route cache for netfilter.
+ *
+ * (C) 2014 Red Hat GmbH
+ *
+ * This program 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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/types.h>
+#include <linux/netfilter.h>
+#include <linux/skbuff.h>
+#include <linux/stddef.h>
+#include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/export.h>
+#include <linux/module.h>
+
+#include <net/dst.h>
+
+#include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_core.h>
+#include <net/netfilter/nf_conntrack_extend.h>
+#include <net/netfilter/nf_conntrack_rtcache.h>
+
+static void __nf_conn_rtcache_destroy(struct nf_conn_rtcache *rtc, int dir)
+{
+ struct dst_entry *dst;
+
+ if (rtc->iif[dir] < 0)
+ return;
+
+ dst = rtc->ptr[dir].dst;
+ pr_debug("release dst %p, refcnt %d, dir %d\n", dst, atomic_read(&dst->__refcnt), dir);
+ dst_release(dst);
+}
+
+static void nf_conn_rtcache_destroy(struct nf_conn *ct)
+{
+ struct nf_conn_rtcache *rtc = nf_ct_rtcache_find(ct);
+
+ if (!rtc)
+ return;
+
+ __nf_conn_rtcache_destroy(rtc, IP_CT_DIR_ORIGINAL);
+ __nf_conn_rtcache_destroy(rtc, IP_CT_DIR_REPLY);
+}
+
+static void nf_ct_rtcache_ext_add(struct nf_conn *ct)
+{
+ struct nf_conn_rtcache *rtc;
+
+ rtc = nf_ct_ext_add(ct, NF_CT_EXT_RTCACHE, GFP_ATOMIC);
+ if (rtc) {
+ rtc->iif[IP_CT_DIR_ORIGINAL] = -1;
+ rtc->iif[IP_CT_DIR_REPLY] = -1;
+ }
+}
+
+static bool nf_rtcache_ignore_ct(struct nf_conn *ct)
+{
+ if (nf_ct_is_untracked(ct))
+ return true;
+
+ if (!test_bit(IPS_ASSURED_BIT, &ct->status))
+ return true;
+ return false;
+}
+
+static struct nf_conn_rtcache *nf_ct_rtcache_find_usable(struct nf_conn *ct)
+{
+ if (nf_rtcache_ignore_ct(ct))
+ return NULL;
+ return nf_ct_rtcache_find(ct);
+}
+
+static struct dst_entry *
+nf_conn_rtcache_dst_get(const struct nf_conn_rtcache *rtc,
+ enum ip_conntrack_dir dir)
+{
+ return rtc->ptr[dir].dst;
+}
+
+static void nf_conn_rtcache_dst_set(struct nf_conn_rtcache *rtc,
+ struct dst_entry *dst,
+ enum ip_conntrack_dir dir, int iif)
+{
+ if (rtc->iif[dir] != iif)
+ rtc->iif[dir] = iif;
+
+ if (rtc->ptr[dir].dst != dst) {
+ struct dst_entry *old;
+
+ dst_hold(dst);
+
+ old = xchg(&rtc->ptr[dir].dst, dst);
+ dst_release(old);
+ }
+}
+
+static void nf_conn_rtcache_dst_obsolete(struct nf_conn_rtcache *rtc,
+ enum ip_conntrack_dir dir)
+{
+ struct dst_entry *old;
+
+ pr_debug("Invalidate iif %d for dir %d on cache %p\n",
+ rtc->iif[dir], dir, rtc);
+
+ old = xchg(&rtc->ptr[dir].dst, NULL);
+ dst_release(old);
+ rtc->iif[dir] = -1;
+}
+
+static unsigned int nf_rtcache_in(const struct nf_hook_ops *ops,
+ struct sk_buff *skb,
+ const struct net_device *in,
+ const struct net_device *out,
+ int (*okfn)(struct sk_buff *))
+{
+ struct nf_conn_rtcache *ct_rtcache;
+ enum ip_conntrack_info ctinfo;
+ enum ip_conntrack_dir dir;
+ struct dst_entry *dst;
+ struct nf_conn *ct;
+ int iif;
+
+ if (in == NULL || skb_dst(skb))
+ return NF_ACCEPT;
+
+ ct = nf_ct_get(skb, &ctinfo);
+ if (!ct)
+ return NF_ACCEPT;
+
+ ct_rtcache = nf_ct_rtcache_find_usable(ct);
+ if (!ct_rtcache)
+ return NF_ACCEPT;
+
+ /* if iif changes, don't use cache and let ip stack
+ * do route lookup.
+ *
+ * If rp_filter is enabled it might toss skb, so
+ * we don't want to avoid these checks.
+ */
+ dir = CTINFO2DIR(ctinfo);
+ iif = nf_conn_rtcache_iif_get(ct_rtcache, dir);
+ if (in->ifindex != iif) {
+ pr_debug("iif %d did not match cached iif %d of ct %p\n\n", iif, in->ifindex, ct);
+ return NF_ACCEPT;
+ }
+ dst = nf_conn_rtcache_dst_get(ct_rtcache, dir);
+ if (dst == NULL)
+ return NF_ACCEPT;
+ dst = dst_check(dst, 0);
+ if (likely(dst)) {
+ pr_debug("using cached dst %p for skb %p\n", dst, skb);
+ skb_dst_set_noref_force(skb, dst);
+ } else {
+ nf_conn_rtcache_dst_obsolete(ct_rtcache, dir);
+ }
+ return NF_ACCEPT;
+}
+
+static unsigned int nf_rtcache_forward(const struct nf_hook_ops *ops,
+ struct sk_buff *skb,
+ const struct net_device *in,
+ const struct net_device *out,
+ int (*okfn)(struct sk_buff *))
+{
+ struct nf_conn_rtcache *ct_rtcache;
+ enum ip_conntrack_info ctinfo;
+ enum ip_conntrack_dir dir;
+ struct dst_entry *dst;
+ struct nf_conn *ct;
+ int iif;
+
+ dst = skb_dst(skb);
+ if (WARN_ON_ONCE(dst == NULL || in == NULL))
+ return NF_ACCEPT;
+
+ ct = nf_ct_get(skb, &ctinfo);
+ if (!ct)
+ return NF_ACCEPT;
+
+ if (!nf_ct_is_confirmed(ct)) {
+ pr_debug("new ct %p, skb %p, adding rtcache extension\n", ct, skb);
+ BUG_ON(nf_ct_rtcache_find(ct));
+ nf_ct_rtcache_ext_add(ct);
+ return NF_ACCEPT;
+ }
+
+ ct_rtcache = nf_ct_rtcache_find_usable(ct);
+ if (!ct_rtcache)
+ return NF_ACCEPT;
+
+ dir = CTINFO2DIR(ctinfo);
+ iif = nf_conn_rtcache_iif_get(ct_rtcache, dir);
+ pr_debug("ct %p, skb %p, dir %d, iif %d, cached iif %d\n", ct, skb, dir, iif, in->ifindex);
+ if (likely(in->ifindex == iif))
+ return NF_ACCEPT;
+
+ nf_conn_rtcache_dst_set(ct_rtcache, dst, dir, in->ifindex);
+ return NF_ACCEPT;
+}
+
+static struct nf_hook_ops rtcache_ops[] = {
+ {
+ .hook = nf_rtcache_in,
+ .owner = THIS_MODULE,
+ .pf = NFPROTO_IPV4,
+ .hooknum = NF_INET_PRE_ROUTING,
+ .priority = NF_IP_PRI_LAST,
+ },
+ {
+ .hook = nf_rtcache_forward,
+ .owner = THIS_MODULE,
+ .pf = NFPROTO_IPV4,
+ .hooknum = NF_INET_FORWARD,
+ .priority = NF_IP_PRI_LAST,
+ },
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_IPV6)
+ {
+ .hook = nf_rtcache_in,
+ .owner = THIS_MODULE,
+ .pf = NFPROTO_IPV6,
+ .hooknum = NF_INET_PRE_ROUTING,
+ .priority = NF_IP_PRI_LAST,
+ },
+ {
+ .hook = nf_rtcache_forward,
+ .owner = THIS_MODULE,
+ .pf = NFPROTO_IPV6,
+ .hooknum = NF_INET_FORWARD,
+ .priority = NF_IP_PRI_LAST,
+ },
+#endif
+};
+
+static struct nf_ct_ext_type rtcache_extend __read_mostly = {
+ .len = sizeof(struct nf_conn_rtcache),
+ .align = __alignof__(struct nf_conn_rtcache),
+ .id = NF_CT_EXT_RTCACHE,
+ .destroy = nf_conn_rtcache_destroy,
+};
+
+int __init nf_conntrack_rtcache_init(void)
+{
+ int ret = nf_ct_extend_register(&rtcache_extend);
+ if (ret < 0) {
+ pr_err("nf_conntrack_rtcache: Unable to register extension\n");
+ return ret;
+ }
+
+ ret = nf_register_hooks(rtcache_ops, ARRAY_SIZE(rtcache_ops));
+ if (ret < 0) {
+ nf_ct_extend_unregister(&rtcache_extend);
+ return ret;
+ }
+
+ return ret;
+}
+
+static int nf_rtcache_ext_remove(struct nf_conn *ct, void *data)
+{
+ struct nf_conn_rtcache *rtc = nf_ct_rtcache_find(ct);
+ return rtc != NULL;
+}
+
+void __exit nf_conntrack_rtcache_fini(void)
+{
+ struct net *net;
+ bool wait;
+ int cpu, count = 0;
+
+ /* first remove hooks so no new connections get rtcache extension */
+ nf_unregister_hooks(rtcache_ops, ARRAY_SIZE(rtcache_ops));
+
+ synchronize_net();
+
+ /* zap all conntracks with rtcache extension */
+ for_each_net(net)
+ nf_ct_iterate_cleanup(net, nf_rtcache_ext_remove, NULL, 0, 0);
+
+ /* again, so that affected conntracks on the unconfirmed list are now
+ * free'd or on dying list */
+ synchronize_net();
+
+ /* ... and wait until dying list contains no affected items any more */
+ do {
+ wait = false;
+ for_each_possible_cpu(cpu) {
+ struct nf_conntrack_tuple_hash *h;
+ struct hlist_nulls_node *n;
+ struct nf_conn *ct;
+ struct ct_pcpu *pcpu = per_cpu_ptr(net->ct.pcpu_lists, cpu);
+
+ rcu_read_lock();
+ spin_lock_bh(&pcpu->lock);
+
+ hlist_nulls_for_each_entry(h, n, &pcpu->unconfirmed, hnnode) {
+ ct = nf_ct_tuplehash_to_ctrack(h);
+ if (nf_ct_rtcache_find(ct) != NULL) {
+ wait = true;
+ break;
+ }
+ }
+ spin_unlock_bh(&pcpu->lock);
+ rcu_read_unlock();
+
+ if (wait) {
+ /* conntrack on dying list, i.e. waiting for
+ * some event, e.g. reinject or whatever */
+ msleep(200);
+ WARN_ONCE(++count > 25, "Waiting for all rtcache conntracks to go away\n");
+ }
+ }
+ } while (wait);
+
+ nf_ct_extend_unregister(&rtcache_extend);
+}
+module_init(nf_conntrack_rtcache_init);
+module_exit(nf_conntrack_rtcache_fini);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Florian Westphal <fw@strlen.de>");
+MODULE_DESCRIPTION("Conntrack route cache extension");
--
2.0.4
^ permalink raw reply related
* Re: [PATCH] ioc3: fix incorrect use of htons/ntohs
From: Lino Sanfilippo @ 2014-12-02 1:31 UTC (permalink / raw)
To: Ben Hutchings; +Cc: ralf, linux-mips, netdev, linux-kernel
In-Reply-To: <1417406976.7215.126.camel@decadent.org.uk>
On 01.12.2014 05:09, Ben Hutchings wrote:
> On Sun, 2014-11-30 at 11:40 +0100, Lino Sanfilippo wrote:
>> The protocol type in the ip header struct is a single byte variable. So there
>> is no need to swap bytes depending on host endianness.
>>
>> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
>> ---
>>
>> Please note that I could not test this, since I dont have access to the
>> concerning hardware.
>>
>> drivers/net/ethernet/sgi/ioc3-eth.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
>> index 7a254da..0bb303d 100644
>> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
>> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
>> @@ -540,8 +540,7 @@ static void ioc3_tcpudp_checksum(struct sk_buff *skb, uint32_t hwsum, int len)
>>
>> /* Same as tx - compute csum of pseudo header */
>> csum = hwsum +
>> - (ih->tot_len - (ih->ihl << 2)) +
>> - htons((uint16_t)ih->protocol) +
>> + (ih->tot_len - (ih->ihl << 2)) + ih->protocol +
>> (ih->saddr >> 16) + (ih->saddr & 0xffff) +
>> (ih->daddr >> 16) + (ih->daddr & 0xffff);
>>
>
> The pseudo-header is specified as:
>
> +--------+--------+--------+--------+
> | Source Address |
> +--------+--------+--------+--------+
> | Destination Address |
> +--------+--------+--------+--------+
> | zero | PTCL | TCP Length |
> +--------+--------+--------+--------+
>
> The current code zero-extends the protocol number to produce the 5th
> 16-bit word of the pseudo-header, then uses htons() to put it in
> big-endian order, consistent with the other fields. (Yes, it's doing
> addition on big-endian words; this works even on little-endian machines
> due to the way the checksum is specified.)
>
> The driver should not be doing this at all, though. It should set
> skb->csum = hwsum; skb->ip_summed = CHECKSUM_COMPLETE; and let the
> network stack adjust the hardware checksum.
>
>> @@ -1417,7 +1416,7 @@ static int ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev)
>> */
>> if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> const struct iphdr *ih = ip_hdr(skb);
>> - const int proto = ntohs(ih->protocol);
>> + const int proto = ih->protocol;
>> unsigned int csoff;
>> uint32_t csum, ehsum;
>> uint16_t *eh;
>
> This should logically be __be16 proto = htons(ih->protocol), but the
> current version should work in practice.
>
> However, the driver should really use skb->csum_start and
> skb->csum_offset to work out where the checksum belongs and which bytes
> to cancel out.
>
> Ben.
>
Hi Ben,
youre right, the use of htons/ntohs is correct in this case. So thank
you for the explanation and please ignore that patch.
Regards,
Lino
^ permalink raw reply
* Re: [RFC PATCH] netfilter: conntrack: cache route for forwarded connections
From: Eric Dumazet @ 2014-12-02 1:36 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, brouer, netdev
In-Reply-To: <1417480114-3002-1-git-send-email-fw@strlen.de>
On Tue, 2014-12-02 at 01:28 +0100, Florian Westphal wrote:
> ... to avoid per-packet FIB lookup if possible.
>
> The cached dst is re-used provided the input interface
> is the same as that of the previous packet in the same direction.
>
> If not, the cached dst is invalidated.
>
> This should speed up forwarding when conntrack is already in use
> anyway, especially when using reverse path filtering -- active RPF
> enforces two FIB lookups for each packet.
>
> Before the routing cache removal this didn't matter since RPF
> was performed only when route cache didn't yield a result; but without
> route cache it comes at high price.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
Seems a good idea (but you might need some IPv6 care, as ( dst =
dst_check(dst, 0); ) seems to handle IPv4 only)
Another idea would be to re-use TCP ehash so that regular IP early demux
can be used, with a single lookup for both local and forwarded sessions.
(That would probably require a bit more memory, as you would need to
insert into TCP ehash some kind of 'tiny sockets' )
^ permalink raw reply
* [PATCH net-next] udp: Neaten and reduce size of compute_score functions
From: Joe Perches @ 2014-12-02 1:39 UTC (permalink / raw)
To: netdev; +Cc: LKML
The compute_score functions are a bit difficult to read.
Neaten them a bit to reduce object sizes and make them a
bit more intelligible.
Return early to avoid indentation and avoid unnecessary
initializations.
(allyesconfig, but w/ -O2 and no profiling)
$ size net/ipv[46]/udp.o.*
text data bss dec hex filename
28680 1184 25 29889 74c1 net/ipv4/udp.o.new
28756 1184 25 29965 750d net/ipv4/udp.o.old
17600 1010 2 18612 48b4 net/ipv6/udp.o.new
17632 1010 2 18644 48d4 net/ipv6/udp.o.old
Signed-off-by: Joe Perches <joe@perches.com>
---
net/ipv4/udp.c | 111 +++++++++++++++++++++++++++++++-------------------------
net/ipv6/udp.c | 113 ++++++++++++++++++++++++++++++++-------------------------
2 files changed, 125 insertions(+), 99 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index b2d6068..227e6ad 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -336,38 +336,45 @@ int udp_v4_get_port(struct sock *sk, unsigned short snum)
return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal, hash2_nulladdr);
}
-static inline int compute_score(struct sock *sk, struct net *net, __be32 saddr,
- unsigned short hnum,
- __be16 sport, __be32 daddr, __be16 dport, int dif)
+static inline int compute_score(struct sock *sk, struct net *net,
+ __be32 saddr, unsigned short hnum, __be16 sport,
+ __be32 daddr, __be16 dport, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!(net_eq(sock_net(sk), net) &&
+ udp_sk(sk)->udp_port_hash == hnum &&
+ !ipv6_only_sock(sk)))
+ return -1;
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_rcv_saddr) {
- if (inet->inet_rcv_saddr != daddr)
- return -1;
- score += 4;
- }
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr) {
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ score += 4;
+ }
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
+ return -1;
+ score += 4;
+ }
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score += 4;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
}
+
return score;
}
@@ -378,33 +385,39 @@ static inline int compute_score2(struct sock *sk, struct net *net,
__be32 saddr, __be16 sport,
__be32 daddr, unsigned int hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!(net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)))
+ return -1;
- if (inet->inet_rcv_saddr != daddr)
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ if (inet->inet_num != hnum)
+ return -1;
+
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
return -1;
- if (inet->inet_num != hnum)
+ score += 4;
+ }
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
+ score += 4;
+ }
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
}
+
return score;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 7cfb5d7..0b9644a 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -148,72 +148,85 @@ static inline int compute_score(struct sock *sk, struct net *net,
const struct in6_addr *daddr, __be16 dport,
int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!(net_eq(sock_net(sk), net) &&
+ udp_sk(sk)->udp_port_hash == hnum &&
+ sk->sk_family == PF_INET6))
+ return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+ score++;
+ }
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
#define SCORE2_MAX (1 + 1 + 1)
static inline int compute_score2(struct sock *sk, struct net *net,
- const struct in6_addr *saddr, __be16 sport,
- const struct in6_addr *daddr, unsigned short hnum,
- int dif)
+ const struct in6_addr *saddr, __be16 sport,
+ const struct in6_addr *daddr,
+ unsigned short hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!(net_eq(sock_net(sk), net) &&
+ udp_sk(sk)->udp_port_hash == hnum &&
+ sk->sk_family == PF_INET6))
+ return -1;
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
-
/* called with read_rcu_lock() */
static struct sock *udp6_lib_lookup2(struct net *net,
const struct in6_addr *saddr, __be16 sport,
^ permalink raw reply related
* Re: [PATCH net-next] udp: Neaten and reduce size of compute_score functions
From: Eric Dumazet @ 2014-12-02 2:59 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
In-Reply-To: <1417484341.4894.6.camel@perches.com>
On Mon, 2014-12-01 at 17:39 -0800, Joe Perches wrote:
> The compute_score functions are a bit difficult to read.
>
> Neaten them a bit to reduce object sizes and make them a
> bit more intelligible.
>
> Return early to avoid indentation and avoid unnecessary
> initializations.
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> net/ipv4/udp.c | 111 +++++++++++++++++++++++++++++++-------------------------
> net/ipv6/udp.c | 113 ++++++++++++++++++++++++++++++++-------------------------
> 2 files changed, 125 insertions(+), 99 deletions(-)
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index b2d6068..227e6ad 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -336,38 +336,45 @@ int udp_v4_get_port(struct sock *sk, unsigned short snum)
> return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal, hash2_nulladdr);
> }
>
> -static inline int compute_score(struct sock *sk, struct net *net, __be32 saddr,
> - unsigned short hnum,
> - __be16 sport, __be32 daddr, __be16 dport, int dif)
> +static inline int compute_score(struct sock *sk, struct net *net,
> + __be32 saddr, unsigned short hnum, __be16 sport,
> + __be32 daddr, __be16 dport, int dif)
> {
> - int score = -1;
> + int score;
> + struct inet_sock *inet;
>
> - if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
> - !ipv6_only_sock(sk)) {
> - struct inet_sock *inet = inet_sk(sk);
> + if (!(net_eq(sock_net(sk), net) &&
> + udp_sk(sk)->udp_port_hash == hnum &&
> + !ipv6_only_sock(sk)))
> + return -1
Or even better :
if (!net_eq(sock_net(sk), net) ||
udp_sk(sk)->udp_port_hash != hnum ||
ipv6_only_sock(sk))
return -1;
^ permalink raw reply
* Re: [PATCH RFC v4 net-next 1/5] virtio_net: enable tx interrupt
From: Jason Wang @ 2014-12-02 3:09 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pagupta, netdev, linux-kernel, virtualization, davem
In-Reply-To: <20141201103536.GA16108@redhat.com>
On Mon, Dec 1, 2014 at 6:35 PM, Michael S. Tsirkin <mst@redhat.com>
wrote:
> On Mon, Dec 01, 2014 at 06:17:04PM +0800, Jason Wang wrote:
>> On newer hosts that support delayed tx interrupts,
>> we probably don't have much to gain from orphaning
>> packets early.
>>
>> Note: this might degrade performance for
>> hosts without event idx support.
>> Should be addressed by the next patch.
>>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> Could you document the changes from the RFC I sent please?
Okay, I will sent a V5 add document more.
>
> Are there optimizations?
Two optimizations.
- Don't do tx packets free in ndo_start_xmit(), tests shows it reduce
the effect of coalescing.
- Let ethtool can change the number of packets freed in one tx napi run
through tx-frames-irq. This is necessary, since user may coalesce more
than 64 packets per irq.
>
> If yes, it might be easier to review (at least for me), if you
> refactor this,
> e.g. applying the straight-forward rfc patch and then optimizations if
> any on top. If it's taking a different approach, pls feel free to
> disregard this.
>
>
>> ---
>> drivers/net/virtio_net.c | 132
>> +++++++++++++++++++++++++++++++----------------
>> 1 file changed, 88 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index ec2a8b4..f68114e 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -72,6 +72,8 @@ struct send_queue {
>>
>> /* Name of the send queue: output.$index */
>> char name[40];
>> +
>> + struct napi_struct napi;
>> };
>>
>> /* Internal representation of a receive virtqueue */
>> @@ -137,6 +139,9 @@ struct virtnet_info {
>>
>> /* CPU hot plug notifier */
>> struct notifier_block nb;
>> +
>> + /* Budget for polling tx completion */
>> + u32 tx_work_limit;
>> };
>>
>> struct skb_vnet_hdr {
>> @@ -211,15 +216,41 @@ static struct page *get_a_page(struct
>> receive_queue *rq, gfp_t gfp_mask)
>> return p;
>> }
>>
>> +static unsigned int free_old_xmit_skbs(struct netdev_queue *txq,
>> + struct send_queue *sq, int budget)
>> +{
>> + struct sk_buff *skb;
>> + unsigned int len;
>> + struct virtnet_info *vi = sq->vq->vdev->priv;
>> + struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
>> + unsigned int packets = 0;
>> +
>> + while (packets < budget &&
>> + (skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
>> + pr_debug("Sent skb %p\n", skb);
>> +
>> + u64_stats_update_begin(&stats->tx_syncp);
>> + stats->tx_bytes += skb->len;
>> + stats->tx_packets++;
>> + u64_stats_update_end(&stats->tx_syncp);
>> +
>> + dev_kfree_skb_any(skb);
>> + packets++;
>> + }
>> +
>> + if (sq->vq->num_free >= 2+MAX_SKB_FRAGS)
>> + netif_tx_start_queue(txq);
>> +
>> + return packets;
>> +}
>> +
>> static void skb_xmit_done(struct virtqueue *vq)
>> {
>> struct virtnet_info *vi = vq->vdev->priv;
>> + struct send_queue *sq = &vi->sq[vq2txq(vq)];
>>
>> - /* Suppress further interrupts. */
>> - virtqueue_disable_cb(vq);
>> -
>> - /* We were probably waiting for more output buffers. */
>> - netif_wake_subqueue(vi->dev, vq2txq(vq));
>> + virtqueue_disable_cb(sq->vq);
>> + napi_schedule(&sq->napi);
>> }
>>
>> static unsigned int mergeable_ctx_to_buf_truesize(unsigned long
>> mrg_ctx)
>> @@ -777,6 +808,32 @@ again:
>> return received;
>> }
>>
>> +static int virtnet_poll_tx(struct napi_struct *napi, int budget)
>> +{
>> + struct send_queue *sq =
>> + container_of(napi, struct send_queue, napi);
>> + struct virtnet_info *vi = sq->vq->vdev->priv;
>> + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev,
>> vq2txq(sq->vq));
>> + u32 limit = vi->tx_work_limit;
>> + unsigned int sent;
>> +
>> + __netif_tx_lock(txq, smp_processor_id());
>> + sent = free_old_xmit_skbs(txq, sq, limit);
>> + if (sent < limit) {
>> + napi_complete(napi);
>> + /* Note: we must enable cb *after* napi_complete, because
>> + * napi_schedule calls from callbacks that trigger before
>> + * napi_complete are ignored.
>> + */
>> + if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
>> + virtqueue_disable_cb(sq->vq);
>> + napi_schedule(&sq->napi);
>> + }
>> + }
>> + __netif_tx_unlock(txq);
>> + return sent < limit ? 0 : budget;
>> +}
>> +
>
> Unlike the patch I sent, this seems to ignore the budget,
> and always poll the full napi_weight.
> Seems strange. What is the reason for this?
>
The budget were in fact the tx_work_limit (by default 64).
This could be tuned by ethtool tx-frames-irq to control
how many packets at most could be processed by on tx napi.
Use may want to coalesce more than 64 packets per irq, so
something like this is necessary for user.
>
>
>> #ifdef CONFIG_NET_RX_BUSY_POLL
>> /* must be called with local_bh_disable()d */
>> static int virtnet_busy_poll(struct napi_struct *napi)
>> @@ -825,30 +882,12 @@ static int virtnet_open(struct net_device
>> *dev)
>> if (!try_fill_recv(&vi->rq[i], GFP_KERNEL))
>> schedule_delayed_work(&vi->refill, 0);
>> virtnet_napi_enable(&vi->rq[i]);
>> + napi_enable(&vi->sq[i].napi);
>> }
>>
>> return 0;
>> }
>>
>> -static void free_old_xmit_skbs(struct send_queue *sq)
>> -{
>> - struct sk_buff *skb;
>> - unsigned int len;
>> - struct virtnet_info *vi = sq->vq->vdev->priv;
>> - struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
>> -
>> - while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
>> - pr_debug("Sent skb %p\n", skb);
>> -
>> - u64_stats_update_begin(&stats->tx_syncp);
>> - stats->tx_bytes += skb->len;
>> - stats->tx_packets++;
>> - u64_stats_update_end(&stats->tx_syncp);
>> -
>> - dev_kfree_skb_any(skb);
>> - }
>> -}
>> -
>> static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
>> {
>> struct skb_vnet_hdr *hdr;
>> @@ -912,7 +951,9 @@ static int xmit_skb(struct send_queue *sq,
>> struct sk_buff *skb)
>> sg_set_buf(sq->sg, hdr, hdr_len);
>> num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
>> }
>> - return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb,
>> GFP_ATOMIC);
>> +
>> + return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb,
>> + GFP_ATOMIC);
>> }
>>
>> static netdev_tx_t start_xmit(struct sk_buff *skb, struct
>> net_device *dev)
>> @@ -924,8 +965,7 @@ static netdev_tx_t start_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>> struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
>> bool kick = !skb->xmit_more;
>>
>> - /* Free up any pending old buffers before queueing new ones. */
>> - free_old_xmit_skbs(sq);
>> + virtqueue_disable_cb(sq->vq);
>>
>> /* Try to transmit */
>> err = xmit_skb(sq, skb);
>> @@ -941,27 +981,19 @@ static netdev_tx_t start_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>> return NETDEV_TX_OK;
>> }
>>
>> - /* Don't wait up for transmitted skbs to be freed. */
>> - skb_orphan(skb);
>> - nf_reset(skb);
>> -
>> /* Apparently nice girls don't return TX_BUSY; stop the queue
>> * before it gets out of hand. Naturally, this wastes entries. */
>> - if (sq->vq->num_free < 2+MAX_SKB_FRAGS) {
>> + if (sq->vq->num_free < 2+MAX_SKB_FRAGS)
>> netif_stop_subqueue(dev, qnum);
>> - if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
>> - /* More just got used, free them then recheck. */
>> - free_old_xmit_skbs(sq);
>> - if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
>> - netif_start_subqueue(dev, qnum);
>> - virtqueue_disable_cb(sq->vq);
>> - }
>> - }
>> - }
>>
>> if (kick || netif_xmit_stopped(txq))
>> virtqueue_kick(sq->vq);
>>
>> + if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
>> + virtqueue_disable_cb(sq->vq);
>> + napi_schedule(&sq->napi);
>> + }
>> +
>> return NETDEV_TX_OK;
>> }
>>
>> @@ -1138,8 +1170,10 @@ static int virtnet_close(struct net_device
>> *dev)
>> /* Make sure refill_work doesn't re-enable napi! */
>> cancel_delayed_work_sync(&vi->refill);
>>
>> - for (i = 0; i < vi->max_queue_pairs; i++)
>> + for (i = 0; i < vi->max_queue_pairs; i++) {
>> napi_disable(&vi->rq[i].napi);
>> + napi_disable(&vi->sq[i].napi);
>> + }
>>
>> return 0;
>> }
>> @@ -1452,8 +1486,10 @@ static void virtnet_free_queues(struct
>> virtnet_info *vi)
>> {
>> int i;
>>
>> - for (i = 0; i < vi->max_queue_pairs; i++)
>> + for (i = 0; i < vi->max_queue_pairs; i++) {
>> netif_napi_del(&vi->rq[i].napi);
>> + netif_napi_del(&vi->sq[i].napi);
>> + }
>>
>> kfree(vi->rq);
>> kfree(vi->sq);
>> @@ -1607,6 +1643,8 @@ static int virtnet_alloc_queues(struct
>> virtnet_info *vi)
>> netif_napi_add(vi->dev, &vi->rq[i].napi, virtnet_poll,
>> napi_weight);
>> napi_hash_add(&vi->rq[i].napi);
>> + netif_napi_add(vi->dev, &vi->sq[i].napi, virtnet_poll_tx,
>> + napi_weight);
>>
>> sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
>> ewma_init(&vi->rq[i].mrg_avg_pkt_len, 1, RECEIVE_AVG_WEIGHT);
>> @@ -1790,6 +1828,8 @@ static int virtnet_probe(struct virtio_device
>> *vdev)
>> if (err)
>> goto free_stats;
>>
>> + vi->tx_work_limit = napi_weight;
>> +
>> #ifdef CONFIG_SYSFS
>> if (vi->mergeable_rx_bufs)
>> dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
>> @@ -1904,8 +1944,10 @@ static int virtnet_freeze(struct
>> virtio_device *vdev)
>> if (netif_running(vi->dev)) {
>> for (i = 0; i < vi->max_queue_pairs; i++) {
>> napi_disable(&vi->rq[i].napi);
>> + napi_disable(&vi->sq[i].napi);
>> napi_hash_del(&vi->rq[i].napi);
>> netif_napi_del(&vi->rq[i].napi);
>> + netif_napi_del(&vi->sq[i].napi);
>> }
>> }
>>
>> @@ -1930,8 +1972,10 @@ static int virtnet_restore(struct
>> virtio_device *vdev)
>> if (!try_fill_recv(&vi->rq[i], GFP_KERNEL))
>> schedule_delayed_work(&vi->refill, 0);
>>
>> - for (i = 0; i < vi->max_queue_pairs; i++)
>> + for (i = 0; i < vi->max_queue_pairs; i++) {
>> virtnet_napi_enable(&vi->rq[i]);
>> + napi_enable(&vi->sq[i].napi);
>> + }
>> }
>>
>> netif_device_attach(vi->dev);
>> --
>> 1.8.3.1
^ permalink raw reply
* Re: [PATCH net-next] udp: Neaten and reduce size of compute_score functions
From: Joe Perches @ 2014-12-02 3:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, LKML
In-Reply-To: <1417489141.4442.24.camel@edumazet-glaptop2.roam.corp.google.com>
On Mon, 2014-12-01 at 18:59 -0800, Eric Dumazet wrote:
> On Mon, 2014-12-01 at 17:39 -0800, Joe Perches wrote:
> > The compute_score functions are a bit difficult to read.
> >
> > Neaten them a bit to reduce object sizes and make them a
> > bit more intelligible.
> >
> > Return early to avoid indentation and avoid unnecessary
> > initializations.
[]
> > + if (!(net_eq(sock_net(sk), net) &&
> > + udp_sk(sk)->udp_port_hash == hnum &&
> > + !ipv6_only_sock(sk)))
> > + return -1
>
> Or even better :
>
>
> if (!net_eq(sock_net(sk), net) ||
> udp_sk(sk)->udp_port_hash != hnum ||
> ipv6_only_sock(sk))
> return -1;
Hi Eric.
Yeah, I thought about it but thought it
simpler to not change the logic.
Either way is fine with me.
David?
btw: the same thing can be done for the v6 block too:
+ if (!(net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)))
+ return -1;
- if (inet->inet_rcv_saddr != daddr)
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ if (inet->inet_num != hnum)
+ return -1;
to:
if (!net_eq(sock_net(sk, net) ||
ipv6_only_sock(sk))
return -1;
inet = inet_sk(sk);
if (inet->inet_rcv_saddr != daddr ||
inet->inet_num != hnum)
return -1;
^ permalink raw reply
* Re: [PATCH RFC v4 net-next 0/5] virtio_net: enabling tx interrupts
From: Jason Wang @ 2014-12-02 3:15 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pagupta, netdev, davem, linux-kernel, virtualization
In-Reply-To: <20141201104223.GB16108@redhat.com>
On Mon, Dec 1, 2014 at 6:42 PM, Michael S. Tsirkin <mst@redhat.com>
wrote:
> On Mon, Dec 01, 2014 at 06:17:03PM +0800, Jason Wang wrote:
>> Hello:
>>
>> We used to orphan packets before transmission for virtio-net. This
>> breaks
>> socket accounting and can lead serveral functions won't work, e.g:
>>
>> - Byte Queue Limit depends on tx completion nofication to work.
>> - Packet Generator depends on tx completion nofication for the last
>> transmitted packet to complete.
>> - TCP Small Queue depends on proper accounting of sk_wmem_alloc to
>> work.
>>
>> This series tries to solve the issue by enabling tx interrupts. To
>> minize
>> the performance impacts of this, several optimizations were used:
>>
>> - In guest side, virtqueue_enable_cb_delayed() was used to delay
>> the tx
>> interrupt untile 3/4 pending packets were sent.
>> - In host side, interrupt coalescing were used to reduce tx
>> interrupts.
>>
>> Performance test results[1] (tx-frames 16 tx-usecs 16) shows:
>>
>> - For guest receiving. No obvious regression on throughput were
>> noticed. More cpu utilization were noticed in few cases.
>> - For guest transmission. Very huge improvement on througput for
>> small
>> packet transmission were noticed. This is expected since TSQ and
>> other
>> optimization for small packet transmission work after tx
>> interrupt. But
>> will use more cpu for large packets.
>> - For TCP_RR, regression (10% on transaction rate and cpu
>> utilization) were
>> found. Tx interrupt won't help but cause overhead in this case.
>> Using
>> more aggressive coalescing parameters may help to reduce the
>> regression.
>
> OK, you do have posted coalescing patches - does it help any?
Helps a lot.
For RX, it saves about 5% - 10% cpu. (reduce 60%-90% tx intrs)
For small packet TX, it increases 33% - 245% throughput. (reduce
about 60% inters)
For TCP_RR, it increase the 3%-10% trans.rate. (reduce 40%-80% tx intrs)
>
> I'm not sure the regression is due to interrupts.
> It would make sense for CPU but why would it
> hurt transaction rate?
Anyway guest need to take some cycles to handle tx interrupts.
And transaction rate does increase if we coalesces more tx interurpts.
>
>
> It's possible that we are deferring kicks too much due to BQL.
>
> As an experiment: do we get any of it back if we do
> - if (kick || netif_xmit_stopped(txq))
> - virtqueue_kick(sq->vq);
> + virtqueue_kick(sq->vq);
> ?
I will try, but during TCP_RR, at most 1 packets were pending,
I suspect if BQL can help in this case.
>
>
> If yes, we can just kick e.g. periodically, e.g. after queueing each
> X bytes.
Okay, let me try to see if this help.
>
>> Changes from RFC V3:
>> - Don't free tx packets in ndo_start_xmit()
>> - Add interrupt coalescing support for virtio-net
>> Changes from RFC v2:
>> - clean up code, address issues raised by Jason
>> Changes from RFC v1:
>> - address comments by Jason Wang, use delayed cb everywhere
>> - rebased Jason's patch on top of mine and include it (with some
>> tweaks)
>>
>> Please reivew. Comments were more than welcomed.
>>
>> [1] Performance Test result:
>>
>> Environment:
>> - Two Intel(R) Xeon(R) CPU E5620 @ 2.40GHz machines connected back
>> to back
>> with 82599ES cards.
>> - Both host and guest were net-next.git plus the patch
>> - Coalescing parameters for the card:
>> Adaptive RX: off TX: off
>> rx-usecs: 1
>> rx-frames: 0
>> tx-usecs: 0
>> tx-frames: 0
>> - Vhost_net was enabled and zerocopy was disabled
>> - Tests was done by netperf-2.6
>> - Guest has 2 vcpus with single queue virtio-net
>>
>> Results:
>> - Numbers of square brackets are whose significance is grater than
>> 95%
>>
>> Guest RX:
>>
>> size/sessions/+throughput/+cpu/+per_cpu_throughput/
>> 64/1/+2.0326/[+6.2807%]/-3.9970%/
>> 64/2/-0.2104%/[+3.2012%]/[-3.3058%]/
>> 64/4/+1.5956%/+2.2451%/-0.6353%/
>> 64/8/+1.1732%/+3.5123%/-2.2598%/
>> 256/1/+3.7619%/[+5.8117%]/-1.9372%/
>> 256/2/-0.0661%/[+3.2511%]/-3.2127%/
>> 256/4/+1.1435%/[-8.1842%]/[+10.1591%]/
>> 256/8/[+2.2447%]/[+6.2044%]/[-3.7283%]/
>> 1024/1/+9.1479%/[+12.0997%]/[-2.6332%]/
>> 1024/2/[-17.3341%]/[+0.0000%]/[-17.3341%]/
>> 1024/4/[-0.6284%]/-1.0376%/+0.4135%/
>> 1024/8/+1.1444%/-1.6069%/+2.7961%/
>> 4096/1/+0.0401%/-0.5993%/+0.6433%/
>> 4096/2/[-0.5894%]/-2.2071%/+1.6542%/
>> 4096/4/[-0.5560%]/-1.4969%/+0.9553%/
>> 4096/8/-0.3362%/+2.7086%/-2.9645%/
>> 16384/1/-0.0285%/+0.7247%/-0.7478%/
>> 16384/2/-0.5286%/+0.3287%/-0.8545%/
>> 16384/4/-0.3297%/-2.0543%/+1.7608%/
>> 16384/8/+1.0932%/+4.0253%/-2.8187%/
>> 65535/1/+0.0003%/-0.1502%/+0.1508%/
>> 65535/2/[-0.6065%]/+0.2309%/-0.8355%/
>> 65535/4/[-0.6861%]/[+3.9451%]/[-4.4554%]/
>> 65535/8/+1.8359%/+3.1590%/-1.2825%/
>>
>> Guest RX:
>> size/sessions/+throughput/+cpu/+per_cpu_throughput/
>> 64/1/[+65.0961%]/[-8.6807%]/[+80.7900%]/
>> 64/2/[+6.0288%]/[-2.2823%]/[+8.5052%]/
>> 64/4/[+5.9038%]/[-2.1834%]/[+8.2677%]/
>> 64/8/[+5.4154%]/[-2.1804%]/[+7.7651%]/
>> 256/1/[+184.6462%]/[+4.8906%]/[+171.3742%]/
>> 256/2/[+46.0731%]/[-8.9626%]/[+60.4539%]/
>> 256/4/[+45.8547%]/[-8.3027%]/[+59.0612%]/
>> 256/8/[+45.3486%]/[-8.4024%]/[+58.6817%]/
>> 1024/1/[+432.5372%]/[+3.9566%]/[+412.2689%]/
>> 1024/2/[-1.4207%]/[-23.6426%]/[+29.1025%]/
>> 1024/4/-0.1003%/[-13.6416%]/[+15.6804%]/
>> 1024/8/[+0.2200%]/[+2.0634%]/[-1.8061%]/
>> 4096/1/[+18.4835%]/[-46.1508%]/[+120.0283%]/
>> 4096/2/+0.1770%/[-26.2780%]/[+35.8848%]/
>> 4096/4/-0.1012%/-0.7353%/+0.6388%/
>> 4096/8/-0.6091%/+1.4159%/-1.9968%/
>> 16384/1/-0.0424%/[+11.9373%]/[-10.7021%]/
>> 16384/2/+0.0482%/+2.4685%/-2.3620%/
>> 16384/4/+0.0840%/[+5.3587%]/[-5.0064%]/
>> 16384/8/+0.0048%/[+5.0176%]/[-4.7733%]/
>> 65535/1/-0.0095%/[+10.9408%]/[-9.8705%]/
>> 65535/2/+0.1515%/[+8.1709%]/[-7.4137%]/
>> 65535/4/+0.0203%/[+5.4316%]/[-5.1325%]/
>> 65535/8/+0.1427%/[+6.2753%]/[-5.7705%]/
>>
>> size/sessions/+trans.rate/+cpu/+per_cpu_trans.rate/
>> 64/1/+0.2346%/[+11.5080%]/[-10.1099%]/
>> 64/25/[-10.7893%]/-0.5791%/[-10.2697%]/
>> 64/50/[-11.5997%]/-0.3429%/[-11.2956%]/
>> 256/1/+0.7219%/[+13.2374%]/[-11.0524%]/
>> 256/25/-6.9567%/+0.8887%/[-7.7763%]/
>> 256/50/[-4.8814%]/-0.0338%/[-4.8492%]/
>> 4096/1/-1.6061%/-0.7561%/-0.8565%/
>> 4096/25/[+2.2120%]/[+1.0839%]/+1.1161%/
>> 4096/50/[+5.6180%]/[+3.2116%]/[+2.3315%]/
>>
>> Jason Wang (4):
>> virtio_net: enable tx interrupt
>> virtio-net: optimize free_old_xmit_skbs stats
>> virtio-net: add basic interrupt coalescing support
>> vhost_net: interrupt coalescing support
>>
>> Michael S. Tsirkin (1):
>> virtio_net: bql
>>
>> drivers/net/virtio_net.c | 211
>> ++++++++++++++++++++++++++++++++--------
>> drivers/vhost/net.c | 200
>> +++++++++++++++++++++++++++++++++++--
>> include/uapi/linux/vhost.h | 12 +++
>> include/uapi/linux/virtio_net.h | 12 +++
>> 4 files changed, 383 insertions(+), 52 deletions(-)
>>
>> --
>> 1.8.3.1
^ permalink raw reply
* Re: [PATCH RFC v3 0/3] virtio_net: enabling tx interrupts
From: Jason Wang @ 2014-12-02 3:36 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, netdev
In-Reply-To: <20141201104808.GA16661@redhat.com>
On Mon, Dec 1, 2014 at 6:48 PM, Michael S. Tsirkin <mst@redhat.com>
wrote:
> On Mon, Dec 01, 2014 at 06:14:36PM +0800, Jason Wang wrote:
>>
>>
>> On 10/20/2014 02:52 PM, Michael S. Tsirkin wrote:
>> >RFC patches to enable tx interrupts.
>> >This is to demonstrate how this can be done without
>> >core virtio changes, and to make sure I understand
>> >the new APIs correctly.
>> >
>> >Testing TBD, I was asked for a version for early testing.
>> >
>> >Applies on top of patch: "virtio_net: fix use after free"
>> >that I recently sent.
>> >
>> >Changes from v3:
>> > clean up code, address issues raised by Jason
>> >Changes from v1:
>> > address comments by Jason Wang, use delayed cb everywhere
>> > rebased Jason's patch on top of mine and include it (with
>> some tweaks)
>> >
>> >Jason Wang (1):
>> > virtio-net: optimize free_old_xmit_skbs stats
>> >
>> >Michael S. Tsirkin (2):
>> > virtio_net: enable tx interrupt
>> > virtio_net: bql
>> >
>> > drivers/net/virtio_net.c | 144
>> +++++++++++++++++++++++++++++++++--------------
>> > 1 file changed, 101 insertions(+), 43 deletions(-)
>> >
>>
>> I've run a full tests on this series and see huge regression when
>> zerocopy
>> is disabled. Looks like the reason is zerocopy could coalescing tx
>> completion which greatly reduce the number of tx interrupts.
>
> I think you refer to this code:
>
> /*
> * Trigger polling thread if guest stopped submitting new
> * buffers:
> * in this case, the refcount after decrement will eventually
> * reach 1.
> * We also trigger polling periodically after each 16 packets
> * (the value 16 here is more or less arbitrary, it's tuned to
> * trigger
> * less than 10% of times).
> */
> if (cnt <= 1 || !(cnt % 16))
> vhost_poll_queue(&vq->poll);
>
> ?
> This seems unrelated to interrupt coalescing.
Well, this in fact tries to coalesce 16 packets per tx irq.
More important, zerocopy depends on host nics tx completion.
This means, if host nic coalesces several packets per irq,
zerocopy will probably also do this. vhost_zerocopy_signal_used()
will try to coalesce the tx intrs.
>
> We can easily enable something similar for all tx
> packets, without need for guest configuration.
We can, but we lose the an interface for user to tune for
their applications.
>
> If it's not clear how to do this, let me know, I'll try to put out a
> patch like this in a couple of days.
We can just do this through harding coding the tx-frames to 16
through interrupt coalescing. I don't see obvious differences.
And in my test of RFCv4, I just use tx-frames 16 to get the result.
Without a timer for coalescing, I suspect how much this can help
for e.g 1 session of TCP_RR. It just has at most 1 packet pending
during the test. So in fact no tx completion could be coalesced
in this case.
>
>
>> I will post RFC V4 shortly with interrupt coalescing support. In
>> this
>> version I remove the tx packet cleanup in ndo_start_xmit() since it
>> may
>> reduce the effects of interrupt coalescing.
>
> Maybe split this in a separate patch?
I can, but since just two minor changes compared to V3. Maybe just
document the differences is ok for you?
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Kevin Zhu @ 2014-12-02 3:53 UTC (permalink / raw)
To: Eli Britstein, Enrico Mioso
Cc: Bjørn Mork, Alex Strizhevsky, Midge Shaojun Tan,
youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <B315C8AC-3B33-406B-A728-14F8FAAAE986@audiocodes.com>
Hi,
The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
the other packets are less than that. However, the DHCP queries are not
replied in time either, there's always some delay.
By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
but it returns error when the host sends this command to it. I disabled
this command in NCM driver and tried, but it's the same result.
Regards,
Kevin
On 12/02/2014 06:02 AM, Eli Britstein wrote:
> Hi Enrico and all,
>
> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
> Maybe we can trace to compare them?
>
> Sent from my iPhone
>
>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
>>
>> So ... I have two ideas left for now.
>> We know for sure the problem is in the way we TX frames, not the way we RX them
>> (the way we send, generate them, not the way we receive them).
>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
>> this first one.
>>
>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
>> We might try to take the usb frames generated by Windows, and send them to the
>> device to see if there is any reaction. It should not be science fiction, I saw
>> them do that in the eciadsl old project.
>> 2 - The huawei ndis driver: does it work with these devices?
>> It should be a little bit out-dated now (at least in terms of dates, it might
>> work as well): the code is A LOT but, just in case, to see if there is any
>> chances it'll work. It remains to be seen in which kernels it can actually
>> compile again.
>>
>> If this works we might analyse what's happening and try to debug this out.
>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
>> Thank you.
This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
^ permalink raw reply
* [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
From: Joe Perches @ 2014-12-02 4:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, LKML
In-Reply-To: <1417489760.4894.8.camel@perches.com>
The compute_score functions are a bit difficult to read.
Neaten them a bit to reduce object sizes and make them a
bit more intelligible.
Return early to avoid indentation and avoid unnecessary
initializations.
(allyesconfig, but w/ -O2 and no profiling)
$ size net/ipv[46]/udp.o.*
text data bss dec hex filename
28680 1184 25 29889 74c1 net/ipv4/udp.o.new
28756 1184 25 29965 750d net/ipv4/udp.o.old
17600 1010 2 18612 48b4 net/ipv6/udp.o.new
17632 1010 2 18644 48d4 net/ipv6/udp.o.old
Signed-off-by: Joe Perches <joe@perches.com>
---
Change the && == blocks to || !=
No change in compiled object files.
Keeps Eric happy too.
net/ipv4/udp.c | 111 +++++++++++++++++++++++++++++++-------------------------
net/ipv6/udp.c | 113 ++++++++++++++++++++++++++++++++-------------------------
2 files changed, 125 insertions(+), 99 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index b2d6068..dd8e006 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -336,38 +336,45 @@ int udp_v4_get_port(struct sock *sk, unsigned short snum)
return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal, hash2_nulladdr);
}
-static inline int compute_score(struct sock *sk, struct net *net, __be32 saddr,
- unsigned short hnum,
- __be16 sport, __be32 daddr, __be16 dport, int dif)
+static inline int compute_score(struct sock *sk, struct net *net,
+ __be32 saddr, unsigned short hnum, __be16 sport,
+ __be32 daddr, __be16 dport, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ ipv6_only_sock(sk))
+ return -1;
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_rcv_saddr) {
- if (inet->inet_rcv_saddr != daddr)
- return -1;
- score += 4;
- }
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+ inet = inet_sk(sk);
+
+ if (inet->inet_rcv_saddr) {
+ if (inet->inet_rcv_saddr != daddr)
+ return -1;
+ score += 4;
+ }
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
+ return -1;
+ score += 4;
}
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score += 4;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
+ }
+
return score;
}
@@ -378,33 +385,39 @@ static inline int compute_score2(struct sock *sk, struct net *net,
__be32 saddr, __be16 sport,
__be32 daddr, unsigned int hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
+
+ if (!net_eq(sock_net(sk), net) ||
+ ipv6_only_sock(sk))
+ return -1;
- if (net_eq(sock_net(sk), net) && !ipv6_only_sock(sk)) {
- struct inet_sock *inet = inet_sk(sk);
+ inet = inet_sk(sk);
- if (inet->inet_rcv_saddr != daddr)
+ if (inet->inet_rcv_saddr != daddr ||
+ inet->inet_num != hnum)
+ return -1;
+
+ score = (sk->sk_family == PF_INET) ? 2 : 1;
+
+ if (inet->inet_daddr) {
+ if (inet->inet_daddr != saddr)
return -1;
- if (inet->inet_num != hnum)
+ score += 4;
+ }
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
+ score += 4;
+ }
- score = (sk->sk_family == PF_INET ? 2 : 1);
- if (inet->inet_daddr) {
- if (inet->inet_daddr != saddr)
- return -1;
- score += 4;
- }
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score += 4;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score += 4;
- }
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score += 4;
}
+
return score;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 7cfb5d7..7f964322 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -148,72 +148,85 @@ static inline int compute_score(struct sock *sk, struct net *net,
const struct in6_addr *daddr, __be16 dport,
int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ sk->sk_family != PF_INET6)
+ return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
+ return -1;
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+ score++;
+ }
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
#define SCORE2_MAX (1 + 1 + 1)
static inline int compute_score2(struct sock *sk, struct net *net,
- const struct in6_addr *saddr, __be16 sport,
- const struct in6_addr *daddr, unsigned short hnum,
- int dif)
+ const struct in6_addr *saddr, __be16 sport,
+ const struct in6_addr *daddr,
+ unsigned short hnum, int dif)
{
- int score = -1;
+ int score;
+ struct inet_sock *inet;
- if (net_eq(sock_net(sk), net) && udp_sk(sk)->udp_port_hash == hnum &&
- sk->sk_family == PF_INET6) {
- struct inet_sock *inet = inet_sk(sk);
+ if (!net_eq(sock_net(sk), net) ||
+ udp_sk(sk)->udp_port_hash != hnum ||
+ sk->sk_family != PF_INET6)
+ return -1;
- if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ if (!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+ return -1;
+
+ score = 0;
+ inet = inet_sk(sk);
+
+ if (inet->inet_dport) {
+ if (inet->inet_dport != sport)
return -1;
- score = 0;
- if (inet->inet_dport) {
- if (inet->inet_dport != sport)
- return -1;
- score++;
- }
- if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
- if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
- return -1;
- score++;
- }
- if (sk->sk_bound_dev_if) {
- if (sk->sk_bound_dev_if != dif)
- return -1;
- score++;
- }
+ score++;
}
+
+ if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
+ if (!ipv6_addr_equal(&sk->sk_v6_daddr, saddr))
+ return -1;
+ score++;
+ }
+
+ if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if != dif)
+ return -1;
+ score++;
+ }
+
return score;
}
-
/* called with read_rcu_lock() */
static struct sock *udp6_lib_lookup2(struct net *net,
const struct in6_addr *saddr, __be16 sport,
^ permalink raw reply related
* Re: [PATCH] Documentation: bindings: net: DPAA corenet binding document
From: Scott Wood @ 2014-12-02 4:39 UTC (permalink / raw)
To: madalin.bucur
Cc: devicetree, linuxppc-dev, netdev, Emilian.Medve, Igal.Liberman
In-Reply-To: <1417169426-11823-1-git-send-email-madalin.bucur@freescale.com>
On Fri, 2014-11-28 at 12:10 +0200, Madalin Bucur wrote:
> Add the device tree binding document for the DPAA corenet node
> and DPAA Ethernet nodes.
>
> Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>
> ---
> Documentation/devicetree/bindings/net/fsl-dpaa.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/fsl-dpaa.txt
>
> diff --git a/Documentation/devicetree/bindings/net/fsl-dpaa.txt b/Documentation/devicetree/bindings/net/fsl-dpaa.txt
> new file mode 100644
> index 0000000..822c668
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fsl-dpaa.txt
> @@ -0,0 +1,31 @@
> +*DPAA corenet
> +
> +The corenet bus containing all DPAA Ethernet nodes.
What does this have to do with corenet?
> +Required property
> + - compatible: string property. Must include "fsl,dpaa". Can include
> + also "fsl,<SoC>-dpaa".
No need for the <SoC> part. As we previously discussed, the only
purpose of this node is backwards compatibility with the U-Boot MAC
address fixup -- if U-Boot doesn't look for the <SoC> version, then
don't complicate things.
Though, I can't find where U-Boot references this node. Are you sure
it's not using the ethernet%d aliases like everything else, in which
case why do we need this node at all?
-Scott
^ permalink raw reply
* Re: [PATCH net-next] udp: Neaten and reduce size of compute_score functions
From: Eric Dumazet @ 2014-12-02 4:44 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
In-Reply-To: <1417489760.4894.8.camel@perches.com>
On Mon, 2014-12-01 at 19:09 -0800, Joe Perches wrote:
> On Mon, 2014-12-01 at 18:59 -0800, Eric Dumazet wrote:
> > On Mon, 2014-12-01 at 17:39 -0800, Joe Perches wrote:
> > > The compute_score functions are a bit difficult to read.
> > >
> > > Neaten them a bit to reduce object sizes and make them a
> > > bit more intelligible.
> > >
> > > Return early to avoid indentation and avoid unnecessary
> > > initializations.
> []
> > > + if (!(net_eq(sock_net(sk), net) &&
> > > + udp_sk(sk)->udp_port_hash == hnum &&
> > > + !ipv6_only_sock(sk)))
> > > + return -1
> >
> > Or even better :
> >
> >
> > if (!net_eq(sock_net(sk), net) ||
> > udp_sk(sk)->udp_port_hash != hnum ||
> > ipv6_only_sock(sk))
> > return -1;
>
> Hi Eric.
>
> Yeah, I thought about it but thought it
> simpler to not change the logic.
>
> Either way is fine with me.
Your patch does not change the logic at all.
My suggestion is about avoiding double negates which are really hard to
read. This is simple boolean logic rules.
Code was :
if (a && x == y && !ipv6_only_sock(sk))) {
EXPR1;
} else {
return -1;
}
So the 'logical' way of negating the condition is actually to not add
double negations and use :
if (!a || x != y || ipv6_only_sock(sk)))
return -1;
EXPR1;
Instead of the less readable form :
if (!(a && x == y && !ipv6_only_sock(sk))))
return -1;
EXPR1;
You do not have to ask David permission for this, really.
^ permalink raw reply
* Re: [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
From: Eric Dumazet @ 2014-12-02 5:08 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, LKML
In-Reply-To: <1417494546.4894.12.camel@perches.com>
On Mon, 2014-12-01 at 20:29 -0800, Joe Perches wrote:
> The compute_score functions are a bit difficult to read.
>
> Neaten them a bit to reduce object sizes and make them a
> bit more intelligible.
>
> Return early to avoid indentation and avoid unnecessary
> initializations.
>
> (allyesconfig, but w/ -O2 and no profiling)
hmm... Not sure how you get such large numbers...
>
> $ size net/ipv[46]/udp.o.*
> text data bss dec hex filename
> 28680 1184 25 29889 74c1 net/ipv4/udp.o.new
> 28756 1184 25 29965 750d net/ipv4/udp.o.old
> 17600 1010 2 18612 48b4 net/ipv6/udp.o.new
> 17632 1010 2 18644 48d4 net/ipv6/udp.o.old
Here I have :
# size net/ipv4/udp.o.*
text data bss dec hex filename
21989 616 9 22614 5856 net/ipv4/udp.o.old
21957 616 9 22582 5836 net/ipv4/udp.o.new
With CONFIG_CC_OPTIMIZE_FOR_SIZE=y I even have an opposite result (code
gets bigger after your patch)
# size net/ipv4/udp.o.*
text data bss dec hex filename
17242 600 9 17851 45bb net/ipv4/udp.o.old
17256 600 9 17865 45c9 net/ipv4/udp.o.new
Anyway, your patch looks fine to me, no matter what the code size is.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH V2 net-next] udp: Neaten and reduce size of compute_score functions
From: Joe Perches @ 2014-12-02 5:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, LKML
In-Reply-To: <1417496925.5303.18.camel@edumazet-glaptop2.roam.corp.google.com>
On Mon, 2014-12-01 at 21:08 -0800, Eric Dumazet wrote:
> On Mon, 2014-12-01 at 20:29 -0800, Joe Perches wrote:
> > The compute_score functions are a bit difficult to read.
> >
> > Neaten them a bit to reduce object sizes and make them a
> > bit more intelligible.
> >
> > Return early to avoid indentation and avoid unnecessary
> > initializations.
> >
> > (allyesconfig, but w/ -O2 and no profiling)
>
> hmm... Not sure how you get such large numbers...
Nor I particularly, but I do with gcc 4.9.1
> > $ size net/ipv[46]/udp.o.*
> > text data bss dec hex filename
> > 28680 1184 25 29889 74c1 net/ipv4/udp.o.new
> > 28756 1184 25 29965 750d net/ipv4/udp.o.old
[]
> Here I have :
>
> # size net/ipv4/udp.o.*
> text data bss dec hex filename
> 21989 616 9 22614 5856 net/ipv4/udp.o.old
> 21957 616 9 22582 5836 net/ipv4/udp.o.new
Curious. What gcc version?
with that 4.9.1 gcc version and a defconfig (x86-64) I get:
$ size net/ipv[46]/udp.o*
text data bss dec hex filename
21328 672 9 22009 55f9 net/ipv4/udp.o.new
21312 672 9 21993 55e9 net/ipv4/udp.o.old
14463 588 2 15053 3acd net/ipv6/udp.o.new
14527 588 2 15117 3b0d net/ipv6/udp.o.old
and defconfig x86-32:
$ size net/ipv[46]/udp.o*
text data bss dec hex filename
19626 324 5 19955 4df3 net/ipv4/udp.o.new
19706 324 5 20035 4e43 net/ipv4/udp.o.old
14189 300 2 14491 389b net/ipv6/udp.o.new
14125 300 2 14427 385b net/ipv6/udp.o.old
> With CONFIG_CC_OPTIMIZE_FOR_SIZE=y I even have an opposite result (code
> gets bigger after your patch)
>
> # size net/ipv4/udp.o.*
> text data bss dec hex filename
> 17242 600 9 17851 45bb net/ipv4/udp.o.old
> 17256 600 9 17865 45c9 net/ipv4/udp.o.new
>
> Anyway, your patch looks fine to me, no matter what the code size is.
>
> Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH net-next] rtnetlink: delay RTM_DELLINK notification until after ndo_uninit()
From: Mahesh Bandewar @ 2014-12-02 5:54 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Roopa Prabhu, Toshiaki Makita,
Mahesh Bandewar
The commit 56bfa7ee7c ("unregister_netdevice : move RTM_DELLINK to
until after ndo_uninit") tried to do this ealier but while doing so
it created a problem. Unfortunately the delayed rtmsg_ifinfo() also
delayed call to fill_info(). So this translated into asking driver
to remove private state and then quert it's private state. This
could have catastropic consequences.
This change breaks the rtmsg_ifinfo() into two parts - one fills the
skb by calling fill_info() prior to calling ndo_uninit() and the second
part just sends the message using the skb filled earlier.
It was brought to notice when last link is deleted from an ipvlan device
when it has free-ed the port and the subsequent .fill_info() call is
trying to get the info from the port.
kernel: [ 255.139429] ------------[ cut here ]------------
kernel: [ 255.139439] WARNING: CPU: 12 PID: 11173 at net/core/rtnetlink.c:2238 rtmsg_ifinfo+0x100/0x110()
kernel: [ 255.139493] Modules linked in: ipvlan bonding w1_therm ds2482 wire cdc_acm ehci_pci ehci_hcd i2c_dev i2c_i801 i2c_core msr cpuid bnx2x ptp pps_core mdio libcrc32c
kernel: [ 255.139513] CPU: 12 PID: 11173 Comm: ip Not tainted 3.18.0-smp-DEV #167
kernel: [ 255.139514] Hardware name: Intel RML,PCH/Ibis_QC_18, BIOS 1.0.10 05/15/2012
kernel: [ 255.139515] 0000000000000009 ffff880851b6b828 ffffffff815d87f4 00000000000000e0
kernel: [ 255.139516] 0000000000000000 ffff880851b6b868 ffffffff8109c29c 0000000000000000
kernel: [ 255.139518] 00000000ffffffa6 00000000000000d0 ffffffff81aaf580 0000000000000011
kernel: [ 255.139520] Call Trace:
kernel: [ 255.139527] [<ffffffff815d87f4>] dump_stack+0x46/0x58
kernel: [ 255.139531] [<ffffffff8109c29c>] warn_slowpath_common+0x8c/0xc0
kernel: [ 255.139540] [<ffffffff8109c2ea>] warn_slowpath_null+0x1a/0x20
kernel: [ 255.139544] [<ffffffff8150d570>] rtmsg_ifinfo+0x100/0x110
kernel: [ 255.139547] [<ffffffff814f78b5>] rollback_registered_many+0x1d5/0x2d0
kernel: [ 255.139549] [<ffffffff814f79cf>] unregister_netdevice_many+0x1f/0xb0
kernel: [ 255.139551] [<ffffffff8150acab>] rtnl_dellink+0xbb/0x110
kernel: [ 255.139553] [<ffffffff8150da90>] rtnetlink_rcv_msg+0xa0/0x240
kernel: [ 255.139557] [<ffffffff81329283>] ? rhashtable_lookup_compare+0x43/0x80
kernel: [ 255.139558] [<ffffffff8150d9f0>] ? __rtnl_unlock+0x20/0x20
kernel: [ 255.139562] [<ffffffff8152cb11>] netlink_rcv_skb+0xb1/0xc0
kernel: [ 255.139563] [<ffffffff8150a495>] rtnetlink_rcv+0x25/0x40
kernel: [ 255.139565] [<ffffffff8152c398>] netlink_unicast+0x178/0x230
kernel: [ 255.139567] [<ffffffff8152c75f>] netlink_sendmsg+0x30f/0x420
kernel: [ 255.139571] [<ffffffff814e0b0c>] sock_sendmsg+0x9c/0xd0
kernel: [ 255.139575] [<ffffffff811d1d7f>] ? rw_copy_check_uvector+0x6f/0x130
kernel: [ 255.139577] [<ffffffff814e11c9>] ? copy_msghdr_from_user+0x139/0x1b0
kernel: [ 255.139578] [<ffffffff814e1774>] ___sys_sendmsg+0x304/0x310
kernel: [ 255.139581] [<ffffffff81198723>] ? handle_mm_fault+0xca3/0xde0
kernel: [ 255.139585] [<ffffffff811ebc4c>] ? destroy_inode+0x3c/0x70
kernel: [ 255.139589] [<ffffffff8108e6ec>] ? __do_page_fault+0x20c/0x500
kernel: [ 255.139597] [<ffffffff811e8336>] ? dput+0xb6/0x190
kernel: [ 255.139606] [<ffffffff811f05f6>] ? mntput+0x26/0x40
kernel: [ 255.139611] [<ffffffff811d2b94>] ? __fput+0x174/0x1e0
kernel: [ 255.139613] [<ffffffff814e2129>] __sys_sendmsg+0x49/0x90
kernel: [ 255.139615] [<ffffffff814e2182>] SyS_sendmsg+0x12/0x20
kernel: [ 255.139617] [<ffffffff815df092>] system_call_fastpath+0x12/0x17
kernel: [ 255.139619] ---[ end trace 5e6703e87d984f6b ]---
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Report-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: David S. Miller <davem@davemloft.net>
---
drivers/net/bonding/bond_main.c | 4 ++--
include/linux/rtnetlink.h | 6 +++++-
include/net/bonding.h | 8 ++++----
net/core/dev.c | 26 ++++++++++++++++----------
net/core/rtnetlink.c | 20 ++++++++++++++++----
5 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 184c434ae305..06206a1439a4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1135,7 +1135,7 @@ static int bond_master_upper_dev_link(struct net_device *bond_dev,
if (err)
return err;
slave_dev->flags |= IFF_SLAVE;
- rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL, false);
return 0;
}
@@ -1144,7 +1144,7 @@ static void bond_upper_dev_unlink(struct net_device *bond_dev,
{
netdev_upper_dev_unlink(slave_dev, bond_dev);
slave_dev->flags &= ~IFF_SLAVE;
- rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL, false);
}
static struct slave *bond_alloc_slave(struct bonding *bond)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 6cacbce1a06c..545dd0b8c83d 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -16,7 +16,11 @@ extern int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics);
extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
u32 id, long expires, u32 error);
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change, gfp_t flags);
+struct sk_buff *rtmsg_ifinfo(int type, struct net_device *dev, unsigned change,
+ gfp_t flags, bool fill_only);
+void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev,
+ gfp_t flags);
+
/* RTNL is used as a global lock for all changes to network configuration */
extern void rtnl_lock(void);
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 983a94b86b95..ea09f6c5af51 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -315,7 +315,7 @@ static inline void bond_set_active_slave(struct slave *slave)
{
if (slave->backup) {
slave->backup = 0;
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC, false);
}
}
@@ -323,7 +323,7 @@ static inline void bond_set_backup_slave(struct slave *slave)
{
if (!slave->backup) {
slave->backup = 1;
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC, false);
}
}
@@ -335,7 +335,7 @@ static inline void bond_set_slave_state(struct slave *slave,
slave->backup = slave_state;
if (notify) {
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC, false);
slave->should_notify = 0;
} else {
if (slave->should_notify)
@@ -365,7 +365,7 @@ static inline void bond_slave_state_notify(struct bonding *bond)
bond_for_each_slave(bond, tmp, iter) {
if (tmp->should_notify) {
- rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_ATOMIC, false);
tmp->should_notify = 0;
}
}
diff --git a/net/core/dev.c b/net/core/dev.c
index ac4836241a96..29bc78d5e6cb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1230,7 +1230,7 @@ void netdev_state_change(struct net_device *dev)
change_info.flags_changed = 0;
call_netdevice_notifiers_info(NETDEV_CHANGE, dev,
&change_info.info);
- rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL, false);
}
}
EXPORT_SYMBOL(netdev_state_change);
@@ -1319,7 +1319,7 @@ int dev_open(struct net_device *dev)
if (ret < 0)
return ret;
- rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL, false);
call_netdevice_notifiers(NETDEV_UP, dev);
return ret;
@@ -1396,7 +1396,8 @@ static int dev_close_many(struct list_head *head)
__dev_close_many(head);
list_for_each_entry_safe(dev, tmp, head, close_list) {
- rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING,
+ GFP_KERNEL, false);
call_netdevice_notifiers(NETDEV_DOWN, dev);
list_del_init(&dev->close_list);
}
@@ -5683,7 +5684,7 @@ void __dev_notify_flags(struct net_device *dev, unsigned int old_flags,
unsigned int changes = dev->flags ^ old_flags;
if (gchanges)
- rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges, GFP_ATOMIC);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges, GFP_ATOMIC, false);
if (changes & IFF_UP) {
if (dev->flags & IFF_UP)
@@ -5925,6 +5926,8 @@ static void rollback_registered_many(struct list_head *head)
synchronize_net();
list_for_each_entry(dev, head, unreg_list) {
+ struct sk_buff *skb = NULL;
+
/* Shutdown queueing discipline. */
dev_shutdown(dev);
@@ -5934,6 +5937,10 @@ static void rollback_registered_many(struct list_head *head)
*/
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
+ if (!dev->rtnl_link_ops ||
+ dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
+ skb = rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL, true);
+
/*
* Flush the unicast and multicast chains
*/
@@ -5943,9 +5950,8 @@ static void rollback_registered_many(struct list_head *head)
if (dev->netdev_ops->ndo_uninit)
dev->netdev_ops->ndo_uninit(dev);
- if (!dev->rtnl_link_ops ||
- dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
+ if (skb)
+ rtmsg_ifinfo_send(skb, dev, GFP_KERNEL);
/* Notifier chain MUST detach us all upper devices. */
WARN_ON(netdev_has_any_upper_dev(dev));
@@ -6334,7 +6340,7 @@ int register_netdevice(struct net_device *dev)
*/
if (!dev->rtnl_link_ops ||
dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL, false);
out:
return ret;
@@ -6959,7 +6965,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
rcu_barrier();
call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL, false);
/*
* Flush the unicast and multicast chains
@@ -7000,7 +7006,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
* Prevent userspace races by waiting until the network
* device is fully setup before sending notifications.
*/
- rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL, false);
synchronize_net();
err = 0;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b9b7dfaf202b..1035d8cdbc08 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2220,8 +2220,16 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
- gfp_t flags)
+void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags)
+{
+ struct net *net = dev_net(dev);
+
+ rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
+}
+EXPORT_SYMBOL(rtmsg_ifinfo_send);
+
+struct sk_buff *rtmsg_ifinfo(int type, struct net_device *dev,
+ unsigned int change, gfp_t flags, bool fill_only)
{
struct net *net = dev_net(dev);
struct sk_buff *skb;
@@ -2239,11 +2247,15 @@ void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
kfree_skb(skb);
goto errout;
}
+ if (fill_only)
+ return skb;
+
rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
- return;
+ return NULL;
errout:
if (err < 0)
rtnl_set_sk_err(net, RTNLGRP_LINK, err);
+ return NULL;
}
EXPORT_SYMBOL(rtmsg_ifinfo);
@@ -3011,7 +3023,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_JOIN:
break;
default:
- rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL, false);
break;
}
return NOTIFY_DONE;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-12-02 6:07 UTC (permalink / raw)
To: Eli Britstein
Cc: Kevin Zhu, Bjørn Mork, Alex Strizhevsky, Midge Shaojun Tan,
youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <B315C8AC-3B33-406B-A728-14F8FAAAE986-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3404 bytes --]
Hi Eli,
Hi Kevin,
Hi everyone.
Hoping Kevin answered your question - as he did so in a way I could not have
done better.
It seems a matter of the size, even if I am suspecting this could also all
relate to some strange behaviour from the firmware side.
However - I tried also changing the MTU to smaller values, and "shortcircuit"
the negotiation of some NCM parameters, with no results.
Yes - confirming the device returns an error in this case.
Interesting also to note that the device claims to be
"before NCM errata".
See specs.
On Mon, 1 Dec 2014, Eli Britstein wrote:
==Date: Mon, 1 Dec 2014 23:02:16
==From: Eli Britstein <Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
==To: Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
==Cc: Kevin Zhu <Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>,
== Alex Strizhevsky <alexxst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== Midge Shaojun Tan <ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
== "youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
== "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
==Subject: Re: Is this 32-bit NCM?
==
==Hi Enrico and all,
==
==Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
==Maybe we can trace to compare them?
==
==Sent from my iPhone
==
==> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
==>
==> So ... I have two ideas left for now.
==> We know for sure the problem is in the way we TX frames, not the way we RX them
==> (the way we send, generate them, not the way we receive them).
==> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
==> this first one.
==>
==> 1 - Does a wayexist to "replay" traffic crom a usb capture?
==> We might try to take the usb frames generated by Windows, and send them to the
==> device to see if there is any reaction. It should not be science fiction, I saw
==> them do that in the eciadsl old project.
==> 2 - The huawei ndis driver: does it work with these devices?
==> It should be a little bit out-dated now (at least in terms of dates, it might
==> work as well): the code is A LOT but, just in case, to see if there is any
==> chances it'll work. It remains to be seen in which kernels it can actually
==> compile again.
==>
==> If this works we might analyse what's happening and try to debug this out.
==> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
==> Thank you.
==
==________________________________
==
==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-12-02 6:12 UTC (permalink / raw)
To: Eli Britstein
Cc: Kevin Zhu, Bjørn Mork, Alex Strizhevsky, Midge Shaojun Tan,
youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <B315C8AC-3B33-406B-A728-14F8FAAAE986@audiocodes.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3280 bytes --]
And - let's be precise: Eli is right, in some cases (with some dongles /
firmwares, including the one I was / am testing remotely), sometimes DHCP is
answered.
On forums, people experience intermittent results.
The only thing I am thinking now is thet the firmware cares about the content
of the padding area. Crazy, stupid but... I don't know what to think about it
now.
I looked at the .inf file values: and I would suggest to all of you who can run
the installation of the product under Windows, to look at the registry key that
are set and that the drivers are caring about.
In my opinion here should be something interesting.
Looking at the inf files of the *ecm* drivers and tracking down registry values
might be effectively useful.
Enrico.
On Mon, 1 Dec 2014, Eli Britstein wrote:
==Date: Mon, 1 Dec 2014 23:02:16
==From: Eli Britstein <Eli.Britstein@audiocodes.com>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: Kevin Zhu <Mingying.Zhu@audiocodes.com>, Bjørn Mork <bjorn@mork.no>,
== Alex Strizhevsky <alexxst@gmail.com>,
== Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
== "youtux@gmail.com" <youtux@gmail.com>,
== "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
== "netdev@vger.kernel.org" <netdev@vger.kernel.org>
==Subject: Re: Is this 32-bit NCM?
==
==Hi Enrico and all,
==
==Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
==Maybe we can trace to compare them?
==
==Sent from my iPhone
==
==> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
==>
==> So ... I have two ideas left for now.
==> We know for sure the problem is in the way we TX frames, not the way we RX them
==> (the way we send, generate them, not the way we receive them).
==> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
==> this first one.
==>
==> 1 - Does a wayexist to "replay" traffic crom a usb capture?
==> We might try to take the usb frames generated by Windows, and send them to the
==> device to see if there is any reaction. It should not be science fiction, I saw
==> them do that in the eciadsl old project.
==> 2 - The huawei ndis driver: does it work with these devices?
==> It should be a little bit out-dated now (at least in terms of dates, it might
==> work as well): the code is A LOT but, just in case, to see if there is any
==> chances it'll work. It remains to be seen in which kernels it can actually
==> compile again.
==>
==> If this works we might analyse what's happening and try to debug this out.
==> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
==> Thank you.
==
==________________________________
==
==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-12-02 6:32 UTC (permalink / raw)
To: Kevin Zhu
Cc: Eli Britstein, Bjørn Mork, Alex Strizhevsky,
Midge Shaojun Tan, youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <547D37CA.7050506-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 4009 bytes --]
Hi again Eli,
Hi Kevin,
Hi everyone...
As I understand it anyway - the distinction here tends not to be between
different types of packets.
It tends to be between received and sent packet.
We are not able to generate packets that the device retains valid.
If you manually configure the IP the device would have assigned you via DHCP,
your system will start answering ARP request, saying that the host with IP
aa.bb.cc.dd is at aa:bb:cc:dd:ee (using the MC address of the dongle).
However, the other host (gateway) will never know that. Indeed, it'll continue
asking who-is at aa.bb.cc.dd ?
At least, this is the situation I observed in the test system.
On Tue, 2 Dec 2014, Kevin Zhu wrote:
==Date: Tue, 2 Dec 2014 04:53:53
==From: Kevin Zhu <Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
==To: Eli Britstein <Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
== Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
==Cc: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>, Alex Strizhevsky <alexxst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== Midge Shaojun Tan <ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
== "youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
== "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
==Subject: Re: Is this 32-bit NCM?
==
==Hi,
==
==The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
==the other packets are less than that. However, the DHCP queries are not
==replied in time either, there's always some delay.
==
==By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
==but it returns error when the host sends this command to it. I disabled
==this command in NCM driver and tried, but it's the same result.
==
==Regards,
==Kevin
==
==On 12/02/2014 06:02 AM, Eli Britstein wrote:
==> Hi Enrico and all,
==>
==> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
==> Maybe we can trace to compare them?
==>
==> Sent from my iPhone
==>
==>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
==>>
==>> So ... I have two ideas left for now.
==>> We know for sure the problem is in the way we TX frames, not the way we RX them
==>> (the way we send, generate them, not the way we receive them).
==>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
==>> this first one.
==>>
==>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
==>> We might try to take the usb frames generated by Windows, and send them to the
==>> device to see if there is any reaction. It should not be science fiction, I saw
==>> them do that in the eciadsl old project.
==>> 2 - The huawei ndis driver: does it work with these devices?
==>> It should be a little bit out-dated now (at least in terms of dates, it might
==>> work as well): the code is A LOT but, just in case, to see if there is any
==>> chances it'll work. It remains to be seen in which kernels it can actually
==>> compile again.
==>>
==>> If this works we might analyse what's happening and try to debug this out.
==>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
==>> Thank you.
==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Kevin Zhu @ 2014-12-02 6:43 UTC (permalink / raw)
To: Enrico Mioso
Cc: Eli Britstein, Bjørn Mork, Alex Strizhevsky,
Midge Shaojun Tan, youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <alpine.LNX.2.03.1412020729470.1351@gmail.com>
I also tried to define CDC_NCM_DPT_DATAGRAMS_MAX as 1 to eliminate the
paddings, but it did not work either, not even DHCP.
Regards,
Kevin
On 12/02/2014 02:32 PM, Enrico Mioso wrote:
> Hi again Eli,
> Hi Kevin,
> Hi everyone...
>
> As I understand it anyway - the distinction here tends not to be between
> different types of packets.
> It tends to be between received and sent packet.
> We are not able to generate packets that the device retains valid.
> If you manually configure the IP the device would have assigned you via DHCP,
> your system will start answering ARP request, saying that the host with IP
> aa.bb.cc.dd is at aa:bb:cc:dd:ee (using the MC address of the dongle).
> However, the other host (gateway) will never know that. Indeed, it'll continue
> asking who-is at aa.bb.cc.dd ?
> At least, this is the situation I observed in the test system.
>
>
> On Tue, 2 Dec 2014, Kevin Zhu wrote:
>
> ==Date: Tue, 2 Dec 2014 04:53:53
> ==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
> ==To: Eli Britstein <Eli.Britstein@audiocodes.com>,
> == Enrico Mioso <mrkiko.rs@gmail.com>
> ==Cc: Bjørn Mork <bjorn@mork.no>, Alex Strizhevsky <alexxst@gmail.com>,
> == Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>,
> == "youtux@gmail.com" <youtux@gmail.com>,
> == "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
> == "netdev@vger.kernel.org" <netdev@vger.kernel.org>
> ==Subject: Re: Is this 32-bit NCM?
> ==
> ==Hi,
> ==
> ==The DHCP packets have the maximum size of dwNtbOutMaxSize=16384, while
> ==the other packets are less than that. However, the DHCP queries are not
> ==replied in time either, there's always some delay.
> ==
> ==By the way, though the device claims to support GET_MAX_DATAGRAM_SIZE,
> ==but it returns error when the host sends this command to it. I disabled
> ==this command in NCM driver and tried, but it's the same result.
> ==
> ==Regards,
> ==Kevin
> ==
> ==On 12/02/2014 06:02 AM, Eli Britstein wrote:
> ==> Hi Enrico and all,
> ==>
> ==> Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
> ==> Maybe we can trace to compare them?
> ==>
> ==> Sent from my iPhone
> ==>
> ==>> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
> ==>>
> ==>> So ... I have two ideas left for now.
> ==>> We know for sure the problem is in the way we TX frames, not the way we RX them
> ==>> (the way we send, generate them, not the way we receive them).
> ==>> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
> ==>> this first one.
> ==>>
> ==>> 1 - Does a wayexist to "replay" traffic crom a usb capture?
> ==>> We might try to take the usb frames generated by Windows, and send them to the
> ==>> device to see if there is any reaction. It should not be science fiction, I saw
> ==>> them do that in the eciadsl old project.
> ==>> 2 - The huawei ndis driver: does it work with these devices?
> ==>> It should be a little bit out-dated now (at least in terms of dates, it might
> ==>> work as well): the code is A LOT but, just in case, to see if there is any
> ==>> chances it'll work. It remains to be seen in which kernels it can actually
> ==>> compile again.
> ==>>
> ==>> If this works we might analyse what's happening and try to debug this out.
> ==>> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
> ==>> Thank you.
> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
> ==
> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
> ==
This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
^ 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