Added manually configuring socket prio #33
Conversation
|
Code review in progress. |
|
Hi there, can you add some explanation about your change in the commit message? |
|
Besides, I think the code inside this commit https://github.com/intel/iotg_tsn_ref_sw/pull/33/commits/4f02c48f2bb0f37521d9b958ebe5b6621a113b19 seems like not part of the original code. Can you rebase to the latest code? |
| @@ -132,6 +132,8 @@ static struct argp_option options[] = { | |||
| {"wakeup", 'w', 0, 0, "enable need_wakeup"}, | |||
| {"vlan-prio", 'q', "NUM", 0, "packet vlan priority, also socket priority\n" | |||
| " Def: 0 | Min: 0 | Max: 7"}, | |||
| {"socket-prio", 'g', "NUM", 0, "packet socket priority\n" | |||
There was a problem hiding this comment.
unwanted indentation between socket priority
| len = strlen(arg); | ||
| res = strtol((const char *)arg, &str_end, 10); | ||
| if (errno || res < 0 || res >= 15 || str_end != &arg[len]) | ||
| exit_with_error("Invalid queue number/socket priority. Check --help"); |
There was a problem hiding this comment.
if this specifically for socket priority only then queue number need to be removed
| @@ -188,6 +190,15 @@ static error_t parser(int key, char *arg, struct argp_state *state) | |||
| #endif | |||
| opt->vlan_prio = opt->socket_prio * 32; | |||
| break; | |||
There was a problem hiding this comment.
if the option 'g' specifically for socket priority then the socket priority need to be decouple out from option 'q' to prevent any error occur.
|
Hi @nilsalon100, Could you please squash all the commits into one? This would help make the history much cleaner and more useful for later inspection and analysis, such as when using |
There was a problem hiding this comment.
@nilsalon100 - Kindly Squash all the patches, otherwise we can't proceed on the PR.
The PR review is to check for sustainability and correctness. Sustainability is actually more business critical as correctness is largely tested into the code over time. Its useful to keep in mind that SW often outlives the HW it was written for and engineers move from job to job so it is critical that code developed for Intel be supportable across many years. It is up to the submitter and reviewer to look at the code from a perspective of what if we have to debug this 3 years from now after the author is no longer available and defect databases have been lost. Yes, that happens all the time when we are working with time scales of more than 2 years. When reviewing your code it is important to look at it from this perspective.
Author Mandatory (to be filled by PR Author/Submitter)
PULL DESCRIPTION (to be filled by PR Author/Submitter)
Provide a 1-2 line brief overview of the changes submitted through the Pull Request...
REFERENCES (to be filled by PR Author/Submitter)
Reference URL for issue tracking (JIRA/HSD/Github): <URL to be filled>
Note-1: Depending on complexity of code changes, use the suitable word for complexity: Low/Medium/High
Example: PR for Slim boot loader project with medium complexity can have the label as: ISDM_Medium
CODE MAINTAINABILITY (to be filled by PR Author/Submitter)
UPSTREAM EXPECTATIONS (NA for STAR)
Maintainer Mandatory (to be filled by PR Reviewer/Approving Maintainer)
QUALITY CHECKS (to be filled by PR Reviewer/Approving Maintainer)
CODE REVIEW IMPACT (Optional - need to discuss with ISDM forum)
Note P1/P2/P3/P4 denotes severity of defects found (Showstopper/High/Medium/Low) and xx denotes number of defects found
SECURITY CHECKS (NA for STAR)
Please check if your PR fulfills the following requirements:
Instead, clarify the check condition using defined behaviors. Example:
if (x > INT_MAX-100)instead ofif (x + 100 < x)Code must act as a teacher for future developers