On Thu, Apr 27, 2023 at 03:47:40PM +0300, Vitaliy Makkoveev wrote:
>
>
> > On 27 Apr 2023, at 15:40, Alexander Bluhm <[email protected]> wrote:
> >
> > On Thu, Apr 27, 2023 at 03:22:10PM +0300, Vitaliy Makkoveev wrote:
> >>> On 27 Apr 2023, at 15:16, Alexander Bluhm <[email protected]> wrote:
> >>>
> >>> On Wed, Apr 26, 2023 at 11:17:37PM +0300, Vitaliy Makkoveev wrote:
> >>>> Route timers and route labels protected by corresponding mutexes. `ifa'
> >>>> uses references counting for protection. No protection required for `rt'
> >>>> passed to rt_mpls_clear() because only current thread owns it.
> >>>>
> >>>> ok?
> >>>
> >>> I have tested your diff and it works for me. But I don't have a
> >>> MPLS setup. And there is a rt_mpls_clear(rt) which your diff does
> >>> not show.
> >>>
> >>> if (rt->rt_llinfo != NULL)
> >>> free(rt->rt_llinfo);
> >>> rt->rt_llinfo = NULL;
> >>>
> >>> and rt->rt_flags &= ~RTF_MPLS are not mpsafe.
> >>>
> >>> What about this? Compile tested only due to lacking MPLS setup.
> >>
> >> This is not required. We hold the last reference of this dying `rt???.
> >
> > You are right. OK bluhm@ for rtfree unlocking.
> >
> > But I am still not convinced with rt_mpls_clear().
> >
> > What about htis call path? soreceive -> pru_send -> route_send ->
> > route_output -> rtm_output -> rt_mpls_clear
> >
> > Since solock() takes rw_enter_write(&so->so_lock) for route sockets,
> > we don't have the kernel lock anymore. I would feel safer with
> > this kenrel lock hammer for mpls. Of course rt_mpls_set() would
> > need something simmilar.
>
> This path is netlock protected.
Ah yes. It is the exclusive netlock, so it is sufficient.
> 911 rtm_output(struct rt_msghdr *rtm, struct rtentry **prt,
> 912 struct rt_addrinfo *info, uint8_t prio, unsigned int tableid)
> 913 {
> /* skip */
> 1132 #ifdef MPLS
> 1133 if (rtm->rtm_flags & RTF_MPLS) {
> 1134 NET_LOCK();
> 1135 error = rt_mpls_set(rt,
> 1136 info->rti_info[RTAX_SRC], info->rti_mpls);
> 1137 NET_UNLOCK();
> 1138 if (error)
> 1139 break;
> 1140 } else if (newgate || (rtm->rtm_fmask & RTF_MPLS)) {
> 1141 NET_LOCK();
> 1142 /* if gateway changed remove MPLS information */
> 1143 rt_mpls_clear(rt);
> 1144 NET_UNLOCK();
> 1145 }
> 1146 #endif
>
