* Re: [PATCH] pkt_sched: fq: clear time_next_packet for reused flows
From: Eric Dumazet @ 2013-10-27 23:35 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: David Miller, netdev
In-Reply-To: <CAGK4HS945jCtfOoa_ENhTwZQvnSRJpHdWVDyhF+wjE56bjT8Pw@mail.gmail.com>
On Sun, 2013-10-27 at 15:06 -0700, Vijay Subramanian wrote:
> Eric,
> By the way, it looks like fq_flow_set_throttled() uses
> f->time_next_packet in an incorrect way.
> It is used as key to insert the flow into the delayed rb-tree but does
> not seem to check for duplicates.
> This could lead to rb tree corruption. I assume it is possible for
> different flows to have same f->time_next_packet.
> Do we at least we need a BUG_ON(f->time_next_packet ==
> aux->time_next_packet) here?
>
What exact RB corruptions do you see ?
RB trees allow items with the same key.
HTB and netem use this property and I never saw any corruption,
but you could be right so please elaborate, thanks !
^ permalink raw reply
* Re: extending ndo_add_rx_vxlan_port
From: John Fastabend @ 2013-10-27 23:41 UTC (permalink / raw)
To: Or Gerlitz
Cc: Joseph Gasparakis, Or Gerlitz, John Fastabend, Yan Burman, netdev,
Stephen Hemminger
In-Reply-To: <CAJZOPZ+S6ngqMjV-2ZMcpzkuWzzLWwCQMtd_1_SjDkqWh_tbYg@mail.gmail.com>
On 10/27/2013 01:34 PM, Or Gerlitz wrote:
> On Sun, Oct 27, 2013 at 7:25 PM, Joseph Gasparakis
> <joseph.gasparakis@intel.com> wrote:
>>
>>
>> On Sun, 27 Oct 2013, Or Gerlitz wrote:
>>
>>> Hi,
>>>
>>> So with commit 53cf527513eed6e7170e9dceacd198f9267171b0 "vxlan: Notify drivers
>>> for listening UDP port changes" drivers that have HW offloads for vxlan can be
>>> notified on which UDP port to listen. Taking this further, some HW may need to
>>> know the multicast address and/or the VNID used by the vxlan instance/s set
>>> above them. In that respect, do we prefer to extend ndo_add_rx_vxlan_port() or
>>> introduce new ndo?
>>>
>>> Or.
>>>
>>
>> The way this patch works is to notify the drivers when a VXLAN UDP port
>> comes up or down. This way drivers do not need to do any sort of accounting. As
>
> Could you elaborate why do we want to notify all the netdev instances
> in the system (on a certain name-space)
> that vxlan instance/s are set to listen on certain UDP port and not
> only the device over which the
> vxlan device is being set? say the HW can listen limited amount of UDP
> ports and few vxlan instances are created
> one of top of each "real" netdev in the system and each on different
> port. Each netdevice will get all callbacks on port addition
> and at some point will start to fail adding them into the HW when the
> HW limit is met.
>
> Or.
> --
The issue is how to determine "the device over which the vlan device is
being set". Because the vxlan device is a layer three device we do not
have a 1:1 binding between vxlan port and l2 net_device. In the
mentioned patches the work around for this was to add the UDP port to
every l2 net_device. I agree it is a bit of a big hammer.
It might be better to only add the UDP port to the default route if it
is specified then add the ndo call to the snooping logic (vxlan_snoop)
and the explicit added netlink fdb entries. This would be a bit more
precise than what we have today.
On the other hand the HW table will eventually be full and even with
this optimization may fail. At this point the user has no way to set
which ports are added and which are failed. Joseph, wanted this to
be automatic so users did not have to configure it so we end up with
this case. The other alternative is to provide a hook into the driver
via rtnetlink I'm not sure its worth the effort though. With this
and a feature flag you could let users manage the device manually.
But is this a problem in practice? My feeling is typically the
number of UDP ports in use is smaller than the number the HW supports.
Thanks,
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [gpio:for-next 67/67] pch_gbe_main.c:undefined reference to `devm_gpio_request_one'
From: Darren Hart @ 2013-10-28 0:04 UTC (permalink / raw)
To: David Cohen
Cc: Linus Walleij, David S. Miller, netdev@vger.kernel.org,
Fengguang Wu, Alexandre Courbot, linux-gpio@vger.kernel.org
In-Reply-To: <526CA546.7040406@linux.intel.com>
On Sat, 2013-10-26 at 22:31 -0700, David Cohen wrote:
> On 10/26/2013 02:15 PM, Darren Hart wrote:
> > On Sat, 2013-10-26 at 21:33 +0100, Darren Hart wrote:
> >> On Fri, 2013-10-25 at 14:25 -0700, David Cohen wrote:
> >>> On 10/25/2013 02:21 PM, David Cohen wrote:
> >>>> Hi Linus,
> >>>>
> >>>> On 10/25/2013 03:49 AM, Linus Walleij wrote:
> >>>>> On Fri, Oct 25, 2013 at 12:41 PM, Linus Walleij
> >>>>> <linus.walleij@linaro.org> wrote:
> >>>>>
> >>>>>>> I wouldn't object to adding a dependency to GPIO_PCH and GPIOLIB
> >>>>>>> unconditionally for PCH_GBE as GPIO_PCH is the same chip... but I don't
> >>>>>>> know if David Miller would be amenable to that.
> >>>>>>
> >>>>>> Well we should probably just stick a dependency to GPIOLIB in there.
> >>>>>>
> >>>>>> - It #includes <linux/gpio.h>
> >>>>>> - It uses gpiolib functions to do something vital
> >>>>>>
> >>>>>> It was just happy that dummy versions were slotted in until now.
> >>>>>
> >>>>> ...or maybe I'm just confused now?
> >>>>>
> >>>>> Should we just add a static inline stub of devm_gpio_request_one()?
> >>>>
> >>>> I am not familiar with the HW. But checking the code, platform
> >>>> initialization should fail with a dummy devm_gpio_request_one()
> >>>> implementation. IMO it makes more sense to depend on GPIOLIB.
> >>>
> >>> Actually, forget about it. Despite driver_data->platform_init() may
> >>> return error, probe() never checks for it. I think the driver must be
> >>> fixed, but in meanwhile a static inline stub seems reasonable.
> >>>
> >>
> >> Ah, that's a problem, and one I created :/ I'm traveling a bit through
> >> Europe atm for the conferences. I will try and have a look on the planes
> >> and add a check.
> >
> > Ah, now I remember. The situation is this. If there is a cable plugged
> > into the jack, the PHY will not go to sleep. If there isn't, there is a
> > good chance it will go to sleep before the driver initializes. If it is
> > asleep, the scan for the PHY will fail. If it isn't, the scan will work
> > correctly and the device will be properly setup. The code currently
> > displays a dev error:
> >
> > ret = devm_gpio_request_one(&pdev->dev, gpio, flags,
> > "minnow_phy_reset");
> > if (ret) {
> > dev_err(&pdev->dev,
> > "ERR: Can't request PHY reset GPIO line '%d'\n", gpio);
> >
> > But deliberately does not about the probe because there is a chance
> > things will work just fine. If they do not, the PHY detection code will
> > print display errors about a failure to communicate over RGMII, and the
> > device probe will fail from there.
> >
> > This still seems like the correct approach to me. Does anyone disagree?
>
> Considering this scenario it seems to be correct. But if
> devm_gpio_request_one() may fail for "unfriendly" reasons too, then
> it's incomplete.
>
> >
> > (we still need to sort out the GPIOLIB and GPIO_SCH dependency though of
> > course)
>
> Maybe if GPIOLIB has the static inline stubs returning -ENODEV we could
> use a patch similar to the one attached here.
>
I think you may have inverted your logic on the return?
+ dev_warn(&pdev->dev,
+ "ERR: Can't request PHY reset GPIO line '%d'\n", gpio);
+ return ret == -ENODEV ? ret : 0;
Did you mean:
+ /*
* Things may still work if the GPIO driver wasn't
* compiled in
*/
+ return ret == -ENODEV ? 0 : ret;
The concern here of course would be someone added another GPIO
controller over i2c over the expansion connector or something similar
and did not enable the GPIO_SCH driver, then it could conceivably grab
the wrong GPIO pin.... or would those never map to GPIO 13?
--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
^ permalink raw reply
* Re: [PATCH] netfilter: ipvs: ip_vs_sync: Remove unused variable
From: Simon Horman @ 2013-10-28 1:04 UTC (permalink / raw)
To: Fabio Estevam; +Cc: ja, netdev, Fabio Estevam
In-Reply-To: <1381180142-3388-1-git-send-email-festevam@gmail.com>
On Mon, Oct 07, 2013 at 06:09:02PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Variable 'ret' is not used anywhere and it causes the following build warning:
>
> net/netfilter/ipvs/ip_vs_sync.c: In function 'sync_thread_master':
> net/netfilter/ipvs/ip_vs_sync.c:1640:8: warning: unused variable 'ret' [-Wunused-variable]
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> net/netfilter/ipvs/ip_vs_sync.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index f63c238..d258125 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -1637,7 +1637,7 @@ static int sync_thread_master(void *data)
> continue;
> }
> while (ip_vs_send_sync_msg(tinfo->sock, sb->mesg) < 0) {
> - int ret = __wait_event_interruptible(*sk_sleep(sk),
> + __wait_event_interruptible(*sk_sleep(sk),
> sock_writeable(sk) ||
> kthread_should_stop());
> if (unlikely(kthread_should_stop()))
Hi Fabio,
this patch does not seem to apply against ipvs-next.
Could you please rework it and repost if you believe it is still needed.
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] bonding: patchset for rcu use in bonding
From: Ding Tianhong @ 2013-10-28 1:15 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <20131027225317.GB11209@redhat.com>
On 2013/10/28 6:53, Veaceslav Falico wrote:
> On Thu, Oct 24, 2013 at 11:08:35AM +0800, Ding Tianhong wrote:
>> Hi:
>>
>> The slave list will add and del by bond_master_upper_dev_link() and bond_upper_dev_unlink(),
>> which will call call_netdevice_notifiers(), even it is safe to call it in write bond lock now,
>> but we can't sure that whether it is safe later, because other drivers may deal NETDEV_CHANGEUPPER
>> in sleep way, so I didn't admit move the bond_upper_dev_unlink() in write bond lock.
>>
>> now the bond_for_each_slave only protect by rtnl_lock(), maybe use bond_for_each_slave_rcu is a good
>> way to protect slave list for bond, but as a system slow path, it is no need to transform bond_for_each_slave()
>> to bond_for_each_slave_rcu() in slow path, so in the patchset, I will remove the unused read bond lock
>> for monitor function, maybe it is a better way, I will wait to accept any relay for it.
>>
>> Thanks for the Veaceslav Falico opinion.
>>
>> v2: add and modify commit for patchset and patch, it will be the first step for the whole patchset.
>>
>> Ding Tianhong (5):
>> bonding: remove bond read lock for bond_mii_monitor()
>> bonding: remove bond read lock for bond_alb_monitor()
>> bonding: remove bond read lock for bond_loadbalance_arp_mon()
>> bonding: remove bond read lock for bond_activebackup_arp_mon()
>
> This patch introduces a regression by boot-test with active backup mode:
>
> bond_activebackup_arp_mon() is already not holding the bond->lock, however
> it might call bond_change_active_slave(), which does (in case of new_active):
>
> 912 write_unlock_bh(&bond->curr_slave_lock);
> 913 read_unlock(&bond->lock);
> 914 915 call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
> 916 if (should_notify_peers)
> 917 call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
> 918 bond->dev);
> 919 920 read_lock(&bond->lock);
> 921 write_lock_bh(&bond->curr_slave_lock);
>
> so it drops the bond->lock (which wasn't taken previously), and then takes
> it (without anyone dropping it afterwards).
>
> I don't know how to fix it - cause a lot of other callers already take it,
> and we can't just drop them (we'd race), and we can't remove it here (cause
> we can't call notifiers while atomic).
>
> Which begs the question - was this patchset tested at all?
>
> [ 21.796823] =====================================
> [ 21.796823] [ BUG: bad unlock balance detected! ]
> [ 21.796823] 3.12.0-rc6+ #305 Tainted: G I [ 21.796823] -------------------------------------
> [ 21.796823] kworker/u8:5/59 is trying to release lock (&bond->lock) at:
> [ 21.796823] [<ffffffffa00b6c38>] bond_change_active_slave+0x2c8/0x390 [bonding]
> [ 21.796823] but there are no more locks to release!
> [ 21.796823] [ 21.796823] other info that might help us debug this:
> [ 21.796823] 3 locks held by kworker/u8:5/59:
> [ 21.796823] #0: (%s#4){.+.+..}, at: [<ffffffff810cfeb9>] process_one_work+0x189/0x580
> [ 21.796823] #1: ((&(&bond->arp_work)->work)){+.+...}, at: [<ffffffff810cfeb9>] process_one_work+0x189/0x580
> [ 21.796823] #2: (rtnl_mutex){+.+.+.}, at: [<ffffffff8169ea05>] rtnl_trylock+0x15/0x20
> [ 21.796823] [ 21.796823] stack backtrace:
> [ 21.796823] CPU: 0 PID: 59 Comm: kworker/u8:5 Tainted: G I 3.12.0-rc6+ #305
> [ 21.796823] Hardware name: Hewlett-Packard HP xw4600 Workstation/0AA0h, BIOS 786F3 v01.15 08/28/2008
> [ 21.796823] Workqueue: bond0 bond_activebackup_arp_mon [bonding]
> [ 21.796823] ffffffffa00b6c38 ffff880079ecdae8 ffffffff817aa048 0000000000000002
> [ 21.796823] ffff880079ec4b40 ffff880079ecdb18 ffffffff81129af9 00000000001d5400
> [ 21.796823] ffff880079ec4b40 ffff880078a36c88 ffff880079ec5440 ffff880079ecdba8
> [ 21.796823] Call Trace:
> [ 21.796823] [<ffffffffa00b6c38>] ? bond_change_active_slave+0x2c8/0x390 [bonding]
> [ 21.796823] [<ffffffff817aa048>] dump_stack+0x59/0x81
> [ 21.796823] [<ffffffff81129af9>] print_unlock_imbalance_bug+0xf9/0x100
> [ 21.796823] [<ffffffff8112d67f>] lock_release_non_nested+0x26f/0x3f0
> [ 21.796823] [<ffffffff810f3aa8>] ? sched_clock_cpu+0xb8/0x120
> [ 21.796823] [<ffffffffa00b6c38>] ? bond_change_active_slave+0x2c8/0x390 [bonding]
> [ 21.796823] [<ffffffffa00b6c38>] ? bond_change_active_slave+0x2c8/0x390 [bonding]
> [ 21.796823] [<ffffffff8112d892>] __lock_release+0x92/0x1b0
> [ 21.796823] [<ffffffffa00b6c38>] ? bond_change_active_slave+0x2c8/0x390 [bonding]
> [ 21.796823] [<ffffffff8112da0b>] lock_release+0x5b/0x130
> [ 21.796823] [<ffffffff817b0553>] _raw_read_unlock+0x23/0x50
> [ 21.796823] [<ffffffffa00b6c38>] bond_change_active_slave+0x2c8/0x390 [bonding]
> [ 21.796823] [<ffffffffa00b6df7>] bond_select_active_slave+0xf7/0x1d0 [bonding]
> [ 21.796823] [<ffffffffa00b7006>] bond_ab_arp_commit+0x136/0x200 [bonding]
> [ 21.796823] [<ffffffffa00b9dd8>] bond_activebackup_arp_mon+0xc8/0xd0 [bonding]
> [ 21.796823] [<ffffffff810cff2a>] process_one_work+0x1fa/0x580
> [ 21.796823] [<ffffffff810cfeb9>] ? process_one_work+0x189/0x580
> [ 21.796823] [<ffffffff810d231f>] worker_thread+0x11f/0x3a0
> [ 21.796823] [<ffffffff810d2200>] ? manage_workers+0x170/0x170
> [ 21.796823] [<ffffffff810dbdfe>] kthread+0xee/0x100
> [ 21.796823] [<ffffffff8112d93b>] ? __lock_release+0x13b/0x1b0
> [ 21.796823] [<ffffffff810dbd10>] ? __init_kthread_worker+0x70/0x70
> [ 21.796823] [<ffffffff817ba3ec>] ret_from_fork+0x7c/0xb0
> [ 21.796823] [<ffffffff810dbd10>] ? __init_kthread_worker+0x70/0x70
>
>
>> bonding: remove bond read lock for bond_3ad_state_machine_handler()
>>
>> drivers/net/bonding/bond_3ad.c | 9 ++--
>> drivers/net/bonding/bond_alb.c | 20 ++------
>> drivers/net/bonding/bond_main.c | 100 +++++++++++++---------------------------
>> 3 files changed, 40 insertions(+), 89 deletions(-)
>>
>> --
>> 1.8.2.1
>>
>>
Hi David:
yes, exactly I miss it and make a mistake, the bond_select_active_slave is still have the protect problem and
need to be processed, I miss it, sorry, I will send a patch to fix the bug soon.
Hi Veaceslav:
sorry about the commit, I will pay more attention to the commit and test, thanks for your advise and report the bug,
I have to admin that I was too careless.
>>
>> --
>> 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
* [PATCH net-next] inet: restore gso for vxlan
From: Eric Dumazet @ 2013-10-28 1:18 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David Miller, netdev
In-Reply-To: <CAMEtUuxwi05oX0TqNdMbjy04MT=ZLemLXJQTTcAMrnWuhXLKdQ@mail.gmail.com>
From: Eric Dumazet <edumazet@google.com>
Alexei reported a performance regression on vxlan, caused
by commit 3347c9602955 "ipv4: gso: make inet_gso_segment() stackable"
GSO vxlan packets were not properly segmented, adding IP fragments
while they were not expected.
Rename 'bool tunnel' to 'bool encap', and add a new boolean
to express the fact that UDP should be fragmented.
This fragmentation is triggered by skb->encapsulation being set.
Remove a "skb->encapsulation = 1" added in above commit,
as its not needed, as frags inherit skb->frag from original
GSO skb.
Reported-by: Alexei Starovoitov <ast@plumgrid.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Tested-by: Alexei Starovoitov <ast@plumgrid.com>
---
net/ipv4/af_inet.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index f4a159e705c0..09d78d4a3cff 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1251,8 +1251,8 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
struct sk_buff *segs = ERR_PTR(-EINVAL);
const struct net_offload *ops;
unsigned int offset = 0;
+ bool udpfrag, encap;
struct iphdr *iph;
- bool tunnel;
int proto;
int nhoff;
int ihl;
@@ -1290,8 +1290,8 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
goto out;
__skb_pull(skb, ihl);
- tunnel = SKB_GSO_CB(skb)->encap_level > 0;
- if (tunnel)
+ encap = SKB_GSO_CB(skb)->encap_level > 0;
+ if (encap)
features = skb->dev->hw_enc_features & netif_skb_features(skb);
SKB_GSO_CB(skb)->encap_level += ihl;
@@ -1306,24 +1306,23 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
if (IS_ERR_OR_NULL(segs))
goto out;
+ udpfrag = !!skb->encapsulation && proto == IPPROTO_UDP;
skb = segs;
do {
iph = (struct iphdr *)(skb_mac_header(skb) + nhoff);
- if (!tunnel && proto == IPPROTO_UDP) {
+ if (udpfrag) {
iph->id = htons(id);
iph->frag_off = htons(offset >> 3);
if (skb->next != NULL)
iph->frag_off |= htons(IP_MF);
offset += skb->len - nhoff - ihl;
- } else {
+ } else {
iph->id = htons(id++);
}
iph->tot_len = htons(skb->len - nhoff);
ip_send_check(iph);
- if (tunnel) {
+ if (encap)
skb_reset_inner_headers(skb);
- skb->encapsulation = 1;
- }
skb->network_header = (u8 *)iph - skb->head;
} while ((skb = skb->next));
^ permalink raw reply related
* Re: [PATCH net 0/3] ipv6: use rt6i_gateway as nexthop
From: Simon Horman @ 2013-10-28 1:28 UTC (permalink / raw)
To: David Miller; +Cc: ja, netdev, netfilter-devel, lvs-devel, yoshfuji
In-Reply-To: <20131021.184057.328665907625927061.davem@davemloft.net>
On Mon, Oct 21, 2013 at 06:40:57PM -0400, David Miller wrote:
> From: Julian Anastasov <ja@ssi.bg>
> Date: Sun, 20 Oct 2013 15:43:02 +0300
>
> > I see the following two alternatives for applying these
> > patches:
> >
> > 1. Linger patch 2 in net-next to avoid surprises in the upcoming
> > release. In this case patch 3 can be reworked not to depend on
> > the new rt6_nexthop() definition in patch 2. I guess this is a
> > better option, so that patch 2 can be reviewed and tested for
> > longer time.
> >
> > 2. Include all 3 patches in net tree - more risky because this
> > is my first attempt to change IPv6.
>
> I have decided to merge all three patches into -net right now.
> I've reviewed these patches several times and they look good
> to me.
>
> I'll let them cook upstream for at least a week before submitting them
> to -stable to let any last minute errors show themselves and
> subsequently get resolved.
Thanks Dave,
FWIW, I have verified that these changes resolve the problem
that I reported with IPVS that I believe prompted Julian to write
these changes. That is IPv6 IPVS-DR once again works with these
changes in place.
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] bonding: patchset for rcu use in bonding
From: Veaceslav Falico @ 2013-10-28 1:34 UTC (permalink / raw)
To: Ding Tianhong
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <526DBAC8.5000009@huawei.com>
On Mon, Oct 28, 2013 at 09:15:52AM +0800, Ding Tianhong wrote:
>On 2013/10/28 6:53, Veaceslav Falico wrote:
>> On Thu, Oct 24, 2013 at 11:08:35AM +0800, Ding Tianhong wrote:
>>> Hi:
>>>
>>> The slave list will add and del by bond_master_upper_dev_link() and bond_upper_dev_unlink(),
>>> which will call call_netdevice_notifiers(), even it is safe to call it in write bond lock now,
>>> but we can't sure that whether it is safe later, because other drivers may deal NETDEV_CHANGEUPPER
>>> in sleep way, so I didn't admit move the bond_upper_dev_unlink() in write bond lock.
>>>
>>> now the bond_for_each_slave only protect by rtnl_lock(), maybe use bond_for_each_slave_rcu is a good
>>> way to protect slave list for bond, but as a system slow path, it is no need to transform bond_for_each_slave()
>>> to bond_for_each_slave_rcu() in slow path, so in the patchset, I will remove the unused read bond lock
>>> for monitor function, maybe it is a better way, I will wait to accept any relay for it.
>>>
>>> Thanks for the Veaceslav Falico opinion.
>>>
>>> v2: add and modify commit for patchset and patch, it will be the first step for the whole patchset.
>>>
>>> Ding Tianhong (5):
>>> bonding: remove bond read lock for bond_mii_monitor()
>>> bonding: remove bond read lock for bond_alb_monitor()
>>> bonding: remove bond read lock for bond_loadbalance_arp_mon()
>>> bonding: remove bond read lock for bond_activebackup_arp_mon()
>>
>> This patch introduces a regression by boot-test with active backup mode:
>>
>> bond_activebackup_arp_mon() is already not holding the bond->lock, however
>> it might call bond_change_active_slave(), which does (in case of new_active):
>>
>> 912 write_unlock_bh(&bond->curr_slave_lock);
>> 913 read_unlock(&bond->lock);
>> 914 915 call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
>> 916 if (should_notify_peers)
>> 917 call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
>> 918 bond->dev);
>> 919 920 read_lock(&bond->lock);
>> 921 write_lock_bh(&bond->curr_slave_lock);
>>
>> so it drops the bond->lock (which wasn't taken previously), and then takes
>> it (without anyone dropping it afterwards).
>>
>> I don't know how to fix it - cause a lot of other callers already take it,
>> and we can't just drop them (we'd race), and we can't remove it here (cause
>> we can't call notifiers while atomic).
>>
>> Which begs the question - was this patchset tested at all?
>>
>> [ 21.796823] =====================================
>> [ 21.796823] [ BUG: bad unlock balance detected! ]
... snip ...
>
>Hi David:
>yes, exactly I miss it and make a mistake, the bond_select_active_slave is still have the protect problem and
>need to be processed, I miss it, sorry, I will send a patch to fix the bug soon.
>
>Hi Veaceslav:
>sorry about the commit, I will pay more attention to the commit and test, thanks for your advise and report the bug,
>I have to admin that I was too careless.
I'll ask you once again, even though it seems that my NACK doesn't block
the patchset - try writing commit messages that actually describe why and
how you do it.
It's, actually, not only for the reviewers - it's also really good for you
- cause while writing the commit log you also understand a lot more what
are you doing, and might spot some corner cases (like this one).
Sorry for being negative, however it costs me *much* more time to review
patches without proper commit messages. I've done it once, twice, several
times more - but that's it, I refuse to spend my time on your skipped
homework.
It might also help to split patches into really small steps - as in - do
only one thing at a time, and describe it. This will help evade bugs *a
lot*. It also helps people who'll bisect it, bugfix it and review it -
because every patch will be a small, well-documented change, instead of a
chunk of code with a description 'lets remove bond->lock'.
Thank you and hope that helps.
>
>
>
>
>>>
>>> --
>>> 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: [ovs-dev] [PATCH v2.45 1/4] odp: Allow VLAN actions after MPLS actions
From: Simon Horman @ 2013-10-28 1:41 UTC (permalink / raw)
To: dev, netdev, Jesse Gross, Ben Pfaff; +Cc: Isaku Yamahata, Ravi K
In-Reply-To: <1382711684-17080-2-git-send-email-horms@verge.net.au>
On Fri, Oct 25, 2013 at 03:34:41PM +0100, Simon Horman wrote:
> From: Joe Stringer <joe@wand.net.nz>
>
> OpenFlow 1.1 and 1.2, and 1.3 differ in their handling of MPLS actions in the
> presence of VLAN tags. To allow correct behaviour to be committed in
> each situation, this patch adds a second round of VLAN tag action
> handling to commit_odp_actions(), which occurs after MPLS actions. This
> is implemented with a new field in 'struct xlate_in' called 'vlan_tci'.
>
> When an push_mpls action is composed, the flow's current VLAN state is
> stored into xin->vlan_tci, and flow->vlan_tci is set to 0 (pop_vlan). If
> a VLAN tag is present, it is stripped; if not, then there is no change.
> Any later modifications to the VLAN state is written to xin->vlan_tci.
> When committing the actions, flow->vlan_tci is used before MPLS actions,
> and xin->vlan_tci is used afterwards. This retains the current datapath
> behaviour, but allows VLAN actions to be applied in a more flexible
> manner.
>
> Both before and after this patch MPLS LSEs are pushed onto a packet after
> any VLAN tags that may be present. This is the behaviour described in
> OpenFlow 1.1 and 1.2. OpenFlow 1.3 specifies that MPLS LSEs should be
> pushed onto a packet before any VLAN tags that are present. Support
> for this will be added by a subsequent patch that makes use of
> the infrastructure added by this patch.
Whole changelog message still reflects the intent of the code the
implementation details it describes are no longer correct. I will
fix this and re-post.
>
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
>
> v2.45 [Simon Horman]
> * As pointed out by Ben Pfaff and Joe Stringer
> + Update VLAN handling in the presence of MPLS push
>
> Previously the test for committing ODP VLAN actions after MPLS actions
> was that the VLAN TCI differed before and after the MPLS push action.
> This results in a false negative in some cases including if a VLAN tag
> is pushed after the MPLS push action in such a way that it duplicates
> the VLAN tag present before the MPLS push action.
>
> This is resolved by ensuring the VLAN_CFI bit of the value used to
> track the desired VLAN TCI after an MPLS push action is zero unless
> VLAN actions should be committed after MPLS actions.
>
> + Update tests to use ovs-ofctl monitor "-m" to allow full inspection of
> MPLS LSE and VLAN tags present in packets.
>
> v2.44 [Simon Horman]
> * Rebase for the following changes:
> f47ea02 ("Set datapath mask bits when setting a flow field.")
> 7fdb60a ("Add support for write-actions")
> 7358063 ("odp-util: Elaborate the comment for odp_flow_format() function.")
> * Correct final_vlan_tci and next_vlan_tci initialisation in xlate_actions__()
>
> v2.43 [Simon Horman]
> * As suggested by Ben Pfaff
> Move vlan state into struct xlate_ctx
>
> 1. Add final_vlan_tci member to struct xlate_ctx instead of vlan_tci member
> struct xlate_xin. This seems to be a better palace for it as it does
> not need to be accessible from the caller.
>
> 2. Move local vlan_tci variable of do_xlate_actions() to be the
> next_vlan_tci member of strict xlate_ctx. This allows for it to work
> across resubmit actions and goto table instructions.
>
> v2.42
> * No change
>
> v2.41 [Joe Stringer via Simon Horman]
> * Rework comments to make things a little clearer
>
> v2.40 [Simon Horman]
> * Rebase for removal of mpls_depth from struct flow
>
> v2.38 - v2.39
> * No change
>
> v2.37
> * Rebase
>
> v2.36
> * No change
>
> v2.5 [Joe Stringer]
> * First post
> ---
> lib/odp-util.c | 12 +-
> lib/odp-util.h | 3 +-
> ofproto/ofproto-dpif-xlate.c | 136 ++++++++++++---
> tests/ofproto-dpif.at | 389 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 514 insertions(+), 26 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 6875e01..21b33ac 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -3639,11 +3639,15 @@ commit_set_pkt_mark_action(const struct flow *flow, struct flow *base,
> * used as part of the action.
> *
> * Returns a reason to force processing the flow's packets into the userspace
> - * slow path, if there is one, otherwise 0. */
> + * slow path, if there is one, otherwise 0.
> + *
> + * VLAN actions may be committed twice; If vlan_tci in 'flow' differs from the
> + * one in 'base', then it is committed before MPLS actions. If the VLAN_CFI
> + * bit of 'post_mpls_vlan_tci' is set then it is committed afterwards. */
> enum slow_path_reason
> commit_odp_actions(const struct flow *flow, struct flow *base,
> struct ofpbuf *odp_actions, struct flow_wildcards *wc,
> - int *mpls_depth_delta)
> + int *mpls_depth_delta, ovs_be16 post_mpls_vlan_tci)
> {
> enum slow_path_reason slow;
>
> @@ -3656,6 +3660,10 @@ commit_odp_actions(const struct flow *flow, struct flow *base,
> * that it is no longer IP and thus nw and port actions are no longer valid.
> */
> commit_mpls_action(flow, base, odp_actions, wc, mpls_depth_delta);
> + if (post_mpls_vlan_tci & htons(VLAN_CFI)) {
> + base->vlan_tci = htons(0);
> + commit_vlan_action(post_mpls_vlan_tci, base, odp_actions, wc);
> + }
> commit_set_priority_action(flow, base, odp_actions, wc);
> commit_set_pkt_mark_action(flow, base, odp_actions, wc);
>
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index 821b2c4..636a3ec 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -175,7 +175,8 @@ enum slow_path_reason commit_odp_actions(const struct flow *,
> struct flow *base,
> struct ofpbuf *odp_actions,
> struct flow_wildcards *wc,
> - int *mpls_depth_delta);
> + int *mpls_depth_delta,
> + ovs_be16 post_mpls_vlan_tci);
> \f
> /* ofproto-dpif interface.
> *
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7be691c..d79356f 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -180,6 +180,37 @@ struct xlate_ctx {
> uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
> bool exit; /* No further actions should be processed. */
>
> + /* The post MPLS vlan_tci.
> + *
> + * The value of the vlan TCI prior to the translation of an MPLS push
> + * actions should be stored in 'xin->flow->vlan_tci'.
> + *
> + * If an VLAN or set field action is subsequently translated then
> + * 'post_mpls_vlan_tci' is updated as according to the action.
> + *
> + * If the VLAN_CFI bit of 'post_mpls_vlan_tci' is set then VLAN ODP actions
> + * will be committed after any MPLS actions regardless of whether VLAN
> + * actions were also committed before the MPLS actions or not.
> + *
> + * This mechanism allows a VLAN tag to be popped before pushing
> + * an MPLS LSE and then the same VLAN tag pushed after pushing
> + * the MPLS LSE. In this way it is possible to push an MPLS LSE
> + * after an existing VLAN tag. Moreover this mechanism allows
> + * the order in which VLAN tags and MPLS LSEs are pushed. */
> + ovs_be16 post_mpls_vlan_tci;
> +
> + /* The next vlan_tci state.
> + *
> + * This field pints to the variable update each time an
> + * action updates the VLAN tci.
> + *
> + * This variable initially points to 'xin->flow->vlan_tci' so that ODP
> + * VLAN actions are committed before any MPLS actions. When an MPLS
> + * action is composed 'next_vlan_tci' is updated to point to
> + * 'post_mpls_vlan_tci'. This causes subsequent VLAN actions to be
> + * committed after MPLS actions. */
> + ovs_be16 *next_vlan_tci;
> +
> /* OpenFlow 1.1+ action set.
> *
> * 'action_set' accumulates "struct ofpact"s added by OFPACT_WRITE_ACTIONS.
> @@ -996,11 +1027,38 @@ output_vlan_to_vid(const struct xbundle *out_xbundle, uint16_t vlan)
> }
> }
>
> +static bool mpls_actions_xlated(struct xlate_ctx *ctx)
> +{
> + return ctx->next_vlan_tci == &ctx->post_mpls_vlan_tci;
> +}
> +
> +static ovs_be16
> +vlan_tci_save(struct xlate_ctx *ctx)
> +{
> + ovs_be16 orig_tci = ctx->xin->flow.vlan_tci;
> + /* If MPLS actions were executed after vlan actions then
> + * copy the final vlan_tci out and restore the intermediate VLAN state. */
> + if (mpls_actions_xlated(ctx)) {
> + ctx->xin->flow.vlan_tci = *ctx->next_vlan_tci;
> + }
> + return orig_tci;
> +}
> +
> +static void
> +vlan_tci_restore(struct xlate_ctx *ctx, ovs_be16 orig_tci)
> +{
> + /* If MPLS actions were executed after vlan actions then
> + * copy the final vlan_tci out and restore the intermediate VLAN state. */
> + if (mpls_actions_xlated(ctx)) {
> + ctx->post_mpls_vlan_tci = ctx->xin->flow.vlan_tci;
> + ctx->xin->flow.vlan_tci = orig_tci;
> + }
> +}
> +
> static void
> output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
> uint16_t vlan)
> {
> - ovs_be16 *flow_tci = &ctx->xin->flow.vlan_tci;
> uint16_t vid;
> ovs_be16 tci, old_tci;
> struct xport *xport;
> @@ -1025,18 +1083,18 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
> }
> }
>
> - old_tci = *flow_tci;
> + old_tci = *ctx->next_vlan_tci;
> tci = htons(vid);
> if (tci || out_xbundle->use_priority_tags) {
> - tci |= *flow_tci & htons(VLAN_PCP_MASK);
> + tci |= *ctx->next_vlan_tci & htons(VLAN_PCP_MASK);
> if (tci) {
> tci |= htons(VLAN_CFI);
> }
> }
> - *flow_tci = tci;
> + ctx->xin->flow.vlan_tci = *ctx->next_vlan_tci = tci;
>
> compose_output_action(ctx, xport->ofp_port);
> - *flow_tci = old_tci;
> + ctx->xin->flow.vlan_tci = *ctx->next_vlan_tci = old_tci;
> }
>
> /* A VM broadcasts a gratuitous ARP to indicate that it has resumed after
> @@ -1269,7 +1327,7 @@ xlate_normal(struct xlate_ctx *ctx)
>
> /* Drop malformed frames. */
> if (flow->dl_type == htons(ETH_TYPE_VLAN) &&
> - !(flow->vlan_tci & htons(VLAN_CFI))) {
> + !(*ctx->next_vlan_tci & htons(VLAN_CFI))) {
> if (ctx->xin->packet != NULL) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> VLOG_WARN_RL(&rl, "bridge %s: dropping packet with partial "
> @@ -1293,7 +1351,7 @@ xlate_normal(struct xlate_ctx *ctx)
> }
>
> /* Check VLAN. */
> - vid = vlan_tci_to_vid(flow->vlan_tci);
> + vid = vlan_tci_to_vid(*ctx->next_vlan_tci);
> if (!input_vid_is_valid(vid, in_xbundle, ctx->xin->packet != NULL)) {
> xlate_report(ctx, "disallowed VLAN VID for this input port, dropping");
> return;
> @@ -1551,7 +1609,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
> const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
> struct flow_wildcards *wc = &ctx->xout->wc;
> struct flow *flow = &ctx->xin->flow;
> - ovs_be16 flow_vlan_tci;
> + ovs_be16 flow_vlan_tci, vlan_tci;
> uint32_t flow_pkt_mark;
> uint8_t flow_nw_tos;
> odp_port_t out_port, odp_port;
> @@ -1620,6 +1678,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
> }
>
> flow_vlan_tci = flow->vlan_tci;
> + vlan_tci = *ctx->next_vlan_tci;
> flow_pkt_mark = flow->pkt_mark;
> flow_nw_tos = flow->nw_tos;
>
> @@ -1659,12 +1718,13 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
> wc->masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
> }
> vlandev_port = vsp_realdev_to_vlandev(ctx->xbridge->ofproto, ofp_port,
> - flow->vlan_tci);
> + *ctx->next_vlan_tci);
> if (vlandev_port == ofp_port) {
> out_port = odp_port;
> } else {
> out_port = ofp_port_to_odp_port(ctx->xbridge, vlandev_port);
> flow->vlan_tci = htons(0);
> + ctx->post_mpls_vlan_tci = htons(0);
> }
> }
>
> @@ -1672,7 +1732,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
> ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
> &ctx->xout->odp_actions,
> &ctx->xout->wc,
> - &ctx->mpls_depth_delta);
> + &ctx->mpls_depth_delta,
> + ctx->post_mpls_vlan_tci);
> nl_msg_put_odp_port(&ctx->xout->odp_actions, OVS_ACTION_ATTR_OUTPUT,
> out_port);
>
> @@ -1684,6 +1745,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
> out:
> /* Restore flow */
> flow->vlan_tci = flow_vlan_tci;
> + *ctx->next_vlan_tci = vlan_tci;
> flow->pkt_mark = flow_pkt_mark;
> flow->nw_tos = flow_nw_tos;
> }
> @@ -1838,7 +1900,8 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
> ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
> &ctx->xout->odp_actions,
> &ctx->xout->wc,
> - &ctx->mpls_depth_delta);
> + &ctx->mpls_depth_delta,
> + ctx->post_mpls_vlan_tci);
>
> odp_execute_actions(NULL, packet, &key, ctx->xout->odp_actions.data,
> ctx->xout->odp_actions.size, NULL, NULL);
> @@ -2231,7 +2294,8 @@ xlate_sample_action(struct xlate_ctx *ctx,
> ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
> &ctx->xout->odp_actions,
> &ctx->xout->wc,
> - &ctx->mpls_depth_delta);
> + &ctx->mpls_depth_delta,
> + ctx->post_mpls_vlan_tci);
>
> compose_flow_sample_cookie(os->probability, os->collector_set_id,
> os->obs_domain_id, os->obs_point_id, &cookie);
> @@ -2320,28 +2384,28 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
>
> case OFPACT_SET_VLAN_VID:
> wc->masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
> - flow->vlan_tci &= ~htons(VLAN_VID_MASK);
> - flow->vlan_tci |= (htons(ofpact_get_SET_VLAN_VID(a)->vlan_vid)
> - | htons(VLAN_CFI));
> + *ctx->next_vlan_tci &= ~htons(VLAN_VID_MASK);
> + *ctx->next_vlan_tci |= (htons(ofpact_get_SET_VLAN_VID(a)->vlan_vid)
> + | htons(VLAN_CFI));
> break;
>
> case OFPACT_SET_VLAN_PCP:
> wc->masks.vlan_tci |= htons(VLAN_PCP_MASK | VLAN_CFI);
> - flow->vlan_tci &= ~htons(VLAN_PCP_MASK);
> - flow->vlan_tci |=
> + *ctx->next_vlan_tci &= ~htons(VLAN_PCP_MASK);
> + *ctx->next_vlan_tci |=
> htons((ofpact_get_SET_VLAN_PCP(a)->vlan_pcp << VLAN_PCP_SHIFT)
> | VLAN_CFI);
> break;
>
> case OFPACT_STRIP_VLAN:
> memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
> - flow->vlan_tci = htons(0);
> + *ctx->next_vlan_tci = htons(0);
> break;
>
> case OFPACT_PUSH_VLAN:
> /* XXX 802.1AD(QinQ) */
> memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
> - flow->vlan_tci = htons(VLAN_CFI);
> + *ctx->next_vlan_tci = htons(VLAN_CFI);
> break;
>
> case OFPACT_SET_ETH_SRC:
> @@ -2423,29 +2487,53 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> flow->skb_priority = ctx->orig_skb_priority;
> break;
>
> - case OFPACT_REG_MOVE:
> + case OFPACT_REG_MOVE: {
> + ovs_be16 orig_tci = flow->vlan_tci;
> nxm_execute_reg_move(ofpact_get_REG_MOVE(a), flow, wc);
> + vlan_tci_restore(ctx, orig_tci);
> break;
> + }
>
> - case OFPACT_REG_LOAD:
> + case OFPACT_REG_LOAD: {
> + ovs_be16 orig_tci = vlan_tci_save(ctx);
> nxm_execute_reg_load(ofpact_get_REG_LOAD(a), flow, wc);
> + vlan_tci_restore(ctx, orig_tci);
> break;
> + }
>
> - case OFPACT_STACK_PUSH:
> + case OFPACT_STACK_PUSH: {
> + ovs_be16 orig_tci = vlan_tci_save(ctx);
> nxm_execute_stack_push(ofpact_get_STACK_PUSH(a), flow, wc,
> &ctx->stack);
> + vlan_tci_restore(ctx, orig_tci);
> break;
> + }
>
> - case OFPACT_STACK_POP:
> + case OFPACT_STACK_POP: {
> + ovs_be16 orig_tci = vlan_tci_save(ctx);
> nxm_execute_stack_pop(ofpact_get_STACK_POP(a), flow, wc,
> &ctx->stack);
> + vlan_tci_restore(ctx, orig_tci);
> break;
> + }
>
> case OFPACT_PUSH_MPLS:
> if (compose_mpls_push_action(ctx,
> ofpact_get_PUSH_MPLS(a)->ethertype)) {
> return;
> }
> +
> + /* Save and pop any existing VLAN tags. They will be pushed
> + * back onto the packet after pushing the MPLS LSE. The overall
> + * effect is to push the MPLS LSE after any VLAN tags that may
> + * be present. This is the behaviour described for OpenFlow 1.1
> + * and 1.2. This code needs to be enhanced to make this
> + * conditional to also and support pushing the MPLS LSE before
> + * any VLAN tags that may be present, the behaviour described
> + * for OpenFlow 1.3. */
> + ctx->post_mpls_vlan_tci = *ctx->next_vlan_tci;
> + flow->vlan_tci = htons(0);
> + ctx->next_vlan_tci = &ctx->post_mpls_vlan_tci;
> break;
>
> case OFPACT_POP_MPLS:
> @@ -2786,6 +2874,8 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
> ctx.table_id = 0;
> ctx.exit = false;
> ctx.mpls_depth_delta = 0;
> + ctx.post_mpls_vlan_tci = htons(0);
> + ctx.next_vlan_tci = &ctx.xin->flow.vlan_tci;
>
> if (!xin->ofpacts && !ctx.rule) {
> rule_dpif_lookup(ctx.xbridge->ofproto, flow, wc, &rule);
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index c569463..372bce7 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -965,6 +965,395 @@ done
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([ofproto-dpif - OF1.2 VLAN+MPLS handling])
> +OVS_VSWITCHD_START([dnl
> + add-port br0 p1 -- set Interface p1 type=dummy
> +])
> +ON_EXIT([kill `cat ovs-ofctl.pid`])
> +
> +AT_CAPTURE_FILE([ofctl_monitor.log])
> +AT_DATA([flows.txt], [dnl
> +cookie=0xa dl_src=40:44:44:44:54:50 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],push_vlan:0x8100,mod_vlan_vid:99,mod_vlan_pcp:1,controller
> +cookie=0xa dl_src=40:44:44:44:54:51 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],push_vlan:0x8100,mod_vlan_vid:99,mod_vlan_pcp:1,controller
> +cookie=0xa dl_src=40:44:44:44:54:52 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],push_vlan:0x8100,load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,controller
> +cookie=0xa dl_src=40:44:44:44:54:53 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],push_vlan:0x8100,load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,controller
> +cookie=0xa dl_src=40:44:44:44:54:54 actions=push_vlan:0x8100,mod_vlan_vid:99,mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
> +cookie=0xa dl_src=40:44:44:44:54:55 actions=push_vlan:0x8100,mod_vlan_vid:99,mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
> +cookie=0xa dl_src=40:44:44:44:54:56 actions=push_vlan:0x8100,load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
> +cookie=0xa dl_src=40:44:44:44:54:57 actions=push_vlan:0x8100,load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
> +cookie=0xa dl_src=40:44:44:44:54:58 actions=load:99->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],controller
> +cookie=0xa dl_src=40:44:44:44:54:59 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],mod_vlan_pcp:1,load:99->OXM_OF_VLAN_VID[[]],controller
> +])
> +AT_CHECK([ovs-ofctl --protocols=OpenFlow12 add-flows br0 flows.txt])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, we push the MPLS tag before pushing a VLAN tag, so we see
> +dnl both of these in the final flow
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:50,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
> +done
> +OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:50,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 50 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:50,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 50 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:50,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 50 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl before we push the MPLS and VLAN tags.
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:51,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
> +done
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
> +ovs-appctl -t ovs-ofctl exit
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:51,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 51 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:51,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 51 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:51,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 51 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, we push the MPLS tag before pushing a VLAN tag, so we see
> +dnl both of these in the final flow
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:52,dst=52:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
> +done
> +OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:52,dl_dst=52:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 52 54 00 00 00 07 40 44-44 44 54 52 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:52,dl_dst=52:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 52 54 00 00 00 07 40 44-44 44 54 52 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:52,dl_dst=52:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 52 54 00 00 00 07 40 44-44 44 54 52 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl before we push the MPLS and VLAN tags.
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:53,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
> +done
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
> +ovs-appctl -t ovs-ofctl exit
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:53,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 53 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:53,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 53 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:53,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 53 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, we push the VLAN tag before pushing a MPLS tag, but these
> +dnl actions are reordered, so we see both of these in the final flow.
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:54,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
> +done
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
> +ovs-appctl -t ovs-ofctl exit
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:54,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 54 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:54,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 54 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:54,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 54 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl before we push the MPLS and VLAN tags.
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:55,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
> +done
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
> +ovs-appctl -t ovs-ofctl exit
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:55,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 55 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:55,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 55 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:55,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 55 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, we push the VLAN tag before pushing a MPLS tag, but these
> +dnl actions are reordered, so we see both of these in the final flow.
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:56,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
> +done
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
> +ovs-appctl -t ovs-ofctl exit
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:56,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 56 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:56,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 56 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=68 in_port=1 (via action) data_len=68 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:56,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 56 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +00000040 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl before we push the MPLS and VLAN tags.
> +AT_CHECK([ovs-ofctl monitor br0 -m 65534 -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:57,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
> +done
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
> +ovs-appctl -t ovs-ofctl exit
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:57,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 57 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:57,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 57 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:57,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 57 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl before we push the MPLS and VLAN tags.
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:58,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
> +done
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
> +ovs-appctl -t ovs-ofctl exit
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:58,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 58 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:58,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 58 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:58,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 58 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +])
> +
> +dnl Modified MPLS controller action.
> +dnl In this test, the input packet in vlan-tagged, which should be modified
> +dnl before we push MPLS and VLAN tags.
> +AT_CHECK([ovs-ofctl monitor br0 65534 -m -P nxm --detach --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=40:44:44:44:54:59,dst=50:54:00:00:00:07),eth_type(0x8100),vlan(vid=88,pcp=7),encap(eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no))'
> +done
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
> +ovs-appctl -t ovs-ofctl exit
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:59,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 59 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:59,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 59 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=64 in_port=1 (via action) data_len=64 (unbuffered)
> +mpls,metadata=0,in_port=0,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:59,dl_dst=50:54:00:00:00:07,mpls_label=10,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> +00000000 50 54 00 00 00 07 40 44-44 44 54 59 81 00 20 63
> +00000010 88 47 00 00 a1 40 45 00-00 28 00 00 00 00 40 06
> +00000020 f9 7c c0 a8 00 01 c0 a8-00 02 00 00 00 00 00 00
> +00000030 00 00 00 00 00 00 50 00-00 00 00 00 00 00 00 00
> +])
> +
> +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:50 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],mod_vlan_vid:99,mod_vlan_pcp:1,CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:51 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],mod_vlan_vid:99,mod_vlan_pcp:1,CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:52 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:53 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:54 actions=mod_vlan_vid:99,mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:55 actions=mod_vlan_vid:99,mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:56 actions=load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:57 actions=load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:58 actions=load:0x63->OXM_OF_VLAN_VID[[]],mod_vlan_pcp:1,push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
> + cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:54:59 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],mod_vlan_pcp:1,load:0x63->OXM_OF_VLAN_VID[[]],CONTROLLER:65535
> +NXST_FLOW reply:
> +])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> AT_SETUP([ofproto-dpif - fragment handling])
> OVS_VSWITCHD_START
> ADD_OF_PORTS([br0], [1], [2], [3], [4], [5], [6], [90])
> --
> 1.8.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
^ permalink raw reply
* [PATCH v2 2/2] net/benet: Make lancer_wait_ready() static
From: Gavin Shan @ 2013-10-28 2:12 UTC (permalink / raw)
To: netdev; +Cc: Sathya.Perla, davem, Gavin Shan
In-Reply-To: <1382926367-14968-1-git-send-email-shangw@linux.vnet.ibm.com>
The function needn't to be public, so to make it as static.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
drivers/net/ethernet/emulex/benet/be_cmds.c | 2 +-
drivers/net/ethernet/emulex/benet/be_cmds.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index c08fd32..c30ab8f 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -522,7 +522,7 @@ static u16 be_POST_stage_get(struct be_adapter *adapter)
return sem & POST_STAGE_MASK;
}
-int lancer_wait_ready(struct be_adapter *adapter)
+static int lancer_wait_ready(struct be_adapter *adapter)
{
#define SLIPORT_READY_TIMEOUT 30
u32 sliport_status;
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 108ca8a..dd522b0 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -1983,7 +1983,6 @@ extern int be_cmd_get_ext_fat_capabilites(struct be_adapter *adapter,
extern int be_cmd_set_ext_fat_capabilites(struct be_adapter *adapter,
struct be_dma_mem *cmd,
struct be_fat_conf_params *cfgs);
-extern int lancer_wait_ready(struct be_adapter *adapter);
extern int lancer_physdev_ctrl(struct be_adapter *adapter, u32 mask);
extern int lancer_initiate_dump(struct be_adapter *adapter);
extern bool dump_present(struct be_adapter *adapter);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 1/2] net/benet: Remove interface type
From: Gavin Shan @ 2013-10-28 2:12 UTC (permalink / raw)
To: netdev; +Cc: Sathya.Perla, davem, Gavin Shan
The interrupt type, which is being traced by "struct be_adapter::
if_type", isn't used currently. So we can remove that safely
according to Sathya's comments.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
drivers/net/ethernet/emulex/benet/be.h | 1 -
drivers/net/ethernet/emulex/benet/be_main.c | 5 -----
2 files changed, 6 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index db02023..5e195e7 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -437,7 +437,6 @@ struct be_adapter {
u32 rx_fc; /* Rx flow control */
u32 tx_fc; /* Tx flow control */
bool stats_cmd_sent;
- u32 if_type;
struct {
u32 size;
u32 total_size;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 2c38cc4..992c9ee 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3960,11 +3960,6 @@ static int be_roce_map_pci_bars(struct be_adapter *adapter)
static int be_map_pci_bars(struct be_adapter *adapter)
{
u8 __iomem *addr;
- u32 sli_intf;
-
- pci_read_config_dword(adapter->pdev, SLI_INTF_REG_OFFSET, &sli_intf);
- adapter->if_type = (sli_intf & SLI_INTF_IF_TYPE_MASK) >>
- SLI_INTF_IF_TYPE_SHIFT;
if (BEx_chip(adapter) && be_physfn(adapter)) {
adapter->csr = pci_iomap(adapter->pdev, 2, 0);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net] xen-netback: use jiffies_64 value to calculate credit timeout
From: annie li @ 2013-10-28 2:36 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, netdev, david.vrabel, jbeulich, Ian Campbell,
Jason Luan
In-Reply-To: <1382872313-11675-1-git-send-email-wei.liu2@citrix.com>
On 2013-10-27 19:11, Wei Liu wrote:
> time_after_eq() only works if the delta is < MAX_ULONG/2.
>
> For a 32bit Dom0, if netfront sends packets at a very low rate, the time
> between subsequent calls to tx_credit_exceeded() may exceed MAX_ULONG/2
> and the test for timer_after_eq() will be incorrect. Credit will not be
> replenished and the guest may become unable to send packets (e.g., if
> prior to the long gap, all credit was exhausted).
>
> Use jiffies_64 variant to mitigate this problem for 32bit Dom0.
>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Suggested-by: David Vrabel <david.vrabel@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jason Luan <jianhai.luan@oracle.com>
> ---
> drivers/net/xen-netback/common.h | 1 +
> drivers/net/xen-netback/interface.c | 4 ++--
> drivers/net/xen-netback/netback.c | 13 ++++++-------
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 5715318..400fea1 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -163,6 +163,7 @@ struct xenvif {
> unsigned long credit_usec;
> unsigned long remaining_credit;
> struct timer_list credit_timeout;
> + u64 credit_window_start;
>
> /* Statistics */
> unsigned long rx_gso_checksum_fixup;
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 01bb854..8c45b63 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -312,8 +312,8 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
> vif->credit_bytes = vif->remaining_credit = ~0UL;
> vif->credit_usec = 0UL;
> init_timer(&vif->credit_timeout);
> - /* Initialize 'expires' now: it's used to track the credit window. */
> - vif->credit_timeout.expires = jiffies;
> + /* credit window is tracked in credit_window_start */
> + vif->credit_window_start = get_jiffies_64();
>
> dev->netdev_ops = &xenvif_netdev_ops;
> dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO;
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index f3e591c..1bc0688 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1185,18 +1185,17 @@ out:
>
> static bool tx_credit_exceeded(struct xenvif *vif, unsigned size)
> {
> - unsigned long now = jiffies;
> - unsigned long next_credit =
> - vif->credit_timeout.expires +
> - msecs_to_jiffies(vif->credit_usec / 1000);
> + u64 now = get_jiffies_64();
> + u64 next_credit = vif->credit_window_start +
> + (u64)msecs_to_jiffies(vif->credit_usec / 1000);
You simply replace "credit_timeout.expires" with
"vif->credit_window_start" here, and never update
"vif->credit_window_start" in following code.
>
> /* Timer could already be pending in rare cases. */
> if (timer_pending(&vif->credit_timeout))
> return true;
>
> /* Passed the point where we can replenish credit? */
> - if (time_after_eq(now, next_credit)) {
> - vif->credit_timeout.expires = now;
> + if (time_after_eq64(now, next_credit)) {
> + vif->credit_timeout.expires = (unsigned long)now;
updates credit_window_start as following,
vif->credit_window_start = (unsigned long)now;
> tx_add_credit(vif);
> }
>
> @@ -1207,7 +1206,7 @@ static bool tx_credit_exceeded(struct xenvif *vif, unsigned size)
> vif->credit_timeout.function =
> tx_credit_callback;
> mod_timer(&vif->credit_timeout,
> - next_credit);
> + (unsigned long)next_credit);
vif->credit_timeout.expires is unsigned long, and this still causes
original issue on 32bit system, which works well on 64bit. Or rewriting
code to avoid uses of vif->credit_timeout.expires, but it is complex.
BTW, I prefer Luan's patch which is simple and clear.
Thanks
Annie
^ permalink raw reply
* Re: [Xen-devel] [PATCH net] xen-netback: use jiffies_64 value to calculate credit timeout
From: annie li @ 2013-10-28 2:58 UTC (permalink / raw)
To: Wei Liu; +Cc: Ian Campbell, netdev, xen-devel, david.vrabel, jbeulich,
Jason Luan
In-Reply-To: <526DCDB8.1080908@oracle.com>
On 2013-10-28 10:36, annie li wrote:
>
> On 2013-10-27 19:11, Wei Liu wrote:
>> time_after_eq() only works if the delta is < MAX_ULONG/2.
>>
>> For a 32bit Dom0, if netfront sends packets at a very low rate, the time
>> between subsequent calls to tx_credit_exceeded() may exceed MAX_ULONG/2
>> and the test for timer_after_eq() will be incorrect. Credit will not be
>> replenished and the guest may become unable to send packets (e.g., if
>> prior to the long gap, all credit was exhausted).
>>
>> Use jiffies_64 variant to mitigate this problem for 32bit Dom0.
>>
>> Suggested-by: Jan Beulich <jbeulich@suse.com>
>> Suggested-by: David Vrabel <david.vrabel@citrix.com>
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Jason Luan <jianhai.luan@oracle.com>
>> ---
>> drivers/net/xen-netback/common.h | 1 +
>> drivers/net/xen-netback/interface.c | 4 ++--
>> drivers/net/xen-netback/netback.c | 13 ++++++-------
>> 3 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/xen-netback/common.h
>> b/drivers/net/xen-netback/common.h
>> index 5715318..400fea1 100644
>> --- a/drivers/net/xen-netback/common.h
>> +++ b/drivers/net/xen-netback/common.h
>> @@ -163,6 +163,7 @@ struct xenvif {
>> unsigned long credit_usec;
>> unsigned long remaining_credit;
>> struct timer_list credit_timeout;
>> + u64 credit_window_start;
>> /* Statistics */
>> unsigned long rx_gso_checksum_fixup;
>> diff --git a/drivers/net/xen-netback/interface.c
>> b/drivers/net/xen-netback/interface.c
>> index 01bb854..8c45b63 100644
>> --- a/drivers/net/xen-netback/interface.c
>> +++ b/drivers/net/xen-netback/interface.c
>> @@ -312,8 +312,8 @@ struct xenvif *xenvif_alloc(struct device
>> *parent, domid_t domid,
>> vif->credit_bytes = vif->remaining_credit = ~0UL;
>> vif->credit_usec = 0UL;
>> init_timer(&vif->credit_timeout);
>> - /* Initialize 'expires' now: it's used to track the credit
>> window. */
>> - vif->credit_timeout.expires = jiffies;
>> + /* credit window is tracked in credit_window_start */
>> + vif->credit_window_start = get_jiffies_64();
>> dev->netdev_ops = &xenvif_netdev_ops;
>> dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO;
>> diff --git a/drivers/net/xen-netback/netback.c
>> b/drivers/net/xen-netback/netback.c
>> index f3e591c..1bc0688 100644
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -1185,18 +1185,17 @@ out:
>> static bool tx_credit_exceeded(struct xenvif *vif, unsigned size)
>> {
>> - unsigned long now = jiffies;
>> - unsigned long next_credit =
>> - vif->credit_timeout.expires +
>> - msecs_to_jiffies(vif->credit_usec / 1000);
>> + u64 now = get_jiffies_64();
>> + u64 next_credit = vif->credit_window_start +
>> + (u64)msecs_to_jiffies(vif->credit_usec / 1000);
>
> You simply replace "credit_timeout.expires" with
> "vif->credit_window_start" here, and never update
> "vif->credit_window_start" in following code.
>
>> /* Timer could already be pending in rare cases. */
>> if (timer_pending(&vif->credit_timeout))
>> return true;
>> /* Passed the point where we can replenish credit? */
>> - if (time_after_eq(now, next_credit)) {
>> - vif->credit_timeout.expires = now;
>> + if (time_after_eq64(now, next_credit)) {
>> + vif->credit_timeout.expires = (unsigned long)now;
>
> updates credit_window_start as following,
> vif->credit_window_start = (unsigned long)now;
both credit_window_start and credit_timeout.expires need to be updated
here,
vif->credit_window_start = (unsigned long)now;
vif->credit_timeout.expires = (unsigned long)now;
>
>> tx_add_credit(vif);
>> }
>> @@ -1207,7 +1206,7 @@ static bool tx_credit_exceeded(struct xenvif
>> *vif, unsigned size)
>> vif->credit_timeout.function =
>> tx_credit_callback;
>> mod_timer(&vif->credit_timeout,
>> - next_credit);
>> + (unsigned long)next_credit);
>
> vif->credit_timeout.expires is unsigned long, and this still causes
> original issue on 32bit system, which works well on 64bit. Or
> rewriting code to avoid uses of vif->credit_timeout.expires, but it is
> complex.
My understanding here is wrong, please ignore this.
Thanks
Annie
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] bonding: patchset for rcu use in bonding
From: Ding Tianhong @ 2013-10-28 3:02 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <20131028013428.GC11209@redhat.com>
On 2013/10/28 9:34, Veaceslav Falico wrote:
> On Mon, Oct 28, 2013 at 09:15:52AM +0800, Ding Tianhong wrote:
>> On 2013/10/28 6:53, Veaceslav Falico wrote:
>>> On Thu, Oct 24, 2013 at 11:08:35AM +0800, Ding Tianhong wrote:
>>>> Hi:
>>>>
>>>> The slave list will add and del by bond_master_upper_dev_link() and bond_upper_dev_unlink(),
>>>> which will call call_netdevice_notifiers(), even it is safe to call it in write bond lock now,
>>>> but we can't sure that whether it is safe later, because other drivers may deal NETDEV_CHANGEUPPER
>>>> in sleep way, so I didn't admit move the bond_upper_dev_unlink() in write bond lock.
>>>>
>>>> now the bond_for_each_slave only protect by rtnl_lock(), maybe use bond_for_each_slave_rcu is a good
>>>> way to protect slave list for bond, but as a system slow path, it is no need to transform bond_for_each_slave()
>>>> to bond_for_each_slave_rcu() in slow path, so in the patchset, I will remove the unused read bond lock
>>>> for monitor function, maybe it is a better way, I will wait to accept any relay for it.
>>>>
>>>> Thanks for the Veaceslav Falico opinion.
>>>>
>>>> v2: add and modify commit for patchset and patch, it will be the first step for the whole patchset.
>>>>
>>>> Ding Tianhong (5):
>>>> bonding: remove bond read lock for bond_mii_monitor()
>>>> bonding: remove bond read lock for bond_alb_monitor()
>>>> bonding: remove bond read lock for bond_loadbalance_arp_mon()
>>>> bonding: remove bond read lock for bond_activebackup_arp_mon()
>>>
>>> This patch introduces a regression by boot-test with active backup mode:
>>>
>>> bond_activebackup_arp_mon() is already not holding the bond->lock, however
>>> it might call bond_change_active_slave(), which does (in case of new_active):
>>>
>>> 912 write_unlock_bh(&bond->curr_slave_lock);
>>> 913 read_unlock(&bond->lock);
>>> 914 915 call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
>>> 916 if (should_notify_peers)
>>> 917 call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
>>> 918 bond->dev);
>>> 919 920 read_lock(&bond->lock);
>>> 921 write_lock_bh(&bond->curr_slave_lock);
>>>
>>> so it drops the bond->lock (which wasn't taken previously), and then takes
>>> it (without anyone dropping it afterwards).
>>>
>>> I don't know how to fix it - cause a lot of other callers already take it,
>>> and we can't just drop them (we'd race), and we can't remove it here (cause
>>> we can't call notifiers while atomic).
>>>
>>> Which begs the question - was this patchset tested at all?
>>>
>>> [ 21.796823] =====================================
>>> [ 21.796823] [ BUG: bad unlock balance detected! ]
> ... snip ...
>>
>> Hi David:
>> yes, exactly I miss it and make a mistake, the bond_select_active_slave is still have the protect problem and
>> need to be processed, I miss it, sorry, I will send a patch to fix the bug soon.
>>
>> Hi Veaceslav:
>> sorry about the commit, I will pay more attention to the commit and test, thanks for your advise and report the bug,
>> I have to admin that I was too careless.
>
> I'll ask you once again, even though it seems that my NACK doesn't block
> the patchset - try writing commit messages that actually describe why and
> how you do it.
>
> It's, actually, not only for the reviewers - it's also really good for you
> - cause while writing the commit log you also understand a lot more what
> are you doing, and might spot some corner cases (like this one).
>
> Sorry for being negative, however it costs me *much* more time to review
> patches without proper commit messages. I've done it once, twice, several
> times more - but that's it, I refuse to spend my time on your skipped
> homework.
>
> It might also help to split patches into really small steps - as in - do
> only one thing at a time, and describe it. This will help evade bugs *a
> lot*. It also helps people who'll bisect it, bugfix it and review it -
> because every patch will be a small, well-documented change, instead of a
> chunk of code with a description 'lets remove bond->lock'.
>
> Thank you and hope that helps.
>
sincere receiving all opinions.
thanks.
Regards.
Ding
>>
>>
>>
>>
>>>>
>>>> --
>>>> 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: [PATCH net-next v2 0/5] bonding: patchset for rcu use in bonding
From: David Miller @ 2013-10-28 4:00 UTC (permalink / raw)
To: vfalico; +Cc: dingtianhong, fubar, andy, nikolay, netdev
In-Reply-To: <20131027221027.GA11209@redhat.com>
From: Veaceslav Falico <vfalico@redhat.com>
Date: Sun, 27 Oct 2013 23:10:27 +0100
> On Sun, Oct 27, 2013 at 05:44:58PM -0400, David Miller wrote:
>>I would have preferred that he did all of this in the initial 0/N
>>patch posting, but I can't defer forever.
>
> Maybe I'm too picky. Anyway - understood, thanks.
I'm happy to revert his changes if you find real problems with them,
but I'd rather you two work more closely together in the future so I
don't have to break the deadlock like I did this time.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] bonding: patchset for rcu use in bonding
From: David Miller @ 2013-10-28 4:01 UTC (permalink / raw)
To: vfalico; +Cc: dingtianhong, fubar, andy, nikolay, netdev
In-Reply-To: <20131027225317.GB11209@redhat.com>
From: Veaceslav Falico <vfalico@redhat.com>
Date: Sun, 27 Oct 2013 23:53:17 +0100
> This patch introduces a regression by boot-test with active backup
> mode:
Ok I'm reverting.
^ permalink raw reply
* Re: [PATCH v2] pkt_sched: fq: clear time_next_packet for reused flows
From: David Miller @ 2013-10-28 4:19 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1382916403.4045.17.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 27 Oct 2013 16:26:43 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> When a socket is freed/reallocated, we need to clear time_next_packet
> or else we can inherit a prior value and delay first packets of the
> new flow.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2013-10-28 4:23 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Somnath Kotur, Sathya Perla
[-- Attachment #1: Type: text/plain, Size: 4320 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
drivers/net/ethernet/emulex/benet/be.h between commit e9e2a904ef0a
("be2net: Warn users of possible broken functionality on BE2 cards with
very old FW versions with latest driver") from the net tree and commit
6384a4d0dcf9 ("be2net: add support for ndo_busy_poll") from the net-next
tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/ethernet/emulex/benet/be.h
index c99dac6a9ddf,b2765ebb0268..000000000000
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@@ -696,23 -733,114 +733,123 @@@ static inline int qnq_async_evt_rcvd(st
return adapter->flags & BE_FLAGS_QNQ_ASYNC_EVT_RCVD;
}
+static inline int fw_major_num(const char *fw_ver)
+{
+ int fw_major = 0;
+
+ sscanf(fw_ver, "%d.", &fw_major);
+
+ return fw_major;
+}
+
- extern void be_cq_notify(struct be_adapter *adapter, u16 qid, bool arm,
- u16 num_popped);
- extern void be_link_status_update(struct be_adapter *adapter, u8 link_status);
- extern void be_parse_stats(struct be_adapter *adapter);
- extern int be_load_fw(struct be_adapter *adapter, u8 *func);
- extern bool be_is_wol_supported(struct be_adapter *adapter);
- extern bool be_pause_supported(struct be_adapter *adapter);
- extern u32 be_get_fw_log_level(struct be_adapter *adapter);
+ #ifdef CONFIG_NET_RX_BUSY_POLL
+ static inline bool be_lock_napi(struct be_eq_obj *eqo)
+ {
+ bool status = true;
+
+ spin_lock(&eqo->lock); /* BH is already disabled */
+ if (eqo->state & BE_EQ_LOCKED) {
+ WARN_ON(eqo->state & BE_EQ_NAPI);
+ eqo->state |= BE_EQ_NAPI_YIELD;
+ status = false;
+ } else {
+ eqo->state = BE_EQ_NAPI;
+ }
+ spin_unlock(&eqo->lock);
+ return status;
+ }
+
+ static inline void be_unlock_napi(struct be_eq_obj *eqo)
+ {
+ spin_lock(&eqo->lock); /* BH is already disabled */
+
+ WARN_ON(eqo->state & (BE_EQ_POLL | BE_EQ_NAPI_YIELD));
+ eqo->state = BE_EQ_IDLE;
+
+ spin_unlock(&eqo->lock);
+ }
+
+ static inline bool be_lock_busy_poll(struct be_eq_obj *eqo)
+ {
+ bool status = true;
+
+ spin_lock_bh(&eqo->lock);
+ if (eqo->state & BE_EQ_LOCKED) {
+ eqo->state |= BE_EQ_POLL_YIELD;
+ status = false;
+ } else {
+ eqo->state |= BE_EQ_POLL;
+ }
+ spin_unlock_bh(&eqo->lock);
+ return status;
+ }
+
+ static inline void be_unlock_busy_poll(struct be_eq_obj *eqo)
+ {
+ spin_lock_bh(&eqo->lock);
+
+ WARN_ON(eqo->state & (BE_EQ_NAPI));
+ eqo->state = BE_EQ_IDLE;
+
+ spin_unlock_bh(&eqo->lock);
+ }
+
+ static inline void be_enable_busy_poll(struct be_eq_obj *eqo)
+ {
+ spin_lock_init(&eqo->lock);
+ eqo->state = BE_EQ_IDLE;
+ }
+
+ static inline void be_disable_busy_poll(struct be_eq_obj *eqo)
+ {
+ local_bh_disable();
+
+ /* It's enough to just acquire napi lock on the eqo to stop
+ * be_busy_poll() from processing any queueus.
+ */
+ while (!be_lock_napi(eqo))
+ mdelay(1);
+
+ local_bh_enable();
+ }
+
+ #else /* CONFIG_NET_RX_BUSY_POLL */
+
+ static inline bool be_lock_napi(struct be_eq_obj *eqo)
+ {
+ return true;
+ }
+
+ static inline void be_unlock_napi(struct be_eq_obj *eqo)
+ {
+ }
+
+ static inline bool be_lock_busy_poll(struct be_eq_obj *eqo)
+ {
+ return false;
+ }
+
+ static inline void be_unlock_busy_poll(struct be_eq_obj *eqo)
+ {
+ }
+
+ static inline void be_enable_busy_poll(struct be_eq_obj *eqo)
+ {
+ }
+
+ static inline void be_disable_busy_poll(struct be_eq_obj *eqo)
+ {
+ }
+ #endif /* CONFIG_NET_RX_BUSY_POLL */
+
+ void be_cq_notify(struct be_adapter *adapter, u16 qid, bool arm,
+ u16 num_popped);
+ void be_link_status_update(struct be_adapter *adapter, u8 link_status);
+ void be_parse_stats(struct be_adapter *adapter);
+ int be_load_fw(struct be_adapter *adapter, u8 *func);
+ bool be_is_wol_supported(struct be_adapter *adapter);
+ bool be_pause_supported(struct be_adapter *adapter);
+ u32 be_get_fw_log_level(struct be_adapter *adapter);
int be_update_queues(struct be_adapter *adapter);
int be_poll(struct napi_struct *napi, int budget);
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] inet: restore gso for vxlan
From: David Miller @ 2013-10-28 4:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: ast, netdev
In-Reply-To: <1382923096.13037.10.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 27 Oct 2013 18:18:16 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> Alexei reported a performance regression on vxlan, caused
> by commit 3347c9602955 "ipv4: gso: make inet_gso_segment() stackable"
>
> GSO vxlan packets were not properly segmented, adding IP fragments
> while they were not expected.
>
> Rename 'bool tunnel' to 'bool encap', and add a new boolean
> to express the fact that UDP should be fragmented.
> This fragmentation is triggered by skb->encapsulation being set.
>
> Remove a "skb->encapsulation = 1" added in above commit,
> as its not needed, as frags inherit skb->frag from original
> GSO skb.
>
> Reported-by: Alexei Starovoitov <ast@plumgrid.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Alexei Starovoitov <ast@plumgrid.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
From: David Miller @ 2013-10-28 4:26 UTC (permalink / raw)
To: ben; +Cc: kevin.curtis, linux-kernel, netdev
In-Reply-To: <1382910704.2994.46.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 27 Oct 2013 21:51:44 +0000
> - dbg(DBG_TX, "In fst_tx_dma %p %p %d\n", skb, mem, len);
> + dbg(DBG_TX, "In fst_tx_dma %x %x %d\n", (u32)skb, mem, len);
Please use %p for the skb pointer instead of casting it (which btw
will introduce a warning on 64-bit).
^ permalink raw reply
* Re: [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers
From: David Miller @ 2013-10-28 4:26 UTC (permalink / raw)
To: ben; +Cc: venkat.x.venkatsubra, linux-kernel, rds-devel, netdev
In-Reply-To: <1382910856.2994.51.camel@deadeye.wl.decadent.org.uk>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 27 Oct 2013 21:54:16 +0000
> Most architectures define virt_to_page() as a macro that casts its
> argument such that an argument of type unsigned long will be accepted
> without complaint. However, the proper type is void *, and passing
> unsigned long results in a warning on MIPS.
>
> Compile-tested only.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
This looks fine:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH 1/3] vxlan: silence one build warning
From: David Miller @ 2013-10-28 4:38 UTC (permalink / raw)
To: stephen; +Cc: zwu.kernel, netdev, linux-kernel, wuzhy
In-Reply-To: <20131025084134.6cfa153a@samsung-9>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 25 Oct 2013 08:41:34 -0700
> I would rather not fix the warning this way since it risks masking
> later bugs if this code ever changes.
But this is suboptimally coded, and is asking for the warning.
Anything returning a pointer by reference is asking for trouble
in my opinion.
The correct thing to do is to make create_v{4,6}_sock() return
the "struct socket *" as an error pointer.
No more ambiguous initializations, no more warnings.
^ permalink raw reply
* Re: [PATCH 2/3] net, datagram: fix the uncorrect comment in zerocopy_sg_from_iovec()
From: David Miller @ 2013-10-28 4:39 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, linux-kernel, wuzhy
In-Reply-To: <1382687360-27794-2-git-send-email-zwu.kernel@gmail.com>
"uncorrect" is not a word, you mean to say "incorrect".
Same goes for patch #3.
^ permalink raw reply
* Re: Big performance loss from 3.4.63 to 3.10.13 when routing ipv4
From: David Miller @ 2013-10-28 4:43 UTC (permalink / raw)
To: steffen.klassert; +Cc: eric.dumazet, linux, hannes, netdev, klassert
In-Reply-To: <20131025092043.GD31491@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 25 Oct 2013 11:20:43 +0200
> On Fri, Oct 25, 2013 at 01:50:28AM -0700, Eric Dumazet wrote:
>>
>> 32768 as the default seems fine to me
>>
>> 448 bytes per dst -> thats less than 30 Mbytes of memory if we hit 65536
>> dst.
>>
>
> Ok, I'll add the patch below to to the ipsec tree if everyone is fine
> with that threshold.
No objections from me.
^ permalink raw reply
* Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
From: Ben Hutchings @ 2013-10-28 4:51 UTC (permalink / raw)
To: David Miller; +Cc: kevin.curtis, linux-kernel, netdev
In-Reply-To: <20131028.002627.2095063739326406659.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 743 bytes --]
On Mon, 2013-10-28 at 00:26 -0400, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Sun, 27 Oct 2013 21:51:44 +0000
>
> > - dbg(DBG_TX, "In fst_tx_dma %p %p %d\n", skb, mem, len);
> > + dbg(DBG_TX, "In fst_tx_dma %x %x %d\n", (u32)skb, mem, len);
>
> Please use %p for the skb pointer instead of casting it (which btw
> will introduce a warning on 64-bit).
skb is the DMA address of the data in the sk_buff. Yes, this is really
unusual naming.
Ben.
--
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ 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