* [PATCH RESEND] bnx2fc: do not add shared skbs to the fcoe_rx_list
@ 2014-08-27 13:03 Maurizio Lombardi
0 siblings, 0 replies; 4+ messages in thread
From: Maurizio Lombardi @ 2014-08-27 13:03 UTC (permalink / raw)
To: QLogic-Storage-Upstream; +Cc: eddie.wai, linux-scsi, JBottomley, hch, cdupuis
In some cases, the fcoe_rx_list may contains multiple instances
of the same skb (the so called "shared skbs").
the bnx2fc_l2_rcv thread is a loop that extracts a skb from the list,
modifies (and destroys) its content and the proceed to the next one.
The problem is that if the skb is shared, the remaining instances will
be corrupted.
The solution is to use skb_share_check() before adding the skb to the
fcoe_rx_list.
[ 6286.808725] ------------[ cut here ]------------
[ 6286.808729] WARNING: at include/scsi/fc_frame.h:173 bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]()
[ 6286.808748] Modules linked in: bnx2x(-) mdio dm_service_time bnx2fc cnic uio fcoe libfcoe 8021q garp stp mrp libfc llc scsi_transport_fc scsi_tgt sg iTCO_wdt iTCO_vendor_support coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel e1000e ghash_clmulni_intel aesni_intel lrw gf128mul glue_helper ablk_helper ptp cryptd hpilo serio_raw hpwdt lpc_ich pps_core ipmi_si pcspkr mfd_core ipmi_msghandler shpchp pcc_cpufreq mperf nfsd auth_rpcgss nfs_acl lockd sunrpc dm_multipath xfs libcrc32c ata_generic pata_acpi sd_mod crc_t10dif crct10dif_common mgag200 syscopyarea sysfillrect sysimgblt i2c_algo_bit ata_piix drm_kms_helper ttm drm libata i2c_core hpsa dm_mirror dm_region_hash dm_log dm_mod [last unloaded: mdio]
[ 6286.808750] CPU: 3 PID: 1304 Comm: bnx2fc_l2_threa Not tainted 3.10.0-121.el7.x86_64 #1
[ 6286.808750] Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
[ 6286.808752] 0000000000000000 000000000b36e715 ffff8800deba1e00 ffffffff815ec0ba
[ 6286.808753] ffff8800deba1e38 ffffffff8105dee1 ffffffffa05618c0 ffff8801e4c81888
[ 6286.808754] ffffe8ffff663868 ffff8801f402b180 ffff8801f56bc000 ffff8800deba1e48
[ 6286.808754] Call Trace:
[ 6286.808759] [<ffffffff815ec0ba>] dump_stack+0x19/0x1b
[ 6286.808762] [<ffffffff8105dee1>] warn_slowpath_common+0x61/0x80
[ 6286.808763] [<ffffffff8105e00a>] warn_slowpath_null+0x1a/0x20
[ 6286.808765] [<ffffffffa054f415>] bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]
[ 6286.808767] [<ffffffffa054eff0>] ? bnx2fc_disable+0x90/0x90 [bnx2fc]
[ 6286.808769] [<ffffffff81085aef>] kthread+0xcf/0xe0
[ 6286.808770] [<ffffffff81085a20>] ? kthread_create_on_node+0x140/0x140
[ 6286.808772] [<ffffffff815fc76c>] ret_from_fork+0x7c/0xb0
[ 6286.808773] [<ffffffff81085a20>] ? kthread_create_on_node+0x140/0x140
[ 6286.808774] ---[ end trace c6cdb939184ccb4e ]---
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 79e5c94..72533c5 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -412,6 +412,7 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev,
struct fc_frame_header *fh;
struct fcoe_rcv_info *fr;
struct fcoe_percpu_s *bg;
+ struct sk_buff *tmp_skb;
unsigned short oxid;
interface = container_of(ptype, struct bnx2fc_interface,
@@ -424,6 +425,12 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev,
goto err;
}
+ tmp_skb = skb_share_check(skb, GFP_ATOMIC);
+ if (!tmp_skb)
+ goto err;
+
+ skb = tmp_skb;
+
if (unlikely(eth_hdr(skb)->h_proto != htons(ETH_P_FCOE))) {
printk(KERN_ERR PFX "bnx2fc_rcv: Wrong FC type frame\n");
goto err;
--
Maurizio Lombardi
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH RESEND] bnx2fc: do not add shared skbs to the fcoe_rx_list
@ 2014-11-20 10:17 Maurizio Lombardi
2014-11-20 14:07 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Maurizio Lombardi @ 2014-11-20 10:17 UTC (permalink / raw)
To: hch; +Cc: chad.dupuis, linux-scsi, JBottomley
In some cases, the fcoe_rx_list may contains multiple instances
of the same skb (the so called "shared skbs").
the bnx2fc_l2_rcv thread is a loop that extracts a skb from the list,
modifies (and destroys) its content and then proceed to the next one.
The problem is that if the skb is shared, the remaining instances will
be corrupted.
The solution is to use skb_share_check() before adding the skb to the
fcoe_rx_list.
[ 6286.808725] ------------[ cut here ]------------
[ 6286.808729] WARNING: at include/scsi/fc_frame.h:173 bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]()
[ 6286.808748] Modules linked in: bnx2x(-) mdio dm_service_time bnx2fc cnic uio fcoe libfcoe 8021q garp stp mrp libfc llc scsi_transport_fc scsi_tgt sg iTCO_wdt iTCO_vendor_support coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel e1000e ghash_clmulni_intel aesni_intel lrw gf128mul glue_helper ablk_helper ptp cryptd hpilo serio_raw hpwdt lpc_ich pps_core ipmi_si pcspkr mfd_core ipmi_msghandler shpchp pcc_cpufreq mperf nfsd auth_rpcgss nfs_acl lockd sunrpc dm_multipath xfs libcrc32c ata_generic pata_acpi sd_mod crc_t10dif crct10dif_common mgag200 syscopyarea sysfillrect sysimgblt i2c_algo_bit ata_piix drm_kms_helper ttm drm libata i2c_core hpsa dm_mirror dm_region_hash dm_log dm_mod [last unloaded: mdio]
[ 6286.808750] CPU: 3 PID: 1304 Comm: bnx2fc_l2_threa Not tainted 3.10.0-121.el7.x86_64 #1
[ 6286.808750] Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
[ 6286.808752] 0000000000000000 000000000b36e715 ffff8800deba1e00 ffffffff815ec0ba
[ 6286.808753] ffff8800deba1e38 ffffffff8105dee1 ffffffffa05618c0 ffff8801e4c81888
[ 6286.808754] ffffe8ffff663868 ffff8801f402b180 ffff8801f56bc000 ffff8800deba1e48
[ 6286.808754] Call Trace:
[ 6286.808759] [<ffffffff815ec0ba>] dump_stack+0x19/0x1b
[ 6286.808762] [<ffffffff8105dee1>] warn_slowpath_common+0x61/0x80
[ 6286.808763] [<ffffffff8105e00a>] warn_slowpath_null+0x1a/0x20
[ 6286.808765] [<ffffffffa054f415>] bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]
[ 6286.808767] [<ffffffffa054eff0>] ? bnx2fc_disable+0x90/0x90 [bnx2fc]
[ 6286.808769] [<ffffffff81085aef>] kthread+0xcf/0xe0
[ 6286.808770] [<ffffffff81085a20>] ? kthread_create_on_node+0x140/0x140
[ 6286.808772] [<ffffffff815fc76c>] ret_from_fork+0x7c/0xb0
[ 6286.808773] [<ffffffff81085a20>] ? kthread_create_on_node+0x140/0x140
[ 6286.808774] ---[ end trace c6cdb939184ccb4e ]---
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
Acked-by: Chad Dupuis <chad.dupuis@qlogic.com>
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 79e5c94..72533c5 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -412,6 +412,7 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev,
struct fc_frame_header *fh;
struct fcoe_rcv_info *fr;
struct fcoe_percpu_s *bg;
+ struct sk_buff *tmp_skb;
unsigned short oxid;
interface = container_of(ptype, struct bnx2fc_interface,
@@ -424,6 +425,12 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev,
goto err;
}
+ tmp_skb = skb_share_check(skb, GFP_ATOMIC);
+ if (!tmp_skb)
+ goto err;
+
+ skb = tmp_skb;
+
if (unlikely(eth_hdr(skb)->h_proto != htons(ETH_P_FCOE))) {
printk(KERN_ERR PFX "bnx2fc_rcv: Wrong FC type frame\n");
goto err;
--
Maurizio Lombardi
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] bnx2fc: do not add shared skbs to the fcoe_rx_list
2014-11-20 10:17 [PATCH RESEND] bnx2fc: do not add shared skbs to the fcoe_rx_list Maurizio Lombardi
@ 2014-11-20 14:07 ` Christoph Hellwig
2014-11-21 13:21 ` Maurizio Lombardi
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2014-11-20 14:07 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: hch, chad.dupuis, linux-scsi, JBottomley
Thanks. This should go into 3.18 and -stable, right?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] bnx2fc: do not add shared skbs to the fcoe_rx_list
2014-11-20 14:07 ` Christoph Hellwig
@ 2014-11-21 13:21 ` Maurizio Lombardi
0 siblings, 0 replies; 4+ messages in thread
From: Maurizio Lombardi @ 2014-11-21 13:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: chad.dupuis, linux-scsi, stable, JBottomley
On 11/20/2014 03:07 PM, Christoph Hellwig wrote:
> Thanks. This should go into 3.18 and -stable, right?
>
Yes.
Thanks,
Maurizio Lombardi
On 11/20/2014 11:17 AM, Maurizio Lombardi wrote:
> In some cases, the fcoe_rx_list may contains multiple instances
> of the same skb (the so called "shared skbs").
>
> the bnx2fc_l2_rcv thread is a loop that extracts a skb from the list,
> modifies (and destroys) its content and then proceed to the next one.
> The problem is that if the skb is shared, the remaining instances will
> be corrupted.
>
> The solution is to use skb_share_check() before adding the skb to the
> fcoe_rx_list.
>
> [ 6286.808725] ------------[ cut here ]------------
> [ 6286.808729] WARNING: at include/scsi/fc_frame.h:173 bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]()
> [ 6286.808748] Modules linked in: bnx2x(-) mdio dm_service_time bnx2fc cnic uio fcoe libfcoe 8021q garp stp mrp libfc llc scsi_transport_fc scsi_tgt sg iTCO_wdt iTCO_vendor_support coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel e1000e ghash_clmulni_intel aesni_intel lrw gf128mul glue_helper ablk_helper ptp cryptd hpilo serio_raw hpwdt lpc_ich pps_core ipmi_si pcspkr mfd_core ipmi_msghandler shpchp pcc_cpufreq mperf nfsd auth_rpcgss nfs_acl lockd sunrpc dm_multipath xfs libcrc32c ata_generic pata_acpi sd_mod crc_t10dif crct10dif_common mgag200 syscopyarea sysfillrect sysimgblt i2c_algo_bit ata_piix drm_kms_helper ttm drm libata i2c_core hpsa dm_mirror dm_region_hash dm_log dm_mod [last unloaded: mdio]
> [ 6286.808750] CPU: 3 PID: 1304 Comm: bnx2fc_l2_threa Not tainted 3.10.0-121.el7.x86_64 #1
> [ 6286.808750] Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
> [ 6286.808752] 0000000000000000 000000000b36e715 ffff8800deba1e00 ffffffff815ec0ba
> [ 6286.808753] ffff8800deba1e38 ffffffff8105dee1 ffffffffa05618c0 ffff8801e4c81888
> [ 6286.808754] ffffe8ffff663868 ffff8801f402b180 ffff8801f56bc000 ffff8800deba1e48
> [ 6286.808754] Call Trace:
> [ 6286.808759] [<ffffffff815ec0ba>] dump_stack+0x19/0x1b
> [ 6286.808762] [<ffffffff8105dee1>] warn_slowpath_common+0x61/0x80
> [ 6286.808763] [<ffffffff8105e00a>] warn_slowpath_null+0x1a/0x20
> [ 6286.808765] [<ffffffffa054f415>] bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]
> [ 6286.808767] [<ffffffffa054eff0>] ? bnx2fc_disable+0x90/0x90 [bnx2fc]
> [ 6286.808769] [<ffffffff81085aef>] kthread+0xcf/0xe0
> [ 6286.808770] [<ffffffff81085a20>] ? kthread_create_on_node+0x140/0x140
> [ 6286.808772] [<ffffffff815fc76c>] ret_from_fork+0x7c/0xb0
> [ 6286.808773] [<ffffffff81085a20>] ? kthread_create_on_node+0x140/0x140
> [ 6286.808774] ---[ end trace c6cdb939184ccb4e ]---
>
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> Acked-by: Chad Dupuis <chad.dupuis@qlogic.com>
> ---
> drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index 79e5c94..72533c5 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -412,6 +412,7 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev,
> struct fc_frame_header *fh;
> struct fcoe_rcv_info *fr;
> struct fcoe_percpu_s *bg;
> + struct sk_buff *tmp_skb;
> unsigned short oxid;
>
> interface = container_of(ptype, struct bnx2fc_interface,
> @@ -424,6 +425,12 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct net_device *dev,
> goto err;
> }
>
> + tmp_skb = skb_share_check(skb, GFP_ATOMIC);
> + if (!tmp_skb)
> + goto err;
> +
> + skb = tmp_skb;
> +
> if (unlikely(eth_hdr(skb)->h_proto != htons(ETH_P_FCOE))) {
> printk(KERN_ERR PFX "bnx2fc_rcv: Wrong FC type frame\n");
> goto err;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-11-21 13:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-20 10:17 [PATCH RESEND] bnx2fc: do not add shared skbs to the fcoe_rx_list Maurizio Lombardi
2014-11-20 14:07 ` Christoph Hellwig
2014-11-21 13:21 ` Maurizio Lombardi
-- strict thread matches above, loose matches on Subject: below --
2014-08-27 13:03 Maurizio Lombardi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).