* [PATCH 10/28] net: ethernet: stmicro: stmmac: drop owner assignment from platform_drivers
From: Wolfram Sang @ 2014-12-21 21:14 UTC (permalink / raw)
To: linux-kernel; +Cc: Wolfram Sang, Giuseppe Cavallaro, netdev
In-Reply-To: <1419196495-9626-1-git-send-email-wsa@the-dreams.de>
This platform_driver does not need to set an owner, it will be populated by the
driver core.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
Generated with coccinelle. SmPL file is in the introductory msg. The big
cleanup was pulled in this merge window. This series catches the bits fallen
through. The patches shall go in via the subsystem trees.
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 4032b170fe24..3039de2465ba 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -430,7 +430,6 @@ static struct platform_driver stmmac_pltfr_driver = {
.remove = stmmac_pltfr_remove,
.driver = {
.name = STMMAC_RESOURCE_NAME,
- .owner = THIS_MODULE,
.pm = &stmmac_pltfr_pm_ops,
.of_match_table = of_match_ptr(stmmac_dt_ids),
},
--
2.1.3
^ permalink raw reply related
* [PATCH 11/28] net: wireless: ath: ath5k: drop owner assignment from platform_drivers
From: Wolfram Sang @ 2014-12-21 21:14 UTC (permalink / raw)
To: linux-kernel
Cc: Wolfram Sang, Jiri Slaby, Nick Kossifidis, Luis R. Rodriguez,
Kalle Valo, linux-wireless, ath5k-devel, netdev
In-Reply-To: <1419196495-9626-1-git-send-email-wsa@the-dreams.de>
This platform_driver does not need to set an owner, it will be populated by the
driver core.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
Generated with coccinelle. SmPL file is in the introductory msg. The big
cleanup was pulled in this merge window. This series catches the bits fallen
through. The patches shall go in via the subsystem trees.
drivers/net/wireless/ath/ath5k/ahb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
index 8f387cf67340..2ca88b593e4c 100644
--- a/drivers/net/wireless/ath/ath5k/ahb.c
+++ b/drivers/net/wireless/ath/ath5k/ahb.c
@@ -227,7 +227,6 @@ static struct platform_driver ath_ahb_driver = {
.remove = ath_ahb_remove,
.driver = {
.name = "ar231x-wmac",
- .owner = THIS_MODULE,
},
};
--
2.1.3
^ permalink raw reply related
* Re: [BUG] rtl8192se: panic accessing unmapped memory in skb
From: Larry Finger @ 2014-12-21 23:02 UTC (permalink / raw)
To: Eric Biggers, linux-wireless; +Cc: netdev, linux-kernel
In-Reply-To: <20141221172516.GA12784@zzz>
[-- Attachment #1: Type: text/plain, Size: 2339 bytes --]
On 12/21/2014 11:25 AM, Eric Biggers wrote:
> Hi,
>
> I have a RTL8192SE wireless card, attached via PCI. Usually it works with no
> issues, but I recently had a kernel panic occur in the rtl8192se driver. The
> kernel version is 3.18. Based on my analysis of the panic dump, the panic was
> caused by a memory access violation in this block of code in
> rtl92se_rx_query_desc():
>
> if (stats->decrypted) {
> hdr = (struct ieee80211_hdr *)(skb->data +
> stats->rx_drvinfo_size + stats->rx_bufshift);
>
> if ((_ieee80211_is_robust_mgmt_frame(hdr)) &&
> (ieee80211_has_protected(hdr->frame_control)))
> rx_status->flag &= ~RX_FLAG_DECRYPTED;
> else
> rx_status->flag |= RX_FLAG_DECRYPTED;
> }
>
> Specifically, the violation occurred the first time hdr->frame_control was
> accessed, as part of _ieee80211_is_robust_mgmt_frame().
>
> The panic occurred when the system was under heavy filesystem load but seemingly
> is not easily reproducible.
>
> There was recently a NULL check that was removed from this exact place in the
> code, but it was certainly useless. Instead, what's much more suspect to me is
> that inside _rtl_pci_rx_interrupt(), there is no error checking of the return
> value of _rtl_pci_init_one_rxdesc(), which might fail if the skb couldn't be
> allocated. I am wondering if this could be causing the problem.
Your analysis is probably correct; however, I'm not sure what to do if the
allocate of an skb fails. As the name says, this routine is entered through an
interrupt, and I'm not sure what to do other than to exit.
The attached patch will implement the exit after logging an error. Please patch
your system and report back.
How much RAM does your system have? That info might be useful in trying to
reproduce the problem, which might indeed be difficult. Although pci.c was
extensively reworked in the 3.17 => 3.18 transition, most of the changes were
added to implement the changed descriptor structure for the RTL8192EE, and I do
not remember any changes that would affect any of the other drivers. As a
result, the current structure has been in place for some time, and this problem
has not been reported before.
Larry
[-- Attachment #2: rtlwifi_report_add_new_rxdesc_failure --]
[-- Type: text/plain, Size: 2180 bytes --]
diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
index 846a2e6..c576c71 100644
--- a/drivers/net/wireless/rtlwifi/pci.c
+++ b/drivers/net/wireless/rtlwifi/pci.c
@@ -676,7 +676,7 @@ static int _rtl_pci_init_one_rxdesc(struct ieee80211_hw *hw,
skb = dev_alloc_skb(rtlpci->rxbuffersize);
if (!skb)
- return 0;
+ return -ENOMEM;;
rtlpci->rx_ring[rxring_idx].rx_buf[desc_idx] = skb;
/* just set skb->cb to mapping addr for pci_unmap_single use */
@@ -685,7 +685,7 @@ static int _rtl_pci_init_one_rxdesc(struct ieee80211_hw *hw,
rtlpci->rxbuffersize, PCI_DMA_FROMDEVICE);
bufferaddress = *((dma_addr_t *)skb->cb);
if (pci_dma_mapping_error(rtlpci->pdev, bufferaddress))
- return 0;
+ return -EFAULT;
if (rtlpriv->use_new_trx_flow) {
rtlpriv->cfg->ops->set_desc(hw, (u8 *)entry, false,
HW_DESC_RX_PREPARE,
@@ -701,7 +701,7 @@ static int _rtl_pci_init_one_rxdesc(struct ieee80211_hw *hw,
HW_DESC_RXOWN,
(u8 *)&tmp_one);
}
- return 1;
+ return 0;
}
/* inorder to receive 8K AMSDU we have set skb to
@@ -768,6 +768,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
.signal = 0,
.rate = 0,
};
+ int err;
/*RX NORMAL PKT */
while (count--) {
@@ -912,13 +913,21 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
}
end:
if (rtlpriv->use_new_trx_flow) {
- _rtl_pci_init_one_rxdesc(hw, (u8 *)buffer_desc,
- rxring_idx,
- rtlpci->rx_ring[rxring_idx].idx);
+ err = _rtl_pci_init_one_rxdesc(hw, (u8 *)buffer_desc,
+ rxring_idx,
+ rtlpci->rx_ring[rxring_idx].idx);
+ if (err) {
+ pr_err("%s Failed to init RX descriptor\n");
+ return;
+ }
} else {
- _rtl_pci_init_one_rxdesc(hw, (u8 *)pdesc, rxring_idx,
- rtlpci->rx_ring[rxring_idx].idx);
-
+ err = _rtl_pci_init_one_rxdesc(hw, (u8 *)pdesc,
+ rxring_idx,
+ rtlpci->rx_ring[rxring_idx].idx);
+ if (err) {
+ pr_err("%s Failed to init RX descriptor\n");
+ return;
+ }
if (rtlpci->rx_ring[rxring_idx].idx ==
rtlpci->rxringcount - 1)
rtlpriv->cfg->ops->set_desc(hw, (u8 *)pdesc,
^ permalink raw reply related
* Re: [BUG] rtl8192se: panic accessing unmapped memory in skb
From: Eric Biggers @ 2014-12-21 23:47 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <5497516A.2020400@lwfinger.net>
Hi,
To get your patched version to work at all I had to update
_rtl_pci_init_rx_ring() to account for new return value of
_rtl_pci_init_one_rxdesc(). I will let you know if anything shows up in the
kernel log, but I expect this is a highly sporadic problem. The system has 4 GB
of memory, and I used the 3.18 kernel for 10 days prior to the panic with no
issues. The panic occurred while upgrading system packages, so it's possible
jhat the system was experiencing memory pressure.
I upgraded from 3.17 to 3.18 on Dec 8, so I've actually only had since then to
notice any bugs that may have been introduced since 3.17.
It does appear there were changes made to pci.c between 3.17 and 3.18. It
appears the 3.17 code will drop the incoming packet if a new skb can't be
allocated, whereas the 3.18 code assumes a new skb can always be allocated. The
3.17 behavior seems more logical to me. I don't know how either of these
behaviors compare to other networking drivers, however.
Eric
^ permalink raw reply
* [PATCH] [PATCH v3 1/2] 8139too: Fix the lack of pci_disable_device
From: Jia-Ju Bai @ 2014-12-22 0:28 UTC (permalink / raw)
To: davem, sergei.shtylyov, netdev; +Cc: jgarzik, Jia-Ju Bai
For linux-3.18.0
When pci_request_regions is failed in rtl8139_init_board, pci_disable_device
is not called to disable the device which are enabled by pci_enable_device,
because of disable_dev_on_err is not assigned 1.
This patch fix this problem.
Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
drivers/net/ethernet/realtek/8139too.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 007b38c..49bbcf3 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -783,10 +783,10 @@ static struct net_device *rtl8139_init_board(struct pci_dev *pdev)
if (rc)
goto err_out;
+ disable_dev_on_err = 1;
rc = pci_request_regions (pdev, DRV_NAME);
if (rc)
goto err_out;
- disable_dev_on_err = 1;
pci_set_master (pdev);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 2/2] 8139too: Add netif_napi_del in the driver
From: Jia-Ju Bai @ 2014-12-22 0:43 UTC (permalink / raw)
To: davem, sergei.shtylyov, netdev; +Cc: jgarzik, Jia-Ju Bai
For linux-3.18.0
The driver lacks netif_napi_del in the normal path and error path
to match the call of netif_napi_add in rtl8139_init_one.
This patch fixes this problem.
Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
drivers/net/ethernet/realtek/8139too.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 007b38c..80fe3fb 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -1098,6 +1098,7 @@ static int rtl8139_init_one(struct pci_dev *pdev,
return 0;
err_out:
+ netif_napi_del(&tp->napi);
__rtl8139_cleanup_dev (dev);
pci_disable_device (pdev);
return i;
@@ -1112,6 +1113,7 @@ static void rtl8139_remove_one(struct pci_dev *pdev)
assert (dev != NULL);
cancel_delayed_work_sync(&tp->thread);
+ netif_napi_del(&tp->napi);
unregister_netdev (dev);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 1/2] e100: Fix a null pointer deference in e100_probe
From: Jia-Ju Bai @ 2014-12-22 0:59 UTC (permalink / raw)
To: davem, sergei.shtylyov, netdev; +Cc: e1000-devel, Jia-Ju Bai, linux.nics
For linux-3.18.0
The driver lacks the check of nic->cbs_pool after pci_pool_create
in e100_probe. So when this function is failed, the null pointer
dereference occurs when pci_pool_alloc uses nic->cbs_pool in e100_alloc_cbs.
This patch fixes this problem, and it has been tested in runtime.
Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
drivers/net/ethernet/intel/e100.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 781065e..ba1813f 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -2969,6 +2969,10 @@ static int e100_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
nic->params.cbs.max * sizeof(struct cb),
sizeof(u32),
0);
+ if (!nic->cbs_pool) {
+ err = -ENOMEM;
+ goto err_out_pool;
+ }
netif_info(nic, probe, nic->netdev,
"addr 0x%llx, irq %d, MAC addr %pM\n",
(unsigned long long)pci_resource_start(pdev, use_io ? 1 : 0),
@@ -2976,6 +2980,8 @@ static int e100_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return 0;
+err_out_pool:
+ unregister_netdev(netdev);
err_out_free:
e100_free(nic);
err_out_iounmap:
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related
* Re: [PATCH] bonding: avoid re-entry of bond_release
From: Wengang @ 2014-12-22 1:09 UTC (permalink / raw)
To: Ding Tianhong, Andy Gospodarek; +Cc: netdev
In-Reply-To: <54962A00.6080005@huawei.com>
Hi Andy and Ding,
Thanks for your reviews!
In the ioctl path, removing a interface that is not currently actually a
slave
can happen from user space(by mistake), we should avoid the noisy message.
While, __bond_release_one() has another call path which is from
bond_uninit().
In the later case, it should be treated as an error if the interface is
not with
IFF_SLAVE flag. To notice that error occurred, the message is printed. I
think
the message is needed for this path.
How do you think?
thanks,
wengang
于 2014年12月21日 10:01, Ding Tianhong 写道:
> On 2014/12/19 23:11, Andy Gospodarek wrote:
>> On Fri, Dec 19, 2014 at 04:56:57PM +0800, Wengang Wang wrote:
>>> If bond_release is run against an interface which is already detached from
>>> it's master, then there is an error message shown like
>>> "<master name> cannot release <slave name>".
>>>
>>> The call path is:
>>> bond_do_ioctl()
>>> bond_release()
>>> __bond_release_one()
>>>
>>> Though it does not really harm, the message the message is misleading.
>>> This patch tries to avoid the message.
>>>
>>> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
>>> ---
>>> drivers/net/bonding/bond_main.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>> index 184c434..4a71bbd 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -3256,7 +3256,10 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
>>> break;
>>> case BOND_RELEASE_OLD:
>>> case SIOCBONDRELEASE:
>>> - res = bond_release(bond_dev, slave_dev);
>>> + if (slave_dev->flags & IFF_SLAVE)
>>> + res = bond_release(bond_dev, slave_dev);
>>> + else
>>> + res = 0;
>> Functionally this patch is fine, but I would prefer that you simply
>> change the check in __bond_release_one to not be so noisy. There is a
>> check[1] in bond_enslave to see if a slave is already in a bond and that
>> just prints a message of netdev_dbg (rather than netdev_err) and it
>> seems that would be appropriate for this type of message.
>>
>> [1] from bond_enslave():
>>
>> /* already enslaved */
>> if (slave_dev->flags & IFF_SLAVE) {
>> netdev_dbg(bond_dev, "Error: Device was already enslaved\n");
>> return -EBUSY;
>> }
>>
>>
>>> break;
>>> case BOND_SETHWADDR_OLD:
>>> case SIOCBONDSETHWADDR:
>>> --
> agree ,use netdev_dbg looks more better and enough.
>
> Ding
>
>
^ permalink raw reply
* Re: [PATCH] bonding: move ipoib_header_ops to vmlinux
From: Wengang @ 2014-12-22 1:14 UTC (permalink / raw)
To: David Miller, jay.vosburgh; +Cc: ogerlitz, netdev, linux-rdma
In-Reply-To: <548E3595.60206@oracle.com>
Hi, Anyone please review this patch? David? Jay? please.
thanks,
wengang
于 2014年12月15日 09:12, Wengang 写道:
> Anyone please respond to this?
>
> thanks,
> wengang
>
> 于 2014年12月03日 09:50, Wengang Wang 写道:
>> Hi David and Jay,
>>
>> Then about about the change in this patch?
>>
>> thanks,
>> wengang
>>
>> 在 2014年11月26日 09:30, Wengang 写道:
>>> 于 2014年11月26日 02:44, David Miller 写道:
>>>> From: Jay Vosburgh <jay.vosburgh@canonical.com>
>>>> Date: Tue, 25 Nov 2014 10:41:17 -0800
>>>>
>>>>> Or Gerlitz <ogerlitz@mellanox.com> wrote:
>>>>>
>>>>>> On 11/25/2014 8:07 AM, David Miller wrote:
>>>>>>> IPOIB should not work over bonding as it requires that the device
>>>>>>> use ARPHRD_ETHER.
>>>>>> Hi Dave,
>>>>>>
>>>>>> IPoIB devices can be enslaved to both bonding and teaming in
>>>>>> their HA mode,
>>>>>> the bond device type becomes ARPHRD_INFINIBAND when this happens.
>>>>> The point was that pktgen disallows ARPHRD_INFINIBAND, not that
>>>>> bonding does.
>>>>>
>>>>> Pktgen specifically checks for type != ARPHRD_ETHER, so the
>>>>> IPoIB bond should not be able to be used with pkgten. My
>>>>> suspicion is
>>>>> that pktgen is being configured on the bond first, then an IPoIB
>>>>> slave
>>>>> is added to the bond; this would change its type in a way that pktgen
>>>>> wouldn't notice.
>>>> +1
>>>
>>> I think it go this way:
>>>
>>> 1) bond_master is ready
>>> 2) bond_enslave enslave a IPOIB interface calling bond_setup_by_slave
>>> 3) then bond_setup_by_slave set change master type to
>>> ARPHRD_INFINIBAND.
>>>
>>> code is like this:
>>>
>>> 1 /* enslave device <slave> to bond device <master> */
>>> 2 int bond_enslave(struct net_device *bond_dev, struct net_device
>>> *slave_dev)
>>> 3 {
>>> 4 <snip>...
>>> 5 /* set bonding device ether type by slave - bonding netdevices are
>>> 6 * created with ether_setup, so when the slave type is not
>>> ARPHRD_ETHER
>>> 7 * there is a need to override some of the type dependent
>>> attribs/funcs.
>>> 8 *
>>> 9 * bond ether type mutual exclusion - don't allow slaves of dissimilar
>>> 10 * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the
>>> same bond
>>> 11 */
>>> 12 if (!bond_has_slaves(bond)) {
>>> 13 if (bond_dev->type != slave_dev->type) {
>>> 14 <snip>...
>>> 15 if (slave_dev->type != ARPHRD_ETHER)
>>> 16 bond_setup_by_slave(bond_dev, slave_dev);
>>> 17 else {
>>> 18 ether_setup(bond_dev);
>>> 19 bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
>>> 20 }
>>> 21
>>> 22 call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE,
>>> 23 bond_dev);
>>> 24 }
>>> 25 <snip>...
>>> 26 }
>>> 27
>>> 28 static void bond_setup_by_slave(struct net_device *bond_dev,
>>> 29 struct net_device *slave_dev)
>>> 30 {
>>> 31 bond_dev->header_ops = slave_dev->header_ops;
>>> 32
>>> 33 bond_dev->type = slave_dev->type;
>>> 34 bond_dev->hard_header_len = slave_dev->hard_header_len;
>>> 35 bond_dev->addr_len = slave_dev->addr_len;
>>> 36
>>> 37 memcpy(bond_dev->broadcast, slave_dev->broadcast,
>>> 38 slave_dev->addr_len);
>>> 39 }
>>> 40
>>>
>>> thanks
>>> wengang
>>> --
>
^ permalink raw reply
* [PATCH v3 2/2] e100: Add netif_napi_add in the driver
From: Jia-Ju Bai @ 2014-12-22 1:53 UTC (permalink / raw)
To: davem, sergei.shtylyov, netdev; +Cc: e1000-devel, Jia-Ju Bai, linux.nics
For linux-3.18.0
The driver lacks netif_napi_del in the normal path and error path to
match the call of netif_napi_add in e100_probe.
This patch fixes this problem, and it has been tested in runtime.
Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
drivers/net/ethernet/intel/e100.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 781065e..21c4d0f 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -2985,6 +2985,7 @@ err_out_free_res:
err_out_disable_pdev:
pci_disable_device(pdev);
err_out_free_dev:
+ netif_napi_del(&nic->napi);
free_netdev(netdev);
return err;
}
@@ -2995,6 +2996,7 @@ static void e100_remove(struct pci_dev *pdev)
if (netdev) {
struct nic *nic = netdev_priv(netdev);
+ netif_napi_del(&nic->napi);
unregister_netdev(netdev);
e100_free(nic);
pci_iounmap(pdev, nic->csr);
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related
* Re: [PATCH] bonding: avoid re-entry of bond_release
From: Ding Tianhong @ 2014-12-22 2:05 UTC (permalink / raw)
To: Wengang, Andy Gospodarek; +Cc: netdev
In-Reply-To: <54976F52.30403@oracle.com>
On 2014/12/22 9:09, Wengang wrote:
> Hi Andy and Ding,
>
> Thanks for your reviews!
> In the ioctl path, removing a interface that is not currently actually a slave
> can happen from user space(by mistake), we should avoid the noisy message.
>
> While, __bond_release_one() has another call path which is from bond_uninit().
> In the later case, it should be treated as an error if the interface is not with
> IFF_SLAVE flag. To notice that error occurred, the message is printed. I think
> the message is needed for this path.
>
> How do you think?
>
Just like the bond_enslave(), it is only a warning.
Ding
> thanks,
> wengang
>
> 于 2014年12月21日 10:01, Ding Tianhong 写道:
>> On 2014/12/19 23:11, Andy Gospodarek wrote:
>>> On Fri, Dec 19, 2014 at 04:56:57PM +0800, Wengang Wang wrote:
>>>> If bond_release is run against an interface which is already detached from
>>>> it's master, then there is an error message shown like
>>>> "<master name> cannot release <slave name>".
>>>>
>>>> The call path is:
>>>> bond_do_ioctl()
>>>> bond_release()
>>>> __bond_release_one()
>>>>
>>>> Though it does not really harm, the message the message is misleading.
>>>> This patch tries to avoid the message.
>>>>
>>>> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
>>>> ---
>>>> drivers/net/bonding/bond_main.c | 5 ++++-
>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>>> index 184c434..4a71bbd 100644
>>>> --- a/drivers/net/bonding/bond_main.c
>>>> +++ b/drivers/net/bonding/bond_main.c
>>>> @@ -3256,7 +3256,10 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
>>>> break;
>>>> case BOND_RELEASE_OLD:
>>>> case SIOCBONDRELEASE:
>>>> - res = bond_release(bond_dev, slave_dev);
>>>> + if (slave_dev->flags & IFF_SLAVE)
>>>> + res = bond_release(bond_dev, slave_dev);
>>>> + else
>>>> + res = 0;
>>> Functionally this patch is fine, but I would prefer that you simply
>>> change the check in __bond_release_one to not be so noisy. There is a
>>> check[1] in bond_enslave to see if a slave is already in a bond and that
>>> just prints a message of netdev_dbg (rather than netdev_err) and it
>>> seems that would be appropriate for this type of message.
>>>
>>> [1] from bond_enslave():
>>>
>>> /* already enslaved */
>>> if (slave_dev->flags & IFF_SLAVE) {
>>> netdev_dbg(bond_dev, "Error: Device was already enslaved\n");
>>> return -EBUSY;
>>> }
>>>
>>>
>>>> break;
>>>> case BOND_SETHWADDR_OLD:
>>>> case SIOCBONDSETHWADDR:
>>>> --
>> agree ,use netdev_dbg looks more better and enough.
>>
>> Ding
>>
>>
>
>
> .
>
^ permalink raw reply
* RE: [PATCH net] r8152: drop the tx packet with invalid length
From: Hayes Wang @ 2014-12-22 2:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, David Miller, netdev@vger.kernel.org, nic_swsd,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
In-Reply-To: <1419012837.9773.85.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Saturday, December 20, 2014 2:14 AM
[...]
> Could you try following patch ?
Thank you. I would test it.
Best Regards,
Hayes
^ permalink raw reply
* [PATCH 1/2] MIPS: Hibernate: flush TLB entries earlier
From: Huacai Chen @ 2014-12-22 2:28 UTC (permalink / raw)
To: Giuseppe Cavallaro
Cc: Srinivas Kandagatla, David S. Miller, netdev, Huacai Chen, stable
We found that TLB mismatch not only happens after kernel resume, but
also happens during snapshot restore. So move it to the beginning of
swsusp_arch_suspend().
Cc: <stable@vger.kernel.org>
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
arch/mips/power/hibernate.S | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/mips/power/hibernate.S b/arch/mips/power/hibernate.S
index 32a7c82..e7567c8 100644
--- a/arch/mips/power/hibernate.S
+++ b/arch/mips/power/hibernate.S
@@ -30,6 +30,8 @@ LEAF(swsusp_arch_suspend)
END(swsusp_arch_suspend)
LEAF(swsusp_arch_resume)
+ /* Avoid TLB mismatch during and after kernel resume */
+ jal local_flush_tlb_all
PTR_L t0, restore_pblist
0:
PTR_L t1, PBE_ADDRESS(t0) /* source */
@@ -43,7 +45,6 @@ LEAF(swsusp_arch_resume)
bne t1, t3, 1b
PTR_L t0, PBE_NEXT(t0)
bnez t0, 0b
- jal local_flush_tlb_all /* Avoid TLB mismatch after kernel resume */
PTR_LA t0, saved_regs
PTR_L ra, PT_R31(t0)
PTR_L sp, PT_R29(t0)
--
1.7.7.3
^ permalink raw reply related
* [PATCH 2/2] MIPS: Hibernate: Restructure files and functions
From: Huacai Chen @ 2014-12-22 2:28 UTC (permalink / raw)
To: Giuseppe Cavallaro
Cc: Srinivas Kandagatla, David S. Miller, netdev, Huacai Chen
In-Reply-To: <1419215296-27831-1-git-send-email-chenhc@lemote.com>
This patch has no functional changes, it just to keep the assembler
code to a minimum. Files and functions naming is borrowed from X86.
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
arch/mips/power/Makefile | 2 +-
arch/mips/power/hibernate.c | 10 ++++++++++
arch/mips/power/{hibernate.S => hibernate_asm.S} | 6 ++----
3 files changed, 13 insertions(+), 5 deletions(-)
create mode 100644 arch/mips/power/hibernate.c
rename arch/mips/power/{hibernate.S => hibernate_asm.S} (90%)
diff --git a/arch/mips/power/Makefile b/arch/mips/power/Makefile
index 73d56b8..70bd788 100644
--- a/arch/mips/power/Makefile
+++ b/arch/mips/power/Makefile
@@ -1 +1 @@
-obj-$(CONFIG_HIBERNATION) += cpu.o hibernate.o
+obj-$(CONFIG_HIBERNATION) += cpu.o hibernate.o hibernate_asm.o
diff --git a/arch/mips/power/hibernate.c b/arch/mips/power/hibernate.c
new file mode 100644
index 0000000..19a9af6
--- /dev/null
+++ b/arch/mips/power/hibernate.c
@@ -0,0 +1,10 @@
+#include <asm/tlbflush.h>
+
+extern int restore_image(void);
+
+int swsusp_arch_resume(void)
+{
+ /* Avoid TLB mismatch during and after kernel resume */
+ local_flush_tlb_all();
+ return restore_image();
+}
diff --git a/arch/mips/power/hibernate.S b/arch/mips/power/hibernate_asm.S
similarity index 90%
rename from arch/mips/power/hibernate.S
rename to arch/mips/power/hibernate_asm.S
index e7567c8..b1fab95 100644
--- a/arch/mips/power/hibernate.S
+++ b/arch/mips/power/hibernate_asm.S
@@ -29,9 +29,7 @@ LEAF(swsusp_arch_suspend)
j swsusp_save
END(swsusp_arch_suspend)
-LEAF(swsusp_arch_resume)
- /* Avoid TLB mismatch during and after kernel resume */
- jal local_flush_tlb_all
+LEAF(restore_image)
PTR_L t0, restore_pblist
0:
PTR_L t1, PBE_ADDRESS(t0) /* source */
@@ -60,4 +58,4 @@ LEAF(swsusp_arch_resume)
PTR_L s7, PT_R23(t0)
PTR_LI v0, 0x0
jr ra
-END(swsusp_arch_resume)
+END(restore_image)
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH 1/2] MIPS: Hibernate: flush TLB entries earlier
From: Huacai Chen @ 2014-12-22 2:31 UTC (permalink / raw)
To: Giuseppe Cavallaro
Cc: Srinivas Kandagatla, David S. Miller, netdev, Huacai Chen, stable
In-Reply-To: <1419215296-27831-1-git-send-email-chenhc@lemote.com>
Sorry, send to a wrong place...
On Mon, Dec 22, 2014 at 10:28 AM, Huacai Chen <chenhc@lemote.com> wrote:
> We found that TLB mismatch not only happens after kernel resume, but
> also happens during snapshot restore. So move it to the beginning of
> swsusp_arch_suspend().
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
> arch/mips/power/hibernate.S | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/arch/mips/power/hibernate.S b/arch/mips/power/hibernate.S
> index 32a7c82..e7567c8 100644
> --- a/arch/mips/power/hibernate.S
> +++ b/arch/mips/power/hibernate.S
> @@ -30,6 +30,8 @@ LEAF(swsusp_arch_suspend)
> END(swsusp_arch_suspend)
>
> LEAF(swsusp_arch_resume)
> + /* Avoid TLB mismatch during and after kernel resume */
> + jal local_flush_tlb_all
> PTR_L t0, restore_pblist
> 0:
> PTR_L t1, PBE_ADDRESS(t0) /* source */
> @@ -43,7 +45,6 @@ LEAF(swsusp_arch_resume)
> bne t1, t3, 1b
> PTR_L t0, PBE_NEXT(t0)
> bnez t0, 0b
> - jal local_flush_tlb_all /* Avoid TLB mismatch after kernel resume */
> PTR_LA t0, saved_regs
> PTR_L ra, PT_R31(t0)
> PTR_L sp, PT_R29(t0)
> --
> 1.7.7.3
>
^ permalink raw reply
* Re: [PATCH 2/2] MIPS: Hibernate: Restructure files and functions
From: Huacai Chen @ 2014-12-22 2:32 UTC (permalink / raw)
To: Giuseppe Cavallaro
Cc: Srinivas Kandagatla, David S. Miller, netdev, Huacai Chen
In-Reply-To: <1419215296-27831-2-git-send-email-chenhc@lemote.com>
Sorry, send to a wrong place...
On Mon, Dec 22, 2014 at 10:28 AM, Huacai Chen <chenhc@lemote.com> wrote:
> This patch has no functional changes, it just to keep the assembler
> code to a minimum. Files and functions naming is borrowed from X86.
>
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
> arch/mips/power/Makefile | 2 +-
> arch/mips/power/hibernate.c | 10 ++++++++++
> arch/mips/power/{hibernate.S => hibernate_asm.S} | 6 ++----
> 3 files changed, 13 insertions(+), 5 deletions(-)
> create mode 100644 arch/mips/power/hibernate.c
> rename arch/mips/power/{hibernate.S => hibernate_asm.S} (90%)
>
> diff --git a/arch/mips/power/Makefile b/arch/mips/power/Makefile
> index 73d56b8..70bd788 100644
> --- a/arch/mips/power/Makefile
> +++ b/arch/mips/power/Makefile
> @@ -1 +1 @@
> -obj-$(CONFIG_HIBERNATION) += cpu.o hibernate.o
> +obj-$(CONFIG_HIBERNATION) += cpu.o hibernate.o hibernate_asm.o
> diff --git a/arch/mips/power/hibernate.c b/arch/mips/power/hibernate.c
> new file mode 100644
> index 0000000..19a9af6
> --- /dev/null
> +++ b/arch/mips/power/hibernate.c
> @@ -0,0 +1,10 @@
> +#include <asm/tlbflush.h>
> +
> +extern int restore_image(void);
> +
> +int swsusp_arch_resume(void)
> +{
> + /* Avoid TLB mismatch during and after kernel resume */
> + local_flush_tlb_all();
> + return restore_image();
> +}
> diff --git a/arch/mips/power/hibernate.S b/arch/mips/power/hibernate_asm.S
> similarity index 90%
> rename from arch/mips/power/hibernate.S
> rename to arch/mips/power/hibernate_asm.S
> index e7567c8..b1fab95 100644
> --- a/arch/mips/power/hibernate.S
> +++ b/arch/mips/power/hibernate_asm.S
> @@ -29,9 +29,7 @@ LEAF(swsusp_arch_suspend)
> j swsusp_save
> END(swsusp_arch_suspend)
>
> -LEAF(swsusp_arch_resume)
> - /* Avoid TLB mismatch during and after kernel resume */
> - jal local_flush_tlb_all
> +LEAF(restore_image)
> PTR_L t0, restore_pblist
> 0:
> PTR_L t1, PBE_ADDRESS(t0) /* source */
> @@ -60,4 +58,4 @@ LEAF(swsusp_arch_resume)
> PTR_L s7, PT_R23(t0)
> PTR_LI v0, 0x0
> jr ra
> -END(swsusp_arch_resume)
> +END(restore_image)
> --
> 1.7.7.3
>
^ permalink raw reply
* RE: [PATCH net-next 2/2] r8152: check the status before submittingrx
From: Hayes Wang @ 2014-12-22 2:53 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
In-Reply-To: <20141219.154402.54691026682141762.davem@davemloft.net>
David Miller [mailto:davem@davemloft.net]
> Sent: Saturday, December 20, 2014 4:44 AM
[...]
> > Don't submit the rx if the device is unplugged, linking down,
> > or stopped.
> ...
> > @@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152
> *tp, struct rx_agg *agg, gfp_t mem_flags)
> > {
> > int ret;
> >
> > + /* The rx would be stopped, so skip submitting */
> > + if (test_bit(RTL8152_UNPLUG, &tp->flags) ||
> > + !test_bit(WORK_ENABLE, &tp->flags) || !(tp->speed & LINK_STATUS))
> > + return 0;
> > +
>
> I think netif_carrier_off() should always be true in all three of those
> situations, and would be a much simpler test than what you've coded
> here.
When the device is unplugged or stopped, the linking status
may be true, so I add additional checks to avoid the submission.
Besides, in set_carrier() I set netif_carrier_on() after
ops.enable() to avoid any transmission before I finish
starting the tx/rx.
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, &tp->flags);
netif_carrier_on(netdev);
However, the r8152_submit_rx() would be called in ops.enable(),
and the check of netif_carrier_ok() would be always false. That
is why I use tp->speed, not netif_carrier_ok(), to check the
linking stauts.
Best Regards,
Hayes
^ permalink raw reply
* Re: SRIOV as bridge Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Jamal Hadi Salim @ 2014-12-22 2:59 UTC (permalink / raw)
To: John Fastabend, Roopa Prabhu
Cc: Hubert Sokolowski, netdev@vger.kernel.org, Vlad Yasevich,
Shrijeet Mukherjee
In-Reply-To: <5497250F.2020906@gmail.com>
On 12/21/14 14:52, John Fastabend wrote:
> not a problem thanks for the response. I might try to document this
> somewhere if folks think it would be useful. Something describing
> how it works today would be helpful is my thought. Showing the
> various stacked cases and how messages get propagated. (Some cases
> being with bridge, without bridge, with bridge and multiple uplinks,
> with bridge + VLAN filtering, macvlan, SR-IOV + bridge + VMDQ, etc.)
>
> Its not a small task so likely won't get to it until after the new
> year.
>
I think this would be very useful.
We are looking for an L2 tutorial at netdev01 (I was looking at
Vlad and Roopa so far) so maybe we could split the work. Would
you be interested?
Yes, all those bridge and bridge like things like Vxlan for example
should be part of this de-mystification.
Shrijeet is also putting together a BOF for the offload stuff.
>
> Yes, but I don't think its too late to bring it into the picture here.
>
that would be nice.
>>> The PF port, when acting as the control interface, is actually
>>> TheClassThingy we discuss on/off.
>
> Yep or if you take Jiri's approach any port on the nic could be used
> to manage this.
>
> The VF's may have netdev's if they are in the host. In this case you
> could use 'bridge fdb' to manage them. In many use cases though the
> VFs are directly assigned to VMs and then are outside the hosts
> management domain. For this case you can either let the host tell the
> driver which addresses it would want to receive.
>
Which driver? The PF? I dont think the semantics allow for that;
How do i tell the PF using "fdb add" that a specific mac+vlan should
be sent to vf1 which is now sitting inside some vm?
> Another _idea_ would be to create a "shadow" netdev in the host
> to manage the port even when the VF is direct assigned.
That may work. But there are questions:
Can its name be changed within the container/vm or are those
different namespaces etc.
So if the answer is that "self" implies using the hardware fdb,
what does "master" mean?
>Then you
> could use all your normal commands from the host to set the MTU,
> set any MACs, etc. At the moment as Jamal noted we have a subset
> of 'ip link' commands that we use to work on VFs when they are not
> in the host domain.
>
> 'ip link set ethx vf # ...'
>
I think once it moves management of such things as MTUs should be from
wherever that thing sits at (vm/container) to avoid any suprises.
> In the SR-IOV case you would have a PF and then a set of eth-vf#
> netdev's which are not attached to a VF but act as the management
> interface for the port.
>
you can have more than one PF?
> I think this is not specific to SR-IOV though right.This is the
> same point for both "real" switch ASICs and SR-IOV. Using the netdev
> directly as a management interace (a la rocker) seems to work OK.
> But does it become cleaner to have the switch object represented
> explicitly for management.
>
Indeed it does. In particular if you had to move around a bridge port
to a container or VM.
Now if we introduced the idea of showing up with netdev for each vf
and you had a classthing you dont have to keep the vf1s visible
once migrated - but would be able to add fdb entries pointing to
them (assuming names/ifindices remain valid).
cheers,
jamal
^ permalink raw reply
* Re: SRIOV as bridge Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Jamal Hadi Salim @ 2014-12-22 3:13 UTC (permalink / raw)
To: Roopa Prabhu
Cc: John Fastabend, Hubert Sokolowski, netdev@vger.kernel.org,
Vlad Yasevich, Shrijeet Mukherjee
In-Reply-To: <549731A1.3090307@cumulusnetworks.com>
On 12/21/14 15:46, Roopa Prabhu wrote:
> On 12/21/14, 12:06 PM, Jamal Hadi Salim wrote:
> yes, could be, but its not today ('PF' is physical function and 'VF' is
> virtual function).
> If you introduce a master/slave relationship between the PF and VF (ie
> VF's were assigned PF as the master using 'ip link set dev vf master
> PF), then yes.
When someone says "modprobe igb max_vfs=19" then 19 VFs show up. i.e the
driver creates them. And then there is assumed direct relationship
between VF and PF. The PF being the parent. Adding fdbs goes via PF.
>> And if the path is via is the PF - i think that seems like "master"
>> not self, no?
>
> Today ...when you add fdb...path is not via the PF netdev.
For SRIOV it is. Example to add via pf eth10 an
fdb entry to the igb hardware fdb to point to vf1:
ip link set eth10 vf 1 mac aa:bb:cc:dd:ee:ff vlan 10
That last part "vf 1 mac aa:bb:cc:dd:ee:ff vlan 10" is typically
part of an "fdb add semantic" - but we explicitly call out
eth10, the parent. The PF has control of the hardware fdb.
It maybe
> internally done that way in PF/VF driver.
> so, 'master' does not apply today. But if there were such a relationship
> between PF/VF, yes, 'master' could be used.
>
I am refering if were to get rid of using iplink. There has to be
something pointed to by vf1 that gets called to add the fdb entry in
hardware.
> PF does not really need to have a master relationship with the VF. Its
> better that way. Infact it should be that way even in the case of 'the
> switch device class model' because that will allow switch ports to be
> added to a linux bridge (and hence make use of the linux bridge (cumulus
> model). 'master' will be the 'linux bridge device' in this case).
>
So what do you do if the user sets either one of master/self and it
doesnt make sense?
cheers,
jamal
> Thanks,
> Roopa
^ permalink raw reply
* Re: [PATCH 01/10] core: Split out UFO6 support
From: Vlad Yasevich @ 2014-12-22 4:06 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, Vladislav Yasevich, virtualization, stefanha, ben,
David Miller
In-Reply-To: <20141220210359.GA23262@redhat.com>
On 12/20/2014 04:03 PM, Michael S. Tsirkin wrote:
> On Fri, Dec 19, 2014 at 03:13:20PM -0500, Vlad Yasevich wrote:
>> On 12/18/2014 12:50 PM, Michael S. Tsirkin wrote:
>>> On Thu, Dec 18, 2014 at 07:35:46PM +0200, Michael S. Tsirkin wrote:
>>>>>> We should either generate our own ID,
>>>>>> like we always did, or make sure we don't accept
>>>>>> these packets.
>>>>>> Second option is almost sure to break userspace,
>>>>>> so it seems we must do the first one.
>>>>>>
>>>>>
>>>>> Right. This was missing from packet sockets. I can fix it.
>>>>>
>>>>> -vlad
>>>>
>>>> Also, this can't be a patch on top, since we don't
>>>> want bisect to give us configurations which
>>>> can BUG().
>>>
>>> So how doing this in stages:
>>>
>>> 1. add helper that checks skb GSO type.
>>> If it is SKB_GSO_UDP, check for IPv6, and
>>> generate the fragment ID.
>>>
>>> Call this helper in
>>> - virtio net rx path
>>
>> Why do we need id on rx path? Fragment ID should only be generated on tx.
>
> So that all GSO_UDP6 packets have fragment ID as appropriate.
> It's similar to how we fill it on rx in tun, is it not?
Saying "rx in tun" always hurts my head. We fill it in in tun_get_user()
which then passed the packet to the kernel for forwarding. The is later
used in the forwarding process.
I've been thinking about putting this fragment generation into the GSO
path so there is only 1 spot that ever needs to this. This way it
would only be done if the fragment id is actually needed. For most
guest-to-guest comms, the id isn't needed.
>
>
>>> - tun rx path (get user)
>>> - macvtap tx patch (get user)
>>> - packet socket tx patch (get user)
>>
>> The reset makes sense.
>>>
>>> 2. Put back UFO flag everywhere: virtio,tun,macvtap,packet,
>>> since we can handle IPv6 now even if it's suboptimal.
>>>
>>> Above two seem like smalling patches, appropriate for stable.
>>
>> OK.
>>
>>>
>>> Next, work on a new feature to pass
>>> fragment ID in virtio header:
>>>
>>> 3. split UFO/UFO6 as you did here, but add code
>>> in above 4 places to convert UDP to UDP6,
>>
>> Doing so will adversely impact IPv6 UFO performance for legacy
>> guests. I know how many times I've seen mail wrt patch xyz caused
>> %X regression in my setup and how we've reverted or tried to fixed
>> things to solve this. If we go with approach, the only "fix' would be
>> to upgrade the guest and that's not available to some users.
>>
>> -vlad
>
> I think there's some misunderstanding here.
>
> I merely mean that after split, host should always have
> SKB_GSO_UDP6 set for IPv6.
>
> To make sure legacy userspace/guests don't notice changes,
> whenever we detect SKB_GSO_UDP6 we should set VIRTIO_NET_HDR_GSO_UDP,
> and whenever we get VIRTIO_NET_HDR_GSO_UDP we should set either
> SKB_GSO_UDP6 or SKB_GSO_UDP depending on IP type.
This is the part that introduced the regression. By setting the gso_type
to UDP6, we trigger skb_gso_segment() to actually perform IPv6 fragmentation.
I've seen this when passing UDP traffic from 2 fedora19 systems over the
kernel that does the above.
-vlad
>
> Given this clarification there's no reason to think
> old guests will regress, correct?
>
>>> additionally, add code in
>>> - virtio net tx path
>>> - tun tx path (get user)
>>> - macvtap rx patch (put user)
>>> - packet socket rx patch (put user)
>>> to convert UDP6 to UDP.
>>>
>>> step 3 needs to be bisect-clean, somehow.
>>>
>>> 4. add new field in header, new feature bit for virtio net to gate it,
>>> new ioctls to tun,macvtap,packet socket.
>>>
>>> These two are more like optimizations, so not stable material.
>>>
>>>
^ permalink raw reply
* Re: [PATCH net-next 2/2] r8152: check the status before submittingrx
From: David Miller @ 2014-12-22 5:22 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2ED5A35@RTITMBSV03.realtek.com.tw>
From: Hayes Wang <hayeswang@realtek.com>
Date: Mon, 22 Dec 2014 02:53:42 +0000
> David Miller [mailto:davem@davemloft.net]
>> Sent: Saturday, December 20, 2014 4:44 AM
> [...]
>> > Don't submit the rx if the device is unplugged, linking down,
>> > or stopped.
>> ...
>> > @@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152
>> *tp, struct rx_agg *agg, gfp_t mem_flags)
>> > {
>> > int ret;
>> >
>> > + /* The rx would be stopped, so skip submitting */
>> > + if (test_bit(RTL8152_UNPLUG, &tp->flags) ||
>> > + !test_bit(WORK_ENABLE, &tp->flags) || !(tp->speed & LINK_STATUS))
>> > + return 0;
>> > +
>>
>> I think netif_carrier_off() should always be true in all three of those
>> situations, and would be a much simpler test than what you've coded
>> here.
>
> When the device is unplugged or stopped, the linking status
> may be true, so I add additional checks to avoid the submission.
>
> Besides, in set_carrier() I set netif_carrier_on() after
> ops.enable() to avoid any transmission before I finish
> starting the tx/rx.
>
> tp->rtl_ops.enable(tp);
> set_bit(RTL8152_SET_RX_MODE, &tp->flags);
> netif_carrier_on(netdev);
>
> However, the r8152_submit_rx() would be called in ops.enable(),
> and the check of netif_carrier_ok() would be always false. That
> is why I use tp->speed, not netif_carrier_ok(), to check the
> linking stauts.
I stil think your check is way too complicated for this fast path so I
would ask that you arrange things such that the simpler
netif_carrier_off() test works.
Especially because that is what the core networking stack uses
to decide whether to send packets to us as well.
^ permalink raw reply
* Re: SRIOV as bridge Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if ndo_fdb_dump is defined.
From: Roopa Prabhu @ 2014-12-22 6:24 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: John Fastabend, Hubert Sokolowski, netdev@vger.kernel.org,
Vlad Yasevich, Shrijeet Mukherjee
In-Reply-To: <54978C3C.4040102@mojatatu.com>
On 12/21/14, 7:13 PM, Jamal Hadi Salim wrote:
> On 12/21/14 15:46, Roopa Prabhu wrote:
>> On 12/21/14, 12:06 PM, Jamal Hadi Salim wrote:
>
>> yes, could be, but its not today ('PF' is physical function and 'VF' is
>> virtual function).
>> If you introduce a master/slave relationship between the PF and VF (ie
>> VF's were assigned PF as the master using 'ip link set dev vf master
>> PF), then yes.
>
>
> When someone says "modprobe igb max_vfs=19" then 19 VFs show up. i.e the
> driver creates them. And then there is assumed direct relationship
> between VF and PF. The PF being the parent. Adding fdbs goes via PF.
>
>>> And if the path is via is the PF - i think that seems like "master"
>>> not self, no?
>>
>> Today ...when you add fdb...path is not via the PF netdev.
>
> For SRIOV it is. Example to add via pf eth10 an
> fdb entry to the igb hardware fdb to point to vf1:
> ip link set eth10 vf 1 mac aa:bb:cc:dd:ee:ff vlan 10
> That last part "vf 1 mac aa:bb:cc:dd:ee:ff vlan 10" is typically
> part of an "fdb add semantic" - but we explicitly call out
> eth10, the parent. The PF has control of the hardware fdb.
Ah......i did not know this syntax with 'ip link set'. thanks for
pointing out.
I always thought that you can still use 'bridge fdb add' for vfs.
Curious why its not that way.
>
> It maybe
>> internally done that way in PF/VF driver.
>> so, 'master' does not apply today. But if there were such a relationship
>> between PF/VF, yes, 'master' could be used.
>>
>
> I am refering if were to get rid of using iplink. There has to be
> something pointed to by vf1 that gets called to add the fdb entry in
> hardware.
ok, i assumed we were only talking about 'bridge fdb add'
>
>> PF does not really need to have a master relationship with the VF. Its
>> better that way. Infact it should be that way even in the case of 'the
>> switch device class model' because that will allow switch ports to be
>> added to a linux bridge (and hence make use of the linux bridge (cumulus
>> model). 'master' will be the 'linux bridge device' in this case).
>>
>
> So what do you do if the user sets either one of master/self and it
> doesnt make sense?
Am guessing it will continue to do what it does today. If there is no
master or if there is master and the master does not support the op, it
will return -EOPNOTSUPP. And, self does not make sense in cases where
the port driver does not support the op. In which case again you will
get a -EOPNOTSUPP. Have not thought through all the other cases yet.
Thanks,
Roopa
^ permalink raw reply
* [PATCH net-next v2 0/2] r8152: adjust r8152_submit_rx
From: Hayes Wang @ 2014-12-22 6:52 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-107-Taiwan-albertk@realtek.com>
v2:
Replace the patch #1 with "call rtl_start_rx after netif_carrier_on".
For patch #2, replace checking tp->speed with netif_carrier_ok.
v1:
Avoid r8152_submit_rx() from submitting rx during unexpected
moment. This could reduce the time of stopping rx.
For patch #1, the tp->speed should be updated early. Then,
the patch #2 could use it to check the current linking status.
Hayes Wang (2):
r8152: call rtl_start_rx after netif_carrier_on
r8152: check the status before submitting rx
drivers/net/usb/r8152.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
--
2.1.0
^ permalink raw reply
* [PATCH net-next v2 2/2] r8152: check the status before submitting rx
From: Hayes Wang @ 2014-12-22 6:52 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-110-Taiwan-albertk@realtek.com>
Don't submit the rx if the device is unplugged, stopped, or
linking down.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index cbe450c..8ecc2df 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags)
{
int ret;
+ /* The rx would be stopped, so skip submitting */
+ if (test_bit(RTL8152_UNPLUG, &tp->flags) ||
+ !test_bit(WORK_ENABLE, &tp->flags) || !netif_carrier_ok(tp->netdev))
+ return 0;
+
usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
agg->head, agg_buf_sz,
(usb_complete_t)read_bulk_callback, agg);
--
2.1.0
^ permalink raw reply related
* [PATCH net-next v2 1/2] r8152: call rtl_start_rx after netif_carrier_on
From: Hayes Wang @ 2014-12-22 6:52 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-110-Taiwan-albertk@realtek.com>
Remove rtl_start_rx() from rtl_enable() and put it after calling
netif_carrier_on().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2d1c77e..cbe450c 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2043,7 +2043,7 @@ static int rtl_enable(struct r8152 *tp)
rxdy_gated_en(tp, false);
- return rtl_start_rx(tp);
+ return 0;
}
static int rtl8152_enable(struct r8152 *tp)
@@ -2858,6 +2858,7 @@ static void set_carrier(struct r8152 *tp)
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, &tp->flags);
netif_carrier_on(netdev);
+ rtl_start_rx(tp);
}
} else {
if (tp->speed & LINK_STATUS) {
--
2.1.0
^ permalink raw reply related
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