* Re: [PATCH net-next] tcp: gso: fix truesize tracking
From: David Miller @ 2013-10-28 4:58 UTC (permalink / raw)
To: eric.dumazet; +Cc: ast, edumazet, stephen, netdev
In-Reply-To: <1382747177.4032.21.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 25 Oct 2013 17:26:17 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> commit 6ff50cd55545 ("tcp: gso: do not generate out of order packets")
> had an heuristic that can trigger a warning in skb_try_coalesce(),
> because skb->truesize of the gso segments were exactly set to mss.
>
> This breaks the requirement that
>
> skb->truesize >= skb->len + truesizeof(struct sk_buff);
>
> It can trivially be reproduced by :
>
> ifconfig lo mtu 1500
> ethtool -K lo tso off
> netperf
>
> As the skbs are looped into the TCP networking stack, skb_try_coalesce()
> warns us of these skb under-estimating their truesize.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
> Based on net-next but applies as well on net tree with some fuzz.
>
> David, we might backport this one to 3.12 and stable later, I let you
> decide.
I'm still thinking about what to do with this one.
^ permalink raw reply
* Re: [PATCH net-next] veth: extend features to support tunneling
From: David Miller @ 2013-10-28 4:58 UTC (permalink / raw)
To: eric.dumazet; +Cc: ast, edumazet, stephen, netdev
In-Reply-To: <1382750703.4032.32.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 25 Oct 2013 18:25:03 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> While investigating on a recent vxlan regression, I found veth
> was using a zero features set for vxlan tunnels.
>
> We have to segment GSO frames, copy the payload, and do the checksum.
>
> This patch brings a ~200% performance increase
>
> We probably have to add hw_enc_features support
> on other virtual devices.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ 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
* 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/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: [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 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 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 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
* 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 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
* 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 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: 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: [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] 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
* [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
* [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
* 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
* 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: [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
* [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-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
* 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: [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
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