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

Implement timestamps and fix issues #14

Merged
merged 21 commits into from
Jul 1, 2024
Merged

Conversation

Lahyllas
Copy link

TX, RX 타임스탬프를 구현하고 ioctl과 연동하였으며, 그 과정에서 발견한 몇 가지 문제를 수정하였습니다. 수정사항 및 기타 언급해야 할 사항은 다음과 같습니다.

  1. ptp_settime() 함수에서 cycle_1s를 설정하는 부분을 삭제했었는데, (link) 그것을 복구했습니다. settime() 함수는 싱크를 조절할 때가 아니라 초기화할 때 호출되는 것으로 보이며, 이 부분이 없으면 master에서는 cycle_1s가 한 번도 설정되지 않아 싱크에 문제가 발생합니다. (특히 한 번 slave로 phc2sys를 실행하여 cycle_1s 값이 바뀌었을 경우)

  2. QoS의 기본값을 Qbv를 모두 open한 상태로 설정했습니다. 현재 발생하고 있는 QoS가 없을 때 throughput이 감소하는 문제를 우회하기 위한 것이며, 그대로 놔두면 테스트에 불편함이 있어 임시로 이렇게 해 두었습니다. 이 문제가 해결되면 다시 되돌릴 예정입니다.

  3. PTP 패킷의 타입에 따라 RX Timestamp 적용 여부를 결정하도록 구현은 했지만 (link) 지금은 테스트를 할 수 없어 (latency 측정 중) 검증은 되지 않았습니다. 추후에 PTP로 timestamp 테스트한 후에 수정이 필요하면 반영하도록 하겠습니다.

Copy link

@tribela tribela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 좋습니다. 몇 가지 부분은 수정이 필요해 보입니다

Comment on lines 126 to 132
if (priv->tstamp_config.tx_type != HWTSTAMP_TX_ON) {
metadata->timestamp_id = 0;
} else if (is_gptp) {
metadata->timestamp_id = 2;
} else {
metadata->timestamp_id = 1;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이름을 지어줍시다 (define을 하든 enum을 하든)
TIMESTAMP_ID_NONE = 0
TIMESTAMP_ID_GPTP = 1
TIMESTAMP_ID_NORMAL = 2
이런 식으로요 NONE은 0이고 gPTP가 1이 되는 게 더 좋습니다. 지금은 4번까지 있지만 어디까지 늘어나거나 줄어들지 모르니까 중요한 게 앞으로 가는 게 좋죠

@@ -415,6 +426,21 @@ int tsn_set_qbv(struct pci_dev* pdev, struct tc_taprio_qopt_offload* qopt) {
return -EINVAL;
}

if (!qopt->enable) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 Qav만 설정했다가 비활성화 하는 경우엔 호출이 안 되지 않나요?
qav든 qbv든 마지막에 bake를 호출하고 bake 안에서 설정하는 게 좋을 것 같습니다.
bake_qbv_configbake_qos_config 등으로 qbv에 한정되지 않게 바꿀 필요는 있겠죠

@@ -104,7 +105,7 @@ netdev_tx_t xdma_netdev_start_xmit(struct sk_buff *skb,
/* Check desc count */
netif_stop_queue(ndev);
xdma_debug("xdma_netdev_start_xmit(skb->len : %d)\n", skb->len);
skb->len = (skb->len < ETH_ZLEN) ? (ETH_ZLEN - skb->len) : 0;
skb->len += (skb->len < ETH_ZLEN) ? (ETH_ZLEN - skb->len) : 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

최소값 리밋을 거는 거라면 이게 더 깔끔합니다.

Suggested change
skb->len += (skb->len < ETH_ZLEN) ? (ETH_ZLEN - skb->len) : 0;
skb->len = max(ETH_ZLEN, skb->len)

Comment on lines 152 to 161
if (true) {
config->qbv.enabled = true;
config->qbv.start = 0;
config->qbv.slot_count = 2;
config->qbv.slots[0].duration_ns = 500000000; // 500ms
config->qbv.slots[0].opened_prios[0] = true;
config->qbv.slots[1].duration_ns = 500000000; // 500ms
config->qbv.slots[1].opened_prios[0] = false;
for (i = 0; i < VLAN_PRIO_COUNT; i++) {
config->qbv.slots[0].opened_prios[i] = true;
config->qbv.slots[1].opened_prios[i] = true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bake에서 처리하는 방식으로 바꾸면 이 부분도 제거 가능합니다

@tribela tribela merged commit 16fd8d8 into burst-xdma Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants