Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: fcoe: return value of skb_linearize should be handled
Zhouyi Zhouwrote: diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index fee1f29..4926d48 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector, total_rx_bytes += ddp_bytes; total_rx_packets += DIV_ROUND_UP(ddp_bytes, mss); - } - if (!ddp_bytes) { + } else { dev_kfree_skb_any(skb); continue; } This is changing the logic by treating a negative ddp_bytes value (an error return) the same as a 0 value. This is probably wrong and inappropriate for this patch in any case. -- Mark Rustad, Networking Division, Intel Corporation signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: fcoe: return value of skb_linearize should be handled
Zhouyi Zhou wrote: diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index fee1f29..4926d48 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector, total_rx_bytes += ddp_bytes; total_rx_packets += DIV_ROUND_UP(ddp_bytes, mss); - } - if (!ddp_bytes) { + } else { dev_kfree_skb_any(skb); continue; } This is changing the logic by treating a negative ddp_bytes value (an error return) the same as a 0 value. This is probably wrong and inappropriate for this patch in any case. -- Mark Rustad, Networking Division, Intel Corporation signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [PATCH 1/1] ixgbe: fcoe: return value of skb_linearize should be handled
Thanks Jeff for your advice, Sorry for the my innocence as a Linux kernel rookie. Zhouyi On Thu, Dec 8, 2016 at 1:30 AM, Jeff Kirsherwrote: > On Wed, 2016-12-07 at 15:43 +0800, Zhouyi Zhou wrote: >> Signed-off-by: Zhouyi Zhou >> Reviewed-by: Cong Wang >> Reviewed-by: Yuval Shaia >> Reviewed-by: Eric Dumazet >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +- >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +-- >> 2 files changed, 6 insertions(+), 3 deletions(-) > > Did Cong, Yuval and Eric give their Reviewed-by offline? I see they made > comments and suggests, but never saw them actually give you their Reviewed- > by. You cannot automatically add their Reviewed-by, Signed-off-by, etc > just because someone provides feedback on your patch.
Re: [PATCH 1/1] ixgbe: fcoe: return value of skb_linearize should be handled
Thanks Jeff for your advice, Sorry for the my innocence as a Linux kernel rookie. Zhouyi On Thu, Dec 8, 2016 at 1:30 AM, Jeff Kirsher wrote: > On Wed, 2016-12-07 at 15:43 +0800, Zhouyi Zhou wrote: >> Signed-off-by: Zhouyi Zhou >> Reviewed-by: Cong Wang >> Reviewed-by: Yuval Shaia >> Reviewed-by: Eric Dumazet >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +- >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +-- >> 2 files changed, 6 insertions(+), 3 deletions(-) > > Did Cong, Yuval and Eric give their Reviewed-by offline? I see they made > comments and suggests, but never saw them actually give you their Reviewed- > by. You cannot automatically add their Reviewed-by, Signed-off-by, etc > just because someone provides feedback on your patch.
Re: [PATCH 1/1] ixgbe: fcoe: return value of skb_linearize should be handled
On Wed, 2016-12-07 at 15:43 +0800, Zhouyi Zhou wrote: > Signed-off-by: Zhouyi Zhou> Reviewed-by: Cong Wang > Reviewed-by: Yuval Shaia > Reviewed-by: Eric Dumazet > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +-- > 2 files changed, 6 insertions(+), 3 deletions(-) Did Cong, Yuval and Eric give their Reviewed-by offline? I see they made comments and suggests, but never saw them actually give you their Reviewed- by. You cannot automatically add their Reviewed-by, Signed-off-by, etc just because someone provides feedback on your patch. signature.asc Description: This is a digitally signed message part
Re: [PATCH 1/1] ixgbe: fcoe: return value of skb_linearize should be handled
On Wed, 2016-12-07 at 15:43 +0800, Zhouyi Zhou wrote: > Signed-off-by: Zhouyi Zhou > Reviewed-by: Cong Wang > Reviewed-by: Yuval Shaia > Reviewed-by: Eric Dumazet > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +-- > 2 files changed, 6 insertions(+), 3 deletions(-) Did Cong, Yuval and Eric give their Reviewed-by offline? I see they made comments and suggests, but never saw them actually give you their Reviewed- by. You cannot automatically add their Reviewed-by, Signed-off-by, etc just because someone provides feedback on your patch. signature.asc Description: This is a digitally signed message part
[PATCH 1/1] ixgbe: fcoe: return value of skb_linearize should be handled
Signed-off-by: Zhouyi ZhouReviewed-by: Cong Wang Reviewed-by: Yuval Shaia Reviewed-by: Eric Dumazet --- drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c index 2a653ec..7b6bdb7 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c @@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter, */ if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) && (fctl & FC_FC_END_SEQ)) { - skb_linearize(skb); + int err; + + err = skb_linearize(skb); + if (err) + return err; crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc)); crc->fcoe_eof = FC_EOF_T; } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index fee1f29..4926d48 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector, total_rx_bytes += ddp_bytes; total_rx_packets += DIV_ROUND_UP(ddp_bytes, mss); - } - if (!ddp_bytes) { + } else { dev_kfree_skb_any(skb); continue; } -- 1.9.1
[PATCH 1/1] ixgbe: fcoe: return value of skb_linearize should be handled
Signed-off-by: Zhouyi Zhou Reviewed-by: Cong Wang Reviewed-by: Yuval Shaia Reviewed-by: Eric Dumazet --- drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c index 2a653ec..7b6bdb7 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c @@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter, */ if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) && (fctl & FC_FC_END_SEQ)) { - skb_linearize(skb); + int err; + + err = skb_linearize(skb); + if (err) + return err; crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc)); crc->fcoe_eof = FC_EOF_T; } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index fee1f29..4926d48 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector, total_rx_bytes += ddp_bytes; total_rx_packets += DIV_ROUND_UP(ddp_bytes, mss); - } - if (!ddp_bytes) { + } else { dev_kfree_skb_any(skb); continue; } -- 1.9.1