Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAI Release notes 1.7.1 checkin for new PR since the old PR1191 had issues w.r.t DCO #1208

Merged
merged 1 commit into from
Apr 21, 2021
Merged

SAI Release notes 1.7.1 checkin for new PR since the old PR1191 had issues w.r.t DCO #1208

merged 1 commit into from
Apr 21, 2021

Conversation

kannankvs
Copy link
Contributor

@rlhui : We are unable to resolve the DCO issue in PR1191. We have created this new PR for the 1.7.1 release notes. We have copied and pasted all the comments (as next comment) and the replies from the PR1191 to this new PR so that the comments are not missed out. We have given reference to this PR from the old PR. We will be closing the old PR after this new PR is merged.

…ssues w.r.t. DCO

Signed-off-by: Kannan KVS <kannan_kvs@dell.com>
@kannankvs
Copy link
Contributor Author

COMMENTS and REPLIES from PR1191

  1. <<rlhui_3rdMar2021>>@tarakreddy1984 , @JaiOCP , @itaibaz , @jasmeetbagga , @kcudnik , @marian-pritsak - would we please target to review/approve this by this week? Thanks.
    <> Waiting for review to be completed.

  2. <<itaibaz_3rdMar2021>>instead of typedef sai_query_stats_capability should say new function sai_query_stats_capability
    <> Addressed.

  3. <<itaibaz_3rdMar2021>>probably a typo, adds the ability to attach
    <> Addressed.

  4. <<JaiOCP_6thMar2021>> There is a typo in the line: This API will return number of vxlan tunnels which can forward with packet action as drop.
    <> Addressed.

  5. <<JaiOCP_6thMar2021>> Please add following line:
    A UDF group that will be used in ACL must be set to SAI_UDF_GROUP_GENERIC as the group type.
    <> Addressed.

  6. <<rlhui_9thMar2021>> @tarakreddy1984 , @jasmeetbagga , @kcudnik , @marian-pritsak - gentle reminder to please review/approve.
    @JaiOCP , @itaibaz - thanks a lot for reviewing. if latest looks good to you, please sign off. Thanks.

  7. <<jasmeetbagga_9thMar2021>>
    For PR 1137
    Add packet allocate, free function to allow for 0 copy packet TX. I would rephrase it as (sorry I can't seem to comment inline on the release notes). This PR enables efficient packet TX, by leveraging zero copy packet memory allocation when available.
    Sending a packet requires allocating it first. Typically, this requires allocating from a special pool of DMAed memory.
    For this a ASIC vendor SDK may support custom allocate and free functions. So mirror those in the SAI spec.
    Without such a ability we are left to allocate heap memory, pass it to send_hostif_packet_fn, which then allocates from the
    aforementioned DMA memory, copies the contents of heap memory and sends the packet along. This causes us to have an extra
    allocation and copy. This can induce a substantial slowdown -~30% in our measurements.

This is achieved by

  • Adding SAI_HOSTIF_PACKET_ATTR_ZERO_COPY_TX in sai_hostif_packet_attr_t
  • Adding functions sai_allocate_hostif_packet_fn, sai_free_hostif_packet_fn
    in saihostif.h

Furthermore total DMA pool size can be queried via SAI_SWITCH_ATTR_PACKET_
DMA_MEMORY_POOL_SIZE in sai_switch_attr_t on saiswitch.h
<> Addressed.

  1. <<rlhui_17thMar2021>>
    The below two were added after v1.6.3 but missing in the release notes. Additional Port Interface Types (Additional Port Interface Types #1098) …
    Policy based hash (Policy based hash #1074) …
    <> Addressed.

  2. <<rlhui_17thMar2021>>
    Please also fix the DCO error.
    <> Could not resolve it in the old PR and hence created this new PR from a fresh fork, which resolved the DCO error.

@kannankvs
Copy link
Contributor Author

@rlhui : All comments have been addressed. There are no further comments for the last few days. Request you to signoff and merge the PR.

@kannankvs
Copy link
Contributor Author

@rlhui , Thanks for your review and sign off. While we tried to merge the PR, we see "opencomputeproject.SAI Expected — Waiting for status to be reported" part of a check. There is no much information on this. Can you guide us on how do we merge the PR clearing this check.

@rlhui
Copy link
Collaborator

rlhui commented Apr 21, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rlhui rlhui merged commit 1e7ae1b into opencomputeproject:master Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants