Commit 51ab31a3 authored by Cedric Roux's avatar Cedric Roux

trigger NR activation based on RRC measurements

An NR-capable UE will transmit some NR measurements.
Once received, the whole process on nr-cell addition is started.
It will fail quickly.

Many things are still missing.

What is not handled properly with this commit:
- in x2ap_eNB_handle_sgNB_add_req we hardcode the gNB ID
- nr triggering is done if we receive a measurement but this measurement is
  not analyzed at all
- no timers (T_dc_overall, T_dc_prep) and as is the code fails and then the
  next triggering after UE reconnection will not work (x2 ID are not well
  handled here and there in the code)

Note also that the gNB part has not been looked at all for this commit.

This is work in progress...
parent ad5706c8
develop 1 128-ues 256_QAM_demod LTE_TRX_on_single_port NCTU_OpinConnect_LDPC NR-PHY-MAC-IF-multi-UE NRUE_usedlschparallel NR_10MHz NR_CSI_reporting NR_DCI_01 NR_DLUL_PF NR_DLUL_PF_4UL NR_DLUL_PF_rebased NR_DL_MIMO NR_DL_sched_fixes NR_DL_scheduler NR_FAPI_beamindex_SSB_RO NR_FDD_FIX NR_FR2_RA NR_FR2_RRC_SSB NR_MAC_CE_GlobalEdge NR_MAC_Multi_Rach_GlobalEdge NR_MAC_SSB_RO_GlobalEdge NR_MAC_SSB_RO_UE_IDCC NR_MAC_SSB_RO_merge NR_MAC_TCI_UCI_GlobalEdge NR_NGAP NR_PDCP_noS1 NR_PUCCH_MultiUE NR_RA_cleanup NR_RA_updates NR_RRCConfiguration NR_RRCConfiguration_FR2 NR_RRCConfiguration_S1U NR_RRCConfiguration_merge_develop NR_RRCConfiguration_sync_source NR_RRCConfiguration_trx_thread NR_RRC_CP_bugfix NR_RRC_PDCP NR_RRC_PRACH_procedures NR_RRC_PRACH_procedures_todevelop NR_RRC_TA NR_RRC_X2AP_AMBR_Change_Global_edge NR_RRC_X2AP_RemoveHardcodings_GlobalEdge NR_RRC_config_simplified NR_RRC_harq NR_RRC_harq_b NR_RRC_harq_hacks NR_RRC_harq_newdcipdu NR_SA_F1AP_RFSIMULATOR NR_SA_F1AP_RFSIMULATOR2 NR_SA_F1AP_dev NR_SA_NGAP_RRC NR_SA_NGAP_RRC_wk42 NR_SA_itti_sim_wk48 NR_SA_itti_sim_wk48_hs NR_SA_itti_sim_wk48_hs1 NR_SCHED NR_SCHED_HARQ NR_SCHED_PDCCH_PUCCH_HARQ NR_SCHED_PDCCH_PUCCH_HARQ_rebased NR_SCHED_fixes NR_SRB_Config NR_TRX_on_single_port NR_TRX_on_single_port2 NR_UE_MAC_scheduler NR_UE_RA_fixes NR_UE_UL_DCI_improvements NR_UE_dlsch_bugfix NR_UE_enable_parallelization NR_UE_stability_fixes NR_UL_FAPI_programming NR_UL_scheduler NR_UL_scheduler_rebased NR_UL_scheduling NR_Wireshark NR_beam_simulation NR_beamforming_test NR_gNB_SCF_Indication NR_ipaccess_testing NR_mac_uci_functions_rework NR_msg2_phytest NR_scheduling_CSIRS NR_scheduling_request NR_scheduling_request2 NR_scheduling_request3 NR_test_S1U_RRC_PRACH_procedures NR_ue_dlsch_dmrs_cdm PUSCH_TA_update RA_CI_test UE_DL_DCI_hotfix bch-fixes-bitmap benetel_5g_prach_fix benetel_driver_uldl_pf_merge benetel_driver_update benetel_phase_rotation benetel_phase_rotation_old bugfix-minor-remove-wrong-log bugfix-nr-bands bugfix-nr-ldpc-post-processing bugfix-nr-ldpc-size-typo bugfix-nr-pdcp-sn-size bugfix-nr-rate-matching-assertion bugfix-x2-SgNBAdditionRequest bupt-sa-merge cce_indexing_fix cce_indexing_fix2 ci-deploy-asterix ci-deploy-docker-compose ci-rd-july-improvements ci-test ci-ul-iperf-from-trf-container clean-5G-scope-round2 cleanup_softmodem_main constant_power debug_branch_init_sync develop-SA-RA develop-SnT develop-oriecpriupdates develop-sib1 develop-sib1-local develop-sib1-lts develop-sib1-update develop-sib1-update-test1 develop-sib1-update-ue develop_inria_ci_deployment develop_inria_ci_deployment_gp develop_integration_2020_w15 develop_integration_2020_w19 develop_stable dfts_alternatives dlsch-all-dlslots dlsch_encode_mthread dlsch_parallel dongzhanyi-zte-develop dongzhanyi-zte-develop2 extend_sharedlibusage2 fapi_for_dmrs_and_ptrs feature-4g-sched feature/make-s1-mme-port-configurable feature/make-s1-mme-port-configurable-with-astyle-fixes fembms-enb-ue finalize-oaicn-integration firas fix-ci-tun fix-itti-segv fix-softmodem-restart fix-warnings fix_NR_DLUL_PF fix_do_ra_data fix_pdsch_low_prb fix_rfsim_mimo fix_rrc_x2_ticking fixes-CE-RLC-PDU-size fixes-mac-sched-nfapi fixes-mac-sched-tun fixes-tun flexran-apps flexran-repair-mme-mgmt fujitsu_lte_contribution fujitsu_lte_contribution-128 gnb-only-test harq-hotfix hotfix-minor-remove-nr-rlc-cppcheck-error hotfix-nr-rlc-tick hotfix-ocp-executable hotfix-ue-musim-compilation improve_nr_modulation improve_ue_stability integration_2020_wk40 integration_2020_wk41 integration_2020_wk42_2 integration_2020_wk45 integration_2020_wk45_2 integration_2020_wk46 integration_2020_wk46_2 integration_2020_wk47 integration_2020_wk48 integration_2020_wk48_2 integration_2020_wk49 integration_2020_wk50 integration_2020_wk50_1 integration_2020_wk51 integration_2020_wk51_2 integration_2021_wk02 integration_2021_wk02_wMR988 integration_2021_wk04 integration_2021_wk05 inter-RRU-final inter-RRU-oairu inter-rru-UE itti-enhancement l2-fixes ldpc-decoder-codegen ldpc-decoder-codegen2 ldpc-offload ldpc_short_codeword_fixes load_gnb lte-ulsch-bugfix lte_uplink_improvement mac-fixes-wk45_2 minor-fix-doc-basic-sim mosaic5g-oai-ran mosaic5g-oai-sim nasmesh_kernel_5.8 new-gtpu new_rlc_2020 nfapi-bugfix nfapi_nr_develop ngap-dlul ngap-support ngap-w48-merge2 ngap-wf ngap-wf-1120 ngap-wf-1120-srb ngap-wf-1120-srb-gtp ngap-wf-1120-srb-gtp-hs ngap-wf-1120-srb-gtp-hs1 ngap-wf-1120-srb-gtp-hs2 ngap-wf-1120-srb-gtp-yhz ngap-wf-1203-yunsdr ngap-wf-liuyu ngap_lfq_1120 ngap_merge noCore nr-dual-connectivity nr-mac-pdu-wireshark nr-mac-remove-ue-list nr-rlc-am-bugfix-w44 nr-rlc-bugfix-w44 nrUE nrUE-hs nrUE-upper-layer nr_bsr nr_dl_dmrs_type2 nr_dl_pf nr_dl_pf2 nr_dl_ul_ptrs nr_fdd_if_fix nr_polar_decoder_improvement nr_prach_fr2 nr_pucch nr_pucch2 nr_ue_msg3 nr_ue_tti_cleanup nr_ul_pf nr_ul_scfdma nrue-multi-thread nrue_msg2_reception nsa-ue nsa_remove_band_hardcodings oai-sim oairu oairu-dockerfile-support oc-docker-october-improvements openxg/develop pdcp-benchmark polar8 ptrs_rrc_config pusch-mthread-scaling-fix pusch-retrans-fix-ue ra-dl-ul remove_nos1_hack_pdcp remove_x2_gnb_hardcoding repair-TA revert_memcpy rh-ci-add-ue-parallelization rh_ci_fix_autoterminate rh_ci_fr1_update rh_ci_oc rh_ci_py rh_ci_ra_fr2 rh_ci_rfsim_ra rh_ci_ue_parallel rh_doc_update_3 rh_fr1_newjenkins rh_fr1_update rh_gnb_compile_fix rh_wk50_debug rlc-v2-bugfix-status-reporting rlc-v2-tick rlc_v2_coverity_fixes rrc-enb-phy-testmode s1-subnormal_rewrite s1_subnormal s1_subnormal-robert s1ap-bugfix-rab_setup sa-demo sa-merge-rrc-srb sa-msg4 sa-msg4-rrc sa-msg4-rrc-yihz sa-msg4-rrc-yihz-hs sa_rrc_yihz small-bugfixes-w40 small-config-change smallcleanup split73 testing_2symb_pdcch testing_with_external_txdata tp-ota-test trx_thread_param ue-csi ue-fixes ue-fixes-ota ue_beam_selection ue_nfapi_mch ul-freq-iq-samps-to-file ul_dl_dci_same_slot ul_harq ulsch_decode_mthread ulsim_changes usrp_fix_adc_shift_and_pps_sync wireshark-T-hack-ueid wireshark-log-scheduling-requests x2-endc-processing xiangwab xiangwan yihongzheng_srb zzs 2021.w04 2021.w02 2020.w51_2 2020.w51 2020.w50 2020.w49 2020.w48_2 2020.w48 2020.w47 2020.w46_2 2020.w46 2020.w45_2 2020.w45 2020.w44 2020.w42_2 2020.w42 2020.w41 2020.w39 2020.w38 2020.w37 2020.w36 2020.w34 2020.w33 2020.w31 2020.w30 2020.w29 2020.w28 2020.w26 2020.w25 2020.w24 2020.w23 2020.w22 2020.w19 2020.w17 2020.w16 2020.w15 benetel_phase_rotation
No related merge requests found
...@@ -357,7 +357,7 @@ typedef struct x2ap_ENDC_sgnb_addition_req_ACK_s { ...@@ -357,7 +357,7 @@ typedef struct x2ap_ENDC_sgnb_addition_req_ACK_s {
int SgNB_ue_x2_id; int SgNB_ue_x2_id;
/* used for RRC->X2AP in source eNB */ /* used for X2AP->RRC in source eNB */
int rnti; int rnti;
uint8_t nb_e_rabs_admitted_tobeadded; uint8_t nb_e_rabs_admitted_tobeadded;
......
...@@ -265,7 +265,8 @@ typedef enum UE_STATE_e { ...@@ -265,7 +265,8 @@ typedef enum UE_STATE_e {
RRC_SI_RECEIVED, RRC_SI_RECEIVED,
RRC_CONNECTED, RRC_CONNECTED,
RRC_RECONFIGURED, RRC_RECONFIGURED,
RRC_HO_EXECUTION RRC_HO_EXECUTION,
RRC_NR_NSA,
} UE_STATE_t; } UE_STATE_t;
typedef enum HO_STATE_e { typedef enum HO_STATE_e {
......
...@@ -4297,6 +4297,10 @@ rrc_eNB_process_MeasurementReport( ...@@ -4297,6 +4297,10 @@ rrc_eNB_process_MeasurementReport(
ctxt_pP->subframe); ctxt_pP->subframe);
break; break;
case 7:
LOG_D(RRC, "NR event frame %d subframe %d\n", ctxt_pP->frame, ctxt_pP->subframe);
break;
default: default:
LOG_D(RRC,"Other event report frame %d and subframe %d \n", ctxt_pP->frame, ctxt_pP->subframe); LOG_D(RRC,"Other event report frame %d and subframe %d \n", ctxt_pP->frame, ctxt_pP->subframe);
break; break;
...@@ -4310,6 +4314,22 @@ rrc_eNB_process_MeasurementReport( ...@@ -4310,6 +4314,22 @@ rrc_eNB_process_MeasurementReport(
ue_context_pP->ue_context.measResults->measResultPCell.rsrqResult/2 - 20); ue_context_pP->ue_context.measResults->measResultPCell.rsrqResult/2 - 20);
} }
/* TODO: improve NR triggering */
if (measResults2->measId == 7) {
if (ue_context_pP->ue_context.Status != RRC_NR_NSA) {
MessageDef *msg;
ue_context_pP->ue_context.Status = RRC_NR_NSA;
msg = itti_alloc_new_message(TASK_RRC_ENB, X2AP_ENDC_SGNB_ADDITION_REQ);
X2AP_ENDC_SGNB_ADDITION_REQ(msg).rnti = ctxt_pP->rnti;
LOG_I(RRC,
"[eNB %d] frame %d subframe %d: UE rnti %x switching to NSA mode\n",
ctxt_pP->module_id, ctxt_pP->frame, ctxt_pP->subframe, ctxt_pP->rnti);
itti_send_msg_to_task(TASK_X2AP, ENB_MODULE_ID_TO_INSTANCE(ctxt_pP->module_id), msg);
return;
}
}
if (measResults2->measResultNeighCells == NULL) if (measResults2->measResultNeighCells == NULL)
return; return;
...@@ -8399,7 +8419,7 @@ void rrc_eNB_process_AdditionResponseInformation(const module_id_t enb_mod_idP, ...@@ -8399,7 +8419,7 @@ void rrc_eNB_process_AdditionResponseInformation(const module_id_t enb_mod_idP,
PROTOCOL_CTXT_SET_BY_INSTANCE(&ctxt, PROTOCOL_CTXT_SET_BY_INSTANCE(&ctxt,
0, 0,
ENB_FLAG_YES, ENB_FLAG_YES,
4660,//ue_context->ue_context.rnti, m->rnti,
0, 0); 0, 0);
size = rrc_eNB_generate_RRCConnectionReconfiguration_endc(&ctxt, ue_context, buffer, 8192, scg_CellGroupConfig, nr1_conf); size = rrc_eNB_generate_RRCConnectionReconfiguration_endc(&ctxt, ue_context, buffer, 8192, scg_CellGroupConfig, nr1_conf);
......
...@@ -446,6 +446,37 @@ void x2ap_eNB_handle_handover_req_ack(instance_t instance, ...@@ -446,6 +446,37 @@ void x2ap_eNB_handle_handover_req_ack(instance_t instance,
x2ap_eNB_generate_x2_handover_request_ack(instance_p, target, x2ap_handover_req_ack); x2ap_eNB_generate_x2_handover_request_ack(instance_p, target, x2ap_handover_req_ack);
} }
static
void x2ap_eNB_handle_sgNB_add_req(instance_t instance,
x2ap_ENDC_sgnb_addition_req_t *x2ap_ENDC_sgnb_addition_req)
{
x2ap_id_manager *id_manager;
x2ap_eNB_instance_t *instance_p;
x2ap_eNB_data_t *x2ap_eNB_data;
int ue_id;
/* TODO: remove hardcoded value */
x2ap_eNB_data = x2ap_is_eNB_id_in_list(3584);
DevAssert(x2ap_eNB_data != NULL);
instance_p = x2ap_eNB_get_instance(instance);
DevAssert(instance_p != NULL);
/* allocate x2ap ID */
id_manager = &instance_p->id_manager;
ue_id = x2ap_allocate_new_id(id_manager);
if (ue_id == -1) {
X2AP_ERROR("could not allocate a new X2AP UE ID\n");
/* TODO: cancel NSA: send (to be defined) message to RRC */
exit(1);
}
/* id_source is ue_id, id_target is unknown yet */
x2ap_set_ids(id_manager, ue_id, x2ap_ENDC_sgnb_addition_req->rnti, ue_id, -1);
x2ap_id_set_state(id_manager, ue_id, X2ID_STATE_NSA_PREPARE);
x2ap_eNB_generate_ENDC_x2_SgNB_addition_request(instance_p, x2ap_eNB_data, ue_id);
}
static static
void x2ap_gNB_trigger_sgNB_add_req_ack(instance_t instance, void x2ap_gNB_trigger_sgNB_add_req_ack(instance_t instance,
x2ap_ENDC_sgnb_addition_req_ACK_t *x2ap_ENDC_sgnb_addition_req_ACK) x2ap_ENDC_sgnb_addition_req_ACK_t *x2ap_ENDC_sgnb_addition_req_ACK)
...@@ -552,11 +583,15 @@ void *x2ap_task(void *arg) { ...@@ -552,11 +583,15 @@ void *x2ap_task(void *arg) {
&X2AP_UE_CONTEXT_RELEASE(received_msg)); &X2AP_UE_CONTEXT_RELEASE(received_msg));
break; break;
case X2AP_ENDC_SGNB_ADDITION_REQ:
x2ap_eNB_handle_sgNB_add_req(ITTI_MESSAGE_GET_INSTANCE(received_msg),
&X2AP_ENDC_SGNB_ADDITION_REQ(received_msg));
break;
case X2AP_ENDC_SGNB_ADDITION_REQ_ACK: case X2AP_ENDC_SGNB_ADDITION_REQ_ACK:
x2ap_gNB_trigger_sgNB_add_req_ack(ITTI_MESSAGE_GET_INSTANCE(received_msg), x2ap_gNB_trigger_sgNB_add_req_ack(ITTI_MESSAGE_GET_INSTANCE(received_msg),
&X2AP_ENDC_SGNB_ADDITION_REQ_ACK(received_msg)); &X2AP_ENDC_SGNB_ADDITION_REQ_ACK(received_msg));
LOG_I(X2AP, "Received elements for X2AP_ENDC_SGNB_ADDITION_REQ_ACK \n"); LOG_I(X2AP, "Received elements for X2AP_ENDC_SGNB_ADDITION_REQ_ACK \n");
break; break;
case SCTP_INIT_MSG_MULTI_CNF: case SCTP_INIT_MSG_MULTI_CNF:
......
...@@ -1569,7 +1569,7 @@ int x2ap_eNB_generate_ENDC_x2_SgNB_addition_request( ...@@ -1569,7 +1569,7 @@ int x2ap_eNB_generate_ENDC_x2_SgNB_addition_request(
ie->id = X2AP_ProtocolIE_ID_id_MeNB_UE_X2AP_ID; //Not sure about that ie->id = X2AP_ProtocolIE_ID_id_MeNB_UE_X2AP_ID; //Not sure about that
ie->criticality= X2AP_Criticality_reject; ie->criticality= X2AP_Criticality_reject;
ie->value.present = X2AP_SgNBAdditionRequest_IEs__value_PR_UE_X2AP_ID; ie->value.present = X2AP_SgNBAdditionRequest_IEs__value_PR_UE_X2AP_ID;
ie->value.choice.UE_X2AP_ID = ue_id; //x2ap_id_get_id_source(&instance_p->id_manager, ue_id); ie->value.choice.UE_X2AP_ID = x2ap_id_get_id_source(&instance_p->id_manager, ue_id);
ASN_SEQUENCE_ADD(&out->protocolIEs.list, ie); ASN_SEQUENCE_ADD(&out->protocolIEs.list, ie);
ie = (X2AP_SgNBAdditionRequest_IEs_t *)calloc(1, sizeof(X2AP_SgNBAdditionRequest_IEs_t)); ie = (X2AP_SgNBAdditionRequest_IEs_t *)calloc(1, sizeof(X2AP_SgNBAdditionRequest_IEs_t));
......
...@@ -1482,9 +1482,7 @@ x2ap_eNB_handle_ENDC_x2_setup_request(instance_t instance, ...@@ -1482,9 +1482,7 @@ x2ap_eNB_handle_ENDC_x2_setup_request(instance_t instance,
instance_p = x2ap_eNB_get_instance(instance); instance_p = x2ap_eNB_get_instance(instance);
DevAssert(instance_p != NULL); DevAssert(instance_p != NULL);
x2ap_eNB_generate_ENDC_x2_setup_response(instance_p, x2ap_eNB_data); return x2ap_eNB_generate_ENDC_x2_setup_response(instance_p, x2ap_eNB_data);
return x2ap_eNB_generate_ENDC_x2_SgNB_addition_request(instance_p, x2ap_eNB_data,0); // Not the right place to call the X2 function. Used only initially for testing.
} }
int int
...@@ -1804,6 +1802,9 @@ int x2ap_eNB_handle_ENDC_sGNB_addition_response (instance_t instance, ...@@ -1804,6 +1802,9 @@ int x2ap_eNB_handle_ENDC_sGNB_addition_response (instance_t instance,
x2ap_eNB_data_t *x2ap_eNB_data; x2ap_eNB_data_t *x2ap_eNB_data;
MessageDef *msg; MessageDef *msg;
int ue_id; int ue_id;
int id_source;
int id_target;
int rnti;
DevAssert (pdu != NULL); DevAssert (pdu != NULL);
x2SgNBAdditionRequest_ack = &pdu->choice.successfulOutcome.value.choice.SgNBAdditionRequestAcknowledge; x2SgNBAdditionRequest_ack = &pdu->choice.successfulOutcome.value.choice.SgNBAdditionRequestAcknowledge;
...@@ -1835,6 +1836,8 @@ int x2ap_eNB_handle_ENDC_sGNB_addition_response (instance_t instance, ...@@ -1835,6 +1836,8 @@ int x2ap_eNB_handle_ENDC_sGNB_addition_response (instance_t instance,
return -1; return -1;
} }
id_source = ie->value.choice.UE_X2AP_ID;
X2AP_ENDC_SGNB_ADDITION_REQ_ACK(msg).MeNB_ue_x2_id = ie->value.choice.UE_X2AP_ID; X2AP_ENDC_SGNB_ADDITION_REQ_ACK(msg).MeNB_ue_x2_id = ie->value.choice.UE_X2AP_ID;
...@@ -1846,6 +1849,24 @@ int x2ap_eNB_handle_ENDC_sGNB_addition_response (instance_t instance, ...@@ -1846,6 +1849,24 @@ int x2ap_eNB_handle_ENDC_sGNB_addition_response (instance_t instance,
return -1; return -1;
} }
id_target = ie->value.choice.SgNB_UE_X2AP_ID;
ue_id = id_source;
if (ue_id != x2ap_find_id_from_id_source(&instance_p->id_manager, id_source)) {
X2AP_WARN("incorrect/unknown X2AP IDs for UE (old ID %d new ID %d), ignoring sGNB addition response\n",
id_source, id_target);
itti_free(ITTI_MSG_ORIGIN_ID(msg), msg);
return 0;
}
rnti = x2ap_id_get_rnti(&instance_p->id_manager, ue_id);
/* id_target is a new information, store it */
x2ap_set_ids(&instance_p->id_manager, ue_id, rnti, id_source, id_target);
X2AP_ENDC_SGNB_ADDITION_REQ_ACK(msg).rnti = rnti;
X2AP_ENDC_SGNB_ADDITION_REQ_ACK(msg).SgNB_ue_x2_id = ie->value.choice.SgNB_UE_X2AP_ID; X2AP_ENDC_SGNB_ADDITION_REQ_ACK(msg).SgNB_ue_x2_id = ie->value.choice.SgNB_UE_X2AP_ID;
/* X2AP_ProtocolIE_ID_id_E_RABs_ToBeAdded_SgNBAddReqList */ /* X2AP_ProtocolIE_ID_id_E_RABs_ToBeAdded_SgNBAddReqList */
...@@ -1907,10 +1928,7 @@ int x2ap_eNB_handle_ENDC_sGNB_addition_response (instance_t instance, ...@@ -1907,10 +1928,7 @@ int x2ap_eNB_handle_ENDC_sGNB_addition_response (instance_t instance,
itti_send_msg_to_task(TASK_RRC_ENB, instance_p->instance, msg); itti_send_msg_to_task(TASK_RRC_ENB, instance_p->instance, msg);
x2ap_eNB_generate_ENDC_x2_SgNB_reconfiguration_complete(instance_p, x2ap_eNB_data,0,0); // Not the right place to call the X2 function. Used only initially for testing.
return 0; return 0;
} }
static static
......
...@@ -37,7 +37,8 @@ ...@@ -37,7 +37,8 @@
typedef enum { typedef enum {
X2ID_STATE_SOURCE_PREPARE, X2ID_STATE_SOURCE_PREPARE,
X2ID_STATE_SOURCE_OVERALL, X2ID_STATE_SOURCE_OVERALL,
X2ID_STATE_TARGET X2ID_STATE_TARGET,
X2ID_STATE_NSA_PREPARE,
} x2id_state_t; } x2id_state_t;
typedef struct { typedef struct {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment