I have tested this patch on local setup, basic I/O is running fine.

Taken care of all review comments.

Matt Joras, Sean Bruno, Stephen Hurd,

So with D13312 committed, is there still a need for a change? If not, please abandon this review.

Verified the change with D13312 alone, it seems to be working fine, I can abandon this now. Thank you!!

bnxt: Removed Tx padding logic from earlier patch

since iflib supporting it as part of D13324.

Thanks to Stephen,
Tested with Cumulus A0 NIC, test passed with this patch.
I'll delete the old workaround (used for padding) from driver.

How are you testing this padding?

<Chenna> ARP response pkt is less than 60 bytes, right? I'm able to repro using ARP response pkt only.

Please ignore Crash as that is not related to this particulate issue.

Hi Stephen,

1. Added support for Short-HWRM since it is required for stratus to support SR-IOV
2. Since few BRCM A0 NICs can not handle < 52 byte Tx pkts, implemented padding logic.
3. Taken care of other minor issues while claiming VFs.

Matt Joras,

Looks good to me.

Thanks Stephen,

Quick update:-

I agree with you!! Thanks for the suggestion.

Problem description:-
Ifconfig showing media as 'Other' only for bnxt interfaces, where as for ix1 (Intel 10G NIC) it is showing properly.

For now, taken care of all of your review comments, I'm sorry for couple of duplicate entries / incompatible entries in earlier patch.

Taken care of all review comments.

Taken care of review comments.

Modify IFLIB_MAX_RX_SEGS to 66 which is theoretical maximum for BRCM NIC (64 + 1 TPA_START + 1 TPA_END)

How many are required when max_agg_segs == 7? With that value change in future hardware designs?

Though I'm seeing max of 45 in my setup, Theoretical max num_frags is 66 (64 frags + 1 TPA_Start + 1 TPA_End).

Taken care of review comments.

Taken care of review comments.

Can you please review the patch and commit if no changes required?

Thank you, Stephen.

Trimmed switch-case block a lot and allowing a speed only when it is supported by it's phy_type.
Can you please review and let me know if still there are any issues?

Taken care of Stephen's review comments by removing speed / phy_type combinations which doesn't exist currently or technically not possible.

Fixed issue of double notification (in the kernel logs) after changing flow control setting in case of pause_autoneg.

Thanks for the review, Stephen.

Hi Stephen,

Hi Stephen,

Hi Stephen,

Looks good, though the other one should be renamed as well.

<Chenna> Taken care!!

Thanks Stephen,

Thank you for the review, Stephen. Please find my response below.

Not really. It's easiest/best to make it only modifiable when !(if_getdrvflags() & IFF_DRV_RUNNING)

<Chenna> Taken care!!

I need some more time to get back on 'default hw_lro options' / ip_forwarding.

Looks like this patch causing issues with bnxt driver unload.

Stephen,

Please find my few inline explanations which may helps in the review.

Thank you, Stephen.

Taken care of review comments.

Thanks a lot, Stephen.

when using iflib, software LRO is handled by iflib, not through the driver using tcp_lro_rx()/tcp_lro_flush_all().

<Chenna> Thanks for the info.

Taken care of all review comments.

Taken care of all review comments.

Corrected patch by removing debug code.

Combining them into one allocation seems like a needlessly awkward way to save 4k.

<Chenna> I agree with this, separated the allocations.

Sean Bruno,

Thanks Stephen, Already opened a mail thread with Firmware team regarding that and will update the status once get back.

Thanks a lot for such detailed explanation, It really helped me for better understanding.

Thank you, Stephen.

You should update the summary to remove item #2 so it's useful as a commit message.

<Chenna> Done!!

Sean Bruno,

Thanks Stephen!! Taken care of your review comments.

Hi Stephen,