* Re: [v8 net-next 0/4] Refactor vxlan and l2tp to use new common UDP tunnel APIs
From: David Miller @ 2014-09-19 19:58 UTC (permalink / raw)
To: azhou; +Cc: netdev
In-Reply-To: <1410913787-31599-1-git-send-email-azhou@nicira.com>
From: Andy Zhou <azhou@nicira.com>
Date: Tue, 16 Sep 2014 17:29:43 -0700
> This patch series add a few more UDP tunnel APIs and refactoring current
> UDP tunnel based protocols, vxlan and l2tp to make use of the new APIs.
>
> The added APIs are setup_udp_tunnel_sock(), udp_tunnel_xmit_skb() and
> udp_tunnel_sock_release(). Those implementation logics already exist in
> current vxlan and l2tp implementation. Move them to common APIs to reduce
> code duplications.
>
> Also split udp_tunnel.c into net/ipv4/udp_tunnel.c and
> net/ipv6/ip6_udp_tunnel.c to maintain proper IP protocol separation.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/8] net: phy: Broadcom BCM7xxx PHY workaround update
From: David Miller @ 2014-09-19 19:55 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
In-Reply-To: <20140919.154104.286842667424203847.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Fri, 19 Sep 2014 15:41:04 -0400 (EDT)
> Series applied, thanks Florian.
Actually I had to revert:
net/dsa/slave.c: In function ‘dsa_slave_phy_setup’:
net/dsa/slave.c:395:42: error: ‘port’ undeclared (first use in this function)
phy_flags = ds->drv->get_phy_flags(ds, port);
^
net/dsa/slave.c:395:42: note: each undeclared identifier is reported only once for each function it appears in
^ permalink raw reply
* Re: [PATCH net-next 0/8] net: phy: Broadcom BCM7xxx PHY workaround update
From: David Miller @ 2014-09-19 19:41 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
In-Reply-To: <1410911584-1559-1-git-send-email-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 16 Sep 2014 16:52:56 -0700
> This patch sets the change to of_phy_connect() that you have seen before,
> this time with the full context of why it is useful and applicable here.
>
> Due to some design decision, the internal PHY on Broadcom BCM7xxx chips
> is not entirely self contained and does not report its internal revision
> through MII_PHYSID2, that is left to external PHY designs.
>
> This forces us to get the PHY revision from the GENET and SF2 switch drivers
> because those two peripherals integrate such a PHY and do contain the PHY
> revision in their registers.
>
> The approach taken here is hopefully easy to extend to similar needs for
> other chips/ as well.
Series applied, thanks Florian.
^ permalink raw reply
* Re: [Patch net-next v3 0/4] net: fec: add interrupt coalescence
From: David Miller @ 2014-09-19 19:37 UTC (permalink / raw)
To: Frank.Li; +Cc: b38611, netdev, lznuaa, shawn.guo, linux-arm-kernel
In-Reply-To: <1410902334-7827-1-git-send-email-Frank.Li@freescale.com>
From: <Frank.Li@freescale.com>
Date: Wed, 17 Sep 2014 05:18:50 +0800
> From: Frank Li <Frank.Li@freescale.com>
> improve error handle when parse queue number.
> add interrupt coalescence feature.
>
> Change from v2 to v3
> - add error check in fec_enet_set_coalesce
> - fix a run time warning to get clock rate in interrupt
> - fix commit message use TKT number
>
> Change from v1 to v2
> - fix indention
> - use errata number instead of TKT
Series applied, thanks Frank.
^ permalink raw reply
* Re: [PATCH net-next] sparc: bpf_jit: add SKF_AD_PKTTYPE support to JIT
From: David Miller @ 2014-09-19 19:34 UTC (permalink / raw)
To: ast; +Cc: dborkman, hannes, netdev
In-Reply-To: <1410896135-9867-1-git-send-email-ast@plumgrid.com>
From: Alexei Starovoitov <ast@plumgrid.com>
Date: Tue, 16 Sep 2014 12:35:35 -0700
> commit 233577a22089 ("net: filter: constify detection of pkt_type_offset")
> allows us to implement simple PKTTYPE support in sparc JIT
>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
Applied, thanks for doing this work Alexei.
^ permalink raw reply
* Bridge IGMP snooping question
From: Thomas Martitz @ 2014-09-19 18:55 UTC (permalink / raw)
To: herbert, netdev
Hello Linux folks,
I have one question regarding IGMP snooping on bridges, specifically
about leaving multicast groups.
br_multicast_leave_group() has this check early on:
if (!netif_running(br->dev) ||
(port && port->state == BR_STATE_DISABLED) ||
timer_pending(&querier->timer)) /* <- THIS */
goto out;
I'm wondering why the bridge code prevents group leaves if if the
querier timer is pending. From my understanding the timer acts as an
indication whether the local system is the network's querier or not (if
pending then another router is querier). Therefore this check prevents
to leave groups if there is another querier.
I'm wondering what's the rationale for this, if any. It seems to be that
the decision whether an attached client gets mc forwarded is independent
on whether the local system is querier or not. Git log does not reveal
the answer as this check is there from the beginning.
PS: This behavior conflicts with something I'm working.
Thank you very much in advance.
Best regards.
^ permalink raw reply
* Re: [PATCH net-next v2] fec: Staticize fec_enet_select_queue()
From: Zhi Li @ 2014-09-19 18:55 UTC (permalink / raw)
To: Fabio Estevam
Cc: David Miller, Frank.Li@freescale.com, netdev@vger.kernel.org
In-Reply-To: <1411149574-26296-1-git-send-email-fabio.estevam@freescale.com>
On Fri, Sep 19, 2014 at 12:59 PM, Fabio Estevam
<fabio.estevam@freescale.com> wrote:
> fec_enet_select_queue() is only used locally, so mark it as 'static' to fix the
> following sparse warning:
>
> drivers/net/ethernet/freescale/fec_main.c:2707:5: warning: symbol 'fec_enet_select_queue' was not declared. Should it be static?
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Acked-by: Frank Li <Frank.Li@freescale.com>
> ---
> Changes since v1:
> - Make checkpatch happy
>
> drivers/net/ethernet/freescale/fec_main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 6e93336..2389dff 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -2704,8 +2704,9 @@ static int fec_set_features(struct net_device *netdev,
> return 0;
> }
>
> -u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
> - void *accel_priv, select_queue_fallback_t fallback)
> +static u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
> + void *accel_priv,
> + select_queue_fallback_t fallback)
> {
> return skb_tx_hash(ndev, skb);
> }
> --
> 1.9.1
>
> --
> 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: [net-next PATCH 29/29] fm10k: Add support for PTP
From: Alexander Duyck @ 2014-09-19 18:32 UTC (permalink / raw)
To: Richard Cochran
Cc: davem, nhorman, netdev, john.fastabend, matthew.vick,
jeffrey.t.kirsher, sassmann
In-Reply-To: <20140919173504.GA4136@localhost.localdomain>
On 09/19/2014 10:35 AM, Richard Cochran wrote:
> On Thu, Sep 18, 2014 at 06:40:46PM -0400, Alexander Duyck wrote:
>
>> +static s32 fm10k_1588_msg_vf(struct fm10k_hw *hw, u32 **results,
>> + struct fm10k_mbx_info *mbx)
>> +{
>> + struct fm10k_intfc *interface = container_of(hw,
>> + struct fm10k_intfc,
>> + hw);
>
> This looks really funny to me here and in the other spot. Why not this?
>
> struct fm10k_intfc *interface = container_of(hw, struct fm10k_intfc, hw);
>
Because doing it that way it extends over 80 characters.
> Its only one over the 80 km/h speed limit.
>
>> + u64 timestamp;
>> + s32 err;
>> +
>> + err = fm10k_tlv_attr_get_u64(results[FM10K_1588_MSG_TIMESTAMP],
>> + ×tamp);
>> + if (err)
>> + return err;
>> +
>> + fm10k_ts_tx_hwtstamp(interface, 0, timestamp);
>> +
>> + return 0;
>> +}
>
>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c
>> new file mode 100644
>> index 0000000..41da724
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c
>
>> +/* We use a 64b counter so overflow is extremely seldom. Just
>> + * to keep things sane we should check for overflow once per day
>> + */
>
> Hm...
>
>> +void fm10k_ts_start_cc(struct fm10k_intfc *interface)
>> +{
>> + struct fm10k_hw *hw = &interface->hw;
>> +
>> + /* Initialize cycle counter */
>> + interface->cc.read = (hw->mac.type == fm10k_mac_pf) ? fm10k_cc_read_pf :
>> + fm10k_cc_read_vf;
>> + interface->cc.mask = CLOCKSOURCE_MASK(64);
>> + interface->cc.mult = 1;
>
> So shift = 0 and multi = 1. Your clock counts nanoseconds. Why not use
> it directly? Then you won't need the timecounter stuff or the overflow
> watchdog either.
Because the value cannot be adjusted directly. The timesource for the
switch is shared by all ports and host interfaces. As such we have to
maintain a software offset per host to avoid corrupting the other clocks.
>> +
>> + /* Initialize lock protecting register access */
>> + rwlock_init(&interface->tsreg_lock);
>> +
>> + /* Initialize skb queue for pending timestamp requests */
>> + skb_queue_head_init(&interface->ts_tx_skb_queue);
>> +
>> + /* Initialize the clock */
>> + fm10k_ts_reset_cc(interface);
>> +
>> + /* Initialize the overflow work */
>> + INIT_DELAYED_WORK(&interface->ts_overflow_work,
>> + fm10k_ts_overflow_check);
>> + schedule_delayed_work(&interface->ts_overflow_work,
>> + FM10K_SYSTIME_OVERFLOW_PERIOD);
>> +}
>
>> +static int fm10k_ptp_enable(struct ptp_clock_info *ptp,
>> + struct ptp_clock_request *rq, int on)
>> +{
>> + struct fm10k_intfc *interface =
>> + container_of(ptp, struct fm10k_intfc, ptp_caps);
>> + struct ptp_clock_time *t = &rq->perout.period;
>> + struct fm10k_hw *hw = &interface->hw;
>> + u64 period;
>> + u32 step;
>> +
>> + /* we can only support periodic output */
>> + if (rq->type != PTP_CLK_REQ_PEROUT)
>> + return -EINVAL;
>> +
>> + /* verify the requested channel is there */
>> + if (rq->perout.index >= ptp->n_per_out)
>> + return -EINVAL;
>> +
>> + /* we simply cannot support the operation if we don't have BAR4 */
>> + if (!hw->sw_addr)
>> + return -ENOTSUPP;
>> +
>> + /* we cannot enforce start time as there is no
>> + * mechanism for that in the hardware, we can only control
>> + * the period.
>> + */
>
> Is this because of the timecounter in the way? Another reason to use
> the 64 bit nanosecond counter directly.
The problem is we cannot modify the counter as it is in use by multiple
clocks. So if one adjusted the value it would mess up all of the others.
Also in our case the hardware design doesn't give us a register we can
plug the start time into. We can only control the frequency of the pulse.
>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_type.h b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
>> index 5055bef..dac5b79 100644
>> --- a/drivers/net/ethernet/intel/fm10k/fm10k_type.h
>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
>> @@ -225,11 +225,7 @@ struct fm10k_hw;
>> #define FM10K_STATS_NODESC_DROP 0x3807
>>
>> /* Timesync registers */
>> -#define FM10K_RRTIME_CFG 0x3808
>> -#define FM10K_RRTIME_LIMIT(_n) ((_n) + 0x380C)
>> -#define FM10K_RRTIME_COUNT(_n) ((_n) + 0x3810)
>> #define FM10K_SYSTIME 0x3814
>> -#define FM10K_SYSTIME0 0x3816
>> #define FM10K_SYSTIME_CFG 0x3818
>> #define FM10K_SYSTIME_CFG_STEP_MASK 0x0000000F
>>
>> @@ -368,9 +364,6 @@ struct fm10k_hw;
>> #define FM10K_VFITR(_n) ((_n) + 0x00060)
>>
>> /* Registers contained in BAR 4 for Switch management */
>> -#define FM10K_SW_SYSTIME_CFG 0x0224C
>> -#define FM10K_SW_SYSTIME_CFG_STEP_SHIFT 4
>> -#define FM10K_SW_SYSTIME_CFG_ADJUST_MASK 0xFF000000
>
> You added these three lines in the previous patch.
>
>> #define FM10K_SW_SYSTIME_ADJUST 0x0224D
>> #define FM10K_SW_SYSTIME_ADJUST_MASK 0x3FFFFFFF
>> #define FM10K_SW_SYSTIME_ADJUST_DIR_NEGATIVE 0x80000000
>>
Yeah, I noticed that after you asked your original question. When I had
gone through to clean up the defines that weren't used I cleaned these
up in the wrong patch.
If/when I submit a v2 I will pull them out of the previous patch in the
sequence since that is where they are added. I already have this
resolved in my local copy as of an hour or so ago.
Thanks,
Alex
^ permalink raw reply
* Re: [net-next PATCH 00/29] Add support for the Intel FM10000 Ethernet Switch Host Interface
From: Alexander Duyck @ 2014-09-19 18:22 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jamal Hadi Salim, David S. Miller, nhorman,
netdev@vger.kernel.org, john fastabend, matthew.vick,
jeffrey.t.kirsher, sassmann
In-Reply-To: <CAADnVQKuzQ82HgMXaKcjxZXwGXB6FchhC536u=KfnBLP3+U-Gw@mail.gmail.com>
On 09/19/2014 09:58 AM, Alexei Starovoitov wrote:
> On Fri, Sep 19, 2014 at 7:54 AM, Alexander Duyck
> <alexander.h.duyck@intel.com> wrote:
>>
>> Since the switch can support up to 9 of these we are treating the host
>> interface driver as though it is a NIC driver. We need to usually ship
>
> Am I reading it right that 9 hosts can be attached to this one physical device
> and they see it as different 'physical' nics ?
>
No, there are up to 9 independent host interfaces that are attached to
the switch. Each host device shows up as a separate PCI device capable
of SR-IOV. If SR-IOV is enabled with 64 VFs then one Host interface
will appear as 1 PF PCI device, and 64 VF PCI devices.
Thanks,
Alex
^ permalink raw reply
* [PATCH net-next v2] fec: Staticize fec_enet_select_queue()
From: Fabio Estevam @ 2014-09-19 17:59 UTC (permalink / raw)
To: davem; +Cc: Frank.Li, netdev, Fabio Estevam
fec_enet_select_queue() is only used locally, so mark it as 'static' to fix the
following sparse warning:
drivers/net/ethernet/freescale/fec_main.c:2707:5: warning: symbol 'fec_enet_select_queue' was not declared. Should it be static?
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Make checkpatch happy
drivers/net/ethernet/freescale/fec_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 6e93336..2389dff 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2704,8 +2704,9 @@ static int fec_set_features(struct net_device *netdev,
return 0;
}
-u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
- void *accel_priv, select_queue_fallback_t fallback)
+static u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
+ void *accel_priv,
+ select_queue_fallback_t fallback)
{
return skb_tx_hash(ndev, skb);
}
--
1.9.1
^ permalink raw reply related
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Jamal Hadi Salim @ 2014-09-19 17:57 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
roopa, linville, dev, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck
In-Reply-To: <20140919154946.GH1980@nanopsycho.orion>
On 09/19/14 11:49, Jiri Pirko wrote:
> Fri, Sep 19, 2014 at 05:25:48PM CEST, jhs@mojatatu.com wrote:
>> Is this just a temporary test tool? Otherwise i dont see reason
>> for its existence (or the API that it feeds on).
>
> Please read the conversation I had with Pravin and Jesse in v1 thread.
> Long story short they like to have the api separated from ovs datapath
> so ovs daemon can use it to directly communicate with driver. Also John
> Fastabend requested a way to work with driver flows without using ovs ->
> that was the original reason I created switchdev genl api.
>
> Regarding the "sw" tool, yes it is for testing purposes now. ovs daemon
> will use directly switchdev genl api.
>
> I hope I cleared this out.
>
It is - thanks Jiri.
cheers,
jamal
^ permalink raw reply
* Re: [net-next PATCH 29/29] fm10k: Add support for PTP
From: Richard Cochran @ 2014-09-19 17:35 UTC (permalink / raw)
To: Alexander Duyck
Cc: davem, nhorman, netdev, john.fastabend, matthew.vick,
jeffrey.t.kirsher, sassmann
In-Reply-To: <20140918224042.10373.93162.stgit@ahduyck-bv4.jf.intel.com>
On Thu, Sep 18, 2014 at 06:40:46PM -0400, Alexander Duyck wrote:
> +static s32 fm10k_1588_msg_vf(struct fm10k_hw *hw, u32 **results,
> + struct fm10k_mbx_info *mbx)
> +{
> + struct fm10k_intfc *interface = container_of(hw,
> + struct fm10k_intfc,
> + hw);
This looks really funny to me here and in the other spot. Why not this?
struct fm10k_intfc *interface = container_of(hw, struct fm10k_intfc, hw);
Its only one over the 80 km/h speed limit.
> + u64 timestamp;
> + s32 err;
> +
> + err = fm10k_tlv_attr_get_u64(results[FM10K_1588_MSG_TIMESTAMP],
> + ×tamp);
> + if (err)
> + return err;
> +
> + fm10k_ts_tx_hwtstamp(interface, 0, timestamp);
> +
> + return 0;
> +}
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c
> new file mode 100644
> index 0000000..41da724
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c
> +/* We use a 64b counter so overflow is extremely seldom. Just
> + * to keep things sane we should check for overflow once per day
> + */
Hm...
> +void fm10k_ts_start_cc(struct fm10k_intfc *interface)
> +{
> + struct fm10k_hw *hw = &interface->hw;
> +
> + /* Initialize cycle counter */
> + interface->cc.read = (hw->mac.type == fm10k_mac_pf) ? fm10k_cc_read_pf :
> + fm10k_cc_read_vf;
> + interface->cc.mask = CLOCKSOURCE_MASK(64);
> + interface->cc.mult = 1;
So shift = 0 and multi = 1. Your clock counts nanoseconds. Why not use
it directly? Then you won't need the timecounter stuff or the overflow
watchdog either.
> +
> + /* Initialize lock protecting register access */
> + rwlock_init(&interface->tsreg_lock);
> +
> + /* Initialize skb queue for pending timestamp requests */
> + skb_queue_head_init(&interface->ts_tx_skb_queue);
> +
> + /* Initialize the clock */
> + fm10k_ts_reset_cc(interface);
> +
> + /* Initialize the overflow work */
> + INIT_DELAYED_WORK(&interface->ts_overflow_work,
> + fm10k_ts_overflow_check);
> + schedule_delayed_work(&interface->ts_overflow_work,
> + FM10K_SYSTIME_OVERFLOW_PERIOD);
> +}
> +static int fm10k_ptp_enable(struct ptp_clock_info *ptp,
> + struct ptp_clock_request *rq, int on)
> +{
> + struct fm10k_intfc *interface =
> + container_of(ptp, struct fm10k_intfc, ptp_caps);
> + struct ptp_clock_time *t = &rq->perout.period;
> + struct fm10k_hw *hw = &interface->hw;
> + u64 period;
> + u32 step;
> +
> + /* we can only support periodic output */
> + if (rq->type != PTP_CLK_REQ_PEROUT)
> + return -EINVAL;
> +
> + /* verify the requested channel is there */
> + if (rq->perout.index >= ptp->n_per_out)
> + return -EINVAL;
> +
> + /* we simply cannot support the operation if we don't have BAR4 */
> + if (!hw->sw_addr)
> + return -ENOTSUPP;
> +
> + /* we cannot enforce start time as there is no
> + * mechanism for that in the hardware, we can only control
> + * the period.
> + */
Is this because of the timecounter in the way? Another reason to use
the 64 bit nanosecond counter directly.
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_type.h b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
> index 5055bef..dac5b79 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_type.h
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
> @@ -225,11 +225,7 @@ struct fm10k_hw;
> #define FM10K_STATS_NODESC_DROP 0x3807
>
> /* Timesync registers */
> -#define FM10K_RRTIME_CFG 0x3808
> -#define FM10K_RRTIME_LIMIT(_n) ((_n) + 0x380C)
> -#define FM10K_RRTIME_COUNT(_n) ((_n) + 0x3810)
> #define FM10K_SYSTIME 0x3814
> -#define FM10K_SYSTIME0 0x3816
> #define FM10K_SYSTIME_CFG 0x3818
> #define FM10K_SYSTIME_CFG_STEP_MASK 0x0000000F
>
> @@ -368,9 +364,6 @@ struct fm10k_hw;
> #define FM10K_VFITR(_n) ((_n) + 0x00060)
>
> /* Registers contained in BAR 4 for Switch management */
> -#define FM10K_SW_SYSTIME_CFG 0x0224C
> -#define FM10K_SW_SYSTIME_CFG_STEP_SHIFT 4
> -#define FM10K_SW_SYSTIME_CFG_ADJUST_MASK 0xFF000000
You added these three lines in the previous patch.
> #define FM10K_SW_SYSTIME_ADJUST 0x0224D
> #define FM10K_SW_SYSTIME_ADJUST_MASK 0x3FFFFFFF
> #define FM10K_SW_SYSTIME_ADJUST_DIR_NEGATIVE 0x80000000
>
> --
> 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
Thanks,
Richard
^ permalink raw reply
* [PATCH net-next] fec: Staticize fec_enet_select_queue()
From: Fabio Estevam @ 2014-09-19 16:40 UTC (permalink / raw)
To: davem; +Cc: Frank.Li, netdev, Fabio Estevam
fec_enet_select_queue() is only used locally, so mark it as 'static' to fix the
following sparse warning:
drivers/net/ethernet/freescale/fec_main.c:2707:5: warning: symbol 'fec_enet_select_queue' was not declared. Should it be static?
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 6e93336..eea4f98 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2704,7 +2704,7 @@ static int fec_set_features(struct net_device *netdev,
return 0;
}
-u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
+static u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
void *accel_priv, select_queue_fallback_t fallback)
{
return skb_tx_hash(ndev, skb);
--
1.9.1
^ permalink raw reply related
* Re: [net-next PATCH 00/29] Add support for the Intel FM10000 Ethernet Switch Host Interface
From: Alexei Starovoitov @ 2014-09-19 16:58 UTC (permalink / raw)
To: Alexander Duyck
Cc: Jamal Hadi Salim, David S. Miller, nhorman,
netdev@vger.kernel.org, john fastabend, matthew.vick,
jeffrey.t.kirsher, sassmann
In-Reply-To: <541C43AE.3030402@intel.com>
On Fri, Sep 19, 2014 at 7:54 AM, Alexander Duyck
<alexander.h.duyck@intel.com> wrote:
>
> Since the switch can support up to 9 of these we are treating the host
> interface driver as though it is a NIC driver. We need to usually ship
Am I reading it right that 9 hosts can be attached to this one physical device
and they see it as different 'physical' nics ?
^ permalink raw reply
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Jiri Pirko @ 2014-09-19 15:49 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
roopa, linville, dev, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck
In-Reply-To: <541C4AFC.8060500@mojatatu.com>
Fri, Sep 19, 2014 at 05:25:48PM CEST, jhs@mojatatu.com wrote:
>On 09/19/14 09:49, Jiri Pirko wrote:
>>This patch exposes switchdev API using generic Netlink.
>>Example userspace utility is here:
>>https://github.com/jpirko/switchdev
>>
>
>Is this just a temporary test tool? Otherwise i dont see reason
>for its existence (or the API that it feeds on).
Please read the conversation I had with Pravin and Jesse in v1 thread.
Long story short they like to have the api separated from ovs datapath
so ovs daemon can use it to directly communicate with driver. Also John
Fastabend requested a way to work with driver flows without using ovs ->
that was the original reason I created switchdev genl api.
Regarding the "sw" tool, yes it is for testing purposes now. ovs daemon
will use directly switchdev genl api.
I hope I cleared this out.
^ permalink raw reply
* Re: [net-next PATCH 28/29] fm10k: Add support for ptp to hw specific files
From: Alexander Duyck @ 2014-09-19 15:34 UTC (permalink / raw)
To: Richard Cochran
Cc: davem, nhorman, netdev, john.fastabend, matthew.vick,
jeffrey.t.kirsher, sassmann
In-Reply-To: <20140919151926.GA542@netboy>
On 09/19/2014 08:19 AM, Richard Cochran wrote:
> On Fri, Sep 19, 2014 at 07:36:56AM -0700, Alexander Duyck wrote:
>> On 09/19/2014 12:38 AM, Richard Cochran wrote:
>>> On Thu, Sep 18, 2014 at 06:40:30PM -0400, Alexander Duyck wrote:
>>>
>>>> +static s32 fm10k_adjust_systime_pf(struct fm10k_hw *hw, s32 ppb)
>>>> +{
>>>> + u64 systime_adjust;
>>>> +
>>>> + /* if sw_addr is not set we don't have switch register access */
>>>> + if (!hw->sw_addr)
>>>> + return ppb ? FM10K_ERR_PARAM : 0;
>>>> +
>>>> + /* we must convert the value from parts per billion to parts per
>>>> + * 2^48 cycles. In addition we can only use the upper 30 bits of
>>>> + * the value when making the change so that restricts us futher.
>>>> + * The math for this is equivilent to ABS(pps) * 2^40 / 10 ^ 9,
>
> ...
>
>> 2. The value is in 2 ^ 48 units, and the OS wants to give us a value in
>> parts per billion, not parts per 256 * 2 ^ 40. So in order to simplify
>> things I dropped the lower 8 bits and only update
>
> So what does the comment in the code about "upper 30 bits" mean?
>
The value is measured in parts per 2^48 units, but the field we are
given to provide adjustment units is 38 bits wide, with the 30 most
significant bits in the main adjustment register, and the 8 least
significant bits in another register. The direction is a separate bit
at bit 31 in the upper register with a 1 bit gap between it and the value.
Thanks,
Alex
^ permalink raw reply
* [PATCH net-next] tcp: add coalescing attempt in tcp_ofo_queue()
From: Eric Dumazet @ 2014-09-19 15:26 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng
From: Eric Dumazet <edumazet@google.com>
In order to make TCP more resilient in presence of reorders, we need
to allow coalescing to happen when skbs from out of order queue are
transferred into receive queue. LRO/GRO can be completely canceled
in some pathological cases, like per packet load balancing on aggregated
links.
I had to move tcp_try_coalesce() up in the file above tcp_ofo_queue()
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Note: I am not sure we really need this part in tcp_try_coalesce(),
as I doubt we need ack_seq in the skbs stored in receive queues ?
TCP_SKB_CB(to)->ack_seq = TCP_SKB_CB(from)->ack_seq;
net/ipv4/tcp_input.c | 89 +++++++++++++++++++++--------------------
1 file changed, 47 insertions(+), 42 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index ea92f23ffaf1..44ff50aac96c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4060,6 +4060,44 @@ static void tcp_sack_remove(struct tcp_sock *tp)
tp->rx_opt.num_sacks = num_sacks;
}
+/**
+ * tcp_try_coalesce - try to merge skb to prior one
+ * @sk: socket
+ * @to: prior buffer
+ * @from: buffer to add in queue
+ * @fragstolen: pointer to boolean
+ *
+ * Before queueing skb @from after @to, try to merge them
+ * to reduce overall memory use and queue lengths, if cost is small.
+ * Packets in ofo or receive queues can stay a long time.
+ * Better try to coalesce them right now to avoid future collapses.
+ * Returns true if caller should free @from instead of queueing it
+ */
+static bool tcp_try_coalesce(struct sock *sk,
+ struct sk_buff *to,
+ struct sk_buff *from,
+ bool *fragstolen)
+{
+ int delta;
+
+ *fragstolen = false;
+
+ /* Its possible this segment overlaps with prior segment in queue */
+ if (TCP_SKB_CB(from)->seq != TCP_SKB_CB(to)->end_seq)
+ return false;
+
+ if (!skb_try_coalesce(to, from, fragstolen, &delta))
+ return false;
+
+ atomic_add(delta, &sk->sk_rmem_alloc);
+ sk_mem_charge(sk, delta);
+ NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPRCVCOALESCE);
+ TCP_SKB_CB(to)->end_seq = TCP_SKB_CB(from)->end_seq;
+ TCP_SKB_CB(to)->ack_seq = TCP_SKB_CB(from)->ack_seq;
+ TCP_SKB_CB(to)->tcp_flags |= TCP_SKB_CB(from)->tcp_flags;
+ return true;
+}
+
/* This one checks to see if we can put data from the
* out_of_order queue into the receive_queue.
*/
@@ -4067,7 +4105,8 @@ static void tcp_ofo_queue(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
__u32 dsack_high = tp->rcv_nxt;
- struct sk_buff *skb;
+ struct sk_buff *skb, *tail;
+ bool fragstolen, eaten;
while ((skb = skb_peek(&tp->out_of_order_queue)) != NULL) {
if (after(TCP_SKB_CB(skb)->seq, tp->rcv_nxt))
@@ -4080,9 +4119,9 @@ static void tcp_ofo_queue(struct sock *sk)
tcp_dsack_extend(sk, TCP_SKB_CB(skb)->seq, dsack);
}
+ __skb_unlink(skb, &tp->out_of_order_queue);
if (!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt)) {
SOCK_DEBUG(sk, "ofo packet was already received\n");
- __skb_unlink(skb, &tp->out_of_order_queue);
__kfree_skb(skb);
continue;
}
@@ -4090,11 +4129,15 @@ static void tcp_ofo_queue(struct sock *sk)
tp->rcv_nxt, TCP_SKB_CB(skb)->seq,
TCP_SKB_CB(skb)->end_seq);
- __skb_unlink(skb, &tp->out_of_order_queue);
- __skb_queue_tail(&sk->sk_receive_queue, skb);
+ tail = skb_peek_tail(&sk->sk_receive_queue);
+ eaten = tail && tcp_try_coalesce(sk, tail, skb, &fragstolen);
tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
+ if (!eaten)
+ __skb_queue_tail(&sk->sk_receive_queue, skb);
if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
tcp_fin(sk);
+ if (eaten)
+ kfree_skb_partial(skb, fragstolen);
}
}
@@ -4121,44 +4164,6 @@ static int tcp_try_rmem_schedule(struct sock *sk, struct sk_buff *skb,
return 0;
}
-/**
- * tcp_try_coalesce - try to merge skb to prior one
- * @sk: socket
- * @to: prior buffer
- * @from: buffer to add in queue
- * @fragstolen: pointer to boolean
- *
- * Before queueing skb @from after @to, try to merge them
- * to reduce overall memory use and queue lengths, if cost is small.
- * Packets in ofo or receive queues can stay a long time.
- * Better try to coalesce them right now to avoid future collapses.
- * Returns true if caller should free @from instead of queueing it
- */
-static bool tcp_try_coalesce(struct sock *sk,
- struct sk_buff *to,
- struct sk_buff *from,
- bool *fragstolen)
-{
- int delta;
-
- *fragstolen = false;
-
- /* Its possible this segment overlaps with prior segment in queue */
- if (TCP_SKB_CB(from)->seq != TCP_SKB_CB(to)->end_seq)
- return false;
-
- if (!skb_try_coalesce(to, from, fragstolen, &delta))
- return false;
-
- atomic_add(delta, &sk->sk_rmem_alloc);
- sk_mem_charge(sk, delta);
- NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPRCVCOALESCE);
- TCP_SKB_CB(to)->end_seq = TCP_SKB_CB(from)->end_seq;
- TCP_SKB_CB(to)->ack_seq = TCP_SKB_CB(from)->ack_seq;
- TCP_SKB_CB(to)->tcp_flags |= TCP_SKB_CB(from)->tcp_flags;
- return true;
-}
-
static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
{
struct tcp_sock *tp = tcp_sk(sk);
^ permalink raw reply related
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Jamal Hadi Salim @ 2014-09-19 15:25 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, sfeldma, f.fainelli, roopa, linville,
dev, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh,
aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye, simon.horman,
alexander.h.duyck
In-Reply-To: <1411134590-4586-9-git-send-email-jiri@resnulli.us>
On 09/19/14 09:49, Jiri Pirko wrote:
> This patch exposes switchdev API using generic Netlink.
> Example userspace utility is here:
> https://github.com/jpirko/switchdev
>
Is this just a temporary test tool? Otherwise i dont see reason
for its existence (or the API that it feeds on).
cheers,
jamal
^ permalink raw reply
* Re: [net-next PATCH 28/29] fm10k: Add support for ptp to hw specific files
From: Richard Cochran @ 2014-09-19 15:19 UTC (permalink / raw)
To: Alexander Duyck
Cc: davem, nhorman, netdev, john.fastabend, matthew.vick,
jeffrey.t.kirsher, sassmann
In-Reply-To: <541C3F88.2090004@intel.com>
On Fri, Sep 19, 2014 at 07:36:56AM -0700, Alexander Duyck wrote:
> On 09/19/2014 12:38 AM, Richard Cochran wrote:
> > On Thu, Sep 18, 2014 at 06:40:30PM -0400, Alexander Duyck wrote:
> >
> >> +static s32 fm10k_adjust_systime_pf(struct fm10k_hw *hw, s32 ppb)
> >> +{
> >> + u64 systime_adjust;
> >> +
> >> + /* if sw_addr is not set we don't have switch register access */
> >> + if (!hw->sw_addr)
> >> + return ppb ? FM10K_ERR_PARAM : 0;
> >> +
> >> + /* we must convert the value from parts per billion to parts per
> >> + * 2^48 cycles. In addition we can only use the upper 30 bits of
> >> + * the value when making the change so that restricts us futher.
> >> + * The math for this is equivilent to ABS(pps) * 2^40 / 10 ^ 9,
...
> 2. The value is in 2 ^ 48 units, and the OS wants to give us a value in
> parts per billion, not parts per 256 * 2 ^ 40. So in order to simplify
> things I dropped the lower 8 bits and only update
So what does the comment in the code about "upper 30 bits" mean?
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next] icmp: add a global rate limitation
From: Eric Dumazet @ 2014-09-19 15:11 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, netdev
In-Reply-To: <1411138613.24444.13.camel@joe-AO725>
On Fri, 2014-09-19 at 07:56 -0700, Joe Perches wrote:
> On Fri, 2014-09-19 at 07:38 -0700, Eric Dumazet wrote:
> > Current ICMP rate limiting uses inetpeer cache, which is an RBL tree
> > protected by a lock, meaning that hosts can be stuck hard if all cpus
> > want to check ICMP limits.
> >
> > When say a DNS or NTP server process is restarted, inetpeer tree grows
> > quick and machine comes to its knees.
> >
> > iptables can not help because the bottleneck happens before ICMP
> > messages are even cooked and sent.
> >
> > This patch adds a new global limitation, using a token bucket filter,
> > controlled by two new sysctl :
> >
> > icmp_msgs_per_sec - INTEGER
> > Limit maximal number of ICMP packets sent per second from this host.
> > Only messages whose type matches icmp_ratemask are
> > controlled by this limit.
> > Default: 1000
> >
> > icmp_msgs_burst - INTEGER
> > icmp_msgs_per_sec controls number of ICMP packets sent per second,
> > while icmp_msgs_burst controls the burst size of these packets.
> > Default: 50
>
> nice.
>
> > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> []
> > @@ -231,12 +231,62 @@ static inline void icmp_xmit_unlock(struct sock *sk)
> > spin_unlock_bh(&sk->sk_lock.slock);
> > }
> >
> > +int sysctl_icmp_msgs_per_sec __read_mostly = 1000;
> > +int sysctl_icmp_msgs_burst __read_mostly = 50;
> > +
> > +static struct {
> > + spinlock_t lock;
> > + u32 credit;
> > + u32 stamp;
> > +} icmp_global = {
> > + .lock = __SPIN_LOCK_UNLOCKED(icmp_global.lock),
> > +};
>
> Is there any real benefit using a u32 stamp
> instead of unsigned long?
This is because the computation and sysctl are 32bit wide.
We clamp the delta to 1 sec anyway, so there is no value having 64bit
wide stamp.
Note that I do not expect anybody trying to overflow the computations,
so I did not bother using u64 to perform the (x * y / HZ) operation.
>
> The stamp comparisons are to jiffies and now
> have slightly odd (u32) casts.
Thats because I am planning to eventually use a common helper for the
inetpeer token bucket, or the hashed one I mention at the end of
changelog. Keeping u32 would avoid taking too much room.
>
> > +
> > +/**
> > + * icmp_global_allow - Are we allowed to send one more ICMP message ?
> > + *
> > + * Uses a token bucket to limit our ICMP messages to sysctl_icmp_msgs_per_sec.
> > + * Returns false if we reached the limit and can not send another packet.
> > + * Note: called with BH disabled
> > + */
> > +bool icmp_global_allow(void)
> > +{
> > + u32 credit, delta, incr = 0, now = (u32)jiffies;
>
> Doesn't casting jiffies costs a couple cycles
> on a 64 bit machine?
No, thats a word access, instead of a qword.
^ permalink raw reply
* Re: [PATCH net-next] icmp: add a global rate limitation
From: Joe Perches @ 2014-09-19 14:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1411137520.26859.13.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, 2014-09-19 at 07:38 -0700, Eric Dumazet wrote:
> Current ICMP rate limiting uses inetpeer cache, which is an RBL tree
> protected by a lock, meaning that hosts can be stuck hard if all cpus
> want to check ICMP limits.
>
> When say a DNS or NTP server process is restarted, inetpeer tree grows
> quick and machine comes to its knees.
>
> iptables can not help because the bottleneck happens before ICMP
> messages are even cooked and sent.
>
> This patch adds a new global limitation, using a token bucket filter,
> controlled by two new sysctl :
>
> icmp_msgs_per_sec - INTEGER
> Limit maximal number of ICMP packets sent per second from this host.
> Only messages whose type matches icmp_ratemask are
> controlled by this limit.
> Default: 1000
>
> icmp_msgs_burst - INTEGER
> icmp_msgs_per_sec controls number of ICMP packets sent per second,
> while icmp_msgs_burst controls the burst size of these packets.
> Default: 50
nice.
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
[]
> @@ -231,12 +231,62 @@ static inline void icmp_xmit_unlock(struct sock *sk)
> spin_unlock_bh(&sk->sk_lock.slock);
> }
>
> +int sysctl_icmp_msgs_per_sec __read_mostly = 1000;
> +int sysctl_icmp_msgs_burst __read_mostly = 50;
> +
> +static struct {
> + spinlock_t lock;
> + u32 credit;
> + u32 stamp;
> +} icmp_global = {
> + .lock = __SPIN_LOCK_UNLOCKED(icmp_global.lock),
> +};
Is there any real benefit using a u32 stamp
instead of unsigned long?
The stamp comparisons are to jiffies and now
have slightly odd (u32) casts.
> +
> +/**
> + * icmp_global_allow - Are we allowed to send one more ICMP message ?
> + *
> + * Uses a token bucket to limit our ICMP messages to sysctl_icmp_msgs_per_sec.
> + * Returns false if we reached the limit and can not send another packet.
> + * Note: called with BH disabled
> + */
> +bool icmp_global_allow(void)
> +{
> + u32 credit, delta, incr = 0, now = (u32)jiffies;
Doesn't casting jiffies costs a couple cycles
on a 64 bit machine?
^ permalink raw reply
* Re: [net-next PATCH 00/29] Add support for the Intel FM10000 Ethernet Switch Host Interface
From: Alexander Duyck @ 2014-09-19 14:54 UTC (permalink / raw)
To: Jamal Hadi Salim, davem
Cc: nhorman, netdev, john.fastabend, matthew.vick, jeffrey.t.kirsher,
sassmann
In-Reply-To: <541C0C01.6080708@mojatatu.com>
On 09/19/2014 03:57 AM, Jamal Hadi Salim wrote:
> On 09/18/14 18:35, Alexander Duyck wrote:
>> This patch series adds support for the FM10000 Ethernet switch host
>> interface. The Intel FM10000 Ethernet Switch is a 48-port Ethernet
>> switch
>> supporting both Ethernet ports and PCI Express host interfaces. The
>> fm10k
>> driver provides support for the host interface portion of the switch,
>> both
>> PF and VF.
>>
>> As the host interfaces are directly connected to the switch this
>> results in
>> some significant differences versus a standard network driver. For
>> example
>> there is no PHY or MII on the device. Since packets are delivered
>> directly
>> from the switch to the host interface these are unnecessary.
>> Otherwise most
>> of the functionality is very similar to our other network drivers such as
>> ixgbe or igb. For example we support all the standard network offloads,
>> jumbo frames, SR-IOV (64 VFS), PTP, and some VXLAN and NVGRE offloads.
>>
>
> First off Kudos to the intel folks. Hopefully this is one of many such
> drivers coming - and lets hope this is as landscape-changing as the
> e1000 was ;-> Broadcom smell that Tim Horton's coffee.
Thanks. Though just to be clear it is just the host interface, not the
entire switch that this driver supports.
> You described this as a switch but then expose it as a glorified nic.
> Assuming more goodstuff(tm) coming?
Since the switch can support up to 9 of these we are treating the host
interface driver as though it is a NIC driver. We need to usually ship
NIC drivers well ahead of the silicon so that it can make it into the
distributions before the silicon is actually released, thus avoiding the
need for a driver disk to do a network install on a system with one of
these on board.
I don't have any hard dates on when anything else related to the switch
might be released. That is being managed by another team and I don't
have visibility to that information.
Thanks,
Alex
^ permalink raw reply
* Re: [net-next PATCH 00/29] Add support for the Intel FM10000 Ethernet Switch Host Interface
From: Alexander Duyck @ 2014-09-19 14:43 UTC (permalink / raw)
To: Jiri Pirko
Cc: davem, nhorman, netdev, john.fastabend, matthew.vick,
jeffrey.t.kirsher, sassmann
In-Reply-To: <20140919075556.GC1980@nanopsycho.orion>
On 09/19/2014 12:55 AM, Jiri Pirko wrote:
> Fri, Sep 19, 2014 at 12:35:37AM CEST, alexander.h.duyck@intel.com wrote:
>> This patch series adds support for the FM10000 Ethernet switch host
>> interface. The Intel FM10000 Ethernet Switch is a 48-port Ethernet switch
>> supporting both Ethernet ports and PCI Express host interfaces. The fm10k
>> driver provides support for the host interface portion of the switch, both
>> PF and VF.
>>
>> As the host interfaces are directly connected to the switch this results in
>> some significant differences versus a standard network driver. For example
>> there is no PHY or MII on the device. Since packets are delivered directly
>>from the switch to the host interface these are unnecessary. Otherwise most
>> of the functionality is very similar to our other network drivers such as
>> ixgbe or igb. For example we support all the standard network offloads,
>> jumbo frames, SR-IOV (64 VFS), PTP, and some VXLAN and NVGRE offloads.
>
> I'm very happy to see this patchset in the wind. Great news! I have
> couple of questions:
>
> Do you also plan to introduce support for FM6000?
>
> From what I understand, there is one netdev instance for the whole switch (PF).
> How can user get stats and info for particular ports? This topic was
> discussed many times and I believe that general consensus is to have 1
> netdev instance to represent one switch port (that is for example how we
> do it in rocker driver).
>
> Thanks.
>
Just to be clear this isn't the entire switch. This is only the host
interface that this driver supports. The host interface is essentially
just an embedded NIC on the switch.
In regards to your question about the netdev instances for the ports
that would be up to the switch driver. We are still debating some of
the implementation details of that. For example we are still trying to
decide if the switch should be a part of the fm10k driver or if it
should be a separate driver onto itself similar to how the DSA driver
model works. My preference is the latter, but that isn't my decision as
I am only responsible for the host interface.
As far as the FM6000 goes I can't say. I don't have visibility into
what the team responsible for that switch plans to do about their host
interface. It would be nice to have both switches end up following the
same model, but I can't speak for that team.
Thanks,
Alex
^ permalink raw reply
* [PATCH net-next] icmp: add a global rate limitation
From: Eric Dumazet @ 2014-09-19 14:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Current ICMP rate limiting uses inetpeer cache, which is an RBL tree
protected by a lock, meaning that hosts can be stuck hard if all cpus
want to check ICMP limits.
When say a DNS or NTP server process is restarted, inetpeer tree grows
quick and machine comes to its knees.
iptables can not help because the bottleneck happens before ICMP
messages are even cooked and sent.
This patch adds a new global limitation, using a token bucket filter,
controlled by two new sysctl :
icmp_msgs_per_sec - INTEGER
Limit maximal number of ICMP packets sent per second from this host.
Only messages whose type matches icmp_ratemask are
controlled by this limit.
Default: 1000
icmp_msgs_burst - INTEGER
icmp_msgs_per_sec controls number of ICMP packets sent per second,
while icmp_msgs_burst controls the burst size of these packets.
Default: 50
Note that if we really want to send millions of ICMP messages per
second, we might extend idea and infra added in commit 04ca6973f7c1a
("ip: make IP identifiers less predictable") :
add a token bucket in the ip_idents hash and no longer rely on inetpeer.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Documentation/networking/ip-sysctl.txt | 13 ++++
include/net/ip.h | 4 +
net/ipv4/icmp.c | 64 +++++++++++++++++++++--
net/ipv4/sysctl_net_ipv4.c | 16 +++++
net/ipv6/icmp.c | 20 ++++---
5 files changed, 105 insertions(+), 12 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 1b5581a30d77..c7a81ace35d0 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -769,8 +769,21 @@ icmp_ratelimit - INTEGER
icmp_ratemask (see below) to specific targets.
0 to disable any limiting,
otherwise the minimal space between responses in milliseconds.
+ Note that another sysctl, icmp_msgs_per_sec limits the number
+ of ICMP packets sent on all targets.
Default: 1000
+icmp_msgs_per_sec - INTEGER
+ Limit maximal number of ICMP packets sent per second from this host.
+ Only messages whose type matches icmp_ratemask (see below) are
+ controlled by this limit.
+ Default: 1000
+
+icmp_msgs_burst - INTEGER
+ icmp_msgs_per_sec controls number of ICMP packets sent per second,
+ while icmp_msgs_burst controls the burst size of these packets.
+ Default: 50
+
icmp_ratemask - INTEGER
Mask made of ICMP types for which rates are being limited.
Significant bits: IHGFEDCBA9876543210
diff --git a/include/net/ip.h b/include/net/ip.h
index 14bfc8e1bcf9..fcd9068fb8c3 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -548,6 +548,10 @@ void ip_icmp_error(struct sock *sk, struct sk_buff *skb, int err, __be16 port,
void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 dport,
u32 info);
+bool icmp_global_allow(void);
+extern int sysctl_icmp_msgs_per_sec;
+extern int sysctl_icmp_msgs_burst;
+
#ifdef CONFIG_PROC_FS
int ip_misc_proc_init(void);
#endif
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index ea7d4afe8205..ba86c146a4ab 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -231,12 +231,62 @@ static inline void icmp_xmit_unlock(struct sock *sk)
spin_unlock_bh(&sk->sk_lock.slock);
}
+int sysctl_icmp_msgs_per_sec __read_mostly = 1000;
+int sysctl_icmp_msgs_burst __read_mostly = 50;
+
+static struct {
+ spinlock_t lock;
+ u32 credit;
+ u32 stamp;
+} icmp_global = {
+ .lock = __SPIN_LOCK_UNLOCKED(icmp_global.lock),
+};
+
+/**
+ * icmp_global_allow - Are we allowed to send one more ICMP message ?
+ *
+ * Uses a token bucket to limit our ICMP messages to sysctl_icmp_msgs_per_sec.
+ * Returns false if we reached the limit and can not send another packet.
+ * Note: called with BH disabled
+ */
+bool icmp_global_allow(void)
+{
+ u32 credit, delta, incr = 0, now = (u32)jiffies;
+ bool rc = false;
+
+ /* Check if token bucket is empty and cannot be refilled
+ * without taking the spinlock.
+ */
+ if (!icmp_global.credit) {
+ delta = min_t(u32, now - icmp_global.stamp, HZ);
+ if (delta < HZ / 50)
+ return false;
+ }
+
+ spin_lock(&icmp_global.lock);
+ delta = min_t(u32, now - icmp_global.stamp, HZ);
+ if (delta >= HZ / 50) {
+ incr = sysctl_icmp_msgs_per_sec * delta / HZ ;
+ if (incr)
+ icmp_global.stamp = now;
+ }
+ credit = min_t(u32, icmp_global.credit + incr, sysctl_icmp_msgs_burst);
+ if (credit) {
+ credit--;
+ rc = true;
+ }
+ icmp_global.credit = credit;
+ spin_unlock(&icmp_global.lock);
+ return rc;
+}
+EXPORT_SYMBOL(icmp_global_allow);
+
/*
* Send an ICMP frame.
*/
-static inline bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
- struct flowi4 *fl4, int type, int code)
+static bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
+ struct flowi4 *fl4, int type, int code)
{
struct dst_entry *dst = &rt->dst;
bool rc = true;
@@ -253,8 +303,14 @@ static inline bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
goto out;
/* Limit if icmp type is enabled in ratemask. */
- if ((1 << type) & net->ipv4.sysctl_icmp_ratemask) {
- struct inet_peer *peer = inet_getpeer_v4(net->ipv4.peers, fl4->daddr, 1);
+ if (!((1 << type) & net->ipv4.sysctl_icmp_ratemask))
+ goto out;
+
+ rc = false;
+ if (icmp_global_allow()) {
+ struct inet_peer *peer;
+
+ peer = inet_getpeer_v4(net->ipv4.peers, fl4->daddr, 1);
rc = inet_peer_xrlim_allow(peer,
net->ipv4.sysctl_icmp_ratelimit);
if (peer)
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 1599966f4639..8a25509c35b3 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -731,6 +731,22 @@ static struct ctl_table ipv4_table[] = {
.extra2 = &one,
},
{
+ .procname = "icmp_msgs_per_sec",
+ .data = &sysctl_icmp_msgs_per_sec,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ },
+ {
+ .procname = "icmp_msgs_burst",
+ .data = &sysctl_icmp_msgs_burst,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ },
+ {
.procname = "udp_mem",
.data = &sysctl_udp_mem,
.maxlen = sizeof(sysctl_udp_mem),
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 394bb824fe4b..141e1f3ab74e 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -170,11 +170,11 @@ static bool is_ineligible(const struct sk_buff *skb)
/*
* Check the ICMP output rate limit
*/
-static inline bool icmpv6_xrlim_allow(struct sock *sk, u8 type,
- struct flowi6 *fl6)
+static bool icmpv6_xrlim_allow(struct sock *sk, u8 type,
+ struct flowi6 *fl6)
{
- struct dst_entry *dst;
struct net *net = sock_net(sk);
+ struct dst_entry *dst;
bool res = false;
/* Informational messages are not limited. */
@@ -199,16 +199,20 @@ static inline bool icmpv6_xrlim_allow(struct sock *sk, u8 type,
} else {
struct rt6_info *rt = (struct rt6_info *)dst;
int tmo = net->ipv6.sysctl.icmpv6_time;
- struct inet_peer *peer;
/* Give more bandwidth to wider prefixes. */
if (rt->rt6i_dst.plen < 128)
tmo >>= ((128 - rt->rt6i_dst.plen)>>5);
- peer = inet_getpeer_v6(net->ipv6.peers, &rt->rt6i_dst.addr, 1);
- res = inet_peer_xrlim_allow(peer, tmo);
- if (peer)
- inet_putpeer(peer);
+ if (icmp_global_allow()) {
+ struct inet_peer *peer;
+
+ peer = inet_getpeer_v6(net->ipv6.peers,
+ &rt->rt6i_dst.addr, 1);
+ res = inet_peer_xrlim_allow(peer, tmo);
+ if (peer)
+ inet_putpeer(peer);
+ }
}
dst_release(dst);
return res;
^ permalink raw reply related
* Re: [net-next PATCH 28/29] fm10k: Add support for ptp to hw specific files
From: Alexander Duyck @ 2014-09-19 14:36 UTC (permalink / raw)
To: Richard Cochran
Cc: davem, nhorman, netdev, john.fastabend, matthew.vick,
jeffrey.t.kirsher, sassmann
In-Reply-To: <20140919073820.GA5954@netboy>
On 09/19/2014 12:38 AM, Richard Cochran wrote:
> On Thu, Sep 18, 2014 at 06:40:30PM -0400, Alexander Duyck wrote:
>
>> +static s32 fm10k_adjust_systime_pf(struct fm10k_hw *hw, s32 ppb)
>> +{
>> + u64 systime_adjust;
>> +
>> + /* if sw_addr is not set we don't have switch register access */
>> + if (!hw->sw_addr)
>> + return ppb ? FM10K_ERR_PARAM : 0;
>> +
>> + /* we must convert the value from parts per billion to parts per
>> + * 2^48 cycles. In addition we can only use the upper 30 bits of
>> + * the value when making the change so that restricts us futher.
>> + * The math for this is equivilent to ABS(pps) * 2^40 / 10 ^ 9,
>
> Huh? Converting ppb to parts per 2^48 should be (ppb * 2^48 / 10^9),
> shouldn't it?
>
> Did you mean, "we must convert ... to parts per 2^40 cycles" ?
>
> Also, the comment about using the upper 30 bits is not clear. Do you
> mean that the hardware ignores the two least significant bits?
So to break it all down.
1. The hardware provides a value that can be specified in parts per
2^48, however that value spans across 2 registers starting at bit 24 in
the first register. You can actually make out a bit more if you look at
the end of the patch. The layout for the registers look something like
this:
/* Registers contained in BAR 4 for Switch management */
#define FM10K_SW_SYSTIME_CFG 0x0224C
#define FM10K_SW_SYSTIME_CFG_ADJUST_MASK 0xFF000000
#define FM10K_SW_SYSTIME_ADJUST 0x0224D
#define FM10K_SW_SYSTIME_ADJUST_MASK 0x3FFFFFFF
#define FM10K_SW_SYSTIME_ADJUST_DIR_NEGATIVE 0x80000000
2. The value is in 2 ^ 48 units, and the OS wants to give us a value in
parts per billion, not parts per 256 * 2 ^ 40. So in order to simplify
things I dropped the lower 8 bits and only update
FM10K_SW_SYSTIME_ADJUST register giving me an adjust value multiplied by
256. So the adjustment value I am using now is:
(256 / (256 * (2 ^ 40)))
this simplifies down to
1 / (2 ^ 40)
The value 1 / (2 ^ 40) implies that we are still looking at something
close to parts per trillion, however since I am now only adjusting one
register instead of 2 it meets my needs since those lower 8 bits are an
even smaller part of the whole.
3. The last bit in the adjustment is to deal with the the fact that we
have a power of 2, not a power of 10 and the adjustments are in parts
per billion. So we would need to convert by 1024 ^ 3 / 1000 ^ 3. So
the whole thing broken down would be:
value / (10 ^ 9) == adj_val / (2 ^ 40)
so if I solve for adj_val
value * (2 ^ 40) / (10 ^ 9) == adj_val
then reduce it by dropping the shared values of 2
value * (2 ^ 31) / (5 ^ 9) == adj_val
Hopefully that explains how I came to that value.
Thanks,
Alex
^ 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