> On 27 Apr 2023, at 15:40, Alexander Bluhm <alexander.bl...@gmx.net> wrote:
>
> On Thu, Apr 27, 2023 at 03:22:10PM +0300, Vitaliy Makkoveev wrote:
>>> On 27 Apr 2023, at 15:16, Alexander Bluhm <alexander.bl...@gmx.net> 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.
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
