* Re: [RFCv2 2/3] ath10k: report per-station tx/rate rates to mac80211
From: Michal Kazior @ 2016-03-24 12:31 UTC (permalink / raw)
To: Mohammed Shafi Shajakhan
Cc: linux-wireless, Felix Fietkau, Emmanuel Grumbach,
Network Development, Dave Taht,
ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
codel-JXvr2/1DY2fm6VMwtOF2vx4hnT+Y9+D1,
make-wifi-fast-JXvr2/1DY2fm6VMwtOF2vx4hnT+Y9+D1, Johannes Berg,
Tim Shepard
In-Reply-To: <20160324122317.GB30872@atheros-ThinkPad-T61>
On 24 March 2016 at 13:23, Mohammed Shafi Shajakhan
<mohammed-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On Thu, Mar 24, 2016 at 08:49:12AM +0100, Michal Kazior wrote:
>> On 24 March 2016 at 08:19, Mohammed Shafi Shajakhan
>> <mohammed-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> > Hi Michal,
>> >
>> > On Wed, Mar 16, 2016 at 11:17:57AM +0100, Michal Kazior wrote:
>> >> The rate control is offloaded by firmware so it's
>> >> challanging to provide expected throughput value
>> >> for given station.
>> >>
>> >> This approach is naive as it reports last tx rate
>> >> used for given station as provided by firmware
>> >> stat event.
>> >>
>> >> This should be sufficient for airtime estimation
>> >> used for fq-codel-in-mac80211 tx scheduling
>> >> purposes now.
>> >>
>> >> This patch uses a very hacky way to get the stats.
>> >> This is sufficient for proof-of-concept but must
>> >> be cleaned up properly eventually.
>> >>
>> >> Signed-off-by: Michal Kazior <michal.kazior-++hxYGjEMp0AvxtiuMwx3w@public.gmane.org>
>> >> ---
>> >> drivers/net/wireless/ath/ath10k/core.h | 5 +++
>> >> drivers/net/wireless/ath/ath10k/debug.c | 61 +++++++++++++++++++++++++++++----
>> >> drivers/net/wireless/ath/ath10k/mac.c | 26 ++++++++------
>> >> drivers/net/wireless/ath/ath10k/wmi.h | 2 +-
>> >> 4 files changed, 76 insertions(+), 18 deletions(-)
>> >>
>> >> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
>> >> index 23ba03fb7a5f..3f76669d44cf 100644
>> >> --- a/drivers/net/wireless/ath/ath10k/core.h
>> >> +++ b/drivers/net/wireless/ath/ath10k/core.h
>> >> @@ -331,6 +331,9 @@ struct ath10k_sta {
>> >> /* protected by conf_mutex */
>> >> bool aggr_mode;
>> >> u64 rx_duration;
>> >> +
>> >> + u32 tx_rate_kbps;
>> >> + u32 rx_rate_kbps;
>> >> #endif
>> >> };
>> >>
>> >> @@ -372,6 +375,8 @@ struct ath10k_vif {
>> >> s8 def_wep_key_idx;
>> >>
>> >> u16 tx_seq_no;
>> >> + u32 tx_rate_kbps;
>> >> + u32 rx_rate_kbps;
>> >>
>> >> union {
>> >> struct {
>> >> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
>> >> index 076d29b53ddf..cc7ebf04ae00 100644
>> >> --- a/drivers/net/wireless/ath/ath10k/debug.c
>> >> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>> >> @@ -316,6 +316,58 @@ static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
>> >> spin_unlock_bh(&ar->data_lock);
>> >> }
>> >>
>> >> +static void ath10k_mac_update_txrx_rate_iter(void *data,
>> >> + u8 *mac,
>> >> + struct ieee80211_vif *vif)
>> >> +{
>> >> + struct ath10k_fw_stats_peer *peer = data;
>> >> + struct ath10k_vif *arvif;
>> >> +
>> >> + if (memcmp(vif->addr, peer->peer_macaddr, ETH_ALEN))
>> >> + return;
>> >> +
>> >> + arvif = (void *)vif->drv_priv;
>> >> + arvif->tx_rate_kbps = peer->peer_tx_rate;
>> >> + arvif->rx_rate_kbps = peer->peer_rx_rate;
>> >> +}
>> >> +
>> >> +static void ath10k_mac_update_txrx_rate(struct ath10k *ar,
>> >> + struct ath10k_fw_stats *stats)
>> >> +{
>> >> + struct ieee80211_hw *hw = ar->hw;
>> >> + struct ath10k_fw_stats_peer *peer;
>> >> + struct ath10k_sta *arsta;
>> >> + struct ieee80211_sta *sta;
>> >> + const u8 *localaddr = NULL;
>> >> +
>> >> + rcu_read_lock();
>> >> +
>> >> + list_for_each_entry(peer, &stats->peers, list) {
>> >> + /* This doesn't account for multiple STA connected on different
>> >> + * vifs. Unfortunately there's no way to derive that from the available
>> >> + * information.
>> >> + */
>> >> + sta = ieee80211_find_sta_by_ifaddr(hw,
>> >> + peer->peer_macaddr,
>> >> + localaddr);
>> >> + if (!sta) {
>> >> + /* This tries to update multicast rates */
>> >> + ieee80211_iterate_active_interfaces_atomic(
>> >> + hw,
>> >> + IEEE80211_IFACE_ITER_NORMAL,
>> >> + ath10k_mac_update_txrx_rate_iter,
>> >> + peer);
>> >> + continue;
>> >> + }
>> >> +
>> >> + arsta = (void *)sta->drv_priv;
>> >> + arsta->tx_rate_kbps = peer->peer_tx_rate;
>> >> + arsta->rx_rate_kbps = peer->peer_rx_rate;
>> >> + }
>> >> +
>> >> + rcu_read_unlock();
>> >> +}
>> >> +
>> >> void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>> >> {
>> >> struct ath10k_fw_stats stats = {};
>> >> @@ -335,6 +387,8 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>> >> goto free;
>> >> }
>> >>
>> >> + ath10k_mac_update_txrx_rate(ar, &stats);
>> >> +
>> >> /* Stat data may exceed htc-wmi buffer limit. In such case firmware
>> >> * splits the stats data and delivers it in a ping-pong fashion of
>> >> * request cmd-update event.
>> >> @@ -351,13 +405,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>> >> if (peer_stats_svc)
>> >> ath10k_sta_update_rx_duration(ar, &stats.peers);
>> >>
>> >> - if (ar->debug.fw_stats_done) {
>> >> - if (!peer_stats_svc)
>> >> - ath10k_warn(ar, "received unsolicited stats update event\n");
>> >> -
>> >> - goto free;
>> >> - }
>> >> -
>> >
>> > [shafi] As you had suggested previously, should we completely clean up this ping
>> > - pong response approach for f/w stats, (or) this should be retained to support
>> > backward compatibility and also for supporting ping - pong response when user
>> > cats for fw-stats (via debugfs) (i did see in the commit message this needs to
>> > be cleaned up)
>>
>> I think it makes sense to remove the ping-pong logic and rely on
>> periodic updates alone, including fw_stats and ethstats handling.
>>
>>
>> >> - if (test_bit(WMI_SERVICE_PEER_STATS, ar->wmi.svc_map)) {
>> >> - param = ar->wmi.pdev_param->peer_stats_update_period;
>> >> - ret = ath10k_wmi_pdev_set_param(ar, param,
>> >> - PEER_DEFAULT_STATS_UPDATE_PERIOD);
>> >> - if (ret) {
>> >> - ath10k_warn(ar,
>> >> - "failed to set peer stats period : %d\n",
>> >> - ret);
>> >> - goto err_core_stop;
>> >> - }
>> >> + param = ar->wmi.pdev_param->peer_stats_update_period;
>> >> + ret = ath10k_wmi_pdev_set_param(ar, param,
>> >> + PEER_DEFAULT_STATS_UPDATE_PERIOD);
>> >> + if (ret) {
>> >> + ath10k_warn(ar,
>> >> + "failed to set peer stats period : %d\n",
>> >> + ret);
>> >> + goto err_core_stop;
>> >> }
>> >
>> > [shafi] If i am correct this change requires 'PEER_STATS' to be enabled by
>> > default.
>>
>> No, it does not. Periodic stats have been available since forever.
>
> [shafi] Michal, sorry i was talking about enabling WMI_PEER_STATS feature for
> 10.2, and we have a patch pushed recently to reduce the number of peers if
> 'WMI_PEER_STATS' feature is enabled(avoiding f/w crash due to memory
> constraints) . But this patch requires the feature to be
> turned ON always (with periodic stats update as well for evey 100ms). Please
> correct me if my understanding is wrong.
Periodic stats and extended stats are two separate things.
WMI_PEER_STATS is a feature which prompts firmware to gather more
statistics and then report them to host via stat update event and
includes, e.g. rx_duration. Due to how rx_duration was designed in
firmware it needs to be combined with reading out the stats often to
make it usable. Periodic stats were used instead of explicit
ping-pong.
Periodic stats is just one of two ways (the other being ping-pong)
asking firmware for stat update event. It has been in firmware since
forever.
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] stmmac: Fix phy without MDIO subnode
From: Giuseppe CAVALLARO @ 2016-03-24 12:56 UTC (permalink / raw)
To: John Keeping; +Cc: Gabriel Fernandez, netdev, linux-kernel
In-Reply-To: <1458817012-14293-1-git-send-email-john@metanate.com>
Hi John
This should be fixed by some work done some days
ago and not yet committed.
Pls see "stmmac: MDIO fixes" patch-set and let me know
if ok on your side.
Regards
Peppe
On 3/24/2016 11:56 AM, John Keeping wrote:
> Since commit 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found'
> regression") we no longer allocate mdio_bus_data unless there is a MDIO
> subnode. This breaks the ethernet on the Radxa Rock2 (using
> rk3288-rock2-square.dts) which does not have an MDIO subnode.
>
> That commit was correct that the phy_bus_name test is unhelpful since we
> allocate "plat" in the same function and never set phy_bus_name so let's
> just drop the test which restores the previous behaviour.
>
> Fixes: 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found' regression")
> Signed-off-by: John Keeping <john@metanate.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index dcbd2a1..e0fa060 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -189,7 +189,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> if (of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr) == 0)
> dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");
>
> - if ((plat->phy_node && !of_phy_is_fixed_link(np)) || !plat->mdio_node)
> + if ((plat->phy_node && !of_phy_is_fixed_link(np)))
> plat->mdio_bus_data = NULL;
> else
> plat->mdio_bus_data =
>
^ permalink raw reply
* Re: [PATCH] stmmac: Fix phy without MDIO subnode
From: Sergei Shtylyov @ 2016-03-24 13:00 UTC (permalink / raw)
To: John Keeping, Giuseppe Cavallaro; +Cc: Gabriel Fernandez, netdev, linux-kernel
In-Reply-To: <1458817012-14293-1-git-send-email-john@metanate.com>
Hello.
On 3/24/2016 1:56 PM, John Keeping wrote:
> Since commit 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found'
> regression") we no longer allocate mdio_bus_data unless there is a MDIO
> subnode. This breaks the ethernet on the Radxa Rock2 (using
> rk3288-rock2-square.dts) which does not have an MDIO subnode.
>
> That commit was correct that the phy_bus_name test is unhelpful since we
> allocate "plat" in the same function and never set phy_bus_name so let's
> just drop the test which restores the previous behaviour.
>
> Fixes: 88f8b1bb41c6 ("stmmac: Fix 'eth0: No PHY found' regression")
> Signed-off-by: John Keeping <john@metanate.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index dcbd2a1..e0fa060 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -189,7 +189,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> if (of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr) == 0)
> dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");
>
> - if ((plat->phy_node && !of_phy_is_fixed_link(np)) || !plat->mdio_node)
> + if ((plat->phy_node && !of_phy_is_fixed_link(np)))
Too many parens...
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] net: mlxsw: avoid unused variable warnings
From: Arnd Bergmann @ 2016-03-24 13:14 UTC (permalink / raw)
To: David Miller; +Cc: andrew, jiri, idosch, eladr, netdev, linux-kernel
In-Reply-To: <20160323.143805.310789862506977932.davem@davemloft.net>
On Wednesday 23 March 2016 14:38:05 David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Wed, 23 Mar 2016 17:51:11 +0100
>
> > On Wed, Mar 23, 2016 at 05:37:38PM +0100, Arnd Bergmann wrote:
> >> dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
> >> not set, which can lead to unused variable warnings:
> >>
> >> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
> >> ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
> >> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
> >> ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]
> >>
> >> This changes the mlxsw driver to remove the local variables we get
> >> warnings for and instead pass the device directly into the API.
> >
> > Hi Arnd
> >
> > Would it not be better to fix the macro?
> >
> > I think the issue is that dev_dbg_ratelimited calls no_printk(),
> > without making use of dev. So how about:
> >
> > #define dev_dbg_ratelimited(dev, fmt, ...) \
> > ({ \
> > if (0) \
> > dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
> > })
> >
> > This follows the pattern for other macros for when DEBUG is not defined.
>
> Yeah, this is probably a better way to fix this problem.
Makes sense. I was thrown off by how a related patch recently
modified the no_printk() definition in Fixes: fe22cd9b7c98 ("printk:
help pr_debug and pr_devel to optimize out arguments") around the
same time I ran into the problem in the mlxsw driver.
I'll test build the patch below for a while and submit that if it doesn't
cause any other problems.
diff --git a/include/linux/device.h b/include/linux/device.h
index 002c59728dbe..07f74c246cac 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1293,8 +1293,11 @@ do { \
dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
} while (0)
#else
-#define dev_dbg_ratelimited(dev, fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define dev_dbg_ratelimited(dev, fmt, ...) \
+do { \
+ if (0) \
+ dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
+} while (0)
#endif
#ifdef VERBOSE_DEBUG
Thanks,
Arnd
^ permalink raw reply related
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Andrew Lunn @ 2016-03-24 13:29 UTC (permalink / raw)
To: Vishal Thanki; +Cc: Florian Fainelli, Matus Ujhelyi, netdev
In-Reply-To: <CAC3a_SBjvQFT5XP2bH3TO_4wHnjZ3a3+XvfBy8StxQieEsavQQ@mail.gmail.com>
On Wed, Mar 23, 2016 at 10:24:45PM +0100, Vishal Thanki wrote:
> On Wed, Mar 23, 2016 at 10:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Wed, Mar 23, 2016 at 09:24:00PM +0100, Vishal Thanki wrote:
> >> Hi,
> >>
> >> > My suggestion was that the hardware needs to control the LEDs. You
> >> > have software doing it. You might be able to do this with the PHY
> >> > state machine for link. But activity is never going to be accepted if
> >> > software control.
> >> >
> >> > The LED trigger attached to an LED should be used to configure the
> >> > hardware to drive the LED as wanted.
> >> >
> >>
> >> The eth-phy-activity trigger uses the blink_set which I think uses the
> >> hardware acceleration if available. I am not sure how to handles LEDs
> >> which does not have hardware acceleration for this (eth-phy-activity)
> >> trigger.
> >
> > We want the LED to blink on activity, real packets coming in and
> > out. The PHY can do this, so let the PHY control the LED. In this
> > case, the trigger is just mechanism for the user to say what the LED
> > should be used for. The trigger is not itself controlling the LED, it
> > has no idea about packets coming and going.
> >
>
> Yes, I understand that. But PHY can only control the LEDs attached to
> it directly. The at803x led driver configures the PHY to blink the
> activity LED based on traffic but I think it is not possible for PHY
> to control other LEDs in system, for example some other LEDs in system
> controlled only via GPIO. In such cases, putting PHY activity trigger
> on the GPIO LEDs would not make sense. Correct me if I am wrong.
Hi Vishal
All correct. Which is why i said in my original email, you need to
extend the LED core to associate triggers to LEDs. You can then
associate the eth-phy-activity trigger to only PHY leds which can
implement that functionality.
drivers/leds/led-triggers.c contains a global list
LIST_HEAD(trigger_list) which triggers get added to using
led_trigger_register(). You could add a second list to the
led_classdev structure, and add an led_trigger_register_to_led()
function which registers a trigger to a specific LED, on its own
trigger list. led_trigger_store() and led_trigger_show() would use
both lists.
Andrew
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: Request 'reset' GPIO only for AT8030 PHY
From: Sergei Shtylyov @ 2016-03-24 13:40 UTC (permalink / raw)
To: Sebastian Frias, Uwe Kleine-König
Cc: Daniel Mack, David S. Miller, netdev, lkml, mason,
Florian Fainelli, Mans Rullgard, Fabio Estevam,
Martin Blumenstingl, Linus Walleij
In-Reply-To: <56F3BD28.4080908@laposte.net>
On 03/24/2016 01:10 PM, Sebastian Frias wrote:
>>> What I don't understand is why the link_change_notify() method ptr is
>>> populated for all 3 supported chips while only being needed on 8030...
>>
>> You are right.
>
> I made the patch but I'm unsure about it because it could conflict with
> yours.
> I mean, I think you submitted a patch to change the GPIO handling on the
> link_change_notify() function, right?
> Well, if we only register the callback for the AT8030, then there is no
> more need for the callback to check the PHY ID.
> However, if I change that, the whole block moves as I remove one
> indentation level (the one required by the PHY ID check).
>
> Any suggestions on how to create a patch that won't conflict? I probably
> need to use a tree that already has your patch applied.
My patch is already in Linus' tree, so should be merged back from net.git
into net-next.git
soon. I suggest that you wait until net-next is open again.
> Best regards,
>
> Sebastian
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Eric Dumazet @ 2016-03-24 14:13 UTC (permalink / raw)
To: Willy Tarreau
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <20160324061222.GA6807@1wt.eu>
On Thu, 2016-03-24 at 07:12 +0100, Willy Tarreau wrote:
> Hi,
>
> On Wed, Mar 23, 2016 at 10:10:06PM -0700, Tolga Ceylan wrote:
> > I apologize for not properly following up on this. I had the
> > impression that we did not want to merge my original patch and then I
> > also noticed that it fails to keep the hash consistent. Recently, I
> > read the follow ups on it as well as Willy's patch/proposals.
> >
> > Is there any update on Willy's SO_REUSEPORT patch? IMHO, it solves the
> > problem and it is simpler than adding new sock option.
>
> no, Craig's changes were merged, and I haven't checked yet if my patch
> needs to be rebased or still applies. Feel free to check it and resubmit
> if you have time.
No need for a patch AFAIK.
BPF solution is generic enough.
All user space needs to do is to update the BPF filter so that the
listener needing to be dismantled does not receive any new packet.
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Willy Tarreau @ 2016-03-24 14:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <1458828813.10868.65.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
On Thu, Mar 24, 2016 at 07:13:33AM -0700, Eric Dumazet wrote:
> On Thu, 2016-03-24 at 07:12 +0100, Willy Tarreau wrote:
> > Hi,
> >
> > On Wed, Mar 23, 2016 at 10:10:06PM -0700, Tolga Ceylan wrote:
> > > I apologize for not properly following up on this. I had the
> > > impression that we did not want to merge my original patch and then I
> > > also noticed that it fails to keep the hash consistent. Recently, I
> > > read the follow ups on it as well as Willy's patch/proposals.
> > >
> > > Is there any update on Willy's SO_REUSEPORT patch? IMHO, it solves the
> > > problem and it is simpler than adding new sock option.
> >
> > no, Craig's changes were merged, and I haven't checked yet if my patch
> > needs to be rebased or still applies. Feel free to check it and resubmit
> > if you have time.
>
> No need for a patch AFAIK.
>
> BPF solution is generic enough.
>
> All user space needs to do is to update the BPF filter so that the
> listener needing to be dismantled does not receive any new packet.
But that means that any software making use of SO_REUSEPORT needs to
also implement BPF on Linux to achieve the same as what it does on
other OSes ? Also I found a case where a dying process would still
cause trouble in the accept queue, maybe it's not redistributed, I
don't remember, all I remember is that my traffic stopped after a
segfault of only one of them :-/ I'll have to dig a bit regarding
this.
Thanks,
Willy
^ permalink raw reply
* Re: [RFD] workqueue: WQ_MEM_RECLAIM usage in network drivers
From: Johannes Berg @ 2016-03-24 14:22 UTC (permalink / raw)
To: Tejun Heo, J. Bruce Fields
Cc: Jeff Layton, David S. Miller, Trond Myklebust, Anna Schumaker,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, Amitoj Kaur Chawla,
kernel-team-b10kYP2dOMg, Johannes Weiner, Eva Rachel Retuya,
Bhaktipriya Shridhar, linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160320185507.GT20028-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Sun, 2016-03-20 at 14:55 -0400, Tejun Heo wrote:
> If everything else is working, I'd be happy to throw in
> WQ_MEM_RECLAIM but I really don't want to add it if it doesn't
> actually achieve the goal. Can a wireless person chime in here?
>
I think for many wireless devices the workqueue, like for iwldvm that
was just changed, isn't in the packet path and thus less relevant.
For some, like SDIO based chips, it's more likely to be in the packet
path, so it would make sense to keep it.
Of course there's always a possibility of getting disconnected and then
not being able to re-establish the connection in memory pressure, but
that's not something we can control/predict (the disconnection). Can of
course also happen with wired if somebody pulls out the cable, but it's
less reliant on memory allocations to get back to working there, I
guess :)
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/1] net: macb: remove BUG_ON() and reset the queue to handle RX errors
From: Cyrille Pitchen @ 2016-03-24 14:37 UTC (permalink / raw)
To: nicolas.ferre, davem, linux-arm-kernel, netdev, soren.brinkmann,
narmstrong
Cc: linux-kernel, Cyrille Pitchen
This patch removes two BUG_ON() used to notify about RX queue corruptions
on macb (not gem) hardware without actually handling the error.
The new code skips corrupted frames but still processes faultless frames.
Then it resets the RX queue before restarting the reception from a clean
state.
This patch is a rework of an older patch proposed by Neil Armstrong:
http://patchwork.ozlabs.org/patch/371525/
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
drivers/net/ethernet/cadence/macb.c | 59 ++++++++++++++++++++++++++++++-------
1 file changed, 49 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 6619178ed77b..39447a337149 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -917,7 +917,10 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
unsigned int frag_len = bp->rx_buffer_size;
if (offset + frag_len > len) {
- BUG_ON(frag != last_frag);
+ if (unlikely(frag != last_frag)) {
+ dev_kfree_skb_any(skb);
+ return -1;
+ }
frag_len = len - offset;
}
skb_copy_to_linear_data_offset(skb, offset,
@@ -945,11 +948,26 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
return 0;
}
+static inline void macb_init_rx_ring(struct macb *bp)
+{
+ int i;
+ dma_addr_t addr;
+
+ addr = bp->rx_buffers_dma;
+ for (i = 0; i < RX_RING_SIZE; i++) {
+ bp->rx_ring[i].addr = addr;
+ bp->rx_ring[i].ctrl = 0;
+ addr += bp->rx_buffer_size;
+ }
+ bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
+}
+
static int macb_rx(struct macb *bp, int budget)
{
int received = 0;
unsigned int tail;
int first_frag = -1;
+ int reset_rx_queue = 0;
for (tail = bp->rx_tail; budget > 0; tail++) {
struct macb_dma_desc *desc = macb_rx_desc(bp, tail);
@@ -972,10 +990,18 @@ static int macb_rx(struct macb *bp, int budget)
if (ctrl & MACB_BIT(RX_EOF)) {
int dropped;
- BUG_ON(first_frag == -1);
+
+ if (unlikely(first_frag == -1)) {
+ reset_rx_queue = 1;
+ continue;
+ }
dropped = macb_rx_frame(bp, first_frag, tail);
first_frag = -1;
+ if (unlikely(dropped < 0)) {
+ reset_rx_queue = 1;
+ continue;
+ }
if (!dropped) {
received++;
budget--;
@@ -983,6 +1009,26 @@ static int macb_rx(struct macb *bp, int budget)
}
}
+ if (unlikely(reset_rx_queue)) {
+ unsigned long flags;
+ u32 ctrl;
+
+ netdev_err(bp->dev, "RX queue corruption: reset it\n");
+
+ spin_lock_irqsave(&bp->lock, flags);
+
+ ctrl = macb_readl(bp, NCR);
+ macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+
+ macb_init_rx_ring(bp);
+ macb_writel(bp, RBQP, bp->rx_ring_dma);
+
+ macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+
+ spin_unlock_irqrestore(&bp->lock, flags);
+ return received;
+ }
+
if (first_frag != -1)
bp->rx_tail = first_frag;
else
@@ -1523,15 +1569,8 @@ static void gem_init_rings(struct macb *bp)
static void macb_init_rings(struct macb *bp)
{
int i;
- dma_addr_t addr;
- addr = bp->rx_buffers_dma;
- for (i = 0; i < RX_RING_SIZE; i++) {
- bp->rx_ring[i].addr = addr;
- bp->rx_ring[i].ctrl = 0;
- addr += bp->rx_buffer_size;
- }
- bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
+ macb_init_rx_ring(bp);
for (i = 0; i < TX_RING_SIZE; i++) {
bp->queues[0].tx_ring[i].addr = 0;
--
1.8.2.2
^ permalink raw reply related
* [PATCH 1/1] net: macb: replace macb_writel() call by queue_writel() to update queue ISR
From: Cyrille Pitchen @ 2016-03-24 14:40 UTC (permalink / raw)
To: nicolas.ferre, davem, linux-arm-kernel, netdev, soren.brinkmann
Cc: linux-kernel, Cyrille Pitchen
macb_interrupt() should not use macb_writel(bp, ISR, <value>) but only
queue_writel(queue, ISR, <value>).
There is one IRQ and one set of {ISR, IER, IDR, IMR} [1] registers per
queue on gem hardware, though only queue0 is actually used for now to
receive frames: other queues can already be used to transmit frames.
The queue_readl() and queue_writel() helper macros are designed to access
the relevant IRQ registers.
[1]
ISR: Interrupt Status Register
IER: Interrupt Enable Register
IDR: Interrupt Disable Register
IMR: Interrupt Mask Register
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Fixes: bfbb92c44670 ("net: macb: Handle the RXUBR interrupt on all devices")
---
drivers/net/ethernet/cadence/macb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 39447a337149..c9c6b2762a39 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1146,7 +1146,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
- macb_writel(bp, ISR, MACB_BIT(RXUBR));
+ queue_writel(queue, ISR, MACB_BIT(RXUBR));
}
if (status & MACB_BIT(ISR_ROVR)) {
--
1.8.2.2
^ permalink raw reply related
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Eric Dumazet @ 2016-03-24 14:45 UTC (permalink / raw)
To: Willy Tarreau
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <20160324142222.GB7237@1wt.eu>
On Thu, 2016-03-24 at 15:22 +0100, Willy Tarreau wrote:
> Hi Eric,
> But that means that any software making use of SO_REUSEPORT needs to
> also implement BPF on Linux to achieve the same as what it does on
> other OSes ? Also I found a case where a dying process would still
> cause trouble in the accept queue, maybe it's not redistributed, I
> don't remember, all I remember is that my traffic stopped after a
> segfault of only one of them :-/ I'll have to dig a bit regarding
> this.
Hi Willy
Problem is : If we add a SO_REUSEPORT_LISTEN_OFF, this wont work with
BPF.
BPF makes a decision without knowing individual listeners states.
Or we would need to extend BPF to access these kind of states.
Doable certainly, but we need to be convinced it is necessary.
And yes, if a listener is closed while children are still in accept
queue, we drop all the children, we do not care of redistributing them
to another listeners. Really too complex to be worth it.
For example, we could probably revert
70da268b569d32a9fddeea85dc18043de9d89f89
("net: SO_INCOMING_CPU setsockopt() support") as this can be handled by
BPF as well, and would remove extra tests in fast path (when
SO_REUSEPORT is not used at all)
^ permalink raw reply
* Re: [PATCH 1/1] net: macb: remove BUG_ON() and reset the queue to handle RX errors
From: Neil Armstrong @ 2016-03-24 14:53 UTC (permalink / raw)
To: Cyrille Pitchen, nicolas.ferre, davem, linux-arm-kernel, netdev,
soren.brinkmann
Cc: linux-kernel
In-Reply-To: <1458830232-6159-1-git-send-email-cyrille.pitchen@atmel.com>
On 03/24/2016 03:37 PM, Cyrille Pitchen wrote:
> This patch removes two BUG_ON() used to notify about RX queue corruptions
> on macb (not gem) hardware without actually handling the error.
>
> The new code skips corrupted frames but still processes faultless frames.
> Then it resets the RX queue before restarting the reception from a clean
> state.
>
> This patch is a rework of an older patch proposed by Neil Armstrong:
> http://patchwork.ozlabs.org/patch/371525/
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 59 ++++++++++++++++++++++++++++++-------
> 1 file changed, 49 insertions(+), 10 deletions(-)
Hi Cyrille,
Thanks for the rework, we solved this situation by moving the descriptors to
an internal RAM with lower latencies, we suspected our AHB-AXI bridge + DDR Controller
to delay the descriptor writes while the interrupt was handled in the meantime.
But the error case was still not handled.
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Thanks,
Neil
^ permalink raw reply
* [PATCH net] net: ipv4: Multipath needs to handle unreachable nexthops
From: David Ahern @ 2016-03-24 15:25 UTC (permalink / raw)
To: netdev
Multipath route lookups should consider knowledge about next hops and not
select a hop that is known to be failed.
Example:
[h2] [h3] 15.0.0.5
| |
3| 3|
[SP1] [SP2]--+
1 2 1 2
| | /-------------+ |
| \ / |
| X |
| / \ |
| / \---------------\ |
1 2 1 2
12.0.0.2 [TOR1] 3-----------------3 [TOR2] 12.0.0.3
4 4
\ /
\ /
\ /
-------| |-----/
1 2
[TOR3]
3|
|
[h1] 12.0.0.1
host h1 with IP 12.0.0.1 has 2 paths to host h3 at 15.0.0.5:
root@h1:~# ip ro ls
...
12.0.0.0/24 dev swp1 proto kernel scope link src 12.0.0.1
15.0.0.0/16
nexthop via 12.0.0.2 dev swp1 weight 1
nexthop via 12.0.0.3 dev swp1 weight 1
...
If the link between tor3 and tor1 is down and the link between tor1
and tor2 then tor1 is effectively cut-off from h1. Yet the route lookups
in h1 are alternating between the 2 routes: ping 15.0.0.5 gets one and
ssh 15.0.0.5 gets the other. Connections that attempt to use the
12.0.0.2 nexthop fail since that neighbor is not reachable:
root@h1:~# ip neigh show
...
12.0.0.3 dev swp1 lladdr 00:02:00:00:00:1b REACHABLE
12.0.0.2 dev swp1 FAILED
...
The failed path can be avoided by considering known neighbor information
when selecting next hops. If the neighbor lookups fails we have no
knowledge about the nexthop, so give it a shot. If there is an entry
then only select the nexthop if the state is sane. This is similar to
what fib_detect_death does for some single path cases.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
net/ipv4/fib_semantics.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index d97268e8ff10..28fc6700c2b1 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1563,13 +1563,43 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
void fib_select_multipath(struct fib_result *res, int hash)
{
struct fib_info *fi = res->fi;
+ struct neighbour *n;
+ int state;
for_nexthops(fi) {
if (hash > atomic_read(&nh->nh_upper_bound))
continue;
- res->nh_sel = nhsel;
- return;
+ state = NUD_NONE;
+ n = neigh_lookup(&arp_tbl, &nh->nh_gw, fi->fib_dev);
+ if (n) {
+ state = n->nud_state;
+ neigh_release(n);
+ }
+ if (!n || (state == NUD_REACHABLE) || (state & NUD_VALID)) {
+ res->nh_sel = nhsel;
+ return;
+ }
+ } endfor_nexthops(fi);
+
+ /* try the nexthops again, but covering the entries
+ * skipped by the hash
+ */
+ fi = res->fi;
+ for_nexthops(fi) {
+ if (hash <= atomic_read(&nh->nh_upper_bound))
+ continue;
+
+ state = NUD_NONE;
+ n = neigh_lookup(&arp_tbl, &nh->nh_gw, fi->fib_dev);
+ if (n) {
+ state = n->nud_state;
+ neigh_release(n);
+ }
+ if (!n || (state == NUD_REACHABLE) || (state & NUD_VALID)) {
+ res->nh_sel = nhsel;
+ return;
+ }
} endfor_nexthops(fi);
/* Race condition: route has just become dead. */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Willy Tarreau @ 2016-03-24 15:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <1458830744.10868.72.camel@edumazet-glaptop3.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]
Hi Eric,
(just lost my e-mail, trying not to forget some points)
On Thu, Mar 24, 2016 at 07:45:44AM -0700, Eric Dumazet wrote:
> On Thu, 2016-03-24 at 15:22 +0100, Willy Tarreau wrote:
> > Hi Eric,
>
> > But that means that any software making use of SO_REUSEPORT needs to
> > also implement BPF on Linux to achieve the same as what it does on
> > other OSes ? Also I found a case where a dying process would still
> > cause trouble in the accept queue, maybe it's not redistributed, I
> > don't remember, all I remember is that my traffic stopped after a
> > segfault of only one of them :-/ I'll have to dig a bit regarding
> > this.
>
> Hi Willy
>
> Problem is : If we add a SO_REUSEPORT_LISTEN_OFF, this wont work with
> BPF.
I wasn't for adding SO_REUSEPORT_LISTEN_OFF either. Instead the idea was
just to modify the score in compute_score() so that a socket which disables
SO_REUSEPORT scores less than one which still has it. The application
wishing to terminate just has to clear the SO_REUSEPORT flag and wait for
accept() reporting EAGAIN. The patch simply looked like this (copy-pasted
hence space-mangled) :
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -189,6 +189,8 @@ static inline int compute_score(struct sock *sk, struct net *net,
return -1;
score += 4;
}
+ if (sk->sk_reuseport)
+ score++;
if (sk->sk_incoming_cpu == raw_smp_processor_id())
score++;
}
> BPF makes a decision without knowing individual listeners states.
But is the decision taken without considering compute_score() ? The point
really was to be the least possibly intrusive and quite logical for the
application : "disable SO_REUSEPORT when you don't want to participate to
incoming load balancing anymore".
> Or we would need to extend BPF to access these kind of states.
> Doable certainly, but we need to be convinced it is necessary.
You know that I don't like complex designs to address simple issues if
possible :-)
> And yes, if a listener is closed while children are still in accept
> queue, we drop all the children, we do not care of redistributing them
> to another listeners. Really too complex to be worth it.
Forget this, I mixed two issues here. Yes I know that redistributing is
almost impossible, I've read that code a year ago or so and realized how
complex this would be, without providing even 100% success rate. I wasn't
speaking about redistribution of incoming queue but about an issue I've
met where when I have 4 processes bound to the same port, if one dies,
its share of incoming traffic is definitely lost. The only fix was to
restart the processes to create new listeners. But I don't remember the
conditions where I met this case, I don't even remember if it was on an
-rc kernel or a final one, so I'd prefer to discuss this only once I have
more elements.
Cheers,
Willy
[-- Attachment #2: 0001-net-make-lingering-sockets-score-less-in-compute_sco.patch --]
[-- Type: text/plain, Size: 2647 bytes --]
>From c060a5db92274402a0178d7c777a1e37c15eadb9 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Tue, 15 Dec 2015 16:40:00 +0100
Subject: net: make lingering sockets score less in compute_score()
When multiple processes use SO_REUSEPORT for a seamless restart
operation, there's a tiny window during which both the old and the new
process are bound to the same port, and there's no way for the old
process to gracefully stop receiving connections without dropping
the few that are left in the queue between the last poll() and the
shutdown() or close() operation.
Incoming connections are distributed between multiple listening sockets
in inet_lookup_listener() according to multiple criteria. The first
criterion is a score based on a number of attributes for each socket,
then a hash computation ensures that the connections are evenly
distributed between sockets of equal weight.
This patch provides a simple approach by which the old process can
simply decrease its score by disabling SO_REUSEPORT on its listening
sockets. Thus, the sockets from the new process always score higher
and are always preferred.
The old process can then safely drain incoming connections and stop
after meeting the -1 EAGAIN condition, as shown in the example below :
process A (old one) | process B (new one)
|
setsockopt(SO_REUSEPORT, 1) |
listen() >= 0 |
... |
accept() |
... | setsockopt(SO_REUSEPORT, 1)
... | listen()
From now on, both processes receive incoming connections
... | kill(process A, go_away)
setsockopt(SO_REUSEPORT, 0) | accept() >= 0
Here process A stops receiving new connections
accept() >= 0 | accept() >= 0
... |
accept() = -1 EAGAIN | accept() >= 0
close() |
exit() |
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
net/ipv4/inet_hashtables.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index ccc5980..1c950ba 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -189,6 +189,8 @@ static inline int compute_score(struct sock *sk, struct net *net,
return -1;
score += 4;
}
+ if (sk->sk_reuseport)
+ score++;
if (sk->sk_incoming_cpu == raw_smp_processor_id())
score++;
}
--
1.7.12.1
^ permalink raw reply related
* Re: [PATCH 1/1] net: macb: replace macb_writel() call by queue_writel() to update queue ISR
From: Nicolas Ferre @ 2016-03-24 15:39 UTC (permalink / raw)
To: Cyrille Pitchen, davem, linux-arm-kernel, netdev, soren.brinkmann
Cc: linux-kernel
In-Reply-To: <1458830404-6213-1-git-send-email-cyrille.pitchen@atmel.com>
Le 24/03/2016 15:40, Cyrille Pitchen a écrit :
> macb_interrupt() should not use macb_writel(bp, ISR, <value>) but only
> queue_writel(queue, ISR, <value>).
>
> There is one IRQ and one set of {ISR, IER, IDR, IMR} [1] registers per
> queue on gem hardware, though only queue0 is actually used for now to
> receive frames: other queues can already be used to transmit frames.
>
> The queue_readl() and queue_writel() helper macros are designed to access
> the relevant IRQ registers.
>
> [1]
> ISR: Interrupt Status Register
> IER: Interrupt Enable Register
> IDR: Interrupt Disable Register
> IMR: Interrupt Mask Register
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Fixes: bfbb92c44670 ("net: macb: Handle the RXUBR interrupt on all devices")
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Thanks!
> ---
> drivers/net/ethernet/cadence/macb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 39447a337149..c9c6b2762a39 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -1146,7 +1146,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
> macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
>
> if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> - macb_writel(bp, ISR, MACB_BIT(RXUBR));
> + queue_writel(queue, ISR, MACB_BIT(RXUBR));
> }
>
> if (status & MACB_BIT(ISR_ROVR)) {
>
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH 1/1] net: macb: remove BUG_ON() and reset the queue to handle RX errors
From: Nicolas Ferre @ 2016-03-24 15:42 UTC (permalink / raw)
To: Cyrille Pitchen, davem, linux-arm-kernel, netdev, soren.brinkmann,
narmstrong
Cc: linux-kernel
In-Reply-To: <1458830232-6159-1-git-send-email-cyrille.pitchen@atmel.com>
Le 24/03/2016 15:37, Cyrille Pitchen a écrit :
> This patch removes two BUG_ON() used to notify about RX queue corruptions
> on macb (not gem) hardware without actually handling the error.
>
> The new code skips corrupted frames but still processes faultless frames.
> Then it resets the RX queue before restarting the reception from a clean
> state.
>
> This patch is a rework of an older patch proposed by Neil Armstrong:
> http://patchwork.ozlabs.org/patch/371525/
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Thanks for this rework of Neil's patch that was standing for a long time
in my backlog ;-).
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Bye,
> ---
> drivers/net/ethernet/cadence/macb.c | 59 ++++++++++++++++++++++++++++++-------
> 1 file changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 6619178ed77b..39447a337149 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -917,7 +917,10 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> unsigned int frag_len = bp->rx_buffer_size;
>
> if (offset + frag_len > len) {
> - BUG_ON(frag != last_frag);
> + if (unlikely(frag != last_frag)) {
> + dev_kfree_skb_any(skb);
> + return -1;
> + }
> frag_len = len - offset;
> }
> skb_copy_to_linear_data_offset(skb, offset,
> @@ -945,11 +948,26 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> return 0;
> }
>
> +static inline void macb_init_rx_ring(struct macb *bp)
> +{
> + int i;
> + dma_addr_t addr;
> +
> + addr = bp->rx_buffers_dma;
> + for (i = 0; i < RX_RING_SIZE; i++) {
> + bp->rx_ring[i].addr = addr;
> + bp->rx_ring[i].ctrl = 0;
> + addr += bp->rx_buffer_size;
> + }
> + bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
> +}
> +
> static int macb_rx(struct macb *bp, int budget)
> {
> int received = 0;
> unsigned int tail;
> int first_frag = -1;
> + int reset_rx_queue = 0;
>
> for (tail = bp->rx_tail; budget > 0; tail++) {
> struct macb_dma_desc *desc = macb_rx_desc(bp, tail);
> @@ -972,10 +990,18 @@ static int macb_rx(struct macb *bp, int budget)
>
> if (ctrl & MACB_BIT(RX_EOF)) {
> int dropped;
> - BUG_ON(first_frag == -1);
> +
> + if (unlikely(first_frag == -1)) {
> + reset_rx_queue = 1;
> + continue;
> + }
>
> dropped = macb_rx_frame(bp, first_frag, tail);
> first_frag = -1;
> + if (unlikely(dropped < 0)) {
> + reset_rx_queue = 1;
> + continue;
> + }
> if (!dropped) {
> received++;
> budget--;
> @@ -983,6 +1009,26 @@ static int macb_rx(struct macb *bp, int budget)
> }
> }
>
> + if (unlikely(reset_rx_queue)) {
> + unsigned long flags;
> + u32 ctrl;
> +
> + netdev_err(bp->dev, "RX queue corruption: reset it\n");
> +
> + spin_lock_irqsave(&bp->lock, flags);
> +
> + ctrl = macb_readl(bp, NCR);
> + macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> +
> + macb_init_rx_ring(bp);
> + macb_writel(bp, RBQP, bp->rx_ring_dma);
> +
> + macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> +
> + spin_unlock_irqrestore(&bp->lock, flags);
> + return received;
> + }
> +
> if (first_frag != -1)
> bp->rx_tail = first_frag;
> else
> @@ -1523,15 +1569,8 @@ static void gem_init_rings(struct macb *bp)
> static void macb_init_rings(struct macb *bp)
> {
> int i;
> - dma_addr_t addr;
>
> - addr = bp->rx_buffers_dma;
> - for (i = 0; i < RX_RING_SIZE; i++) {
> - bp->rx_ring[i].addr = addr;
> - bp->rx_ring[i].ctrl = 0;
> - addr += bp->rx_buffer_size;
> - }
> - bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
> + macb_init_rx_ring(bp);
>
> for (i = 0; i < TX_RING_SIZE; i++) {
> bp->queues[0].tx_ring[i].addr = 0;
>
--
Nicolas Ferre
^ permalink raw reply
* [PATCH iproute2 master 0/2] setting flow label support
From: Daniel Borkmann @ 2016-03-24 15:49 UTC (permalink / raw)
To: stephen; +Cc: netdev, Daniel Borkmann
On top of Jesse's csum patches for vxlan, geneve:
- http://patchwork.ozlabs.org/patch/599746/
- http://patchwork.ozlabs.org/patch/599747/
Thanks!
Daniel Borkmann (2):
vxlan: add support to set flow label
geneve: add support to set flow label
ip/ip_common.h | 4 ++++
ip/iplink_geneve.c | 29 ++++++++++++++++++++++++-----
ip/iplink_vxlan.c | 29 ++++++++++++++++++++++++-----
man/man8/ip-link.8.in | 12 ++++++++++++
4 files changed, 64 insertions(+), 10 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH iproute2 master 1/2] vxlan: add support to set flow label
From: Daniel Borkmann @ 2016-03-24 15:49 UTC (permalink / raw)
To: stephen; +Cc: netdev, Daniel Borkmann
In-Reply-To: <cover.1458833866.git.daniel@iogearbox.net>
Follow-up for kernel commit e7f70af111f0 ("vxlan: support setting
IPv6 flow label") to allow setting the label for the device config.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
ip/ip_common.h | 4 ++++
ip/iplink_vxlan.c | 29 ++++++++++++++++++++++++-----
man/man8/ip-link.8.in | 6 ++++++
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/ip/ip_common.h b/ip/ip_common.h
index 815487a..b7361a8 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -92,3 +92,7 @@ void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len);
#ifndef INFINITY_LIFE_TIME
#define INFINITY_LIFE_TIME 0xFFFFFFFFU
#endif
+
+#ifndef LABEL_MAX_MASK
+#define LABEL_MAX_MASK 0xFFFFFU
+#endif
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 77aabba..bfebe09 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -24,7 +24,7 @@
static void print_explain(FILE *f)
{
fprintf(f, "Usage: ... vxlan id VNI [ { group | remote } IP_ADDRESS ] [ local ADDR ]\n");
- fprintf(f, " [ ttl TTL ] [ tos TOS ] [ dev PHYS_DEV ]\n");
+ fprintf(f, " [ ttl TTL ] [ tos TOS ] [ flowlabel LABEL ] [ dev PHYS_DEV ]\n");
fprintf(f, " [ dstport PORT ] [ srcport MIN MAX ]\n");
fprintf(f, " [ [no]learning ] [ [no]proxy ] [ [no]rsc ]\n");
fprintf(f, " [ [no]l2miss ] [ [no]l3miss ]\n");
@@ -33,10 +33,11 @@ static void print_explain(FILE *f)
fprintf(f, " [ [no]remcsumtx ] [ [no]remcsumrx ]\n");
fprintf(f, " [ [no]external ] [ gbp ]\n");
fprintf(f, "\n");
- fprintf(f, "Where: VNI := 0-16777215\n");
- fprintf(f, " ADDR := { IP_ADDRESS | any }\n");
- fprintf(f, " TOS := { NUMBER | inherit }\n");
- fprintf(f, " TTL := { 1..255 | inherit }\n");
+ fprintf(f, "Where: VNI := 0-16777215\n");
+ fprintf(f, " ADDR := { IP_ADDRESS | any }\n");
+ fprintf(f, " TOS := { NUMBER | inherit }\n");
+ fprintf(f, " TTL := { 1..255 | inherit }\n");
+ fprintf(f, " LABEL := 0-1048575\n");
}
static void explain(void)
@@ -58,6 +59,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
unsigned int link = 0;
__u8 tos = 0;
__u8 ttl = 0;
+ __u32 label = 0;
__u8 learning = 1;
__u8 proxy = 0;
__u8 rsc = 0;
@@ -146,6 +148,15 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
tos = uval;
} else
tos = 1;
+ } else if (!matches(*argv, "label") ||
+ !matches(*argv, "flowlabel")) {
+ __u32 uval;
+
+ NEXT_ARG();
+ if (get_u32(&uval, *argv, 0) ||
+ (uval & ~LABEL_MAX_MASK))
+ invarg("invalid flowlabel", *argv);
+ label = htonl(uval);
} else if (!matches(*argv, "ageing")) {
NEXT_ARG();
if (strcmp(*argv, "none") == 0)
@@ -280,6 +291,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
if (link)
addattr32(n, 1024, IFLA_VXLAN_LINK, link);
+ addattr32(n, 1024, IFLA_VXLAN_LABEL, label);
addattr8(n, 1024, IFLA_VXLAN_TTL, ttl);
addattr8(n, 1024, IFLA_VXLAN_TOS, tos);
addattr8(n, 1024, IFLA_VXLAN_LEARNING, learning);
@@ -426,6 +438,13 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
fprintf(f, "ttl %d ", ttl);
}
+ if (tb[IFLA_VXLAN_LABEL]) {
+ __u32 label = rta_getattr_u32(tb[IFLA_VXLAN_LABEL]);
+
+ if (label)
+ fprintf(f, "flowlabel %#x ", ntohl(label));
+ }
+
if (tb[IFLA_VXLAN_AGEING]) {
__u32 age = rta_getattr_u32(tb[IFLA_VXLAN_AGEING]);
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 2cd93b0..f115c19 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -396,6 +396,8 @@ the following additional arguments are supported:
] [
.BI tos " TOS "
] [
+.BI flowlabel " FLOWLABEL "
+] [
.BI dstport " PORT "
] [
.BI srcport " MIN MAX "
@@ -460,6 +462,10 @@ parameter.
- specifies the TOS value to use in outgoing packets.
.sp
+.BI flowlabel " FLOWLABEL"
+- specifies the flow label to use in outgoing packets.
+
+.sp
.BI dstport " PORT"
- specifies the UDP destination port to communicate to the remote VXLAN tunnel endpoint.
--
1.9.3
^ permalink raw reply related
* [PATCH iproute2 master 2/2] geneve: add support to set flow label
From: Daniel Borkmann @ 2016-03-24 15:49 UTC (permalink / raw)
To: stephen; +Cc: netdev, Daniel Borkmann
In-Reply-To: <cover.1458833866.git.daniel@iogearbox.net>
Follow-up for kernel commit 8eb3b99554b8 ("geneve: support setting
IPv6 flow label") to allow setting the label for the device config.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
ip/iplink_geneve.c | 29 ++++++++++++++++++++++++-----
man/man8/ip-link.8.in | 6 ++++++
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/ip/iplink_geneve.c b/ip/iplink_geneve.c
index 16e70fd..685a0eb 100644
--- a/ip/iplink_geneve.c
+++ b/ip/iplink_geneve.c
@@ -18,14 +18,15 @@
static void print_explain(FILE *f)
{
fprintf(f, "Usage: ... geneve id VNI remote ADDR\n");
- fprintf(f, " [ ttl TTL ] [ tos TOS ]\n");
+ fprintf(f, " [ ttl TTL ] [ tos TOS ] [ flowlabel LABEL ]\n");
fprintf(f, " [ dstport PORT ] [ [no]external ]\n");
fprintf(f, " [ [no]udpcsum ] [ [no]udp6zerocsumtx ] [ [no]udp6zerocsumrx ]\n");
fprintf(f, "\n");
- fprintf(f, "Where: VNI := 0-16777215\n");
- fprintf(f, " ADDR := IP_ADDRESS\n");
- fprintf(f, " TOS := { NUMBER | inherit }\n");
- fprintf(f, " TTL := { 1..255 | inherit }\n");
+ fprintf(f, "Where: VNI := 0-16777215\n");
+ fprintf(f, " ADDR := IP_ADDRESS\n");
+ fprintf(f, " TOS := { NUMBER | inherit }\n");
+ fprintf(f, " TTL := { 1..255 | inherit }\n");
+ fprintf(f, " LABEL := 0-1048575\n");
}
static void explain(void)
@@ -40,6 +41,7 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
int vni_set = 0;
__u32 daddr = 0;
struct in6_addr daddr6 = IN6ADDR_ANY_INIT;
+ __u32 label = 0;
__u8 ttl = 0;
__u8 tos = 0;
__u16 dstport = 0;
@@ -90,6 +92,15 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
tos = uval;
} else
tos = 1;
+ } else if (!matches(*argv, "label") ||
+ !matches(*argv, "flowlabel")) {
+ __u32 uval;
+
+ NEXT_ARG();
+ if (get_u32(&uval, *argv, 0) ||
+ (uval & ~LABEL_MAX_MASK))
+ invarg("invalid flowlabel", *argv);
+ label = htonl(uval);
} else if (!matches(*argv, "dstport")) {
NEXT_ARG();
if (get_u16(&dstport, *argv, 0))
@@ -150,6 +161,7 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
addattr_l(n, 1024, IFLA_GENEVE_REMOTE, &daddr, 4);
if (memcmp(&daddr6, &in6addr_any, sizeof(daddr6)) != 0)
addattr_l(n, 1024, IFLA_GENEVE_REMOTE6, &daddr6, sizeof(struct in6_addr));
+ addattr32(n, 1024, IFLA_GENEVE_LABEL, label);
addattr8(n, 1024, IFLA_GENEVE_TTL, ttl);
addattr8(n, 1024, IFLA_GENEVE_TOS, tos);
if (dstport)
@@ -214,6 +226,13 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
fprintf(f, "tos %#x ", tos);
}
+ if (tb[IFLA_GENEVE_LABEL]) {
+ __u32 label = rta_getattr_u32(tb[IFLA_GENEVE_LABEL]);
+
+ if (label)
+ fprintf(f, "flowlabel %#x ", ntohl(label));
+ }
+
if (tb[IFLA_GENEVE_PORT])
fprintf(f, "dstport %u ",
ntohs(rta_getattr_u16(tb[IFLA_GENEVE_PORT])));
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index f115c19..8055114 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -752,6 +752,8 @@ the following additional arguments are supported:
.BI ttl " TTL "
] [
.BI tos " TOS "
+] [
+.BI flowlabel " FLOWLABEL "
]
.in +8
@@ -771,6 +773,10 @@ the following additional arguments are supported:
.BI tos " TOS"
- specifies the TOS value to use in outgoing packets.
+.sp
+.BI flowlabel " FLOWLABEL"
+- specifies the flow label to use in outgoing packets.
+
.in -8
.TP
--
1.9.3
^ permalink raw reply related
* [PATCH net] switchdev: fix typo in comments/doc
From: Nicolas Dichtel @ 2016-03-24 15:50 UTC (permalink / raw)
To: davem; +Cc: netdev, Nicolas Dichtel
Two minor typo.
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
Documentation/networking/switchdev.txt | 2 +-
net/switchdev/switchdev.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
index fad63136ee3e..2f659129694b 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -386,7 +386,7 @@ used. First phase is to "prepare" anything needed, including various checks,
memory allocation, etc. The goal is to handle the stuff that is not unlikely
to fail here. The second phase is to "commit" the actual changes.
-Switchdev provides an inftrastructure for sharing items (for example memory
+Switchdev provides an infrastructure for sharing items (for example memory
allocations) between the two phases.
The object created by a driver in "prepare" phase and it is queued up by:
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 8b5833c1ff2e..2b9b98f1c2ff 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -1079,7 +1079,7 @@ nla_put_failure:
* @filter_dev: filter device
* @idx:
*
- * Delete FDB entry from switch device.
+ * Dump FDB entries from switch device.
*/
int switchdev_port_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
struct net_device *dev,
--
2.4.2
^ permalink raw reply related
* [PATCH] netpoll: Fix extra refcount release in netpoll_cleanup()
From: Bjorn Helgaas @ 2016-03-24 16:13 UTC (permalink / raw)
To: David S. Miller
Cc: Nikolay Aleksandrov, netdev, Neil Horman, Alexander Duyck,
linux-kernel
netpoll_setup() does a dev_hold() on np->dev, the netpoll device. If it
fails, it correctly does a dev_put() but leaves np->dev set. If we call
netpoll_cleanup() after the failure, np->dev is still set so we do another
dev_put(), which decrements the refcount an extra time.
It's questionable to call netpoll_cleanup() after netpoll_setup() fails,
but it can be difficult to find the problem, and we can easily avoid it in
this case. The extra decrements can lead to hangs like this:
unregister_netdevice: waiting for bond0 to become free. Usage count = -3
In __netpoll_setup(), don't set np->dev until we know we're going to
succeed.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
net/core/netpoll.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 94acfc8..32e373e 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -603,7 +603,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
const struct net_device_ops *ops;
int err;
- np->dev = ndev;
strlcpy(np->dev_name, ndev->name, IFNAMSIZ);
INIT_WORK(&np->cleanup_work, netpoll_async_cleanup);
@@ -628,7 +627,7 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
atomic_set(&npinfo->refcnt, 1);
- ops = np->dev->netdev_ops;
+ ops = ndev->netdev_ops;
if (ops->ndo_netpoll_setup) {
err = ops->ndo_netpoll_setup(ndev, npinfo);
if (err)
@@ -639,6 +638,7 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
atomic_inc(&npinfo->refcnt);
}
+ np->dev = ndev;
npinfo->netpoll = np;
/* last thing to do is link it to the net device structure */
^ permalink raw reply related
* Attn My Dear
From: Mrs. Ann @ 2016-03-24 16:29 UTC (permalink / raw)
To: Recipients
Attn My Dear
We have finally arranged to deliver your package worth $5.8m USD
Through (DHL) Company. We were able to accomplish this through the help
Of IMF director Anderson Morgan and every necessary arrangement has
Been made successfully.
Contact Dr.mark milliams
Telephone: (+229-99653283)
Email: (dhlbenin35@yahoo.in)
Contact the (DHL) with your delivery information such as:
(1) Full names ---
(2) Phone line ---
(3) Country -----
(4) Age/sex ------
(5) Occupation:---
(6)Home address:--
And also be informed that delivery agent will leave to this country as
soon as you proceed with (DHL) Company and please ask them how much is they Security Keeping fee to enable the agent live this country by tomorrow morning please I am waiting to hear from you thank you and God bless
Sincerely
Mrs. Ann Godwin
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Eric Dumazet @ 2016-03-24 16:33 UTC (permalink / raw)
To: Willy Tarreau
Cc: Tolga Ceylan, Tom Herbert, cgallek, Josh Snyder, Aaron Conole,
David S. Miller, Linux Kernel Network Developers
In-Reply-To: <20160324153053.GA7569@1wt.eu>
On Thu, 2016-03-24 at 16:30 +0100, Willy Tarreau wrote:
> Hi Eric,
>
> (just lost my e-mail, trying not to forget some points)
>
> On Thu, Mar 24, 2016 at 07:45:44AM -0700, Eric Dumazet wrote:
> > On Thu, 2016-03-24 at 15:22 +0100, Willy Tarreau wrote:
> > > Hi Eric,
> >
> > > But that means that any software making use of SO_REUSEPORT needs to
> > > also implement BPF on Linux to achieve the same as what it does on
> > > other OSes ? Also I found a case where a dying process would still
> > > cause trouble in the accept queue, maybe it's not redistributed, I
> > > don't remember, all I remember is that my traffic stopped after a
> > > segfault of only one of them :-/ I'll have to dig a bit regarding
> > > this.
> >
> > Hi Willy
> >
> > Problem is : If we add a SO_REUSEPORT_LISTEN_OFF, this wont work with
> > BPF.
>
> I wasn't for adding SO_REUSEPORT_LISTEN_OFF either. Instead the idea was
> just to modify the score in compute_score() so that a socket which disables
> SO_REUSEPORT scores less than one which still has it. The application
> wishing to terminate just has to clear the SO_REUSEPORT flag and wait for
> accept() reporting EAGAIN. The patch simply looked like this (copy-pasted
> hence space-mangled) :
>
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -189,6 +189,8 @@ static inline int compute_score(struct sock *sk, struct net *net,
> return -1;
> score += 4;
> }
> + if (sk->sk_reuseport)
> + score++;
This wont work with BPF
> if (sk->sk_incoming_cpu == raw_smp_processor_id())
> score++;
This one does not work either with BPF
> }
>
> > BPF makes a decision without knowing individual listeners states.
>
> But is the decision taken without considering compute_score() ? The point
> really was to be the least possibly intrusive and quite logical for the
> application : "disable SO_REUSEPORT when you don't want to participate to
> incoming load balancing anymore".
Whole point of BPF was to avoid iterate through all sockets [1],
and let user space use whatever selection logic it needs.
[1] This was okay with up to 16 sockets. But with 128 it does not scale.
If you really look at how BPF works, implementing another 'per listener' flag
would break the BPF selection.
You can certainly implement the SO_REUSEPORT_LISTEN_OFF by loading an
updated BPF, why should we add another way in the kernel to do the same,
in a way that would not work in some cases ?
^ permalink raw reply
* Attn My Dear
From: Mrs. Ann @ 2016-03-24 16:19 UTC (permalink / raw)
To: Recipients
Attn My Dear
We have finally arranged to deliver your package worth $5.8m USD
Through (DHL) Company. We were able to accomplish this through the help
Of IMF director Anderson Morgan and every necessary arrangement has
Been made successfully.
Contact Dr.mark milliams
Telephone: (+229-99653283)
Email: (dhlbenin35@yahoo.in)
Contact the (DHL) with your delivery information such as:
(1) Full names ---
(2) Phone line ---
(3) Country -----
(4) Age/sex ------
(5) Occupation:---
(6)Home address:--
And also be informed that delivery agent will leave to this country as
soon as you proceed with (DHL) Company and please ask them how much is they Security Keeping fee to enable the agent live this country by tomorrow morning please I am waiting to hear from you thank you and God bless
Sincerely
Mrs. Ann Godwin
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ 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